Skip to content
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

WIP: Add inline syntax highlighting using docutils :code: role #6916

Closed
wants to merge 3 commits into from

Conversation

jessetan
Copy link
Contributor

@jessetan jessetan commented Dec 12, 2019

Subject: support inline syntax highlighting in HTML and LaTeX without extensions

Feature or Bugfix

  • Feature

Purpose

This PR adds the ability to do syntax highlighting of inline text using the docutils :code: role.

Changes:

  • Change default of docutils setting that controls the type of Pygments classes that are added (long names or short). Anyone adding custom CSS
  • Add a CSS class to enable highlighting
  • Changes LaTeX output to use \PYG for inline code

Things this PR does not do (yet):

  • Passing inline code to PygmentsBridge for adding classes. Classes are added by docutils instead. This can cause a small difference in what classes are added, but I could not find a nice way to do this.
  • A toggle switch to enable/disable this. It is now always on for :code:
  • Verification with a wide array of input
  • Documentation (will need to be added if approach is OK)
  • Automated tests (will need to be added if approach is OK)

Relates

@codecov
Copy link

codecov bot commented Dec 12, 2019

Codecov Report

Merging #6916 into master will decrease coverage by 0.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6916      +/-   ##
==========================================
- Coverage   84.59%   84.58%   -0.02%     
==========================================
  Files         266      266              
  Lines       41250    41263      +13     
  Branches     5963     5967       +4     
==========================================
+ Hits        34897    34903       +6     
- Misses       5034     5038       +4     
- Partials     1319     1322       +3
Impacted Files Coverage Δ
sphinx/environment/__init__.py 86.56% <ø> (ø) ⬆️
sphinx/writers/html.py 84.87% <100%> (-0.29%) ⬇️
sphinx/writers/html5.py 90.92% <100%> (+0.05%) ⬆️
sphinx/writers/latex.py 83.9% <40%> (-0.27%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ec567b...ccc069f. Read the comment docs.

Copy link
Member

@tk0miya tk0miya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love this implementation. Let's go with this! I'm waiting for update :-)

sphinx/environment/__init__.py Show resolved Hide resolved
sphinx/writers/latex.py Outdated Show resolved Hide resolved
@tk0miya
Copy link
Member

tk0miya commented Dec 15, 2019

Passing inline code to PygmentsBridge for adding classes. Classes are added by docutils instead. This can cause a small difference in what classes are added, but I could not find a nice way to do this.

I'll also check this. It would be nice if both are same.

A toggle switch to enable/disable this. It is now always on for :code:

AFAIK, there are no way to disable it for code-blocks. So I feel it is not needed.

@jessetan
Copy link
Contributor Author

A toggle switch to enable/disable this. It is now always on for :code:

AFAIK, there are no way to disable it for code-blocks. So I feel it is not needed.

You are correct that there is no option to disable it per :code: role. I meant an entry in conf.py to disable it document-wide.

@tk0miya
Copy link
Member

tk0miya commented Dec 15, 2019

What I meant is that we also don't have such option for code-blocks document-wide. I consider it is not requested.

Copy link
Contributor Author

@jessetan jessetan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline syntax highlighting extension actually does use the Pygments bridge, and then applies a regex to remove the unnecessary <div> that Pygments wraps it in.
Is that any better than using the lexed code from docutils?

@tk0miya
Copy link
Member

tk0miya commented Dec 17, 2019

Oh, I don't know that extension. It seems better to me in HTML case. It seems HtmlFormatter has "nowrap" option not to generate div and pre tag. It would be nice for this case.
https://pygments.org/docs/formatters/#HtmlFormatter

But, on the other hand, there are no such option for LaTeXFormatter... So it is hard to use this implementation to sphinx-core.

@tk0miya
Copy link
Member

tk0miya commented Dec 21, 2019

I posted a PR for adding nowrap option to LatexFormatter to pygments. It will help us if merged.
pygments/pygments#1343

@jessetan
Copy link
Contributor Author

Thanks @tk0miya, I will change the implementation to use PygmentsBridge

@jessetan jessetan changed the title Add inline syntax highlighting using docutils :code: role WIP: Add inline syntax highlighting using docutils :code: role Dec 23, 2019
@tk0miya tk0miya added this to the 4.0.0 milestone Jul 23, 2020
@tk0miya tk0miya modified the milestones: 4.0.0, 4.1.0 Mar 16, 2021
@tk0miya tk0miya modified the milestones: 4.1.0, 4.2.0 Jul 10, 2021
@tk0miya tk0miya modified the milestones: 4.2.0, 4.3.0 Sep 12, 2021
@tk0miya tk0miya modified the milestones: 4.3.0, 4.4.0 Nov 8, 2021
@tk0miya tk0miya modified the milestones: 4.4.0, 4.5.0 Jan 15, 2022
@tk0miya tk0miya modified the milestones: 4.5.0, 5.0.0 Mar 27, 2022
@tk0miya
Copy link
Member

tk0miya commented May 5, 2022

I adopted #10251 now instead of this. Thank you for your work.

@tk0miya tk0miya closed this May 5, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inline code is not syntax highlighted
2 participants