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

💥 Role / Directive Refactor #184

Merged
merged 48 commits into from
Feb 16, 2023
Merged

💥 Role / Directive Refactor #184

merged 48 commits into from
Feb 16, 2023

Conversation

fwkoch
Copy link
Collaborator

@fwkoch fwkoch commented Feb 6, 2023

markdown-it-myst is a new package to generically parse roles/directives into tokens. It does a nested parse to tokenize args/options/content as well, passing along both the raw role/directive content and the tokenized content.

This package does not define custom role/directive types; those are defined downstream as AST transforms on content made available here.

This package is the first step in addressing #181


Update

This PR has consolidated the work on markdown-it-myst with myst-parser #193 and myst-roles/directives #206 - it addresses all the refactoring described in #181

@fwkoch fwkoch changed the title 📦 New markdown-it-myst package 💥 Role / Directive Refactor Feb 15, 2023
@rowanc1
Copy link
Member

rowanc1 commented Feb 16, 2023

I think that the only bugs that are left are around the card headers.

image

It would be great to have tests on the remaining directives, but also want to get this in and out the door! I think there is another pass on validation we need to do, but that can come in next, I don't think we actually loose that much?

@rowanc1
Copy link
Member

rowanc1 commented Feb 16, 2023

Some known things:

  • Alignment of images is a regression.
  • And better validation of width/height.
  • figure classes did a bit more on pulling off 'center' and making that the align key.

Anything else?

I think those can easily come in a validation pass.

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 this pull request may close these issues.

2 participants