Skip to content

Commit

Permalink
Fix two relative link bugs in inheritance diagrams (#11634)
Browse files Browse the repository at this point in the history
Co-authored-by: Adam Turner <9087854+aa-turner@users.noreply.github.com>
  • Loading branch information
ayshih and AA-Turner authored Aug 30, 2023
1 parent ca0fc7a commit 4692208
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 31 deletions.
3 changes: 3 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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)
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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

0 comments on commit 4692208

Please sign in to comment.