♻️ Make needextend argument declarative

docs/directives/needextend.rst

@@ -4,7 +4,7 @@ needextend
 ==========
 .. versionadded:: 0.7.0
 
-``needextend`` allows to modify existing needs. It doesn’t provide any output, as the modifications
+``needextend`` allows to modify existing needs. It doesn't provide any output, as the modifications
 get presented at the original location of the changing need, for example:
 
 .. code-block:: rst
@@ -20,7 +20,12 @@ The following modifications are supported:
 * ``+option``: add new value to an existing value of an option.
 * ``-option``: delete a complete option.
 
-The argument of ``needextend`` must be a :ref:`filter_string` which defines the needs to modify.
+The argument of ``needextend`` will be taken as, by order of priority:
+
+- a single need ID, if it is enclosed by ``<>``,
+- a :ref:`filter_string` if it is enclosed by ``""``,
+- a single need ID, if it is a single word (no spaces),
+- a :ref:`filter_string` otherwise.
 
 ``needextend`` can modify all string-based and list-based options.
 Also, you can add links or delete tags.
@@ -40,11 +45,26 @@ Also, you can add links or delete tags.
       | And a tag was added.
       | Finally all links got removed.
 
-   .. needextend:: id == "extend_test_001"
+   .. req:: needextend Example 2
+      :id: extend_test_002
+      :tags: extend_example
+      :status: open
+
+      Contents
+
+   .. needextend:: extend_test_001
       :status: closed
       :+author: and me
+
+   .. needextend:: <extend_test_001>
       :+tags: new_tag
 
+   .. needextend:: id == "extend_test_002"
+      :status: New status
+
+   .. needextend:: ""extend_example" in tags"
+      :+tags: other
+
 Options
 -------
 
@@ -72,26 +92,6 @@ Default: false
     We have a configuration (conf.py) option called :ref:`needs_needextend_strict`
     that deactivates or activates the ``:strict:`` option behaviour for all ``needextend`` directives in a project.
 
-
-Single need modification
-------------------------
-If only one single need shall get modified, the argument of ``needextend`` can just be the need-id.
-
-.. need-example::
-
-    .. req:: needextend Example 2
-       :id: extend_test_002
-       :status: open
-
-    .. needextend:: extend_test_002
-       :status: New status
-
-.. attention::
-
-    The given argument must fully match the regular expression defined in
-    :ref:`needs_id_regex` and a need with this ID must exist!
-    Otherwise the argument is taken as normal filter string.
-
 Setting default option values
 -----------------------------
 You can use ``needextend``'s filter string to set default option values for a group of needs.

sphinx_needs/data.py

@@ -566,9 +566,11 @@ class NeedsBarType(NeedsBaseDataType):
 class NeedsExtendType(NeedsBaseDataType):
     """Data to modify existing need(s)."""
 
