-
-
Notifications
You must be signed in to change notification settings - Fork 16
Add support for inferring phrasing of embedded hast #74
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
Conversation
This comment has been minimized.
This comment has been minimized.
lib/util/wrap.js
Outdated
|
||
/** | ||
* @param {MdastNode} node | ||
* @returns {boolean} |
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.
You could make this a type guard btw, something along the lines of:
* @returns {node is MdastPhrasingContent}
should do it I believe? That means the cast later would no longer be needed.
This is a nice to have, not required. If it doesn’t work (well), the changes in this PR look good as is!
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.
My JSDoc-fu is inexistent so I just implemented your change.
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.
It’s more a TS thing, although written inside JSDoc, the grammar is TS!
Co-authored-by: Titus <tituswormer@gmail.com>
Co-authored-by: Titus <tituswormer@gmail.com>
@@ -80,7 +96,7 @@ function runs(nodes, onphrasing, onnonphrasing) { | |||
|
|||
if (phrasing(node)) { | |||
if (!queue) queue = [] | |||
queue.push(node) | |||
queue.push(/** @type {MdastPhrasingContent} */ (node)) |
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 believe this should not be needed anymore. I’ll check myself.
@@ -45,6 +45,7 @@ | |||
"hast-util-is-element": "^2.0.0", | |||
"hast-util-to-text": "^3.0.0", | |||
"mdast-util-phrasing": "^3.0.0", | |||
"hast-util-phrasing": "^2.0.1", |
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.
In unified, we prefer to keep loose ranges, if possible, so 2.0.0
. It helps resolving packages by making the ranges loose. And it keeps package lock diffs cleaner.
I’ll fix this myself here, but just an FYI for the future!
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.
Ok, thanks. I'm learning as I go along with npm, package.json as well as git.
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.
no problem, I’ll try to keep that mind, and do let me know when there’s something you’re wondering about in all this!
This comment has been minimized.
This comment has been minimized.
Thanks, released! |
Initial checklist
Description of changes
Expand check if mdast node is phrasing content to also check node.data.tagName agains hast phrasing. This allows correct classification in conversion of hast nodes into custom mdast nodes.
Notes