diff --git a/sphinx_needs/directives/needflow/_graphviz.py b/sphinx_needs/directives/needflow/_graphviz.py index aa2572921..39b05805a 100644 --- a/sphinx_needs/directives/needflow/_graphviz.py +++ b/sphinx_needs/directives/needflow/_graphviz.py @@ -4,6 +4,7 @@ import textwrap from functools import lru_cache from typing import Callable, Literal, TypedDict +from urllib.parse import urlparse from docutils import nodes from sphinx.application import Sphinx @@ -17,9 +18,9 @@ from sphinx_needs.config import LinkOptionsType, NeedsSphinxConfig from sphinx_needs.data import NeedsInfoType, SphinxNeedsData from sphinx_needs.debug import measure_time -from sphinx_needs.diagrams_common import calculate_link from sphinx_needs.directives.needflow._directive import NeedflowGraphiz from sphinx_needs.directives.utils import no_needs_found_paragraph +from sphinx_needs.errors import NoUri from sphinx_needs.filter_common import ( filter_single_need, process_filters, @@ -159,7 +160,7 @@ def process_needflow_graphviz( node, needs_view, needs_config, - lambda n: calculate_link(app, n, fromdocname, relative="."), + lambda n: _get_link_to_need(app, fromdocname, n), id_comp_to_need, rendered_nodes, ) @@ -191,6 +192,31 @@ def process_needflow_graphviz( node.parent.parent.insert(node.parent.parent.index(node.parent) + 1, code) +def _get_link_to_need( + app: Sphinx, docname: str, need_info: NeedsInfoType +) -> str | None: + """Compute the link to a need, relative to a document. + + It is of note that the links are computed relative to the document that the graph is in. + For PNGs, the links are defined as https://developer.mozilla.org/en-US/docs/Web/HTML/Element/map in the document, so this correct. + For SVGs, the graphs are extracted to external files, and in this case the links are modified to be relative to the SVG file + (from sphinx 7.2 onwards, see: https://github.com/sphinx-doc/sphinx/pull/11078) + """ + if need_info["is_external"]: + if need_info["external_url"] and urlparse(need_info["external_url"]).scheme: + return need_info["external_url"] + elif need_info["docname"]: + try: + rel_uri = app.builder.get_relative_uri(docname, need_info["docname"]) + if not rel_uri: + # svg relative path fix cannot yet handle empty paths https://github.com/sphinx-doc/sphinx/issues/13078 + rel_uri = app.builder.get_target_uri(docname.split("/")[-1]) + except NoUri: + return None + return rel_uri + "#" + need_info["id_complete"] + return None + + class _RenderedNode(TypedDict): cluster_id: str | None need: NeedsInfoType @@ -206,7 +232,7 @@ def _render_node( node: NeedflowGraphiz, needs_view: NeedsView, config: NeedsSphinxConfig, - calc_link: Callable[[NeedsInfoType], str], + calc_link: Callable[[NeedsInfoType], str | None], id_comp_to_need: dict[str, NeedsInfoType], rendered_nodes: dict[str, _RenderedNode], subgraph: bool = True, @@ -285,7 +311,7 @@ def _render_subgraph( node: NeedflowGraphiz, needs_view: NeedsView, config: NeedsSphinxConfig, - calc_link: Callable[[NeedsInfoType], str], + calc_link: Callable[[NeedsInfoType], str | None], id_comp_to_need: dict[str, NeedsInfoType], rendered_nodes: dict[str, _RenderedNode], ) -> str: @@ -606,7 +632,7 @@ def html_visit_needflow_graphviz(self: HTML5Translator, node: NeedflowGraphiz) - log_warning(LOGGER, "Content has not been resolved", "needflow", location=node) raise nodes.SkipNode attrributes = node.attributes - format = self.builder.config.graphviz_output_format + format: Literal["png", "svg"] = self.builder.config.graphviz_output_format if format not in ("png", "svg"): log_warning( LOGGER, diff --git a/tests/test_needflow.py b/tests/test_needflow.py index a0aa492eb..66afa968c 100644 --- a/tests/test_needflow.py +++ b/tests/test_needflow.py @@ -3,6 +3,7 @@ import pytest from lxml import html as html_parser +from sphinx import version_info from sphinx.config import Config from sphinx.util.console import strip_colors @@ -36,6 +37,7 @@ def _get_svg(config: Config, outdir: Path, file: str, id: str) -> str: }, }, ], + ids=["plantuml", "graphviz"], indirect=True, ) def test_doc_build_html(test_app): @@ -50,32 +52,42 @@ def test_doc_build_html(test_app): assert warnings == "" outdir = Path(app.outdir) + svg = _get_svg(app.config, outdir, "index.html", "needflow-index-0") - for link in ( - "./index.html#SPEC_1", - "./index.html#SPEC_2", - "./index.html#STORY_1", - "./index.html#STORY_1.1", - "./index.html#STORY_1.2", - "./index.html#STORY_1.subspec", - "./index.html#STORY_2", - "./index.html#STORY_2.another_one", - ): - assert link in svg + + if test_app.config.needs_flow_engine == "graphviz" and version_info < (7, 2): + pass # links will be wrong due to https://github.com/sphinx-doc/sphinx/pull/11078 + else: + for link in ( + '"../index.html#SPEC_1"', + '"../index.html#SPEC_2"', + '"../index.html#STORY_1"', + '"../index.html#STORY_1.1"', + '"../index.html#STORY_1.2"', + '"../index.html#STORY_1.subspec"', + '"../index.html#STORY_2"', + '"../index.html#STORY_2.another_one"', + ): + assert link in svg + assert "No needs passed the filters" in Path(app.outdir, "index.html").read_text() svg = _get_svg(app.config, outdir, "page.html", "needflow-page-0") - for link in ( - "./index.html#SPEC_1", - "./index.html#SPEC_2", - "./index.html#STORY_1", - "./index.html#STORY_1.1", - "./index.html#STORY_1.2", - "./index.html#STORY_1.subspec", - "./index.html#STORY_2", - "./index.html#STORY_2.another_one", - ): - assert link in svg + + if test_app.config.needs_flow_engine == "graphviz" and version_info < (7, 2): + pass # links will be wrong due to https://github.com/sphinx-doc/sphinx/pull/11078 + else: + for link in ( + '"../index.html#SPEC_1"', + '"../index.html#SPEC_2"', + '"../index.html#STORY_1"', + '"../index.html#STORY_1.1"', + '"../index.html#STORY_1.2"', + '"../index.html#STORY_1.subspec"', + '"../index.html#STORY_2"', + '"../index.html#STORY_2.another_one"', + ): + assert link in svg svg = _get_svg( app.config, @@ -83,7 +95,7 @@ def test_doc_build_html(test_app): "needflow_with_root_id.html", "needflow-needflow_with_root_id-0", ) - print(svg) + for link in ("SPEC_1", "STORY_1", "STORY_2"): assert link in svg @@ -112,6 +124,7 @@ def test_doc_build_html(test_app): }, }, ], + ids=["plantuml", "graphviz"], indirect=True, ) def test_doc_build_needflow_incl_child_needs(test_app): @@ -128,97 +141,107 @@ def test_doc_build_needflow_incl_child_needs(test_app): outdir = Path(app.outdir) svg = _get_svg(app.config, outdir, "index.html", "needflow-index-0") - for link in ( - "./index.html#STORY_1", - "./index.html#STORY_1.1", - "./index.html#STORY_1.2", - "./index.html#STORY_2", - "./index.html#STORY_2.3", - "./index.html#SPEC_1", - "./index.html#SPEC_2", - "./index.html#SPEC_3", - "./index.html#SPEC_4", - "./index.html#STORY_3", - "./index.html#SPEC_5", - ): - assert link in svg - - svg = _get_svg( - app.config, - outdir, - "single_parent_need_filer.html", - "needflow-single_parent_need_filer-0", - ) - assert "./index.html#STORY_3" in svg - for link in ( - "./index.html#STORY_1", - "./index.html#STORY_1.1", - "./index.html#STORY_1.2", - "./index.html#STORY_2", - "./index.html#STORY_2.3", - "./index.html#SPEC_1", - "./index.html#SPEC_2", - "./index.html#SPEC_3", - "./index.html#SPEC_4", - "./index.html#SPEC_5", - ): - assert link not in svg - svg = _get_svg( - app.config, - outdir, - "single_child_with_child_need_filter.html", - "needflow-single_child_with_child_need_filter-0", - ) - assert "./index.html#STORY_2" in svg - for link in ( - "./index.html#STORY_1", - "./index.html#STORY_1.1", - "./index.html#STORY_1.2", - "./index.html#STORY_2.3", - "./index.html#SPEC_1", - "./index.html#SPEC_2", - "./index.html#SPEC_3", - "./index.html#SPEC_4", - "./index.html#STORY_3", - "./index.html#SPEC_5", - ): - assert link not in svg - - svg = _get_svg( - app.config, - outdir, - "single_child_need_filter.html", - "needflow-single_child_need_filter-0", - ) - assert "./index.html#SPEC_1" in svg - for link in ( - "./index.html#STORY_1", - "./index.html#STORY_1.1", - "./index.html#STORY_1.2", - "./index.html#STORY_2", - "./index.html#STORY_2.3", - "./index.html#SPEC_2", - "./index.html#SPEC_3", - "./index.html#SPEC_4", - "./index.html#STORY_3", - "./index.html#SPEC_5", - ): - assert link not in svg - - svg = _get_svg( - app.config, outdir, "grandy_and_child.html", "needflow-grandy_and_child-0" - ) - for link in ("./index.html#STORY_1", "./index.html#SPEC_1", "./index.html#SPEC_2"): - assert link in svg - for link in ( - "./index.html#STORY_1.1", - "./index.html#STORY_1.2", - "./index.html#STORY_2", - "./index.html#STORY_2.3", - "./index.html#SPEC_3", - "./index.html#SPEC_4", - "./index.html#STORY_3", - "./index.html#SPEC_5", - ): - assert link not in svg + if test_app.config.needs_flow_engine == "graphviz" and version_info < (7, 2): + pass # links will be wrong due to https://github.com/sphinx-doc/sphinx/pull/11078 + else: + for link in ( + '"../index.html#STORY_1"', + '"../index.html#STORY_1.1"', + '"../index.html#STORY_1.2"', + '"../index.html#STORY_2"', + '"../index.html#STORY_2.3"', + '"../index.html#SPEC_1"', + '"../index.html#SPEC_2"', + '"../index.html#SPEC_3"', + '"../index.html#SPEC_4"', + '"../index.html#STORY_3"', + '"../index.html#SPEC_5"', + ): + assert link in svg + + svg = _get_svg( + app.config, + outdir, + "single_parent_need_filer.html", + "needflow-single_parent_need_filer-0", + ) + + assert '"../index.html#STORY_3"' in svg + for link in ( + '"../index.html#STORY_1"', + '"../index.html#STORY_1.1"', + '"../index.html#STORY_1.2"', + '"../index.html#STORY_2"', + '"../index.html#STORY_2.3"', + '"../index.html#SPEC_1"', + '"../index.html#SPEC_2"', + '"../index.html#SPEC_3"', + '"../index.html#SPEC_4"', + '"../index.html#SPEC_5"', + ): + assert link not in svg + + svg = _get_svg( + app.config, + outdir, + "single_child_with_child_need_filter.html", + "needflow-single_child_with_child_need_filter-0", + ) + + assert '"../index.html#STORY_2"' in svg + for link in ( + '"../index.html#STORY_1"', + '"../index.html#STORY_1.1"', + '"../index.html#STORY_1.2"', + '"../index.html#STORY_2.3"', + '"../index.html#SPEC_1"', + '"../index.html#SPEC_2"', + '"../index.html#SPEC_3"', + '"../index.html#SPEC_4"', + '"../index.html#STORY_3"', + '"../index.html#SPEC_5"', + ): + assert link not in svg + + svg = _get_svg( + app.config, + outdir, + "single_child_need_filter.html", + "needflow-single_child_need_filter-0", + ) + assert '"../index.html#SPEC_1"' in svg + for link in ( + '"../index.html#STORY_1"', + '"../index.html#STORY_1.1"', + '"../index.html#STORY_1.2"', + '"../index.html#STORY_2"', + '"../index.html#STORY_2.3"', + '"../index.html#SPEC_2"', + '"../index.html#SPEC_3"', + '"../index.html#SPEC_4"', + '"../index.html#STORY_3"', + '"../index.html#SPEC_5"', + ): + assert link not in svg + + svg = _get_svg( + app.config, outdir, "grandy_and_child.html", "needflow-grandy_and_child-0" + ) + for link in ( + '"../index.html#STORY_1"', + '"../index.html#SPEC_1"', + '"../index.html#SPEC_2"', + ): + assert link in svg + for link in ( + '"../index.html#STORY_1.1"', + '"../index.html#STORY_1.2"', + '"../index.html#STORY_2"', + '"../index.html#STORY_2.3"', + '"../index.html#SPEC_3"', + '"../index.html#SPEC_4"', + '"../index.html#STORY_3"', + '"../index.html#SPEC_5"', + ): + assert link not in svg