-
Notifications
You must be signed in to change notification settings - Fork 876
Always wrap CodeHilite code in <code> tags #862
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
Always wrap CodeHilite code in <code> tags #862
Conversation
Cool! I hadn't noticed that Pygments added this option. This has always bugged me about Pygments' output. Glad to see it added (in other words, I didn't need all that convincing but its now documented for others so your effort is not waisted).
That addresses my one concern. Thank you.
As it turns out, our next planned release is 3.2, so no problem there anyway. Update the docs, including the release notes, and I'll merge this. |
947b4d5
to
d629d6a
Compare
I updated the docs. Let me know if it's what you're looking for. (I also hope the force push was okay... do you prefer to squash on merge instead?) Also, ... As I've been working on this small section of CodeHilite, I have also noticed some gnarly other bits of it. There are options that only apply with/apply without/apply differently depending on Pygments/ I'd like to take some time to refactor it if you have the appetite for that. I'd also like to make Pygments a required package for Python-Markdown to reduce complexity:
Thoughts? |
I squash except for those rare occasions were a PR contains a perfect commit. With your force push, your commit was good. However, I still used squash so I could add a technical note to the commit message. In other words, as long as the final product is good, I'll happy squash it. GitHub makes that easy with their web interface.
This extension is over a decade old, so yeah, it needs some updating/refactoring which I have not had time to work on. Reviewing some of the open issues/PRs (#334, #775, #652, #822 and perhaps others) will provide some insight into the planned direction. Feel free to ask questions or propose solutions in the various discussions in those issues/PRs. Note that the extension serves two functions:
Note that the extension always performs function 1. However, function 2 can be optionally turned off by the user. Any refactor must maintain this division of functions.
As long as CodeHilite is included with Python-Markdown I am not comfortable with this. We are proud of the fact that Python-Markdown has no dependencies (except Setuptools). Many users of Python-Markdown never use or need CodeHilite. Others may use CodeHilite for function 1 (as described above) but never need Pygments. Therefore, we should not require all users of Python-Markdown to have Pygments installed. We have contemplated breaking extensions out into separate packages. If that were to happen at some point in the future, then yes, requiring Pygments as a dependency would make sense. Until then, I'd prefer to leave the requirement as-is. At this time, we are not planning on breaking out the extensions. |
…n-Markdown/markdown#862). Reset CSS for code tags inside pre blocks.
…n-Markdown/markdown#862). Reset CSS for code tags inside pre blocks.
See Python-Markdown/markdown#862 for more information. * docs/reference/markdown/stylesheets/extra.css: Update 'pre' styles to work for 'pre>code'. * docwriter/tomarkdown.py: Add <code> tag to source code blocks.
Goal
Currently, the HTML generated by CodeHilite
<pre>stuff</pre>
if you are using Pygments<pre><code>stuff</code></pre>
tags if you are not using Pygments.See logic here: https://github.com/Python-Markdown/markdown/blob/master/markdown/extensions/codehilite.py#L106
I argue that we should always be using the nested
<code>
tags in both cases. The code in this PR makes the two cases consistent.Background
So, backing up a bit:
<code>
indicates that the enclosed text is computer code. It can be used inline. In other words,this
is literally rendered with a code tag.<pre>
indicates that the enclosed text's whitespace should be preserved. It creates a block.Using them together?:
Example
Note that there probably no visual difference in your browser, but the underlying HTML structure is different. Use your browser to inspect.
Rationale
We should always be wrapping our code blocks with code tags for a few reasons, I think:
<code>
tags.<code>
tags get produced if you are not using Pygments, but otherwise using Pygments omits them. CodeHilite has been doing it this way for at least 9 years. If the community doesn't like this PR, we should at least be consistent in how we output nonetheless and figure out some other solution to give users control.code
tag nested inside apre
tag.<code>
tag wrapping in itsHTMLFormatter
since version its 2.4 version released May 8, 2019, but we have not been utilizing it.<pre>
tag.Breakages
<code>
tags. In my opinion, I think this will affect a low number of people because of the reasons listed in the rationale. Check the test string updates for how this will look.<code>
tags when no Pygments is present was introduced in a minor version, 2.1.0. I'm not saying that we should necessarily push this and break semantic versioning...again... but if there's some lower bar for this kind of change that I don't know about, then we might be ok.Non-Breakages
Pygments since it's first major version release, 1.0 (again, current is 2.4.3), has been supporting arbitrary options being passed to its
HTMLFormatter
. With how this PR is coded, there should be no errors to users using thewrapcode
option of at least that version, but the option will only have effect for people with Pygments >2.4.This PR's Strategy
wrapcode=True
to the PygmentsHTMLFormatter