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

🐛 FIX needflow graphviz links #1339

Merged
merged 13 commits into from
Oct 28, 2024
Merged

🐛 FIX needflow graphviz links #1339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b077bac
🐛 FIX needflow graphviz links
chrisjsewell Oct 28, 2024
1d44312
update
chrisjsewell Oct 28, 2024
0d7e699
Update _graphviz.py
chrisjsewell Oct 28, 2024
d8df201
Delete page.rst
chrisjsewell Oct 28, 2024
b48ddc9
Update _graphviz.py
chrisjsewell Oct 28, 2024
77fd8a0
Merge branch 'master' into fix-graphviz-links
chrisjsewell Oct 28, 2024
5892b08
Update needflow.rst
chrisjsewell Oct 28, 2024
35e7c7a
Merge branch 'fix-graphviz-links' of https://github.com/useblocks/sph…
chrisjsewell Oct 28, 2024
be82a44
Update _graphviz.py
chrisjsewell Oct 28, 2024
6ff362f
fix
chrisjsewell Oct 28, 2024
6440b38
update tests
chrisjsewell Oct 28, 2024
7b36f27
Update index.rst
chrisjsewell Oct 28, 2024
25af27c
Update test_needflow.py
chrisjsewell Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 31 additions & 5 deletions sphinx_needs/directives/needflow/_graphviz.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -159,7 +160,7 @@
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,
)
Expand Down Expand Up @@ -191,6 +192,31 @@
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"]

Check warning on line 207 in sphinx_needs/directives/needflow/_graphviz.py

View check run for this annotation

Codecov / codecov/patch

sphinx_needs/directives/needflow/_graphviz.py#L206-L207 

Added lines #L206 - L207 were not covered by tests
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

Check warning on line 215 in sphinx_needs/directives/needflow/_graphviz.py

View check run for this annotation

Codecov / codecov/patch

sphinx_needs/directives/needflow/_graphviz.py#L214-L215 

Added lines #L214 - L215 were not covered by tests
return rel_uri + "#" + need_info["id_complete"]
return None

Check warning on line 217 in sphinx_needs/directives/needflow/_graphviz.py

View check run for this annotation

Codecov / codecov/patch

sphinx_needs/directives/needflow/_graphviz.py#L217 

Added line #L217 was not covered by tests


class _RenderedNode(TypedDict):
cluster_id: str | None
need: NeedsInfoType
Expand All @@ -206,7 +232,7 @@
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,
Expand Down Expand Up @@ -285,7 +311,7 @@
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:
Expand Down Expand Up @@ -606,7 +632,7 @@
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,
Expand Down
255 changes: 139 additions & 116 deletions tests/test_needflow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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):
Expand All @@ -50,40 +52,50 @@ 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,
outdir,
"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

Expand Down Expand Up @@ -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):
Expand All @@ -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
Loading