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
Make code role highlighting consistent with code-block directive #10251
Conversation
ab381f9
to
748c346
Compare
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.
-1: It's better to propose adding a new role to the docutils project first. No reason to add a new Sphinx-originated custom role.
Docutils already contains a This PR makes Per your comment here: You seemed to be supportive to fixing this problem in Sphinx itself. You had been waiting on your pygments PR to add a nowrap option to the latex highlighter. However, that PR seems to have stalled, and I was able to implement a fix for latex without needing that option. |
Long term, given that a A |
@AA-Turner I wasn't actually aware that docutils had a This is my understanding of the current situation in sphinx:
It sounds like you are proposing that we eliminate |
Wow, I did not notice reST has already provided the
Could you let me know an example of |
I perfectly understand now. The |
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.
LGTM with nits. But merging HTML4 and HTML5 writers is another topic. So it should be separated from this PR. Could you add the change to each writers, please?
748c346
to
90522ac
Compare
Thanks for your feedback -- I have addressed your comments and also updated the documentation. |
90522ac
to
96fa3ae
Compare
Fixes sphinx-doc#5157 This is factored out of the sphinx-immaterial theme: https://github.com/jbms/sphinx-immaterial/blob/1ef121a612d4f5afc2a9ca9c4e3f20fca89065e8/sphinx_immaterial/inlinesyntaxhighlight.py#L1 See also: sphinx-doc#6916
96fa3ae
to
099b54c
Compare
I changed this to use The problem is that you need to define the option specs via the |
@tk0miya Is there anything left to do on this? |
I'll review this properly over the next week or so. A |
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.
Sorry for the late response. LGTM.
I'll take a look at the design problem of SphinxRole in another time. Thank you for the info.
Just merged. Thank you for your great work! |
Thanks. Looks like this still needs to be merged into 5.x since my pull request targeted 4.x. |
Thanks! I merged this into 5.x branch manually now. |
…inline code Related: sphinx-doc#10251
Feature or Bugfix
Purpose
Fixes #5157
This is factored out of the sphinx-immaterial theme:
https://github.com/jbms/sphinx-immaterial/blob/1ef121a612d4f5afc2a9ca9c4e3f20fca89065e8/sphinx_immaterial/inlinesyntaxhighlight.py#L1
See also: #6916