🏷 Propagate block metatags to code node and output node #407
Conversation
There was a problem hiding this comment.
Thanks for the PR, this is looking great! I added a few minor comments about searching through the MDAST and handling a few other cases of the markdown cells, blocks without output (or perhaps code, but that would be weird).
I also think we want to leave tags on the block that we haven't processed here.
| * Traverse mdast, propagate block tags to code and output | ||
| */ | ||
| export function propagateBlockDataToCode(session: ISession, filename: string, mdast: Root) { | ||
| const blocks = selectAll('block', mdast) as GenericNode[]; |
There was a problem hiding this comment.
I think it is possible that this might grab too many blocks (e.g. the markdown ones, which aren't going to have the code/output). I think we put a kind or something on the block, not sure where that is at the moment!
There was a problem hiding this comment.
If this is changed, it's reasonable to change the liftCodeMetadataToBlock with the same way. This worth a separate pr to do since two tests will be added and their functionality do not have direct link to the meta tag implementation.
There was a problem hiding this comment.
That sounds good -- let me know when you are a bit further on the feature and I can go in and test against a few other projects!
|
For |
|
For This will guide you through the process of noting which libraries need to be patched/released! |
Co-authored-by: Rowan Cockett <[email protected]>
|
From a functionality point of view do you think the feature is now complete? Core-review wise it looks solid! |
|
For |
|
Perfect -- I will try to get to this today! |
|
Hi @rowanc1, if you are planning to merge this in and do a patch release today, I will have some spare time to finish the rest of tasks in myst-theme at this weekend. Not rushing, just want to know is there any blocker. Thanks 😊 |
|
Sure! I think this will be great. Take your time! |
|
Didn't quite get to actually testing, I hope I can get to it this eve -- tests are now passing again though! :) |
| blocks.forEach((block) => { | ||
| if (!block.data || !block.data.tags) return; | ||
| if (!Array.isArray(block.data.tags)) { | ||
| session.log.error(`tags in code-cell directive must be a list of strings`); |
There was a problem hiding this comment.
I am tweaking this a bit to pass in the node and file information for better console tracking:
|
This looks great! I am going to merge and release once the tests go green. We should add some docs after we get the frontend working! Thanks again for your help on this feature, another check on #189. ✅ 🚀 |
rowanc1
changed the title
|
This is released as |
This is the first part of implementation for jupyter meta tags support, as discussed in here.
After merging this, there are at least 4 things on the list:
For this pr, there are still 2 questions, which are mentioned in the comments:
hide-xxandremove-xxappear at the same time?