-
Notifications
You must be signed in to change notification settings - Fork 567
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
Update to Mistune 2.0.2 #1764
Update to Mistune 2.0.2 #1764
Conversation
Thanks for working on this @TiagodePAlves! I think what you've done so far is a great approach to keeping backwards compatibility. I'd say if we get the existing test suite to pass, we should merge this PR. Pinning isn't a long term solution, and vendoring |
Ok, I think the regexes may be the problem. I'll push it as soon as I fix it. |
No rush, thanks again for taking this up! |
I rewrote the regexes and they were passing the |
All green in CI! @bollwyvl @SylvainCorlay I propose we release a 7.0rc0 with this change. |
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.
Changes LGTM!
- 'AXT_HEADING' is now requiring whitespace after the '#', fixed with a new regex - 're.Scanner' is not able to extract the correct group, the text needs trimming before use - 'javascript:...' links considered harmful by 'HTMLRenderer', should we disable it too?
Important to avoid problems with the TeX-style block math where the trailing '$$' is not removed.
Thanks a lot for this! |
Mistune 2.0 was major overhaul, requiring a lot of changes on the
markdown2html_mistune
filter. I understand that the update may noy be desirable (see #1685), but I'm leaving this here as an option. Still, some more testing seems needed.Possible problems
BlockParser
doesn't consider#TITLE
without space a valid header. I've fixed it with a customAXT_HEADING
regex, but there may be other problems hidden in those regexes.re.Scanner
, but I couldn't make it remove the$$
suffix in block math and had to strip the text manually. Can we trustre.Scanner
? It seems to have other unexpected shortcomings too.javascript:...
links harmful and removes them. I've bypassed that, but maybe it should be adopted too.