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

Part extraction bugs #1050

Closed
rowanc1 opened this issue Apr 3, 2024 · 4 comments
Closed

Part extraction bugs #1050

rowanc1 opened this issue Apr 3, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@rowanc1
Copy link
Member

rowanc1 commented Apr 3, 2024

There is a duplication bug:

# testing

+++ {"part": "abstract"}

this is duplicated

image

# testing

## Abstract

this is duplicated

image

And then an error on a blank block (in the theme in myst-common):

# testing

+++ {"part": "abstract"}

this is duplicated

+++

image

@fwkoch
Copy link
Member

fwkoch commented Apr 4, 2024

Ok - the first couple examples are happening when there is nothing else in the file except the abstract. In these cases, when extractParts called remove, it was removing the entire tree and therefore failing. That's easy to fix - just add a cascade: false failsafe if the entire tree is removed. (We already do this exact thing here: https://github.com/executablebooks/mystmd/blob/main/packages/myst-transforms/src/frontmatter.ts#L84-L88)

For the last case, we should definitely add a ?: tree.children?.forEach. But also, blocks should always have children defined... There's a line here where we explicitly remove empty children: https://github.com/executablebooks/mystmd/blob/main/packages/myst-parser/src/tokensToMyst.ts#L577 - I think leaving the empty array is better?

I made these changes here: #1055

For this to fully take effect, myst-theme will need to consume the changes to myst-common

@rowanc1
Copy link
Member Author

rowanc1 commented Apr 4, 2024

Thanks for the PRs, leaving this open til we bring it over to the theme.

@fwkoch
Copy link
Member

fwkoch commented Apr 5, 2024

Version bumps that bring this into the theme are included with this pr: executablebooks/myst-theme#351

@fwkoch
Copy link
Member

fwkoch commented Apr 5, 2024

Fixed in the latest release of themes. You may need to myst clean --templates!

@fwkoch fwkoch closed this as completed Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants