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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 2 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c9d8eac
Don't remove substitution_reference nodes (fix #7953)
brechtm Sep 8, 2020
97e7e09
Fix tests failing on trailing substitution definition
brechtm Sep 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions sphinx/builders/latex/transforms.py
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@

from docutils import nodes
from docutils.nodes import Element, Node
from docutils.transforms.references import Substitutions

from sphinx import addnodes
from sphinx.application import Sphinx
Expand All @@ -38,6 +39,18 @@ def apply(self, **kwargs: Any) -> None:
node['docname'] = self.env.docname


class SubstitutionDefinitionsRemover(SphinxPostTransform):
"""Remove ``substitution_definition node from doctrees."""

# should be invoked after Substitutions process
default_priority = Substitutions.default_priority + 1
builders = ('latex',)

def apply(self, **kwargs: Any) -> None:
for node in self.document.traverse(nodes.substitution_definition):
node.parent.remove(node)


class ShowUrlsTransform(SphinxPostTransform):
"""Expand references to inline text or footnotes.

Expand Down Expand Up @@ -602,6 +615,7 @@ def apply(self, **kwargs: Any) -> None:

def setup(app: Sphinx) -> Dict[str, Any]:
app.add_transform(FootnoteDocnameUpdater)
app.add_post_transform(SubstitutionDefinitionsRemover)
app.add_post_transform(BibliographyTransform)
app.add_post_transform(CitationReferenceTransform)
app.add_post_transform(DocumentTargetTransform)
Expand Down
15 changes: 1 addition & 14 deletions sphinx/transforms/references.py
View file
Open in desktop
Expand Up @@ -10,8 +10,7 @@

from typing import Any, Dict

from docutils import nodes
from docutils.transforms.references import DanglingReferences, Substitutions
from docutils.transforms.references import DanglingReferences

from sphinx.transforms import SphinxTransform

Expand All @@ -20,17 +19,6 @@
from sphinx.application import Sphinx


class SubstitutionDefinitionsRemover(SphinxTransform):
"""Remove ``substitution_definition node from doctrees."""

# should be invoked after Substitutions process
default_priority = Substitutions.default_priority + 1

def apply(self, **kwargs: Any) -> None:
for node in self.document.traverse(nodes.substitution_definition):
node.parent.remove(node)


class SphinxDanglingReferences(DanglingReferences):
"""DanglingReferences transform which does not output info messages."""

Expand All @@ -56,7 +44,6 @@ def apply(self, **kwargs: Any) -> None:


def setup(app: "Sphinx") -> Dict[str, Any]:
app.add_transform(SubstitutionDefinitionsRemover)
app.add_transform(SphinxDanglingReferences)
app.add_transform(SphinxDomains)

Expand Down
9 changes: 9 additions & 0 deletions sphinx/writers/texinfo.py
View file
Open in desktop
Expand Up @@ -1242,6 +1242,15 @@ def visit_legend(self, node: Element) -> None:
def depart_legend(self, node: Element) -> None:
pass

def visit_substitution_reference(self, node: Element) -> None:
pass

def depart_substitution_reference(self, node: Element) -> None:
pass

def visit_substitution_definition(self, node: Element) -> None:
raise nodes.SkipNode

def visit_system_message(self, node: Element) -> None:
self.body.append('\n@verbatim\n'
'<SYSTEM MESSAGE: %s>\n'
Expand Down
3 changes: 3 additions & 0 deletions sphinx/writers/text.py
View file
Open in desktop
Expand Up @@ -1013,6 +1013,9 @@ def visit_index(self, node: Element) -> None:
def visit_toctree(self, node: Element) -> None:
raise nodes.SkipNode

def visit_substitution_definition(self, node: Element) -> None:
raise nodes.SkipNode

def visit_pending_xref(self, node: Element) -> None:
pass

Expand Down
30 changes: 19 additions & 11 deletions tests/test_directive_patch.py
View file
Open in desktop
Expand Up @@ -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.

assert_node(doctree[0], language="default", highlight_args={})

# with language
Expand All @@ -30,7 +31,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))
assert_node(doctree[0], language="python", highlight_args={})

# :number-lines: option
Expand All @@ -40,7 +42,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))
assert_node(doctree[0], language="python", linenos=True, highlight_args={})

# :number-lines: option
Expand All @@ -50,37 +53,42 @@ 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))
assert_node(doctree[0], language="python", linenos=True, highlight_args={'linenostart': 5})


def test_math_directive(app):
# normal case
text = '.. math:: E = mc^2'
doctree = restructuredtext.parse(app, text)
assert_node(doctree, [nodes.document, nodes.math_block, 'E = mc^2\n\n'])
assert_node(doctree, ([nodes.math_block, "E = mc^2\n\n"],
nodes.substitution_definition))

# :name: option
text = ('.. math:: E = mc^2\n'
' :name: eq1\n')
doctree = restructuredtext.parse(app, text)
assert_node(doctree, [nodes.document, (nodes.target,
[nodes.math_block, "E = mc^2\n\n"])])
assert_node(doctree, (nodes.target,
[nodes.math_block, "E = mc^2\n\n"],
nodes.substitution_definition))
assert_node(doctree[1], nodes.math_block, docname='index', label="eq1", number=1)

# :label: option
text = ('.. math:: E = mc^2\n'
' :label: eq2\n')
doctree = restructuredtext.parse(app, text)
assert_node(doctree, [nodes.document, (nodes.target,
[nodes.math_block, 'E = mc^2\n\n'])])
assert_node(doctree, (nodes.target,
[nodes.math_block, "E = mc^2\n\n"],
nodes.substitution_definition))
assert_node(doctree[1], nodes.math_block, docname='index', label="eq2", number=2)

# :label: option without value
text = ('.. math:: E = mc^2\n'
' :label:\n')
doctree = restructuredtext.parse(app, text)
assert_node(doctree, [nodes.document, (nodes.target,
[nodes.math_block, 'E = mc^2\n\n'])])
assert_node(doctree, (nodes.target,
[nodes.math_block, "E = mc^2\n\n"],
nodes.substitution_definition))
assert_node(doctree[1], nodes.math_block, ids=['equation-index-0'],
docname='index', label="index:0", number=3)
4 changes: 3 additions & 1 deletion tests/test_domain_c.py
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@
:license: BSD, see LICENSE for details.
"""
import pytest
from docutils import nodes

