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

Don't remove substitution_reference nodes (fix #7953) #8183

Closed

Conversation

brechtm
Copy link
Contributor

@brechtm brechtm commented Sep 8, 2020

SubstitutionDefinitionsRemover is now a SphinxPostTransform, only
applied in the Sphinx builder, as was originally the case (see #4827).

SubstitutionDefinitionsRemover is now a SphinxPostTransform, only
applied in the Sphinx builder, as was originally the case (see sphinx-doc#4827).
@brechtm brechtm force-pushed the pr/keep_substitution_definition_nodes branch from 636682f to c9d8eac Compare September 8, 2020 15:34
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.
@@ -21,7 +21,8 @@ def test_code_directive(app):
' print("hello world")\n')

doctree = restructuredtext.parse(app, text)
assert_node(doctree, [nodes.document, nodes.literal_block, 'print("hello world")'])
assert_node(doctree, ([nodes.literal_block, 'print("hello world")'],
nodes.substitution_definition))
Copy link
Member

Choose a reason for hiding this comment

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

Finally, I found the reason why the substituion_definition node appears suddenly here. tests/roots/test-root/conf.py contains a substitution definition in its rst_epilog. We need to remove it to be these testcases healthy. I'll work on it.

Copy link
Member

Choose a reason for hiding this comment

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

I just posted #8295 to fix it. So please update this PR. Then I'll review this again.

@tk0miya
Copy link
Member

tk0miya commented Nov 1, 2020

I merged this partially (without unnecessary changes of testings) at #8354.
Thank you for your contribution!

@tk0miya tk0miya closed this Nov 1, 2020
tk0miya added a commit that referenced this pull request Nov 1, 2020
@brechtm
Copy link
Contributor Author

brechtm commented Nov 2, 2020

Thanks, @tk0miya. I didn't find the time yet to get back to this, so you beat me to it. Cheers!

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

Successfully merging this pull request may close these issues.

None yet

2 participants