tools: fix man pages linking regex #17724
Conversation
The change to word boundary was breaking many doc pages. This reverts the word boundary back to space. Fixes: nodejs#17637 Fixes: nodejs#17694 Refs: nodejs#17479
nodejs-github-bot
added
doc
This add the space needed to match the man pages linking regex. Refs: nodejs#17724
|
Maybe replace word boundary |
|
@Trott's suggestion sounds good. In addition to that we should replace the space with the value of the first capturing group |
|
@lpinca Indeed it would be needed, else a space would be removed. |
The change to word boundary was breaking many doc pages. This replace the word boundary with a matching group of space or beginning of line. Fixes: nodejs#17637 Fixes: nodejs#17694 Refs: nodejs#17479
tools/doc/html.js
Outdated
| if (BSD_ONLY_SYSCALLS.has(name)) { | ||
| return ` <a href="https://www.freebsd.org/cgi/man.cgi?query=${name}` + | ||
| return `<a href="https://www.freebsd.org/cgi/man.cgi?query=${name}` + |
There was a problem hiding this comment.
I think beginning should be used here, replacing the space.
There was a problem hiding this comment.
@lpinca Same applies to line 427 too? Or am I mistaken about that?
This moves the beginning regex matching group to the beginning of the resulting HTML man pages link
|
Landing for now as fixes main generation issue. Just noting for posterity that the |
|
Landed in 66e6aff |
The change to word boundary was breaking many doc pages. This reverts the word boundary back to space. PR-URL: nodejs#17724 Fixes: nodejs#17694 Refs: nodejs#17479 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
|
@maclover7 Hmm. The links referenced in #17637 seem to not match the new RegExp, so they should not be erroneously double-linkified now if I do not miss something. |
|
@vsemozhetbyt I think our comments crossed -- see #17637 (comment) |
The change to word boundary was breaking many doc pages. This reverts the word boundary back to space. PR-URL: #17724 Fixes: #17694 Refs: #17479 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
The change to word boundary was breaking many doc pages. This reverts the word boundary back to space. PR-URL: #17724 Fixes: #17694 Refs: #17479 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
The change to word boundary was breaking many doc pages. This reverts the word boundary back to space. PR-URL: #17724 Fixes: #17694 Refs: #17479 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
|
cc @nodejs/lts: Can this be backported to v8 branch? |
The change to word boundary was breaking many doc pages. This reverts the word boundary back to space. PR-URL: #17724 Fixes: #17694 Refs: #17479 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
Done. I assume it doesn't need to go back to the v6.x branch. |
|
@gibfahn Yes, it seems v6.x docs do not have this issue. |
The change to word boundary was breaking many doc pages. This reverts the word boundary back to space. PR-URL: #17724 Fixes: #17694 Refs: #17479 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
The change to word boundary was breaking many doc pages. This reverts the word boundary back to space.
Fixes: #17637
Fixes: #17694
Refs: #17479
Affected core subsystem(s)
tools, doc