-
Notifications
You must be signed in to change notification settings - Fork 197
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
👌 Improve default slug generation for heading anchors #753
Conversation
Thanks for submitting your first pull request! You are awesome! 🤗 |
Thanks @Cimbali! I'll have a proper look probably over the weekend |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #753 +/- ##
==========================================
+ Coverage 90.23% 90.26% +0.03%
==========================================
Files 23 23
Lines 2970 2970
==========================================
+ Hits 2680 2681 +1
+ Misses 290 289 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Hi @chrisjsewell have you been able to have a look? It’s quite simply keeping the trailing |
Ok cool I've checked this and seems good thanks 👌 My main concern was that removing the
So e.g. |
This can be easily tested by visiting the source markdown page on github:
https://github.com/Cimbali/MyST-Parser/blob/master/tests/test_sphinx/sourcedirs/references/index.md#image-in-title-
Fixes #752.
Note that for the
nested $a=1$
case I’ve updated the test to match the new behaviour, to allow improving matching github behaviour bit by bit.Neither the old or new test match Github behaviour:
https://github.com/executablebooks/MyST-Parser/blob/master/tests/test_sphinx/sourcedirs/references/index.md#title-with-nested-a1
The issue seems to be math-to-text conversion (so the trailing
-
added by this PR is “more correct” even though not 100% there).