-    filter: None | str
-    """Single need ID or filter string to select multiple needs."""
-    modifications: dict[str, str]
+    filter: str
+    """Filter string to select needs to extend."""
+    filter_is_id: bool
+    """Whether the filter is a single need ID."""
+    modifications: dict[str, Any]
     """Mapping of field name to new value.
     If the field name starts with a ``+``, the new value is appended to the existing value.
     If the field name starts with a ``-``, the existing value is cleared (new value is ignored).

sphinx_needs/directives/needextend.py

@@ -1,6 +1,5 @@
 from __future__ import annotations
 
-import re
 from collections.abc import Sequence
 from typing import Any, Callable
 
@@ -11,11 +10,7 @@
 from sphinx_needs.api.exceptions import NeedsInvalidFilter
 from sphinx_needs.api.need import _split_list_with_dyn_funcs
 from sphinx_needs.config import NeedsSphinxConfig
-from sphinx_needs.data import (
-    NeedsExtendType,
-    NeedsMutable,
-    SphinxNeedsData,
-)
+from sphinx_needs.data import NeedsExtendType, NeedsMutable, SphinxNeedsData
 from sphinx_needs.filter_common import filter_needs_mutable
 from sphinx_needs.logging import get_logger, log_warning
 from sphinx_needs.utils import add_doc
@@ -44,30 +39,50 @@
     def run(self) -> Sequence[nodes.Node]:
         env = self.env
 
-        id = env.new_serialno("needextend")
-        targetid = f"needextend-{env.docname}-{id}"
-        targetnode = nodes.target("", "", ids=[targetid])
-
-        extend_filter = self.arguments[0] if self.arguments else None
-        if not extend_filter:
-            raise NeedsInvalidFilter(
-                f"Filter of needextend must be set. See {env.docname}:{self.lineno}"
-            )
-
-        strict = NeedsSphinxConfig(self.env.app.config).needextend_strict
+        needs_config = NeedsSphinxConfig(self.env.app.config)
+        strict = needs_config.needextend_strict
         strict_option: str = self.options.get("strict", "").upper()
         if strict_option == "TRUE":
             strict = True
         elif strict_option == "FALSE":
             strict = False
 
+        modifications = self.options.copy()
+        modifications.pop("strict", None)
+
+        extend_filter = (self.arguments[0] if self.arguments else "").strip()
+        if extend_filter.startswith("<") and extend_filter.endswith(">"):
+            filter_is_id = True
+            extend_filter = extend_filter[1:-1]
+        elif extend_filter.startswith('"') and extend_filter.endswith('"'):
+            filter_is_id = False
+            extend_filter = extend_filter[1:-1]
+        elif len(extend_filter.split()) == 1:
+            filter_is_id = True
+        else:
+            filter_is_id = False
+
+        if not extend_filter:
+            log_warning(
+                logger,
+                "Empty ID/filter argument in needextend directive.",
+                "needextend",
+                location=self.get_location(),
+            )
+            return []
+
+        id = env.new_serialno("needextend")
+        targetid = f"needextend-{env.docname}-{id}"
+        targetnode = nodes.target("", "", ids=[targetid])
+
         data = SphinxNeedsData(env).get_or_create_extends()
         data[targetid] = {
             "docname": env.docname,
             "lineno": self.lineno,
             "target_id": targetid,
-            "filter": self.arguments[0] if self.arguments else None,
-            "modifications": self.options,
+            "filter": extend_filter,
+            "filter_is_id": filter_is_id,
+            "modifications": modifications,
             "strict": strict,
         }
 
@@ -91,48 +106,28 @@
 
     for current_needextend in extends.values():
         need_filter = current_needextend["filter"]
-        if need_filter and need_filter in all_needs:
-            # a single known ID
-            found_needs = [all_needs[need_filter]]
-        elif need_filter is not None and re.fullmatch(
-            needs_config.id_regex, need_filter
-        ):
-            # an unknown ID
-            error = f"Provided id {need_filter!r} for needextend does not exist."
-            if current_needextend["strict"]:
-                raise NeedsInvalidFilter(error)
-            else:
-                log_warning(
-                    logger,
-                    error,
-                    "needextend",
-                    location=(
-                        current_needextend["docname"],
-                        current_needextend["lineno"],
-                    ),
-                )
-            continue
+        location = (current_needextend["docname"], current_needextend["lineno"])
+        if current_needextend["filter_is_id"]:
+            try:
+                found_needs = [all_needs[need_filter]]
+            except KeyError:
+                error = f"Provided id {need_filter!r} for needextend does not exist."
+                if current_needextend["strict"]:
+                    raise NeedsInvalidFilter(error)
+                else:
+                    log_warning(logger, error, "needextend", location=location)
+                continue
         else:
-            # a filter string
             try:
                 found_needs = filter_needs_mutable(
-                    all_needs,
-                    needs_config,
-                    need_filter,
-                    location=(
-                        current_needextend["docname"],
-                        current_needextend["lineno"],
-                    ),
+                    all_needs, needs_config, need_filter, location=location
                 )
-            except NeedsInvalidFilter as e:
+            except Exception as e:
                 log_warning(
                     logger,
                     f"Invalid filter {need_filter!r}: {e}",
                     "needextend",
-                    location=(
-                        current_needextend["docname"],
-                        current_needextend["lineno"],
-                    ),
+                    location=location,
                 )
                 continue
 

tests/doc_test/doc_needextend_unknown_id/index.rst

@@ -1,5 +1,5 @@
-needextend unknown id
-=====================
+needextend warnings
+===================
 
 .. story:: needextend Example 3
    :id: extend_test_003
@@ -18,3 +18,9 @@ needextend unknown id
 
 .. needextend:: unknown_id
    :status: open
+
+.. needextend:: <id with space>
+.. needextend:: "bad_filter"
+.. needextend::  bad == filter
+.. needextend:: <>
+.. needextend:: ""

tests/test_needextend.py

@@ -1,4 +1,5 @@
 import json
+import os
 from pathlib import Path
 
 import pytest
@@ -54,13 +55,21 @@ def test_doc_needextend_html(test_app: Sphinx, snapshot):
     ],
     indirect=True,
 )
-def test_doc_needextend_unknown_id(test_app: Sphinx):
+def test_doc_needextend_warnings(test_app: Sphinx):
     app = test_app
     app.build()
 
-    warnings = strip_colors(app._warning.getvalue()).splitlines()
+    warnings = strip_colors(
+        app._warning.getvalue().replace(str(app.srcdir) + os.path.sep, "<srcdir>/")
+    ).splitlines()
+    # print(warnings)
     assert warnings == [
-        f"{Path(str(app.srcdir)) / 'index.rst'}:19: WARNING: Provided id 'unknown_id' for needextend does not exist. [needs.needextend]"
+        "<srcdir>/index.rst:25: WARNING: Empty ID/filter argument in needextend directive. [needs.needextend]",
+        "<srcdir>/index.rst:26: WARNING: Empty ID/filter argument in needextend directive. [needs.needextend]",
+        "<srcdir>/index.rst:19: WARNING: Provided id 'unknown_id' for needextend does not exist. [needs.needextend]",
+        "<srcdir>/index.rst:22: WARNING: Provided id 'id with space' for needextend does not exist. [needs.needextend]",
+        "<srcdir>/index.rst:23: WARNING: Filter 'bad_filter' not valid. Error: name 'bad_filter' is not defined. [needs.filter]",
+        "<srcdir>/index.rst:24: WARNING: Filter 'bad == filter' not valid. Error: name 'bad' is not defined. [needs.filter]",
     ]