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

Implement the only directive through input lines manipulation #12491

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

AA-Turner
Copy link
Member

@AA-Turner AA-Turner commented Jun 28, 2024

This adjusts how .. only:: directives are implemented.

only directives are special, as a supported use-case is conditional inclusion of a section or sections. For example:

Alpha
-----

Lorem ipsum 

.. only:: exemplar_tag

   Bravo
   -----

   dolar sit

Charlie
-------

amet.

However, directives have no way within Docutils to create a new section. The current implementation therefore takes the approach of a post-hoc pass to identify sections that ought be at a greater level in the document hierarchy, and raises those sections accordingly. This approach works, though is somewhat fragile.

The approach suggested in this PR does have fragility concerns, but I believe is a stronger approach to take. We treat the only directive like a pre-processor, and replace the text in the buffer with either the de-indented contents of the directive, or blank lines, depending on the status of the tag condition. This has the benefit of reducing complexity as the only post-transform is no longer needed (although not removed here), as tags are eagerly evaluated.

A

@AA-Turner AA-Turner changed the title Implement the only directive throught input lines manipulation Implement the only directive through input lines manipulation Jun 28, 2024
@AA-Turner AA-Turner mentioned this pull request Jun 28, 2024
@chrisjsewell
Copy link
Member

chrisjsewell commented Jun 28, 2024

Heya, @AA-Turner I don't think this will work, in the context of doctree caching and multiple builds:

In general, doctrees should be cached (after the read phase) in a builder/tag agnostic state, then the cache can be used to build mutiple format/tag configurations.
This is why the content of only is always parsed, then only removed in OnlyNodeTransform(SphinxPostTransform)

With this change, I believe you would now be caching doctrees specific to the initial builder/tag, and so it would no longer work for other builder/tag?

(I agree though that the current only is not particularly great 😄 )

@AA-Turner
Copy link
Member Author

In general, doctrees should be cached (after the read phase) in a builder/tag agnostic state

Assuming my understanding is right, I am unsure if it is possible to cache in a tag-agnostic manner. The tags object (unlike the builder information) is provided to conf.py, which means that content can be included or excluded depending on tags. This is observed in a few of the links in the PR description, where users are seeking to conditionally include documents.

We also currently invalidate the cache if tags change (only for HTML builders) via BuildInfo.

If I am correct that any cache must be aware of tags, it might be worth ensuring that environments are invalidated if tags change, to avoid unforseen scenarios and cache interactions.

I think you've spent more time recently than me on the doctree cache, though, so likely that I have missed points. We can survive without this change, but I think it is a nice quality-of-life improvement and makes reasoning about only nodes significantly easier, as they're dealt with immediatley.

a

@chrisjsewell
Copy link
Member

We also currently invalidate the cache if tags change (only for HTML builders) via BuildInfo.

just did a bit of checking in the source code,
and this doesn't invalidate doctrees, only the output HTML files, i.e. it only affects the write phase, not the read phase

@chrisjsewell
Copy link
Member

Assuming my understanding is right, I am unsure if it is possible to cache in a tag-agnostic manner.

I believe that, if you are only using tags to affect things in post-transforms (i.e. after doctrees have been cached) then it should be fine,
but if you start trying to use them in directives/transforms (i.e. before doctrees are cached), that is when you start getting into trouble

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

Successfully merging this pull request may close these issues.

3 participants