-
Notifications
You must be signed in to change notification settings - Fork 30k
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, docs: fix stability index isssues #20731
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but I suggest to use a shorter version.
tools/doc/html.js
Outdated
'<a href="documentation.html#documentation_stability_index">' + | ||
`${prefix} ${number}</a>${explication}</div>` | ||
.replace(/\n/g, ' '); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to change this to something like:
const link = filename === 'documentation' &&
heading && heading.text === 'Stability Index' ?
`${prefix} ${number}` :
'<a href="documentation.html#documentation_stability_index">' +
`${prefix} ${number}</a>`;
token.text = `<div class="api_stability api_stability_${number}">` +
`${link}${explication}</div>`
.replace(/\n/g, ' ');
It is just a bit shorter due to having less repetition.
tools/doc/html.js
Outdated
} else { | ||
toc += ' '.repeat((depth - 1) * 2) + | ||
`* <a href="#${id}">${token.text}</a>\n`; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same as above.
1. Do not autolink in doc stability section. 2. Do not add void wrapping elements to docs.
I've reduced diff and repetitions, PTAL. CI-lite: https://ci.nodejs.org/job/node-test-pull-request-lite/711/ |
1. Do not autolink in doc stability section. 2. Do not add void wrapping elements to docs. PR-URL: #20731 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Landed in 2773de5 |
1. Do not autolink in doc stability section. 2. Do not add void wrapping elements to docs. PR-URL: #20731 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesChange 1: do not autolink in doc stability section
Currently, stability index descriptions in
documentation.html
contain links to their very section which is redundant. This commit preventstools/doc/html.js
from adding this links.Screenshots of "before" and "after" states:
Before:
After:
Change 2: do not add void wrapping elements to docs
Currently, all the TOC links are wrapped in
span
elements with class according to stability index of their sections. However, most doc sections have not any stability index, so these spans have useless classstability_undefined
. This commit preventstools/doc/html.js
from wrapping TOC links in useless spans.Screenshots of "before" and "after" states (no visible difference):
Before:
After:
This PR eliminates 4402 useless elements in docs and reduces doc size by ~ 176 KB.