Make code role highlighting consistent with code-block directive

[jbms]

Feature or Bugfix

• 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

[tk0miya]

-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.

[jbms]

Docutils already contains a :code: role --- it just doesn't highlight correctly in sphinx due to using the wrong CSS class names, as noted in this long-standing bug: #5157 Additionally though, even if you use one of the workarounds to fix that bug, the highlighting is not consistent with the sphinx code-block directive, which is confusing.

This PR makes :code: just work in the way I think users expect --- the same as the sphinx code-block directive.

Per your comment here:
#6916 (comment)

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.

[AA-Turner]

Long term, given that a code role now exists in Docutils, wouldn't it be better to try and remove the custom code from Sphinx? This would mean all reStructuredText users get the same behaviour.

A

[jbms]

@AA-Turner I wasn't actually aware that docutils had a .. code:: directive in addition to a :code: role.

This is my understanding of the current situation in sphinx:

• :code: role and .. code:: directive: from docutils, under default configuration produces highlighting with long class names not included in sphinx themes and therefore appears unstyled in sphinx-generated html. In practice probably rarely used with Sphinx since it does not work by default.
• .. code-block:: directive: from sphinx, produces styled output with short class names with sphinx-modified styling and is correctly styled by sphinx themes. Widely used with Sphinx.
• Literal blocks: From docutils, but modified by sphinx to take into account the .. highlight directive language setting and optionally perform syntax highlighting. Produces short class names with sphinx-modified styling and is correctly styled by sphinx themes. Widely used with sphinx.

It sounds like you are proposing that we eliminate code-block, literal blocks, all of the sphinx code that handles highlighting, and fix docutils to output class names that work with sphinx themes? Or do you have a different proposal for how to fix this?

[tk0miya]

Wow, I did not notice reST has already provided the code role since v0.9. Okay, let's implement it ASAP.
https://docutils.sphinx-users.jp/docutils/docs/ref/rst/roles.html#code

:code: role and .. code:: directive: from docutils, under default configuration produces highlighting with long class names not included in sphinx themes and therefore appears unstyled in sphinx-generated html. In practice probably rarely used with Sphinx since it does not work by default.

Could you let me know an example of code directive, please? I consider it's supported.

[tk0miya]

I perfectly understand now. The code role is mainly used to define a custom role via role directive. So far I know the technique, but I did not recognize it's based on the role.

[tk0miya]

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?

[jbms]

Thanks for your feedback -- I have addressed your comments and also updated the documentation.

[jbms]

I changed this to use SphinxRole, but then changed it back because I realized SphinxRole does not work correctly with options.

The problem is that you need to define the option specs via the options attribute, but the __call__ method then sets self.options to the actual option values, not the option specs. In general the SphinxRole design seems kind of odd, in that you have a single object representing the role that gets modified each time it is used. Normally it nonetheless works but for options we run into a problem. It would be better to have a design where a fresh object is created for each use of the role. However, we could alternatively fix it by using a different attribute for the option values, e.g. option_values.

[jbms]

@tk0miya Is there anything left to do on this?

[AA-Turner]

I'll review this properly over the next week or so.

A

[tk0miya]

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.

[tk0miya]

Just merged. Thank you for your great work!

[jbms]

Thanks. Looks like this still needs to be merged into 5.x since my pull request targeted 4.x.

[tk0miya]

Thanks! I merged this into 5.x branch manually now.