Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: escape elements swallowed as HTML in markdown #29374

Closed
wants to merge 1 commit into from

Conversation

nschonni
Copy link
Member

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Addresses Markdownlint MD033 issues.
Altering changlog should usually be avoided, but they don't render
currently.
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Aug 30, 2019
`object.passphrase` is optional. Encrypted keys will be decrypted with
`object.passphrase` if provided, or `options.passphrase` if it is not.
buffers, or an array of objects in the form
`{pem: <string|buffer>[, passphrase: <string>]}`. The object form can only
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking across lines for code element renders OK in GitHub and the rendered HTML, but doesn't in IDEs and is flagged by Markdownlint

@@ -587,7 +587,7 @@ $ git push upstream master

Close the pull request with a "Landed in `<commit hash>`" comment. If
your pull request shows the purple merged status then you should still
add the "Landed in <commit hash>..<commit hash>" comment if you added
add the "Landed in \<commit hash>..\<commit hash>" comment if you added
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an improvement for sure (because now it actually renders) so 👍 as-is, but FWIW, it might be good to get the code-display highlighting (like this) from line 588 on this line too or else to remove it from line 588. Either one, but consistent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you thinking like this:

add the "Landed in `<commit hash>`..`<commit hash>`" comment if you added

or

add the "Landed in `<commit hash>..<commit hash>`" comment if you added

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, honestly. Maybe just omitting the backticks entirely?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it landed with just the escape. BTW, the backticks in the examples above were just because I codefence'd the line as markdown to show placement of the backticks as where to start/end the code sections that you were talking about

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the backticks in the examples above were just because I codefence'd the line as markdown

Yes, I understand. I was saying that maybe it makes the most sense to eliminate code sections entirely in that bit because <commit hash>..<commit hash> isn't really code. It's unclear whether to also treat Landed in as code, etc. etc. So, just treat it all like a string. But this is all pretty minor stuff and I'm OK with it not being fixed at all or else being fixed in whatever way someone else wants as long as the end result is consistency.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, I'm going to leave it since it's rendering again :)

@Trott
Copy link
Member

Trott commented Aug 31, 2019

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 31, 2019
danbev pushed a commit that referenced this pull request Sep 2, 2019
Addresses Markdownlint MD033 issues.
Altering changlog should usually be avoided, but they don't render
currently.

PR-URL: #29374
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@danbev
Copy link
Contributor

danbev commented Sep 2, 2019

Landed in 61d973a.

@danbev danbev closed this Sep 2, 2019
@nschonni nschonni deleted the fix-md033 branch September 2, 2019 05:59
BridgeAR pushed a commit that referenced this pull request Sep 3, 2019
Addresses Markdownlint MD033 issues.
Altering changlog should usually be avoided, but they don't render
currently.

PR-URL: #29374
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Sep 3, 2019
BridgeAR pushed a commit that referenced this pull request Sep 4, 2019
Addresses Markdownlint MD033 issues.
Altering changlog should usually be avoided, but they don't render
currently.

PR-URL: #29374
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants