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

Fixed two more bugs with SVG inheritance-diagram links #11634

Merged
merged 12 commits into from Aug 30, 2023
3 changes: 3 additions & 0 deletions CHANGES
Expand Up @@ -20,6 +20,9 @@ Bugs fixed
packages that make use of ``if typing.TYPE_CHECKING:`` to guard circular
imports needed by type checkers.
Patch by Matt Wozniski.
* #11634: Fixed inheritance diagram relative link resolution
for sibling files in a subdirectory.
Patch by Albert Shih.

Testing
-------
Expand Down
28 changes: 17 additions & 11 deletions sphinx/ext/graphviz.py
Expand Up @@ -21,7 +21,7 @@
from sphinx.errors import SphinxError
from sphinx.locale import _, __
from sphinx.util import logging
from sphinx.util.docutils import SphinxDirective, SphinxTranslator
from sphinx.util.docutils import SphinxDirective
from sphinx.util.i18n import search_image_for_language
from sphinx.util.nodes import set_source_info
from sphinx.util.osutil import ensuredir
Expand Down Expand Up @@ -218,7 +218,8 @@ def run(self) -> list[Node]:
return [figure]


def fix_svg_relative_paths(self: SphinxTranslator, filepath: str) -> None:
def fix_svg_relative_paths(self: HTML5Translator | LaTeXTranslator | TexinfoTranslator,
filepath: str) -> None:
"""Change relative links in generated svg files to be relative to imgpath."""
tree = ET.parse(filepath) # NoQA: S314
root = tree.getroot()
Expand All @@ -230,16 +231,20 @@ def fix_svg_relative_paths(self: SphinxTranslator, filepath: str) -> None:
root.findall('.//svg:image[@xlink:href]', ns),
root.findall('.//svg:a[@xlink:href]', ns),
):
scheme, hostname, url, query, fragment = urlsplit(element.attrib[href_name])
scheme, hostname, rel_uri, query, fragment = urlsplit(element.attrib[href_name])
if hostname:
# not a relative link
continue

old_path = path.join(self.builder.outdir, url)
new_path = path.relpath(
old_path,
start=path.join(self.builder.outdir, self.builder.imgpath),
)
docname = self.builder.env.path2doc(self.document["source"])
if docname is None:
# This shouldn't happen!
continue
doc_dir = self.builder.app.outdir.joinpath(docname).resolve().parent

old_path = doc_dir / rel_uri
img_path = doc_dir / self.builder.imgpath
new_path = path.relpath(old_path, start=img_path)
AA-Turner marked this conversation as resolved.
Show resolved Hide resolved
modified_url = urlunsplit((scheme, hostname, new_path, query, fragment))

element.set(href_name, modified_url)
Expand All @@ -249,7 +254,8 @@ def fix_svg_relative_paths(self: SphinxTranslator, filepath: str) -> None:
tree.write(filepath)


