-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
refactor: Theme-agnostic code highlighting, respecting configs #202
Conversation
6862725
to
f84413d
Compare
Infer the highlighter configs from what the current Markdown config is, trying to mimick the two most typical extensions and how they'd highlight a code block in Markdown normally. This improves theme support, as previously only some assumptions based on each theme's defaults were supported, e.g. the readthedocs theme is assumed to rely only on JS highlighting, when in reality you just need to add a Markdown syntax highlight extension and you can use Pygments. Another improvement stemming from this is support for disabling syntax highlighting altogether. (`use_pygments: false`). For that I also had to edit the template for function signature, to avoid all newlines in it, because previously inline highlighting relied on processing of the syntax highlighter to get rid of those.
Note that I intentionally ignore changes from I looked through top repos to find whether anyone relied on this "class highlight", and found these:
Why the change: If the site uses the |
OK, LGTM. The change could be surprising in some repos indeed, but I feel it's a more correct approach, and if a change is needed on the user side it should be fairly small, like changing |
Good to hear! (probably don't merge yet) |
I have no other comment on whether the change is breaking. Don't know your criteria. |
Infer the highlighter configs from what the current Markdown config is, trying to mimick the two most typical extensions and how they'd highlight a code block in Markdown normally.
This improves theme support, as previously only some assumptions based on each theme's defaults were supported, e.g. the readthedocs theme is assumed to rely only on JS highlighting, when in reality you just need to add a Markdown syntax highlight extension and you can use Pygments.
I had commented on this here: #159 (comment)
Another improvement stemming from this is support for disabling syntax highlighting altogether. (
use_pygments: false
). For that I also had to edit the template for function signature, to avoid all newlines in it, because previously inline highlighting relied on processing of the syntax highlighter to get rid of those.Note that, according to the basic logic of this change (inferring highlight config from Markdown config), if there is no highlight extension selected, this one should disable highlighting as well. But I decided to enable highlighting by defauly, unless
use_pygments: False
is specified, because even mkdocstrings repo itself doesn't explicitly specify a highlighter, and would "regress".And personal note: I don't really need this to make it into the upcoming release, would prefer it to not block the release.