🐛 FIX: Centralise splitting of need ID (useblocks#1101)

Ensure the splitting of a need ID, on a `.` into `main.part` IDs, cannot
except (and remove duplication of the logic)

closes useblocks#1096

Author: chrisjsewell

sphinx_needs/directives/need.py

@@ -28,7 +28,7 @@
 from sphinx_needs.logging import get_logger
 from sphinx_needs.need_constraints import process_constraints
 from sphinx_needs.nodes import Need
-from sphinx_needs.utils import add_doc, profile, remove_node_from_tree
+from sphinx_needs.utils import add_doc, profile, remove_node_from_tree, split_need_id
 
 logger = get_logger(__name__)
 
@@ -440,17 +440,14 @@ def check_links(needs: Dict[str, NeedsInfoType], config: NeedsSphinxConfig) -> N
     for need in needs.values():
         for link_type in extra_links:
             dead_links_allowed = link_type.get("allow_dead_links", False)
-            need_link_value = (
+            need_link_value: List[str] = (
                 [need[link_type["option"]]] if isinstance(need[link_type["option"]], str) else need[link_type["option"]]  # type: ignore
             )
-            for link in need_link_value:
-                if "." in link:
-                    need_id, need_part_id = link.split(".")
-                else:
-                    need_id = link
-                    need_part_id = None
-                if need_id not in needs or (
-                    need_id in needs and need_part_id and need_part_id not in needs[need_id]["parts"]
+            for need_id_full in need_link_value:
+                need_id_main, need_id_part = split_need_id(need_id_full)
+
+                if need_id_main not in needs or (
+                    need_id_main in needs and need_id_part and need_id_part not in needs[need_id_main]["parts"]
                 ):
                     need["has_dead_links"] = True
                     if not dead_links_allowed:
@@ -469,23 +466,19 @@ def create_back_links(needs: Dict[str, NeedsInfoType], config: NeedsSphinxConfig
         option_back = f"{option}_back"
 
         for key, need in needs.items():
-            need_link_value = [need[option]] if isinstance(need[option], str) else need[option]  # type: ignore[literal-required]
-            for link in need_link_value:
-                link_main = link.split(".")[0]
-                try:
-                    link_part = link.split(".")[1]
-                except IndexError:
-                    link_part = None
-
-                if link_main in needs:
-                    if key not in needs[link_main][option_back]:  # type: ignore[literal-required]
-                        needs[link_main][option_back].append(key)  # type: ignore[literal-required]
+            need_link_value: List[str] = [need[option]] if isinstance(need[option], str) else need[option]  # type: ignore[literal-required]
+            for need_id_full in need_link_value:
+                need_id_main, need_id_part = split_need_id(need_id_full)
+
+                if need_id_main in needs:
+                    if key not in needs[need_id_main][option_back]:  # type: ignore[literal-required]
+                        needs[need_id_main][option_back].append(key)  # type: ignore[literal-required]
 
                     # Handling of links to need_parts inside a need
-                    if link_part and link_part in needs[link_main]["parts"]:
-                        if option_back not in needs[link_main]["parts"][link_part].keys():
-                            needs[link_main]["parts"][link_part][option_back] = []  # type: ignore[literal-required]
-                        needs[link_main]["parts"][link_part][option_back].append(key)  # type: ignore[literal-required]
+                    if need_id_part and need_id_part in needs[need_id_main]["parts"]:
+                        if option_back not in needs[need_id_main]["parts"][need_id_part].keys():
+                            needs[need_id_main]["parts"][need_id_part][option_back] = []  # type: ignore[literal-required]
+                        needs[need_id_main]["parts"][need_id_part][option_back].append(key)  # type: ignore[literal-required]
 
 
 def _fix_list_dyn_func(list: List[str]) -> List[str]:

sphinx_needs/roles/need_outgoing.py

@@ -8,7 +8,7 @@
 from sphinx_needs.data import SphinxNeedsData
 from sphinx_needs.errors import NoUri
 from sphinx_needs.logging import get_logger
-from sphinx_needs.utils import check_and_calc_base_url_rel_path
+from sphinx_needs.utils import check_and_calc_base_url_rel_path, split_need_id
 
 log = get_logger(__name__)
 
@@ -41,24 +41,21 @@ def process_need_outgoing(
 
         link_list = [links] if isinstance(links, str) else links
 
-        for index, link in enumerate(link_list):
-            link_split = link.split(".")
-            link = link_split[0]
-            try:
-                link_part = link_split[1]
-            except IndexError:
-                link_part = None
+        for index, need_id_full in enumerate(link_list):
+            need_id_main, need_id_part = split_need_id(need_id_full)
 
             # If the need target exists, let's create the reference
-            if (link in needs_all_needs and not link_part) or (
-                link_part and link in needs_all_needs and link_part in needs_all_needs[link]["parts"]
+            if (need_id_main in needs_all_needs and not need_id_part) or (
+                need_id_part
+                and need_id_main in needs_all_needs
+                and need_id_part in needs_all_needs[need_id_main]["parts"]
             ):
                 try:
-                    target_need = needs_all_needs[link]
-                    if link_part and link_part in target_need["parts"]:
-                        part_content = target_need["parts"][link_part]["content"]
+                    target_need = needs_all_needs[need_id_main]
+                    if need_id_part and need_id_part in target_need["parts"]:
+                        part_content = target_need["parts"][need_id_part]["content"]
                         target_title = part_content if len(part_content) < 30 else part_content[:27] + "..."
-                        target_id = ".".join([link, link_part])
+                        target_id = ".".join([need_id_main, need_id_part])
                     else:
                         target_title = target_need["title"]
                         target_id = target_need["id"]
@@ -102,9 +99,9 @@ def process_need_outgoing(
             else:
                 # Let's add a normal text here instead of a link.
                 # So really each link set by the user gets shown.
-                link_text = f"{link}"
-                if link_part:
-                    link_text += f".{link_part}"
+                link_text = f"{need_id_main}"
+                if need_id_part:
+                    link_text += f".{need_id_part}"
                 dead_link_text = nodes.Text(link_text)
                 dead_link_para = nodes.inline(classes=["needs_dead_link"])
                 dead_link_para.append(dead_link_text)
@@ -130,7 +127,7 @@ def process_need_outgoing(
                     if node_need_ref and node_need_ref.line:
                         log.log(
                             log_level,
-                            f"linked need {link} not found "
+                            f"linked need {need_id_main} not found "
                             f"(Line {node_need_ref.line} of file {node_need_ref.source}) [needs]",
                             **kwargs,
                         )
@@ -139,7 +136,7 @@ def process_need_outgoing(
                             log_level,
                             "outgoing linked need {} not found (document: {}, "
                             "source need {} on line {} ) [needs]".format(
-                                link, ref_need["docname"], ref_need["id"], ref_need["lineno"]
+                                need_id_main, ref_need["docname"], ref_need["id"], ref_need["lineno"]
                             ),
                             **kwargs,
                         )

sphinx_needs/roles/need_ref.py

@@ -10,7 +10,7 @@
 from sphinx_needs.data import NeedsInfoType, SphinxNeedsData
 from sphinx_needs.errors import NoUri
 from sphinx_needs.logging import get_logger
-from sphinx_needs.utils import check_and_calc_base_url_rel_path
+from sphinx_needs.utils import check_and_calc_base_url_rel_path, split_need_id
 
 log = get_logger(__name__)
 
@@ -71,25 +71,20 @@ def process_need_ref(app: Sphinx, doctree: nodes.document, fromdocname: str, fou
         prefix = "[["
         postfix = "]]"
 
-        ref_id_complete = node_need_ref["reftarget"]
+        need_id_full = node_need_ref["reftarget"]
+        need_id_main, need_id_part = split_need_id(need_id_full)
 
-        if "." in ref_id_complete:
-            ref_id, part_id = ref_id_complete.split(".")
-        else:
-            ref_id = ref_id_complete
-            part_id = None
-
-        if ref_id in all_needs:
-            target_need = all_needs[ref_id]
+        if need_id_main in all_needs:
+            target_need = all_needs[need_id_main]
 
             dict_need = transform_need_to_dict(target_need)  # Transform a dict in a dict of {str, str}
 
             # We set the id to the complete id maintained in node_need_ref["reftarget"]
-            dict_need["id"] = ref_id_complete
+            dict_need["id"] = need_id_full
 
-            if part_id:
+            if need_id_part:
                 # If part_id, we have to fetch the title from the content.
-                dict_need["title"] = target_need["parts"][part_id]["content"]
+                dict_need["title"] = target_need["parts"][need_id_part]["content"]
 
             # Shorten title, if necessary
             max_length = needs_config.role_need_max_title_length
@@ -100,7 +95,7 @@ def process_need_ref(app: Sphinx, doctree: nodes.document, fromdocname: str, fou
 
             ref_name: Union[None, str, nodes.Text] = node_need_ref.children[0].children[0]  # type: ignore[assignment]
             # Only use ref_name, if it differs from ref_id
-            if str(ref_id_complete) == str(ref_name):
+            if str(need_id_full) == str(ref_name):
                 ref_name = None
 
             if ref_name and prefix in ref_name and postfix in ref_name:

sphinx_needs/utils.py

@@ -109,6 +109,22 @@ class NeedFunctionsType(TypedDict):
 ]
 
 
+def split_need_id(need_id_full: str) -> Tuple[str, Optional[str]]:
+    """A need id can be a combination of a main id and a part id,
+    split by a dot.
+    This function splits them:
+    If there is no dot, the part id is None,
+    otherwise everything before the first dot is the main id,
+    and everything after the first dot is the part id.
+    """
+    if "." in need_id_full:
+        need_id, need_part_id = need_id_full.split(".", maxsplit=1)
+    else:
+        need_id = need_id_full
+        need_part_id = None
+    return need_id, need_part_id
+
+
 def row_col_maker(
     app: Sphinx,
     fromdocname: str,