def render_dot(self: SphinxTranslator, code: str, options: dict, format: str,
def render_dot(self: HTML5Translator | LaTeXTranslator | TexinfoTranslator,
code: str, options: dict, format: str,
prefix: str = 'graphviz', filename: str | None = None,
) -> tuple[str | None, str | None]:
"""Render graphviz code into a PNG or PDF output file."""
Expand Down Expand Up @@ -294,8 +300,8 @@ def render_dot(self: SphinxTranslator, code: str, options: dict, format: str,
logger.warning(__('dot command %r cannot be run (needed for graphviz '
'output), check the graphviz_dot setting'), graphviz_dot)
if not hasattr(self.builder, '_graphviz_warned_dot'):
self.builder._graphviz_warned_dot = {} # type: ignore[attr-defined]
self.builder._graphviz_warned_dot[graphviz_dot] = True # type: ignore[attr-defined]
self.builder._graphviz_warned_dot = {} # type: ignore[union-attr]
self.builder._graphviz_warned_dot[graphviz_dot] = True
return None, None
except CalledProcessError as exc:
raise GraphvizError(__('dot exited with error:\n[stderr]\n%r\n'
Expand Down
3 changes: 2 additions & 1 deletion sphinx/ext/inheritance_diagram.py
Expand Up @@ -36,6 +36,7 @@ class E(B): pass
import re
from collections.abc import Iterable, Sequence
from importlib import import_module
from os import path
from typing import TYPE_CHECKING, Any, cast

from docutils import nodes
Expand Down Expand Up @@ -417,7 +418,7 @@ def html_visit_inheritance_diagram(self: HTML5Translator, node: inheritance_diag

# Create a mapping from fully-qualified class names to URLs.
graphviz_output_format = self.builder.env.config.graphviz_output_format.upper()
current_filename = self.builder.current_docname + self.builder.out_suffix
current_filename = path.basename(self.builder.current_docname + self.builder.out_suffix)
urls = {}
pending_xrefs = cast(Iterable[addnodes.pending_xref], node)
for child in pending_xrefs:
Expand Down
4 changes: 2 additions & 2 deletions tests/roots/test-ext-inheritance_diagram/index.rst
Expand Up @@ -7,12 +7,12 @@ test-ext-inheritance_diagram
.. inheritance-diagram:: test.Foo
:caption: Test Foo!

.. inheritance-diagram:: test.DocLowerLevel
.. inheritance-diagram:: test.DocSubDir2

.. py:class:: test.DocHere
.. py:class:: test.DocMainLevel
.. inheritance-diagram:: subdir.other.Bob
.. inheritance-diagram:: external.other.Bob

.. py:class:: test.Alice
7 changes: 0 additions & 7 deletions tests/roots/test-ext-inheritance_diagram/subdir/index.rst

This file was deleted.

9 changes: 9 additions & 0 deletions tests/roots/test-ext-inheritance_diagram/subdir/page1.rst
@@ -0,0 +1,9 @@
================================================
test-ext-inheritance_diagram subdirectory page 1
================================================

.. inheritance-diagram:: test.DocMainLevel

.. inheritance-diagram:: test.DocSubDir2

.. py:class:: test.DocSubDir1
5 changes: 5 additions & 0 deletions tests/roots/test-ext-inheritance_diagram/subdir/page2.rst
@@ -0,0 +1,5 @@
================================================
test-ext-inheritance_diagram subdirectory page 2
================================================

.. py:class:: test.DocSubDir2
6 changes: 5 additions & 1 deletion tests/roots/test-ext-inheritance_diagram/test.py
Expand Up @@ -6,7 +6,11 @@ class DocHere(Foo):
pass


class DocLowerLevel(DocHere):
class DocSubDir1(DocHere):
pass


class DocSubDir2(DocSubDir1):
pass


Expand Down
19 changes: 10 additions & 9 deletions tests/test_ext_inheritance_diagram.py
Expand Up @@ -138,21 +138,21 @@ def new_run(self):


# An external inventory to test intersphinx links in inheritance diagrams
subdir_inventory = b'''\
external_inventory = b'''\
# Sphinx inventory version 2
# Project: subdir
# Project: external
# Version: 1.0
# The remainder of this file is compressed using zlib.
''' + zlib.compress(b'''\
subdir.other.Bob py:class 1 foo.html#subdir.other.Bob -
external.other.Bob py:class 1 foo.html#external.other.Bob -
''')


@pytest.mark.sphinx('html', testroot='ext-inheritance_diagram')
@pytest.mark.usefixtures('if_graphviz_found')
def test_inheritance_diagram_png_html(tmp_path, app):
inv_file = tmp_path / 'inventory'
inv_file.write_bytes(subdir_inventory)
inv_file.write_bytes(external_inventory)
app.config.intersphinx_mapping = {
'https://example.org': str(inv_file),
}
Expand All @@ -173,7 +173,7 @@ def test_inheritance_diagram_png_html(tmp_path, app):
'title="Link to this image">\xb6</a></p>\n</figcaption>\n</figure>\n')
assert re.search(pattern, content, re.M)

subdir_content = (app.outdir / 'subdir/index.html').read_text(encoding='utf8')
subdir_content = (app.outdir / 'subdir/page1.html').read_text(encoding='utf8')
subdir_maps = re.findall('<map .+\n.+\n</map>', subdir_content)
subdir_maps = [re.sub('href="(\\S+)"', 'href="subdir/\\g<1>"', s) for s in subdir_maps]

Expand All @@ -199,7 +199,7 @@ def test_inheritance_diagram_png_html(tmp_path, app):
@pytest.mark.usefixtures('if_graphviz_found')
def test_inheritance_diagram_svg_html(tmp_path, app):
inv_file = tmp_path / 'inventory'
inv_file.write_bytes(subdir_inventory)
inv_file.write_bytes(external_inventory)
app.config.intersphinx_mapping = {
"subdir": ('https://example.org', str(inv_file)),
}
Expand All @@ -223,7 +223,7 @@ def test_inheritance_diagram_svg_html(tmp_path, app):

assert re.search(pattern, content, re.M)

subdir_content = (app.outdir / 'subdir/index.html').read_text(encoding='utf8')
subdir_content = (app.outdir / 'subdir/page1.html').read_text(encoding='utf8')
subdir_svgs = re.findall('<object data="../(_images/inheritance-\\w+.svg?)"', subdir_content)

# Go through every SVG inheritance diagram
Expand Down Expand Up @@ -268,8 +268,9 @@ def test_inheritance_diagram_latex_alias(app, status, warning):

doc = app.env.get_and_resolve_doctree('index', app)
aliased_graph = doc.children[0].children[3]['graph'].class_info
assert len(aliased_graph) == 3
assert ('test.DocLowerLevel', 'test.DocLowerLevel', ['test.DocHere'], None) in aliased_graph
assert len(aliased_graph) == 4
assert ('test.DocSubDir2', 'test.DocSubDir2', ['test.DocSubDir1'], None) in aliased_graph
assert ('test.DocSubDir1', 'test.DocSubDir1', ['test.DocHere'], None) in aliased_graph
assert ('test.DocHere', 'test.DocHere', ['alias.Foo'], None) in aliased_graph
assert ('alias.Foo', 'alias.Foo', [], None) in aliased_graph

Expand Down