Skip to content

markup/rst: Make syntax highlighting configurable#14841

Open
xndvaz wants to merge 2 commits into
gohugoio:masterfrom
xndvaz:fix/rst-short-syntax-highlight-5349
Open

markup/rst: Make syntax highlighting configurable#14841
xndvaz wants to merge 2 commits into
gohugoio:masterfrom
xndvaz:fix/rst-short-syntax-highlight-5349

Conversation

@xndvaz
Copy link
Copy Markdown
Contributor

@xndvaz xndvaz commented May 4, 2026

Fixes #5349
Supersedes #14659

Summary

  • add markup.rst.syntaxHighlight with Docutils' CLI default (long)
  • pass --syntax-highlight only when configured as short or none
  • document the RST markup configuration and default
  • cover the real rst2html render path without requiring Pygments

Notes

  • Default behavior remains Docutils' long syntax classes.
  • The integration test allow-list now matches the command Hugo actually executes: rst2html/rst2html.py on Unix-like systems, python/python.exe on Windows.

Tests

  • go test ./hugolib -run TestRSTSyntaxHighlightConfigIssue5349 -count=1
  • ./check.sh ./markup/rst/...
  • ./check.sh ./markup/markup_config/...
  • ./check.sh ./hugolib/...

AI assistance disclosure:

  • Developed with AI assistance; I reviewed the final diff, scope, and validation before submission.

@gemini-code-assist
Copy link
Copy Markdown

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

@Qodo-Free-For-OSS
Copy link
Copy Markdown

Hi, The new integration test uses security.exec.allow = ['rst2html'], but allow-list entries are regex patterns; an unanchored pattern can match unintended command names containing rst2html.

Severity: remediation recommended | Category: security

How to fix: Anchor allow-list pattern

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

The test config’s security.exec.allow = ['rst2html'] is interpreted as a regexp, which can match more than the intended binary name.

Issue Context

Whitelist.Accept compiles the patterns as regexps and uses MatchString(name).

Fix Focus Areas

  • hugolib/rst_integration_test.go[51-57]

Fix outline

  • Change allow = ['rst2html'] to allow = ['^rst2html$'] (or '^rst2html(\\.py)?$' if you want to allow both explicitly).

We noticed a couple of other issues in this PR as well - happy to share if helpful.


Found by Qodo code review

@xndvaz
Copy link
Copy Markdown
Contributor Author

xndvaz commented May 6, 2026

Addressed in 22984f4: updated test config to use anchored allow-list regex (^rst2html$).

Comment thread hugolib/rst_integration_test.go Outdated
@xndvaz
Copy link
Copy Markdown
Contributor Author

xndvaz commented May 10, 2026

Agreed, thanks @bep. The previous test used a fake rst2html only to verify the added flag, but I agree that this is better covered with the real executable.

Updated the test to use the real rst2html path installed in CI and added Pygments to the CI setup so the syntax-highlight output is actually exercised.

@xndvaz
Copy link
Copy Markdown
Contributor Author

xndvaz commented May 10, 2026

Follow-up CI note: the current failure is unrelated to this RST PR.

It is the existing TestNPMGlobalInstalls failure already present on master, caused by an exact Tailwind CSS byte-size assertion changing from 4557 to 4479. I opened #14860 separately to fix that CI fragility, and its checks are green.

Keeping this PR scoped to the RST change.

Comment thread .github/workflows/test.yml Outdated
- name: Install docutils
run: |
pip install docutils
pip install docutils Pygments
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I will have to get back to this later, but if this patch requires pygments (I suspect it's lowercase?), I think we need to reconsider doing this as the new default (as many may not want it) ... We could add a config struct for rst similar to what we have for the other markup engines with a sensible option with a comment that you would need to install pygments for it to work. I'm not totally sure about this, though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, thanks.

I changed this to avoid making --syntax-highlight=short the new default. The PR now adds an explicit markup.rst.syntaxHighlight setting instead, with Docutils' default long preserved.

The CI setup also no longer installs pygments; the integration test uses syntaxHighlight = "none" so it verifies the Hugo/rst2html configuration path without adding a new CI dependency. The docs mention that Docutils needs Pygments available when users want highlighted code blocks.

So the short class names are now opt-in via:

[markup.rst]
syntaxHighlight = "short"

Copy link
Copy Markdown
Member

@jmooring jmooring May 14, 2026

Choose a reason for hiding this comment

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

@bep Highlighted code classes require the Pygments library. This is an existing requirement (not related to this PR) if you want to perform syntax highlighting, but since pip install docutils doesn’t include it by default (unlike the python3-docutils apt package), our CI currently lacks it. Without Pygments in CI, we can't run affirmative integration tests for this PR.

Comment thread hugolib/rst_integration_test.go Outdated
Add markup.rst.syntaxHighlight and pass Docutils' --syntax-highlight option only when configured away from the rst2html default.

Fixes gohugoio#5349
@xndvaz xndvaz force-pushed the fix/rst-short-syntax-highlight-5349 branch from 6e9de20 to d0ff51b Compare May 15, 2026 04:05
@xndvaz xndvaz changed the title markup/rst: use short syntax highlighting markup/rst: Make syntax highlighting configurable May 15, 2026
@jmooring
Copy link
Copy Markdown
Member

Although the configuration key syntaxHighlight matches the command line flag, that name doesn't tell me what it does when looking at a Hugo config file. To be consistent with other highlight options, I suggest this instead:

[markup.rst.highlight]
classNaming = 'long'

With this description:

The naming convention for CSS classes generated during syntax highlighting, one of long, short, or none. Default is long.

@xndvaz
Copy link
Copy Markdown
Contributor Author

xndvaz commented May 21, 2026

@bep were you able to verify this PR? Do you think it needs any adjustments?

@jmooring
Copy link
Copy Markdown
Member

See my previous comments. This should not be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

reStructuredText uses long token names for syntax highlighting

4 participants