From 05652f70eab4fe05327b8d58531be2a6f6f39683 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Mon, 27 Jan 2025 14:12:11 +0100 Subject: [PATCH 1/4] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Make=20`needextend`=20?= =?UTF-8?q?argument=20deterministic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/directives/needextend.rst | 46 +++++++++--------- sphinx_needs/data.py | 8 +-- sphinx_needs/directives/needextend.py | 70 +++++++++++++-------------- 3 files changed, 61 insertions(+), 63 deletions(-) diff --git a/docs/directives/needextend.rst b/docs/directives/needextend.rst index dd0e4ed64..b6799ce1b 100644 --- a/docs/directives/needextend.rst +++ b/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:: :+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. diff --git a/sphinx_needs/data.py b/sphinx_needs/data.py index 2260572cd..ec248864b 100644 --- a/sphinx_needs/data.py +++ b/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 extebd.""" + 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). diff --git a/sphinx_needs/directives/needextend.py b/sphinx_needs/directives/needextend.py index 9db1e9d08..c2620c561 100644 --- a/sphinx_needs/directives/needextend.py +++ b/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 @@ -54,20 +53,37 @@ def run(self) -> Sequence[nodes.Node]: 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 = extend_filter.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 + 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 +107,28 @@ def extend_needs_data( 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: log_warning( logger, f"Invalid filter {need_filter!r}: {e}", "needextend", - location=( - current_needextend["docname"], - current_needextend["lineno"], - ), + location=location, ) continue From 330e049af87248bd7cc845f2e712df099d4cc4e7 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Mon, 27 Jan 2025 15:01:21 +0100 Subject: [PATCH 2/4] add warning tests --- sphinx_needs/directives/needextend.py | 33 +++++++++---------- .../doc_needextend_unknown_id/index.rst | 10 ++++-- tests/test_needextend.py | 15 +++++++-- 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/sphinx_needs/directives/needextend.py b/sphinx_needs/directives/needextend.py index c2620c561..e1c485415 100644 --- a/sphinx_needs/directives/needextend.py +++ b/sphinx_needs/directives/needextend.py @@ -10,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 @@ -43,16 +39,6 @@ class NeedextendDirective(SphinxDirective): 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}" - ) - needs_config = NeedsSphinxConfig(self.env.app.config) strict = needs_config.needextend_strict strict_option: str = self.options.get("strict", "").upper() @@ -64,7 +50,7 @@ def run(self) -> Sequence[nodes.Node]: modifications = self.options.copy() modifications.pop("strict", None) - extend_filter = extend_filter.strip() + 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] @@ -76,6 +62,19 @@ def run(self) -> Sequence[nodes.Node]: 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, @@ -123,7 +122,7 @@ def extend_needs_data( found_needs = filter_needs_mutable( 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}", diff --git a/tests/doc_test/doc_needextend_unknown_id/index.rst b/tests/doc_test/doc_needextend_unknown_id/index.rst index 9e87e0279..4e3959c11 100644 --- a/tests/doc_test/doc_needextend_unknown_id/index.rst +++ b/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:: +.. needextend:: "bad_filter" +.. needextend:: bad filter +.. needextend:: <> +.. needextend:: "" diff --git a/tests/test_needextend.py b/tests/test_needextend.py index 788391c04..ae82ec720 100644 --- a/tests/test_needextend.py +++ b/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, "/") + ).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]" + "/index.rst:25: WARNING: Empty ID/filter argument in needextend directive. [needs.needextend]", + "/index.rst:26: WARNING: Empty ID/filter argument in needextend directive. [needs.needextend]", + "/index.rst:19: WARNING: Provided id 'unknown_id' for needextend does not exist. [needs.needextend]", + "/index.rst:22: WARNING: Provided id 'id with space' for needextend does not exist. [needs.needextend]", + "/index.rst:23: WARNING: Filter 'bad_filter' not valid. Error: name 'bad_filter' is not defined. [needs.filter]", + "/index.rst:24: WARNING: Invalid filter 'bad filter': unexpected EOF while parsing (, line 1) [needs.needextend]", ] From 030155de4f971c5cede48ea9bb2e710ead7485af Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Mon, 27 Jan 2025 15:03:21 +0100 Subject: [PATCH 3/4] Update sphinx_needs/data.py Co-authored-by: Marco Heinemann --- sphinx_needs/data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx_needs/data.py b/sphinx_needs/data.py index ec248864b..aae4104a1 100644 --- a/sphinx_needs/data.py +++ b/sphinx_needs/data.py @@ -567,7 +567,7 @@ class NeedsExtendType(NeedsBaseDataType): """Data to modify existing need(s).""" filter: str - """Filter string to select needs to extebd.""" + """Filter string to select needs to extend.""" filter_is_id: bool """Whether the filter is a single need ID.""" modifications: dict[str, Any] From 290255953eac29950f1834599ad9c9af1f6a4b92 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Mon, 27 Jan 2025 15:10:35 +0100 Subject: [PATCH 4/4] update test --- tests/doc_test/doc_needextend_unknown_id/index.rst | 2 +- tests/test_needextend.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/doc_test/doc_needextend_unknown_id/index.rst b/tests/doc_test/doc_needextend_unknown_id/index.rst index 4e3959c11..570937185 100644 --- a/tests/doc_test/doc_needextend_unknown_id/index.rst +++ b/tests/doc_test/doc_needextend_unknown_id/index.rst @@ -21,6 +21,6 @@ needextend warnings .. needextend:: .. needextend:: "bad_filter" -.. needextend:: bad filter +.. needextend:: bad == filter .. needextend:: <> .. needextend:: "" diff --git a/tests/test_needextend.py b/tests/test_needextend.py index ae82ec720..2dc278692 100644 --- a/tests/test_needextend.py +++ b/tests/test_needextend.py @@ -62,14 +62,14 @@ def test_doc_needextend_warnings(test_app: Sphinx): warnings = strip_colors( app._warning.getvalue().replace(str(app.srcdir) + os.path.sep, "/") ).splitlines() - print(warnings) + # print(warnings) assert warnings == [ "/index.rst:25: WARNING: Empty ID/filter argument in needextend directive. [needs.needextend]", "/index.rst:26: WARNING: Empty ID/filter argument in needextend directive. [needs.needextend]", "/index.rst:19: WARNING: Provided id 'unknown_id' for needextend does not exist. [needs.needextend]", "/index.rst:22: WARNING: Provided id 'id with space' for needextend does not exist. [needs.needextend]", "/index.rst:23: WARNING: Filter 'bad_filter' not valid. Error: name 'bad_filter' is not defined. [needs.filter]", - "/index.rst:24: WARNING: Invalid filter 'bad filter': unexpected EOF while parsing (, line 1) [needs.needextend]", + "/index.rst:24: WARNING: Filter 'bad == filter' not valid. Error: name 'bad' is not defined. [needs.filter]", ]