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: mitigate marked bug #20411

Closed
wants to merge 1 commit into from
Closed

doc: mitigate marked bug #20411

wants to merge 1 commit into from

Conversation

vsemozhetbyt
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt commented Apr 29, 2018

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 vsemozhetbyt added the fast-track PRs that do not need to wait for 48 hours to land. label Apr 29, 2018
@nodejs-github-bot 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
@vsemozhetbyt
Copy link
Contributor Author

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

@vsemozhetbyt
Copy link
Contributor Author

Copy link
Member

@trivikr trivikr left a comment

Choose a reason for hiding this comment

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

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

@BridgeAR
Copy link
Member

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

@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Apr 30, 2018

@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.

@vsemozhetbyt
Copy link
Contributor Author

Landed in 536b1fb
Thanks for the reviews.

@vsemozhetbyt 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 <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 4, 2018
PR-URL: #20411
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request May 8, 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. 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