from sphinx import addnodes
from sphinx.addnodes import desc
Expand Down Expand Up @@ -601,6 +602,7 @@ def test_noindexentry(app):
".. c:function:: void g()\n"
" :noindexentry:\n")
doctree = restructuredtext.parse(app, text)
assert_node(doctree, (addnodes.index, desc, addnodes.index, desc))
assert_node(doctree, (addnodes.index, desc, addnodes.index, desc,
nodes.substitution_definition))
assert_node(doctree[0], addnodes.index, entries=[('single', 'f (C function)', 'c.f', '', None)])
assert_node(doctree[2], addnodes.index, entries=[])
4 changes: 3 additions & 1 deletion tests/test_domain_cpp.py
View file
Open in desktop
Expand Up @@ -11,6 +11,7 @@
import re

import pytest
from docutils import nodes

import sphinx.domains.cpp as cppDomain
from sphinx import addnodes
Expand Down Expand Up @@ -1233,6 +1234,7 @@ def test_noindexentry(app):
".. cpp:function:: void g()\n"
" :noindexentry:\n")
doctree = restructuredtext.parse(app, text)
assert_node(doctree, (addnodes.index, desc, addnodes.index, desc))
assert_node(doctree, (addnodes.index, desc, addnodes.index, desc,
nodes.substitution_definition))
assert_node(doctree[0], addnodes.index, entries=[('single', 'f (C++ function)', '_CPPv41fv', '', None)])
assert_node(doctree[2], addnodes.index, entries=[])
15 changes: 10 additions & 5 deletions tests/test_domain_js.py
View file
Open in desktop
Expand Up @@ -176,7 +176,8 @@ def test_js_module(app):
text = ".. js:module:: sphinx"
doctree = restructuredtext.parse(app, text)
assert_node(doctree, (nodes.target,
addnodes.index))
addnodes.index,
nodes.substitution_definition))
assert_node(doctree[0], nodes.target, ids=["module-sphinx"])
assert_node(doctree[1], addnodes.index,
entries=[("single", "sphinx (module)", "module-sphinx", "", None)])
Expand All @@ -188,7 +189,8 @@ def test_js_function(app):
assert_node(doctree, (addnodes.index,
[desc, ([desc_signature, ([desc_name, "sum"],
desc_parameterlist)],
[desc_content, ()])]))
[desc_content, ()])],
nodes.substitution_definition))
assert_node(doctree[1][0][1], [desc_parameterlist, ([desc_parameter, "a"],
[desc_parameter, "b"])])
assert_node(doctree[0], addnodes.index,
Expand All @@ -203,7 +205,8 @@ def test_js_class(app):
[desc, ([desc_signature, ([desc_annotation, "class "],
[desc_name, "Application"],
[desc_parameterlist, ()])],
[desc_content, ()])]))
[desc_content, ()])],
nodes.substitution_definition))
assert_node(doctree[0], addnodes.index,
entries=[("single", "Application() (class)", "Application", "", None)])
assert_node(doctree[1], addnodes.desc, domain="js", objtype="class", noindex=False)
Expand All @@ -214,7 +217,8 @@ def test_js_data(app):
doctree = restructuredtext.parse(app, text)
assert_node(doctree, (addnodes.index,
[desc, ([desc_signature, desc_name, "name"],
[desc_content, ()])]))
[desc_content, ()])],
nodes.substitution_definition))
assert_node(doctree[0], addnodes.index,
entries=[("single", "name (global variable or constant)", "name", "", None)])
assert_node(doctree[1], addnodes.desc, domain="js", objtype="data", noindex=False)
Expand All @@ -225,6 +229,7 @@ def test_noindexentry(app):
".. js:function:: g()\n"
" :noindexentry:\n")
doctree = restructuredtext.parse(app, text)
assert_node(doctree, (addnodes.index, desc, addnodes.index, desc))
assert_node(doctree, (addnodes.index, desc, addnodes.index, desc,
nodes.substitution_definition))
assert_node(doctree[0], addnodes.index, entries=[('single', 'f() (built-in function)', 'f', '', None)])
assert_node(doctree[2], addnodes.index, entries=[])