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

Use \ for escaping in markdown #22169

Closed
wants to merge 1 commit into from
Closed

Use \ for escaping in markdown #22169

wants to merge 1 commit into from

Conversation

ankon
Copy link

@ankon ankon commented Aug 7, 2018

The HTML entities didn't "work", and the documentation contained them literally on the main site.

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

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. inspector Issues and PRs related to the V8 inspector protocol labels Aug 7, 2018
The HTML entities didn't "work", and the documentation contained them literally on the main site.
@aduh95
Copy link
Contributor

aduh95 commented Aug 7, 2018

@Trott
Copy link
Member

Trott commented Aug 7, 2018

Hi, @ankon! Welcome, and thanks for the pull request! This would seem to be a bug in the markdown processing we use as HTML entities should be rendered in this situation.

@rubys @vsemozhetbyt Bug in the new HTML generation stuff that needs to be fixed?

@nodejs/documentation are angle brackets (< >) even the right thing to use in this situation? I'm having a hard time coming up with a better and more intuitive way to express that this is not a literal value, so maybe it is?

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Aug 7, 2018

Thank you for the spotting and an attempt to fix. However, this is a part of a bigger issue that should be fixed in #22140 (probably after one more fix upstream, see comments there).

@rubys
Copy link
Member

rubys commented Aug 7, 2018

I agree with @vsemozhetbyt - the markup is correct, the processing of the markup is what is in error.

@Trott
Copy link
Member

Trott commented Aug 7, 2018

@rubys @vsemozhetbyt How long do we estimate before a fix appears? Is it worth it to do something like this as a temporary measure until the larger fix lands? (My sense is "no" if the larger fix is a few days away, but "perhaps" if it's longer than that.)

@rubys
Copy link
Member

rubys commented Aug 7, 2018

@Trott it should be days. I know what needs to be fixed and will submit the necessary pull requests. I seem to have gotten the attention of the maintainer who will land pull requests and push out new modules in a matter of minutes. The issue here is that Remark is a set of small modules, and this last change will affect at least two of them, as well as the data structure that is passed between them (MDAST).

Meanwhile, my preferred quick fix is here: c9e7083

And the accumulated set of workarounds to date can be found here: #22084

If there is a pressing need, I'm willing to land #22084 while we wait for #22140 (the latter both undoes #22084 as well as prior quick fixes, and depends on Remark.js doing the right thing™).

@ankon
Copy link
Author

ankon commented Aug 8, 2018

Thanks for looking at this, from my point: If you're working on a fix, do that -- the intent of the documentation is clear enough, this is just a cosmetical PR :)

@addaleax
Copy link
Member

addaleax commented Sep 2, 2018

ping @rubys – anything (left) to do here?

@rubys
Copy link
Member

rubys commented Sep 2, 2018

@BridgeAR BridgeAR closed this Sep 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants