-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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, tools: make type parsing more strict #19881
Conversation
* `args` {void\*} A pointer to pass to the callback at exit. | ||
* `callback` <span class="type"><void (\*)(void\*)></span> | ||
A pointer to the function to call at exit. | ||
* `args` <span class="type"><void\*></span> |
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.
were these changed so they don't trigger the type parser?
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.
Yes. Currently, tools/doc/html.js
type-process any non-code text in any section, so it cannot define if this is a js type or something else. But the author of this section wanted the C types to be consistent in CSS style with JS types, so I've tried to conform.
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, just a nit.
tools/doc/type-parser.js
Outdated
typeLinks.push(`<span class="type"><${typeTextFull}></span>`); | ||
throw new Error( | ||
`Unrecognized type: '${typeTextFull | ||
}'. Please, edit the type or update the 'tools/doc/type-parser.js'.` |
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.
Please concat the string instead of using a multi line template string.
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.
Done in the fixup commit.
Rebased after the 0bd3da1. |
Rebased. |
PR-URL: #19881 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Landed in b3bff41 |
PR-URL: #19881 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: nodejs#19881 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesWe do many small correction PRs for doc types format. This change makes type parsing more strict to avoid these post-corrections. Wrong or undefined types would throw on doc build stage.
Main code changes:
any
type linkable (to this section).New parsing mode had detected some issues which have been fixed.
{object}
here.{Cipher}
here.Also, some style nits in
tools/doc/type-parser.js
were fixed.