-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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: non leading-tabs in markdown content (#1559) #2434
Conversation
Only replaces tabs at the beginning of a block construct. Tabs in the middle of the item are unaffected. All tests passing. Tabs in both GFM and CommonMark at 100% fixes markedjs#1559
This pull request is being automatically deployed with Vercel (learn more). marked-website – ./🔍 Inspect: https://vercel.com/markedjs/marked-website/D3uTfybfhra66Kx3bbv8kgdEqMw5 markedjs – ./🔍 Inspect: https://vercel.com/markedjs-legacy/markedjs/A1UvPUv6aE5erAJmpiDKM5my7xeg |
Does this look good now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this! 💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Only question is if there might be some other token that needs its rules.js
updated to handle tabs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did find one example that seems broken.
> test
// tab between > and test
It is actually currently kind of broken (additional spaces are added to the output) but with this PR it creates a code element in the blockquote.
Looks like https://github.com/markedjs/marked/blob/master/src/Tokenizer.js#L154 might have to be updated to remove a tab after >
as well.
Ok seems like that might be a case that's missing a test. I'll see about putting one together for it. |
@calculuschild I think this now passes any test I can think of. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, I approve then. I can't think of anything else either.
🎉 This PR is included in version 4.0.14 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Marked version:
Markdown flavor: CommonMark
Description
Only replaces tabs at the beginning of a block construct. Tabs in the middle of the item are unaffected.
Contributor
Committer
In most cases, this should be a different person than the contributor.