-
-
Notifications
You must be signed in to change notification settings - Fork 44
Wrap footnote definition in a paragraph
#15
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
Hey hey! Sorry about the long wait!
This looks good for text elements, but I’m wondering about a single strong, or other non-block element. What would happen there?
In the reverse program, we have something similar:
Do you think that’s useful to add here too?
I'm pretty sure that would be better, yep.
Since I'll need the list inline nodes in here, it's not ideal to maintain IMO.
A few ideas/solutions:
- creating a
mdastpackage, only exporting nodes infos so that we could e.g. get a list of all inline nodes, or - exporting this from
remark, or - attaching a type to each node besides the node type, e.g.
{type: 'emphasis', element: 'inline'}
Ahh, sorry, late!
I think a version of 1 makes the most sense, but in a separate utility? I think mdast should be used, if ever, for something like unist-builder but for mdast?
However, I think it makes sense to first inline it here, for now, before starting work on another utility?
Or, if it’s the same list as in hast-util-to-mdast, we could create a small utility and depend on it in both these projects?
@vhf Ping! P.S. I know, I know, I’m late all the time 😒
@wooorm I'm also constantly late, no worries. :)
It's the same list as hast-util-to-mdast. What would be a good package name?
HTML doesn’t really have "block" or "inline", as that’s related to CSS, but it does have "phrasing" content (warning: the HTML spec is huge), which contains <a>, <br>, <code>, <em>, <img>, <s>, <strong>, and Text.
So how about mdast-util-phrasing?
@wooorm Here's my suggestion: https://github.com/syntax-tree/mdast-util-phrasing
If you would have preferred I created this repo outside of the org then moved it there upon your approval please say so! :)
No this is perfect! And what the organisation is for! Thank you!!! 🎉
So what are the next steps?
Next step would be to bump the mdast-util-phrasing to 1.0.0 and publish it to NPM, then I'd change this PR to leverage it and also I could also PR hast-util-to-mdast to remove the array checked in here.
Sweet, sounds good! Could you add me on npm when you publish?
Would you like me to do something?
I will. No need, I can handle this. I'll assign the PR reviews to you. :)
All done.
- 1.0.0 released
- gave you access to the NPM package
- This PR is good to go, please merge or request modifications
- hast-util-to-mdast PR is up for review
Now that I look at this again, is there actually a need for checking if there’s one node, and it’s phrasing? If footnote is always inline, why not always wrap it in a paragraph?
I think you're right, footnote is always inline. (I deleted a comment I had just posted, I got confused with footnote and footnoreReference.)
Right?! That’s what I was thinking as well! Oh, I’m so sorry for leading you astray with my first comment #15 (comment)...
In that case, should we just wrap this in a <p>, always?
No big deal! Here it is again. 😄
Sweet!
Released, plus everything I can access that depends on it!
Since : - syntax-tree/mdast-util-to-hast#32 - khttps://github.com/syntax-tree/mdast-util-to-hast/pull/15
Here is a fix for #14
Please tell me what you think @wooorm !