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

fix: more tweaks to improve remark/rehype compatibility #109

Merged
merged 1 commit into from
Feb 17, 2024

Conversation

deer
Copy link
Contributor

@deer deer commented Feb 16, 2024

This does a few things to improve compatibility with remark/rehype:

  1. Start inserting line breaks at the end of heading tags. Looks like the original implementation simply forgot to include the \n at the end. Then there are some test changes to respond to this.

  2. Allow width on td tags. This is specified in the tables.md fixture, but was being sanitized in the output.

  3. Manually insert rel="noopener noreferrer" into tables.md fixture and output. Marked parses this entire thing with a token.type of html, and so the overridden link function in our renderer does not get invoked here. Rather than try to fix this programmatically, I thought it would be easier to just fix the test.

    The reason why this is necessary is because https://github.com/rehypejs/rehype-raw will convert the raw html into an AST. And then later on https://github.com/rehypejs/rehype-external-links will deal with the external links.

    People probably shouldn't be inserting giant raw anchor tags into their markdown, so I suspect this won't impact people. Additionally, not having this attribute is a slight security concern, so people should be doing it anyway.

  4. Add some basic tests covering anchor tags.

  5. A special hack to change how lists live inside summary tags. There's a more detailed comment in mod.ts and you can see the fixture result changes to not have the extra line (in addition to removing whitespace at the start). Again, this is necessary due to https://github.com/rehypejs/rehype-raw converting raw html into an AST. Perhaps this warrants some extra tests around summary tags?

Copy link
Member

@crowlKats crowlKats left a comment

Choose a reason for hiding this comment

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

LGTM!

@crowlKats crowlKats merged commit 4f1f29e into denoland:main Feb 17, 2024
1 check passed
@deer deer deleted the more_remark_compatibility branch February 17, 2024 06:08
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