From 3bf706208171f0f4eadf9b0fe7fcc973bdf38403 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Fri, 1 Sep 2023 12:10:09 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20FIX:=20NeedImport=20logic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sphinx_needs/api/need.py | 21 ++++--- sphinx_needs/directives/need.py | 2 +- sphinx_needs/directives/needimport.py | 91 +++++++++++++-------------- 3 files changed, 56 insertions(+), 58 deletions(-) diff --git a/sphinx_needs/api/need.py b/sphinx_needs/api/need.py index cff05e3eb..8af32bfd7 100644 --- a/sphinx_needs/api/need.py +++ b/sphinx_needs/api/need.py @@ -193,13 +193,6 @@ def run(): if needs_config.id_regex and not re.match(needs_config.id_regex, need_id): raise NeedsInvalidException(f"Given ID '{need_id}' does not match configured regex '{needs_config.id_regex}'") - # Calculate target id, to be able to set a link back - if is_external: - target_node = None - else: - target_node = nodes.target("", "", ids=[need_id], refid=need_id, anonymous="") - external_url = None - # Handle status # Check if status is in needs_statuses. If not raise an error. if needs_config.statuses and status not in [stat["name"] for stat in needs_config.statuses]: @@ -310,7 +303,7 @@ def run(): "doctype": doctype, "lineno": lineno, "target_id": need_id, - "external_url": external_url, + "external_url": external_url if is_external else None, "content_node": None, # gets set after rst parsing "content_id": None, # gets set after rst parsing "type": need_type, @@ -443,8 +436,10 @@ def run(): node_need.line = needs_info["lineno"] if needs_info["hide"]: + # add node to doctree, so we can later compute the containing section(s) + # (for use with section filters) node_need["hidden"] = True - return [target_node, node_need] + return [node_need] node_need_content = _render_template(content, docname, lineno, state) @@ -485,7 +480,13 @@ def run(): # Create a copy of the content needs_info["content_node"] = node_need.deepcopy() - return_nodes = [target_node] + [node_need] + return_nodes = [node_need] + if not is_external: + # Calculate target id, to be able to set a link back + target_node = nodes.target("", "", ids=[need_id], refid=need_id, anonymous="") + # TODO add to document? + return_nodes = [target_node, node_need] + if pre_content: node_need_pre_content = _render_template(pre_content, docname, lineno, state) return_nodes = node_need_pre_content.children + return_nodes diff --git a/sphinx_needs/directives/need.py b/sphinx_needs/directives/need.py index fae4210d5..1bbb55fc6 100644 --- a/sphinx_needs/directives/need.py +++ b/sphinx_needs/directives/need.py @@ -548,7 +548,7 @@ def remove_hidden_needs(app: Sphinx, doctree: nodes.document, fromdocname: str) """Remove hidden needs from the doctree, before it is rendered.""" if fromdocname not in SphinxNeedsData(app.env).get_or_create_docs().get("all", []): return - for node_need in doctree.findall(Need): + for node_need in list(doctree.findall(Need)): if node_need.get("hidden"): node_need.parent.remove(node_need) # type: ignore diff --git a/sphinx_needs/directives/needimport.py b/sphinx_needs/directives/needimport.py index d7bf49f62..cdccd1409 100644 --- a/sphinx_needs/directives/needimport.py +++ b/sphinx_needs/directives/needimport.py @@ -1,7 +1,7 @@ import json import os import re -from typing import Sequence +from typing import Dict, Sequence from urllib.parse import urlparse import requests @@ -12,6 +12,7 @@ from sphinx_needs.api import add_need from sphinx_needs.config import NEEDS_CONFIG, NeedsSphinxConfig +from sphinx_needs.data import NeedsInfoType from sphinx_needs.debug import measure_time from sphinx_needs.filter_common import filter_single_need from sphinx_needs.needsfile import check_needs_file @@ -105,12 +106,11 @@ def run(self) -> Sequence[nodes.Node]: for error in errors.schema: logger.info(f' {error.message} -> {".".join(error.path)}') - with open(correct_need_import_path) as needs_file: - needs_file_content = needs_file.read() try: - needs_import_list = json.loads(needs_file_content) + with open(correct_need_import_path) as needs_file: + needs_import_list = json.load(needs_file) except json.JSONDecodeError as e: - # ToDo: Add exception handling + # TODO: Add exception handling raise e if version is None: @@ -123,8 +123,8 @@ def run(self) -> Sequence[nodes.Node]: if version not in needs_import_list["versions"].keys(): raise VersionNotFound(f"Version {version} not found in needs import file {correct_need_import_path}") - # TODO type this (it uncovers lots of bugs) - needs_list = needs_import_list["versions"][version]["needs"] + # TODO this is not exactly NeedsInfoType, because the export removes/adds some keys + needs_list: Dict[str, NeedsInfoType] = needs_import_list["versions"][version]["needs"] # Filter imported needs needs_list_filtered = {} @@ -136,7 +136,7 @@ def run(self) -> Sequence[nodes.Node]: # Support both ways of addressing the description, as "description" is used in json file, but # "content" is the sphinx internal name for this kind of information - filter_context["content"] = need["description"] + filter_context["content"] = need["description"] # type: ignore[typeddict-item] try: if filter_single_need(self.env.app, filter_context, filter_string): needs_list_filtered[key] = need @@ -158,70 +158,67 @@ def run(self) -> Sequence[nodes.Node]: for id in needs_list: # Manipulate links in all link types for extra_link in extra_links: - if extra_link["option"] in need and id in need[extra_link["option"]]: - for n, link in enumerate(need[extra_link["option"]]): + if extra_link["option"] in need and id in need[extra_link["option"]]: # type: ignore[literal-required] + for n, link in enumerate(need[extra_link["option"]]): # type: ignore[literal-required] if id == link: - need[extra_link["option"]][n] = "".join([id_prefix, id]) + need[extra_link["option"]][n] = "".join([id_prefix, id]) # type: ignore[literal-required] # Manipulate descriptions # ToDo: Use regex for better matches. - need["description"] = need["description"].replace(id, "".join([id_prefix, id])) + need["description"] = need["description"].replace(id, "".join([id_prefix, id])) # type: ignore[typeddict-item] # tags update for need in needs_list.values(): need["tags"] = need["tags"] + tags + known_options = ( + "title", + "status", + "content", + "id", + "tags", + "hide", + "template", + "pre_template", + "post_template", + "collapse", + "style", + "layout", + "need_type", + *[x["option"] for x in extra_links], + *NEEDS_CONFIG.extra_options, + ) need_nodes = [] for need in needs_list.values(): # Set some values based on given option or value from imported need. - need["template"] = self.options.get("template", getattr(need, "template", None)) - need["pre_template"] = self.options.get("pre_template", getattr(need, "pre_template", None)) - need["post_template"] = self.options.get("post_template", getattr(need, "post_template", None)) - need["layout"] = self.options.get("layout", getattr(need, "layout", None)) - need["style"] = self.options.get("style", getattr(need, "style", None)) - need["style"] = self.options.get("style", getattr(need, "style", None)) + need["template"] = self.options.get("template", need.get("template")) + need["pre_template"] = self.options.get("pre_template", need.get("pre_template")) + need["post_template"] = self.options.get("post_template", need.get("post_template")) + need["layout"] = self.options.get("layout", need.get("layout")) + need["style"] = self.options.get("style", need.get("style")) + if "hide" in self.options: need["hide"] = True else: - need["hide"] = getattr(need, "hide", None) - need["collapse"] = self.options.get("collapse", getattr(need, "collapse", None)) + need["hide"] = need.get("hide", False) + need["collapse"] = self.options.get("collapse", need.get("collapse")) # The key needs to be different for add_need() api call. - need["need_type"] = need["type"] + need["need_type"] = need["type"] # type: ignore[typeddict-unknown-key] # Replace id, to get unique ids need["id"] = id_prefix + need["id"] - need["content"] = need["description"] + need["content"] = need["description"] # type: ignore[typeddict-item] + # Remove unknown options, as they may be defined in source system, but not in this sphinx project - extra_link_keys = [x["option"] for x in extra_links] - extra_option_keys = list(NEEDS_CONFIG.extra_options) - default_options = [ - "title", - "status", - "content", - "id", - "tags", - "hide", - "template", - "pre_template", - "post_template", - "collapse", - "style", - "layout", - "need_type", - ] - delete_options = [] - for option in need.keys(): - if option not in default_options + extra_link_keys + extra_option_keys: - delete_options.append(option) - - for option in delete_options: - del need[option] + for option in list(need): + if option not in known_options: + del need[option] # type: ignore need["docname"] = self.docname need["lineno"] = self.lineno - nodes = add_need(self.env.app, self.state, **need) + nodes = add_need(self.env.app, self.state, **need) # type: ignore[call-arg] need_nodes.extend(nodes) add_doc(self.env, self.env.docname)