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

substitution_definition nodes are missing from the doctree #7953

Closed
brechtm opened this issue Jul 13, 2020 · 4 comments
Closed

substitution_definition nodes are missing from the doctree #7953

brechtm opened this issue Jul 13, 2020 · 4 comments
Labels
type:proposal a feature suggestion
Milestone

Comments

@brechtm
Copy link
Contributor

brechtm commented Jul 13, 2020

Since PR #4827 all substitution_definition nodes are removed from the docutils doctree. Reasoning behind this was that this node was not being used by any builders included with Sphinx. However, third-party builders might make use of them. rinohtype, for example parses them to be able to reference the definitions in the a document template.

I think it would be better for the document tree that Sphinx passes to builders to match the standard docutils document tree as much as possible. A single builder (LaTeX in this case) having an issue with the document tree shouldn't be a reason to change the doctree for all builders.

I propose making the ´SubstitutionsDefinitionsRemover´ transform local to the LaTeX builder, as was originally intended. I can provide a PR if you wish.

Please also consider third-party builders when making changes like this (e.g. doctree) in the future. Thanks!

@tk0miya
Copy link
Member

tk0miya commented Jul 14, 2020

I don't think this is a bug. But I can accept the change. Could you send a PR please?

BTW, how does rinohtype use such node? I can't imagine how it works. On the writing phase, there are no substitution_reference nodes. Is it enough to build some document?

@tk0miya tk0miya added this to the 3.2.0 milestone Jul 14, 2020
@tk0miya tk0miya added type:proposal a feature suggestion and removed type:bug labels Jul 14, 2020
@tk0miya tk0miya modified the milestones: 3.2.0, 3.3.0 Aug 2, 2020
@brechtm
Copy link
Contributor Author

brechtm commented Sep 8, 2020

I had a go at reverting to the behavior of only removing the substitition definition nodes for the LaTeX builder (PR #4827) here: brechtm@dfa1976. Unfortunately, the test_latex_show_urls_footnote_and_substitutions test case still fails with these changes in place.

I assume the LaTeX builder has been changed since #4827 was merged, causing the footnote numbers to be determined earlier? I haven't been able to find the cause for this. I have tried calling apply_transforms earlier, to no avail. Perhaps @jfbu can point me in the right direction?

I believe #4293 will also be fixed if we undo removal of the substitution definition nodes.

PS. rinohtype allows referencing substitutions outside of the realm of the reStructuredText input files, e.g. in a document template. This can be used to put customized text in a page header or footer, for example.

@brechtm
Copy link
Contributor Author

brechtm commented Sep 8, 2020

The changes that cause brechtm@dfa1976 not to work have been applied somewhere between v2.0.1 and v2.1.0. With the help of git bisect, I've determined b5959ca to be the culprit.

brechtm added a commit to brechtm/sphinx that referenced this issue Sep 8, 2020
SubstitutionDefinitionsRemover is now a SphinxPostTransform, only
applied in the Sphinx builder, as was originally the case (see sphinx-doc#4827).
@brechtm
Copy link
Contributor Author

brechtm commented Sep 8, 2020

I think I've found the correct way to fix this. See PR #8183.

brechtm added a commit to brechtm/sphinx that referenced this issue Sep 9, 2020
Some of the tests that have rst_epilog or rst_prolog set need to be
adjusted to not fail when encountering substitution definitions which
have reappeared since fixing sphinx-doc#7953.
@tk0miya tk0miya closed this as completed in c9d8eac Nov 1, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type:proposal a feature suggestion
Projects
None yet
Development

No branches or pull requests

2 participants