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

✅ Parse tasklists to an AST #322

Merged
merged 2 commits into from
Mar 21, 2023
Merged

✅ Parse tasklists to an AST #322

merged 2 commits into from
Mar 21, 2023

Conversation

rowanc1
Copy link
Collaborator

@rowanc1 rowanc1 commented Mar 21, 2023

This parses tasklists into an AST, previously they were going in as HTML. This adds a checked property to the AST.

image

@rowanc1 rowanc1 requested a review from fwkoch March 21, 2023 18:40
Copy link
Collaborator

@fwkoch fwkoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, definitely better than leaving the HTML. It's a pretty gross path to get here... markdown -> tokens with "task list" classes -> child html for the checkboxes -> regex on that html to determine attributes to lift up to the list item -> delete html and fix up list item text. This is a case where working at the AST level is more complex than if we just modified the parsing logic itself - but that logic isn't very accessible.

Anyway, this works great, and the test cases are there!

@rowanc1
Copy link
Collaborator Author

rowanc1 commented Mar 21, 2023

Yeah, I looked at unified here and .. it is just super easy. :)

I think that the test cases are the most important here for catching things if we change in the future!

@rowanc1 rowanc1 merged commit 45ecdf8 into main Mar 21, 2023
@rowanc1 rowanc1 deleted the feat/checklists branch March 21, 2023 19:39
@rowanc1 rowanc1 mentioned this pull request Mar 22, 2023
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