Skip to content
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

Remove output directive #1354

Closed
fwkoch opened this issue Jun 24, 2024 · 2 comments · Fixed by #1355
Closed

Remove output directive #1354

fwkoch opened this issue Jun 24, 2024 · 2 comments · Fixed by #1355
Assignees

Comments

@fwkoch
Copy link
Member

fwkoch commented Jun 24, 2024

Previously, when ingesting jupyter notebooks for myst, we created an in-memory markdown "file." For cell outputs, the output data was moved to a cache, keyed off arbitrary id, then we used an output directive in the markdown "file" with that id to inject the output back into the document... Now we just go straight from notebook to mdast; much more straight-forward and understandable, I think.

However, the little output directive with only an id is still lingering in our myst-directives (and the docs!). This directive does not have any user-facing functionality and now has no internal functionality either. I think we should just remove it.

@agoose77 agoose77 self-assigned this Jun 24, 2024
@stevejpurves
Copy link
Member

stevejpurves commented Jun 24, 2024

@fwkoch is there anything downstream that might be using this? a unist select() one of the themes for example?

https://github.com/executablebooks/myst-theme/blob/ec6cd2b8bf0cb53292e99bf3d2bcb212236fc876/packages/jupyter/src/execute/provider.tsx#L90

or do you mean to remove the directive but maintain the node type?

@agoose77
Copy link
Collaborator

@stevejpurves your example is the output AST node which we do use, it's just the directive that's redundant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants