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

🔧 Fix typing of need docname/lineno #1134

Merged
merged 1 commit into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions sphinx_needs/api/need.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,8 @@ def run():

# Add the need and all needed information
needs_info: NeedsInfoType = {
"docname": docname, # type: ignore[typeddict-item]
"lineno": lineno, # type: ignore[typeddict-item]
"docname": docname,
"lineno": lineno,
"doctype": doctype,
"target_id": need_id,
"content_node": None,
Expand Down
10 changes: 4 additions & 6 deletions sphinx_needs/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,10 @@ class NeedsInfoType(TypedDict, total=False):
id: Required[str]
"""ID of the data (same as target_id)"""

# TODO docname and lineno can be None, if the need is external,
# but currently this raises mypy errors for other parts of the code base
docname: Required[str]
"""Name of the document where the need is defined."""
lineno: Required[int]
"""Line number where the need is defined."""
docname: Required[str | None]
"""Name of the document where the need is defined (None if external)"""
lineno: Required[int | None]
"""Line number where the need is defined (None if external)"""

# meta information
full_title: Required[str]
Expand Down
7 changes: 2 additions & 5 deletions sphinx_needs/diagrams_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,12 +195,9 @@ def calculate_link(
if not parsed_url.scheme and not os.path.isabs(need_info["external_url"]):
# only need to add ../ or ..\ to get out of the image folder
link = ".." + os.path.sep + need_info["external_url"]
else:
elif _docname := need_info["docname"]:
link = (
"../"
+ builder.get_target_uri(need_info["docname"])
+ "#"
+ need_info["target_id"]
"../" + builder.get_target_uri(_docname) + "#" + need_info["target_id"]
)
if need_info["is_part"]:
link = f"{link}.{need_info['id']}"
Expand Down
30 changes: 15 additions & 15 deletions sphinx_needs/directives/needfilter.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import os
from contextlib import suppress
from typing import Sequence
from urllib.parse import urlparse

Expand Down Expand Up @@ -179,12 +180,10 @@
# Create a reference
if need_info["hide"]:
line_node += title
else:
elif _docname := need_info["docname"]:
ref = nodes.reference("", "")
ref["refdocname"] = need_info["docname"]
ref["refuri"] = builder.get_relative_uri(
fromdocname, need_info["docname"]
)
ref["refdocname"] = _docname
ref["refuri"] = builder.get_relative_uri(fromdocname, _docname)
ref["refuri"] += "#" + target_id
ref.append(title)
line_node += ref
Expand All @@ -211,16 +210,17 @@
# But the generated link in the svg will be relative to the svg-file location
# (e.g. server.com/docs/_images/sqwxo499cnq329439dfjne.svg)
# and not to current documentation. Therefore we need to add ../ to get out of the image folder.
try:
link = (
"../"
+ builder.get_target_uri(need_info["docname"])
+ "?highlight={}".format(urlparse(need_info["title"]))
+ "#"
+ target_id
) # Gets mostly called during latex generation
except NoUri:
link = ""
link = ""
with suppress(NoUri):

Check warning on line 214 in sphinx_needs/directives/needfilter.py

View check run for this annotation

Codecov / codecov/patch

sphinx_needs/directives/needfilter.py#L213-L214

Added lines #L213 - L214 were not covered by tests
# Gets mostly called during latex generation
if _docname := need_info["docname"]:
link = (

Check warning on line 217 in sphinx_needs/directives/needfilter.py

View check run for this annotation

Codecov / codecov/patch

sphinx_needs/directives/needfilter.py#L216-L217

Added lines #L216 - L217 were not covered by tests
"../"
+ builder.get_target_uri(_docname)
+ "?highlight={}".format(urlparse(need_info["title"]))
+ "#"
+ target_id
)

diagram_template = Template(needs_config.diagram_template)
node_text = diagram_template.render(
Expand Down
8 changes: 3 additions & 5 deletions sphinx_needs/directives/needlist.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,11 @@ def process_needlist(
ref["classes"].append(need_info["external_css"])
ref.append(title)
para += ref
else:
elif _docname := need_info["docname"]:
target_id = need_info["target_id"]
ref = nodes.reference("", "")
ref["refdocname"] = need_info["docname"]
ref["refuri"] = builder.get_relative_uri(
fromdocname, need_info["docname"]
)
ref["refdocname"] = _docname
ref["refuri"] = builder.get_relative_uri(fromdocname, _docname)
ref["refuri"] += "#" + target_id
ref.append(title)
para += ref
Expand Down
2 changes: 1 addition & 1 deletion sphinx_needs/functions/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ def copy(
NeedsSphinxConfig(app.config),
filter,
need,
location=(need["docname"], need["lineno"]),
location=(need["docname"], need["lineno"]) if need["docname"] else None,
)
if result:
need = result[0]
Expand Down
44 changes: 26 additions & 18 deletions sphinx_needs/layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from contextlib import suppress
from functools import lru_cache
from optparse import Values
from typing import Callable
from typing import Callable, cast
from urllib.parse import urlparse

import requests
Expand Down Expand Up @@ -63,7 +63,10 @@
# This is done for original need content automatically.
# But as we are working on a copy, we have to trigger this on our own.
if docname is None:
docname = needs[need_id]["docname"] # needed to calculate relative references
# needed to calculate relative references
# TODO ideally we should not cast here:
# the docname can still be None, if the need is external, although practically these are not rendered
docname = cast(str, needs[need_id]["docname"])

Check warning on line 69 in sphinx_needs/layout.py

View check run for this annotation

Codecov / codecov/patch

sphinx_needs/layout.py#L69

Added line #L69 was not covered by tests

node_container = nodes.container()
# node_container += needs[need_id]["need_node"].children
Expand Down Expand Up @@ -679,16 +682,16 @@
id_container = nodes.inline(classes=["needs-id"])

nodes_id_text = nodes.Text(self.need["id"])
id_ref = make_refnode(
self.app.builder,
# fromdocname=self.need['docname'],
fromdocname=self.fromdocname,
todocname=self.need["docname"],
targetid=self.need["id"],
child=nodes_id_text.deepcopy(),
title=self.need["id"],
)
id_container += id_ref
if self.fromdocname and (_docname := self.need["docname"]):
id_ref = make_refnode(
self.app.builder,
fromdocname=self.fromdocname,
todocname=_docname,
targetid=self.need["id"],
child=nodes_id_text.deepcopy(),
title=self.need["id"],
)
id_container += id_ref
return id_container

def meta_all(
Expand Down Expand Up @@ -924,8 +927,12 @@

url = value

if not is_external and not os.path.isabs(url):
subfolder_amount = self.need["docname"].count("/")
if (
not is_external
and not os.path.isabs(url)
and (docname := self.need["docname"])
):
subfolder_amount = docname.count("/")

Check warning on line 935 in sphinx_needs/layout.py

View check run for this annotation

Codecov / codecov/patch

sphinx_needs/layout.py#L935

Added line #L935 was not covered by tests
url = "../" * subfolder_amount + url

if is_external:
Expand Down Expand Up @@ -963,7 +970,8 @@
# Sphinx voodoo needed here.
# It is not enough to just add a doctuils nodes.image, we also have to register the imag location for sphinx
# Otherwise the images gets not copied to the later build-output location
env.images.add_file(self.need["docname"], url)
if _docname := self.need["docname"]:
env.images.add_file(_docname, url)

data_container.append(image_node)
return data_container
Expand Down Expand Up @@ -1133,10 +1141,10 @@

permalink = self.needs_config.permalink_file
id = self.need["id"]
docname = self.need["docname"]
permalink_url = ""
for _ in range(0, len(docname.split("/")) - 1):
permalink_url += "../"
if docname := self.need["docname"]:
for _ in range(0, len(docname.split("/")) - 1):
permalink_url += "../"

Check warning on line 1147 in sphinx_needs/layout.py

View check run for this annotation

Codecov / codecov/patch

sphinx_needs/layout.py#L1145-L1147

Added lines #L1145 - L1147 were not covered by tests
permalink_url += permalink + "?id=" + id

return self.link(
Expand Down
6 changes: 4 additions & 2 deletions sphinx_needs/roles/need_incoming.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,13 @@ def process_need_incoming(
# link_text += ", "
node_need_backref[0] = nodes.Text(link_text)

if not target_need["is_external"]:
if not target_need["is_external"] and (
_docname := target_need["docname"]
):
new_node_ref = make_refnode(
builder,
fromdocname,
target_need["docname"],
_docname,
target_need["target_id"],
node_need_backref[0].deepcopy(),
node_need_backref["reftarget"],
Expand Down
6 changes: 4 additions & 2 deletions sphinx_needs/roles/need_outgoing.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,13 @@ def process_need_outgoing(

node_need_ref[0] = nodes.Text(link_text)

if not target_need["is_external"]:
if not target_need["is_external"] and (
_docname := target_need["docname"]
):
new_node_ref = make_refnode(
builder,
fromdocname,
target_need["docname"],
_docname,
target_id,
node_need_ref[0].deepcopy(),
node_need_ref["reftarget"],
Expand Down
26 changes: 14 additions & 12 deletions sphinx_needs/roles/need_part.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from docutils import nodes
from sphinx.application import Sphinx
from sphinx.environment import BuildEnvironment
from sphinx.util.nodes import make_refnode

from sphinx_needs.data import NeedsInfoType
from sphinx_needs.logging import get_logger
Expand Down Expand Up @@ -59,7 +60,6 @@ def update_need_with_parts(
env: BuildEnvironment, need: NeedsInfoType, part_nodes: list[NeedPart]
) -> None:
app = env.app
builder = app.builder
for part_node in part_nodes:
content = cast(str, part_node.children[0].children[0]) # ->inline->Text
result = part_pattern.match(content)
Expand Down Expand Up @@ -91,24 +91,26 @@ def update_need_with_parts(
}

part_id_ref = "{}.{}".format(need["id"], inline_id)
part_id_show = inline_id

part_node["reftarget"] = part_id_ref

part_link_text = f" {part_id_show}"
part_link_node = nodes.Text(part_link_text)
part_text_node = nodes.Text(part_content)

from sphinx.util.nodes import make_refnode

part_ref_node = make_refnode(
builder, need["docname"], need["docname"], part_id_ref, part_link_node
)
part_ref_node["classes"] += ["needs-id"]

part_node.children = []
node_need_part_line = nodes.inline(ids=[part_id_ref], classes=["need-part"])
node_need_part_line.append(part_text_node)
node_need_part_line.append(part_ref_node)

if docname := need["docname"]:
part_id_show = inline_id
part_link_text = f" {part_id_show}"
part_link_node = nodes.Text(part_link_text)

part_ref_node = make_refnode(
app.builder, docname, docname, part_id_ref, part_link_node
)
part_ref_node["classes"] += ["needs-id"]
node_need_part_line.append(part_ref_node)

part_node.append(node_need_part_line)


Expand Down
6 changes: 4 additions & 2 deletions sphinx_needs/roles/need_ref.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,13 @@ def process_need_ref(
node_need_ref[0].children[0] = nodes.Text(link_text) # type: ignore[index]

with contextlib.suppress(NoUri):
if not target_need.get("is_external", False):
if not target_need.get("is_external", False) and (
_docname := target_need["docname"]
):
new_node_ref = make_refnode(
builder,
fromdocname,
target_need["docname"],
_docname,
node_need_ref["reftarget"],
node_need_ref[0].deepcopy(),
node_need_ref["reftarget"],
Expand Down
8 changes: 4 additions & 4 deletions sphinx_needs/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,9 @@ def row_col_maker(
)
ref_col["classes"].append(need_info["external_css"])
row_col["classes"].append(need_info["external_css"])
else:
elif _docname := need_info["docname"]:
ref_col["refuri"] = builder.get_relative_uri(
fromdocname, need_info["docname"]
fromdocname, _docname
)
ref_col["refuri"] += "#" + datum
elif ref_lookup:
Expand All @@ -231,9 +231,9 @@ def row_col_maker(
)
ref_col["classes"].append(temp_need["external_css"])
row_col["classes"].append(temp_need["external_css"])
else:
elif _docname := temp_need["docname"]:
ref_col["refuri"] = builder.get_relative_uri(
fromdocname, temp_need["docname"]
fromdocname, _docname
)
ref_col["refuri"] += "#" + temp_need["id"]
if link_part:
Expand Down
Loading