FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: mitigate `marked` bug by vsemozhetbyt · Pull Request #20411 · nodejs/node · GitHub

/ node Public

doc: mitigate marked bug - #20411

Closed
vsemozhetbyt wants to merge 1 commit into
nodejs:masterfrom
vsemozhetbyt:doc-mitigate-marked-bug
Closed

doc: mitigate marked bug#20411
vsemozhetbyt wants to merge 1 commit into
nodejs:masterfrom
vsemozhetbyt:doc-mitigate-marked-bug

Conversation

vsemozhetbyt commented Apr 29, 2018
edited
Loading

Copy link
Copy Markdown
Contributor
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

Status quo

3cb8e64 + 647954d have fixed a method signature, but caused link rendering issues in 2 places of the HTML doc:

In the GitHub docs, both links are rendered properly: see here and here.

The cause

We have rather an old version of marked module and it seems it has a parsing bug for this pattern:

  • link text in code backticks
  • with two sibling pairs of square brackets
  • with one or more nested square brackets in the first sibling.

Minimal reproduction:

'use strict';

const marked = require('tools/doc/node_modules/marked/');

const md = `
A [\`[[]][]\`][ref].

[ref]: #hash
`;

const tokens = marked.lexer(md);
console.log(tokens);

const html = marked.parser(tokens);
console.log(`\n${html}`);
[ { type: 'paragraph', text: 'A [`[[]][]`][ref].' },
  links: { ref: { href: '#hash', title: undefined } } ]

<p>A [<code>[[]][]</code>]<a href="#hash">ref</a>.</p>

In this case, marked renders the first link part as a code fragment in non-code square brackets, and then renders the second part as a link inferring both the text and the URL from it.

What can we do

The bug is fixed in marked master. Compare the output from the tip-of-tree:

[ { type: 'text', text: 'A [`[[]][]`][ref].' },
  { type: 'space' },
  links: { ref: { href: '#hash', title: undefined } } ]

<p>A <a href="#hash"><code>[[]][]</code></a>.</p>

But it seems we cannot update now as marked is still unstable, has possible new bugs and experiences turbulent renewal (see issue with a comment).

So we can use this workaround till marked 1.0 is released. See screenshots of fixed links and here and here.

If we should add any HTML comments in these places with PR URL or TODO instructions, please, suggest the wording and format.

cc @nodejs/documentation

vsemozhetbyt added the fast-track PRs that do not need to wait for 48 hours to land. label Apr 29, 2018
nodejs-github-bot added doc Issues and PRs related to the documentations. net Issues and PRs related to the net subsystem. labels Apr 29, 2018

Copy link
Copy Markdown
Contributor Author

Node.js Collaborators, please, add 👍 here if you approve fast-tracking.

Copy link
Copy Markdown
Contributor Author

trivikr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Confirmed that links appear correctly in net.md in private branch

Copy link
Copy Markdown
Member

@vsemozhetbyt just as a question: did you check intermediate versions as well?

vsemozhetbyt commented Apr 30, 2018
edited
Loading

Copy link
Copy Markdown
Contributor Author

@BridgeAR
For this bug, I've only checked master. But I've checked the last v0.3.19 skin-deep to find out if there were any breaking changes and if we can update painlessly, and there were some quirks. So I've decided to not bother them till 1.0 according to the response.

Copy link
Copy Markdown
Contributor Author

Landed in 536b1fb
Thanks for the reviews.

vsemozhetbyt deleted the doc-mitigate-marked-bug branch May 2, 2018 03:03
vsemozhetbyt added a commit that referenced this pull request May 2, 2018
PR-URL: #20411
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 4, 2018
PR-URL: #20411
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins mentioned this pull request May 8, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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. fast-track PRs that do not need to wait for 48 hours to land. net Issues and PRs related to the net subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL