Skip to content

Conversation

frantic1048
Copy link

Found directive name matching issue in this package when trying to build silverrainz.github.io to validate some subtle changes QaQ

Problem

# fetch repo
git clone --depth 1 https://github.com/SilverRainZ/silverrainz.github.io.git
cd silverrainz.github.io

# dependencies
uv venv
uv pip install -r requirements.txt

# try build <- fail here
uv run make

Error log:

      File "/REDACTED/silverrainz.github.io/.venv/lib/python3.12/site-packages/sphinxnotes/mock/__init__.py", line 49, in run
        raise ValueError('unsupported mock mode')
    ValueError: unsupported mock mode
full log:
Running Sphinx v8.2.3
Deployment: Deployment.Local
loading translations [zh_CN]... done
WARNING: while setting up extension sphinxnotes.snippet: extension 'sphinxnotes.snippet' has no setup() function; is it really a Sphinx extension module?
loading pickled environment... The configuration has changed (10 options: 'any_schemas', 'html_context', 'html_css_files', 'html_domain_indices', 'html_js_files', ...)
done
building [html]: template versionchanges.html has been changed since the previous build, all docs will be rebuilt
building [html]: targets for 270 source files that are out of date
updating environment: 0 added, 85 changed, 0 removed
WARNING: the sphinxnotes.extweb extension does not declare if it is safe for parallel reading, assuming it isn't - please ask the extension author to check and make it explicit
WARNING: doing serial read
dump any domain data to any-objects.json... [100%] term-db1d470


Versions
========

* Platform:         linux; (Linux-6.14.1-arch1-1-x86_64-with-glibc2.41)
* Python version:   3.12.11 (CPython)
* Sphinx version:   8.2.3
* Docutils version: 0.21.2
* Jinja2 version:   3.1.6
* Pygments version: 2.19.2

Last Messages
=============



    dump any domain data to any-objects.json... [100%]
    term-dfcbb9b


    dump any domain data to any-objects.json... [100%]
    term-db1d470

Loaded Extensions
=================

* sphinx.ext.mathjax (8.2.3)
* alabaster (1.0.0)
* sphinxcontrib.applehelp (2.0.0)
* sphinxcontrib.devhelp (2.0.0)
* sphinxcontrib.htmlhelp (2.1.0)
* sphinxcontrib.serializinghtml (2.0.0)
* sphinxcontrib.qthelp (2.0.0)
* sphinxcontrib.email (0.3.6)
* sphinx.ext.githubpages (8.2.3)
* sphinxnotes.strike (1.3)
* sphinxcontrib.plantuml (unknown version)
* sphinxcontrib.asciinema (0.4.2)
* sphinx_copybutton (0.5.2)
* sphinxcontrib.youtube (1.4.1)
* sphinxnotes.extweb (unknown version)
* sphinx_design (0.6.1)
* sphinx_simplepdf (unknown version)
* sphinxnotes.mock (1.0.2)
* sphinx.ext.todo (8.2.3)
* sphinx.ext.extlinks (8.2.3)
* sphinxnotes.any (3.0a7.post1.dev1+g8ca6aef1e)
* ablog (0.11.12)
* sphinxnotes.snippet (unknown version)
* sphinxnotes.lilypond (2.4)
* sphinxnotes.recentupdate (1.0b2)
* sphinxnotes.comboroles (0.1.0)
* sphinxcontrib.globalsubs (0.1.0)
* sphinxnotes.fasthtml (unknown version)
* sphinx.ext.graphviz (8.2.3)
* sphinxnotes.poc (0.0.post1.dev4+g0690743f5)
* sphinx_book_theme (unknown version)
* pydata_sphinx_theme (unknown version)

Traceback
=========

      File "/REDACTED/silverrainz.github.io/.venv/lib/python3.12/site-packages/sphinxnotes/mock/__init__.py", line 49, in run
        raise ValueError('unsupported mock mode')
    ValueError: unsupported mock mode

Analysis and Solution

I found the directive name in runtime could be 'Contents', but the config from the blog is 'contents', resulting mode being resolved as None and raising the error. (I did not investigate why the directive name is 'Contents' instead of 'contents', it looks like Sphinx is normalizing directive name to capital case?)

At the first look we could update the mock_directives to be 'Contents' (with uppercase C) in https://github.com/SilverRainZ/silverrainz.github.io/blob/cf5fcad88b7727871af06c06f0d39ffda51a5606/conf.py#L207.

However according to reST spec:

Directive types are case-insensitive single words (alphanumerics plus isolated internal hyphens, underscores, plus signs, colons, and periods; no whitespace).

We could make the logic in sphinxnotes/mock case-insensitive to avoid such issues in the future.

Another fix is making the directive name matching in MockDirective case-insensive:

if self.name != name:

I found it is redundant to check directive name in MockDirective since app.add_directive(name, cls, ...) already binds a directive name to the given directive class.

Finally, I elimated the redundant directive name checks in MockDirective class and updated the arguments for app.add_directive() to make Sphinx handle the directive name matching for us.

Additional Changes

  • Updated outdated docs

Copy link
Member

@SilverRainZ SilverRainZ left a comment

Choose a reason for hiding this comment

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

prprpr

final_argument_whitespace = True
option_spec = MockOptionSpec()
has_content = True
def _make_mock_directive(mode: str):
Copy link
Member

Choose a reason for hiding this comment

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

Create a wrapper for passing mode is unnecessary, you can access mode by accessing self.config.mock_default_mode.

Copy link
Author

Choose a reason for hiding this comment

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

We are handling mode for specific directive outside of MockDirective, to achieve avoiding resolving mode again inside MockDirective. So we need a way to keep the resolved mode info.

Actually we could avoid creating new MockDirective for each directive here.

Copy link
Author

Choose a reason for hiding this comment

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

Updated the implementation, we do not need to create many MockDirective classes now

@frantic1048 frantic1048 force-pushed the dev/enhance-directive-matching branch from c0a952d to 9622329 Compare October 5, 2025 08:57
@frantic1048 frantic1048 marked this pull request as draft October 5, 2025 09:12
@frantic1048 frantic1048 force-pushed the dev/enhance-directive-matching branch from 9622329 to 73d18a2 Compare October 5, 2025 09:20
@frantic1048 frantic1048 marked this pull request as ready for review October 5, 2025 09:20
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.

2 participants