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

tools: update to mdast-util-to-hast v3.0.2 #22140

Merged
merged 1 commit into from Mar 10, 2019

Conversation

@rubys
Copy link
Member

commented Aug 5, 2018

See syntax-tree/mdast-util-to-hast#21

Fixes #22065

Note: I updated all of the tools/doc dependencies, not just this one,
and removed the previous workaround that was in place until this change
landed.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
@vsemozhetbyt

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2018

There is good news.

This PR fix two known regressions and one previously unknown regression:

  1. Regression with Markdown escapes described in #22065

  2. Regression with HTML entities described in #22084 (comment)

  3. Unknown regression with Markdown backticks rendered in headings instead of code tags (nightly example).

@vsemozhetbyt

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2018

There is bad news.

This PR introduces one new regression. If a heading has two consecutive optional parameters in square brackets, they are not rendered as links, but some preprocess cleaning is done as if they are links:

  1. spaces between them are deleted;
  2. all text in the second pair of brackets is lowercased.

Compare these examples before and after this PR:

11
12

21
22

@vsemozhetbyt

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2018

And there is good bad news)

Comparing the diff helps to find out one issue that is differently exposed before and after this PR.

  1. Before this PR. If a parameter type signature is a 2-dimensional array with a [][] part, this part is rendered as an invisible empty link (nightly example, see records parameter) and the type is not properly shown and linkified.

  2. After this PR. All the type is rendered wrongly verbatim without proper linkification:

3

Compare the doc from the old toolchain: https://nodejs.org/docs/latest-v8.x/api/dns.html#dns_dns_resolvetxt_hostname_callback

One of the concerned code fragments:

// To support type[], type[][] etc., we store the full string
// and use the bracket-less version to lookup the type URL.
const typeTextFull = typeText;
typeText = typeText.replace(arrayPart, '');

@rubys

This comment has been minimized.

Copy link
Member Author

commented Aug 5, 2018

I know what's going on. I'll try to see if i can create another patch for mdast-util-to-hast.

@vsemozhetbyt vsemozhetbyt referenced this pull request Aug 7, 2018
1 of 4 tasks complete
rubys added a commit to rubys/node that referenced this pull request Aug 7, 2018
squash: quick fix for < and >
Preferred long term fix can be found at:
nodejs#22140
@rubys

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2018

Three pull requests created to solve (most) of the problems noted above:

The 2-dimensional array appears to be a separate problem in that remark, et. al., appears to be doing the right thing. Once the above fixes land, I'll look into fixing that problem too.

@Trott

This comment has been minimized.

Copy link
Member

commented Aug 10, 2018

Should this update the corresponding package.json file and not just the package-lock.json? I mean, I guess that's not required, but if we want to be sure we are bumping a dependency up to a certain minimum version, that should be reflected in the package.json?

@rubys

This comment has been minimized.

Copy link
Member Author

commented Aug 10, 2018

@Trott will do... once the above three patches are resolved. But first, there are yaks that need shaving: syntax-tree/mdast#23 (comment)

@vsemozhetbyt vsemozhetbyt referenced this pull request Aug 11, 2018
3 of 3 tasks complete
@rubys rubys referenced this pull request Aug 12, 2018
2 of 2 tasks complete
rubys added a commit to rubys/node that referenced this pull request Aug 13, 2018
squash: quick fix for < and >
Preferred long term fix can be found at:
nodejs#22140
rubys added a commit that referenced this pull request Aug 13, 2018
tools: fix header escaping regression
quick fix for #22065

Preferred long term fix can be found at:
#22140

PR-URL: #22084
Fixes: #22065
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
rvagg added a commit that referenced this pull request Aug 15, 2018
tools: fix header escaping regression
quick fix for #22065

Preferred long term fix can be found at:
#22140

PR-URL: #22084
Fixes: #22065
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@jasnell

This comment has been minimized.

Copy link
Member

commented Sep 10, 2018

Ping @rubys ... what's the status on this one?

@trivikr

This comment has been minimized.

Copy link
Member

commented Oct 1, 2018

The PRs listed in #22140 (comment) need a follow-up
Ping @rubys

@jasnell
jasnell approved these changes Oct 1, 2018
@Trott

This comment has been minimized.

Copy link
Member

commented Dec 5, 2018

@rubys Looks like all upstream yaks have been shaved and this can be rebased, run through CI, and hopefully landed?

@Trott Trott force-pushed the rubys:mdast-util-to-hast-302 branch from 8f1c6ab to 6118207 Dec 14, 2018

@Trott

This comment has been minimized.

Copy link
Member

commented Dec 14, 2018

Took the liberty of rebasing. Hope that's OK. Still have to install the upstream dependencies that have been updated and run through CI?

@BridgeAR

This comment has been minimized.

@refack refack force-pushed the rubys:mdast-util-to-hast-302 branch from 6118207 to ade009e Mar 10, 2019

@refack

This comment has been minimized.

tools: update to mdast-util-to-hast v3.0.2
See syntax-tree/mdast-util-to-hast#21

Note: I updated all of the tools/doc dependencies, not just this one,
and removed the previous workaround that was in place until this change
landed.

PR-URL: #22140
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

@refack refack force-pushed the rubys:mdast-util-to-hast-302 branch from ade009e to cba2c7a Mar 10, 2019

@refack refack merged commit cba2c7a into nodejs:master Mar 10, 2019

1 of 2 checks passed

Travis CI - Pull Request Build Errored
Details
Travis CI - Branch Build Passed
Details

@refack refack removed the author ready label Mar 10, 2019

BridgeAR added a commit that referenced this pull request Mar 13, 2019
tools: update to mdast-util-to-hast v3.0.2
See syntax-tree/mdast-util-to-hast#21

Note: I updated all of the tools/doc dependencies, not just this one,
and removed the previous workaround that was in place until this change
landed.

PR-URL: #22140
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BridgeAR added a commit that referenced this pull request Mar 13, 2019
tools: update to mdast-util-to-hast v3.0.2
See syntax-tree/mdast-util-to-hast#21

Note: I updated all of the tools/doc dependencies, not just this one,
and removed the previous workaround that was in place until this change
landed.

PR-URL: #22140
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BridgeAR added a commit that referenced this pull request Mar 14, 2019
tools: update to mdast-util-to-hast v3.0.2
See syntax-tree/mdast-util-to-hast#21

Note: I updated all of the tools/doc dependencies, not just this one,
and removed the previous workaround that was in place until this change
landed.

PR-URL: #22140
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BethGriggs added a commit that referenced this pull request Apr 16, 2019
tools: update to mdast-util-to-hast v3.0.2
See syntax-tree/mdast-util-to-hast#21

Note: I updated all of the tools/doc dependencies, not just this one,
and removed the previous workaround that was in place until this change
landed.

PR-URL: #22140
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BethGriggs BethGriggs referenced this pull request May 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.