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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃憣 Allow access to the parser during the read phase #12361

Merged
merged 2 commits into from May 21, 2024

Conversation

chrisjsewell
Copy link
Member

@chrisjsewell chrisjsewell commented May 7, 2024

This PR allows access to the Parser instance during parsing, using the read-only property BuildEnvironment.parser.

The primary use case here is for within directives (also to a lesser extent roles).

Context:

To be fair to docutils, it never actually exposes directives as an extension point to users, this is done by sphinx (mainly in Sphinx.add_directive and Domain.directives)

The "problem" with this is that

  1. Directives are currently a pretty leaky abstraction; exposing a lot of the "internals" of the docutils rST parser, particularly when it comes to nested parsing or parsing from other sources (like docstrings).
  2. The docutils rst parser itself is not the greatest when it comes to things such as nested parsing, and also source mapped logging

This has started to lead to a proliferation of "hacks" into the rst parser, to try to overcome these shortcomings.
To name a few:

These are exceptionally difficult to reconcile with other parsers like Myst, or even alternative rST parsers

Allowing access to the parser would allow the possibility for this sort of code to be centralised and abstracted better on the parser implementation


Also somewhat related #12238 by @n-peugnet

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

In principle, I'm fine but does the publisher.parser object have a lot of references (meaning, is there any chance that we have circular references?)

sphinx/builders/__init__.py Outdated Show resolved Hide resolved
@chrisjsewell chrisjsewell requested a review from picnixz May 10, 2024 10:54
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

I'm fine with this approach ! (though we'd still need to think about the temp_data management in general...)

@chrisjsewell chrisjsewell changed the title Allow access to the parser during the read phase 馃憣 Allow access to the parser during the read phase May 21, 2024
@chrisjsewell chrisjsewell merged commit 548f0f9 into sphinx-doc:master May 21, 2024
23 checks passed
@chrisjsewell chrisjsewell deleted the access-parser branch May 21, 2024 12:46
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.

None yet

2 participants