From 976dcd52130d7edcca6b1b073f665fca5cf823da Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Wed, 18 Sep 2024 10:00:26 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Add=20`ndf`=20role,=20deprecate=20`?= =?UTF-8?q?need=5Ffunc`=20&=20`[[...]]`=20in=20need=20content=20(#1269)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There were previously two methods to use dynamic functions within need directive's content: 1. ``[[copy("id")]]``; is problematic, because it does not adhere to the rst / myst syntax specification, and has already shown to cause parsing issues and be surprising to users 2. `` :need_func:`[[copy("id")]]` ``; is better but overly verbose In this commit, these are replaced by the role: `` :ndf:`copy("id")` `` Here we take the entire content to be the function, as so do not require the `[[]]`, reducing verbosity and processing The other two methods now emit a deprecation warning, showing how to convert them to the new role. --- docs/directives/needextend.rst | 4 +- docs/dynamic_functions.rst | 25 ++++++----- docs/needs_templates/spec_template.need | 2 +- docs/roles.rst | 14 ++++-- sphinx_needs/directives/needextend.py | 5 ++- sphinx_needs/directives/needextract.py | 3 +- sphinx_needs/functions/common.py | 14 +++--- sphinx_needs/functions/functions.py | 15 +++++-- sphinx_needs/needs.py | 3 +- sphinx_needs/roles/need_func.py | 44 ++++++++++++++++--- .../doc_test/doc_dynamic_functions/index.rst | 6 ++- .../needextract_with_nested_needs/index.rst | 4 +- tests/test_dynamic_functions.py | 18 +++++++- tests/test_global_options.py | 2 +- tests/test_needextract.py | 18 ++++++-- 15 files changed, 132 insertions(+), 45 deletions(-) diff --git a/docs/directives/needextend.rst b/docs/directives/needextend.rst index c9a4c298a..dd0e4ed64 100644 --- a/docs/directives/needextend.rst +++ b/docs/directives/needextend.rst @@ -35,8 +35,8 @@ Also, you can add links or delete tags. This requirement got modified. - | Status was **open**, now it is **[[copy('status')]]**. - | Also author got changed from **Foo** to **[[copy('author')]]**. + | Status was **open**, now it is :ndf:`copy('status')`. + | Also author got changed from **Foo** to :ndf:`copy('author')`. | And a tag was added. | Finally all links got removed. diff --git a/docs/dynamic_functions.rst b/docs/dynamic_functions.rst index 2f4e36b24..443818c90 100644 --- a/docs/dynamic_functions.rst +++ b/docs/dynamic_functions.rst @@ -3,32 +3,35 @@ Dynamic functions ================= -**Sphinx-Needs** provides a mechanism to set dynamic data for need-options during generation. -We do this by giving an author the possibility to set a function call to a predefined function, which calculates -the final result/value for the option. +Dynamic functions provide a mechanism to specify need fields or content that are calculated at build time, based on other fields or needs. + +We do this by giving an author the possibility to set a function call to a predefined function, which calculates the final value **after all needs have been collected**. For instance, you can use the feature if the status of a requirement depends on linked test cases and their status. Or if you will request specific data from an external server like JIRA. -**needtable** - -The options :ref:`needtable_style_row` of :ref:`needtable` also support -dynamic function execution. In this case, the function gets executed with the found need for each row. - -This allows you to set row and column specific styles such as, set a row background to red, if a need-status is *failed*. +To refer to a dynamic function, you can use the following syntax: +- In a need directive option, wrap the function call in double square brackets: ``function_name(arg)`` +- In a need content, use the :ref:`ndf` role: ``:ndf:\`function_name(arg)\``` .. need-example:: Dynamic function example .. req:: my test requirement :id: df_1 :status: open + :tags: test;[[copy("status")]] - This need has id **[[copy("id")]]** and status **[[copy("status")]]**. + This need has id :ndf:`copy("id")` and status :ndf:`copy("status")`. + +.. deprecated:: 3.1.0 + + The :ref:`ndf` role replaces the use of the ``[[...]]`` syntax in need content. Built-in functions ------------------- -The following functions are available in all **Sphinx-Needs** installations. + +The following functions are available by default. .. note:: diff --git a/docs/needs_templates/spec_template.need b/docs/needs_templates/spec_template.need index 87d395c74..2c2115ff1 100644 --- a/docs/needs_templates/spec_template.need +++ b/docs/needs_templates/spec_template.need @@ -19,5 +19,5 @@ Tags: {# by using dynamic_functions #} Links: {% for link in links %} -| **{{link}}**: [[copy('title', '{{link}}')]] ([[copy('type_name', '{{link}}')]]) +| **{{link}}**: :ndf:`copy('title', '{{link}}')` (:ndf:`copy('type_name', '{{link}}')`) {%- endfor %} diff --git a/docs/roles.rst b/docs/roles.rst index 7c8e8d2cc..428da03df 100644 --- a/docs/roles.rst +++ b/docs/roles.rst @@ -188,10 +188,18 @@ To calculate the ratio of one filter to another filter, you can define two filte need_func --------- -.. versionadded:: 0.6.3 +.. deprecated:: 3.1.0 -Executes :ref:`dynamic_functions` and uses the return values as content. + Use :ref:`ndf` instead. + +.. _ndf: + +ndf +--- +.. versionadded:: 3.1.0 + +Executes a :ref:`need dynamic function ` and uses the return values as content. .. need-example:: - A nice :need_func:`[[echo("first")]] test` for need_func. + A nice :ndf:`echo("first test")` for dynamic functions. diff --git a/sphinx_needs/directives/needextend.py b/sphinx_needs/directives/needextend.py index 0f119d20b..37be5f2a7 100644 --- a/sphinx_needs/directives/needextend.py +++ b/sphinx_needs/directives/needextend.py @@ -71,7 +71,10 @@ def run(self) -> Sequence[nodes.Node]: add_doc(env, env.docname) - return [targetnode, Needextend("")] + node = Needextend("") + self.set_source_info(node) + + return [targetnode, node] RE_ID_FUNC = re.compile(r"\s*((?P\[\[[^\]]*\]\])|(?P[^;,]+))\s*([;,]|$)") diff --git a/sphinx_needs/directives/needextract.py b/sphinx_needs/directives/needextract.py index aaae9678d..e3be851a8 100644 --- a/sphinx_needs/directives/needextract.py +++ b/sphinx_needs/directives/needextract.py @@ -196,6 +196,8 @@ def _build_needextract( dummy_need.extend(need_node.children) + find_and_replace_node_content(dummy_need, env, need_data) + # resolve_references() ignores the given docname and takes the docname from the pending_xref node. # Therefore, we need to manipulate this first, before we can ask Sphinx to perform the normal # reference handling for us. @@ -203,7 +205,6 @@ def _build_needextract( env.resolve_references(dummy_need, extract_data["docname"], app.builder) # type: ignore[arg-type] dummy_need.attributes["ids"].append(need_data["id"]) - find_and_replace_node_content(dummy_need, env, need_data) rendered_node = build_need_repr( dummy_need, # type: ignore[arg-type] need_data, diff --git a/sphinx_needs/functions/common.py b/sphinx_needs/functions/common.py index 5fa4a32aa..105d21565 100644 --- a/sphinx_needs/functions/common.py +++ b/sphinx_needs/functions/common.py @@ -39,16 +39,16 @@ def test( .. req:: test requirement - [[test('arg_1', [1,2,3], my_keyword='awesome')]] + :ndf:`test('arg_1', [1,2,3], my_keyword='awesome')` .. req:: test requirement - [[test('arg_1', [1,2,3], my_keyword='awesome')]] + :ndf:`test('arg_1', [1,2,3], my_keyword='awesome')` :return: single test string """ need_id = "none" if need is None else need["id"] - return f"Test output of need_func; need: {need_id}; args: {args}; kwargs: {kwargs}" + return f"Test output of dynamic function; need: {need_id}; args: {args}; kwargs: {kwargs}" def echo( @@ -67,9 +67,9 @@ def echo( .. code-block:: jinja - A nice :need_func:`[[echo("first")]] test` for need_func. + A nice :ndf:`echo("first test")` for a dynamic function. - **Result**: A nice :need_func:`[[echo("first")]] test` for need_func. + **Result**: A nice :ndf:`echo("first test")` for a dynamic function. """ return text @@ -146,7 +146,7 @@ def copy( The following copy command copies the title of the first need found under the same highest section (headline): - [[copy('title', filter='current_need["sections"][-1]==sections[-1]')]] + :ndf:`copy('title', filter='current_need["sections"][-1]==sections[-1]')` .. test:: test of current_need value :id: copy_4 @@ -154,7 +154,7 @@ def copy( The following copy command copies the title of the first need found under the same highest section (headline): - [[copy('title', filter='current_need["sections"][-1]==sections[-1]')]] + :ndf:`copy('title', filter='current_need["sections"][-1]==sections[-1]')` This filter possibilities get really powerful in combination with :ref:`needs_global_options`. diff --git a/sphinx_needs/functions/functions.py b/sphinx_needs/functions/functions.py index 38926a40c..a849e87cb 100644 --- a/sphinx_needs/functions/functions.py +++ b/sphinx_needs/functions/functions.py @@ -152,7 +152,7 @@ def execute_func( return func_return -func_pattern = re.compile(r"\[\[(.*?)\]\]") # RegEx to detect function strings +FUNC_RE = re.compile(r"\[\[(.*?)\]\]") # RegEx to detect function strings def find_and_replace_node_content( @@ -163,6 +163,9 @@ def find_and_replace_node_content( if found, check if it contains a function string and run/replace it. :param node: Node to analyse + :param env: Sphinx environment + :param need: Need data + :param extract: If True, the function has been called from a needextract node """ new_children = [] if isinstance(node, NeedFunc): @@ -181,7 +184,7 @@ def find_and_replace_node_content( return node else: new_text = node - func_match = func_pattern.findall(new_text) + func_match = FUNC_RE.findall(new_text) for func_string in func_match: # sphinx is replacing ' and " with language specific quotation marks (up and down), which makes # it impossible for the later used AST render engine to detect a python function call in the given @@ -194,6 +197,10 @@ def find_and_replace_node_content( func_string = func_string.replace("‘", "'") # noqa: RUF001 func_string = func_string.replace("’", "'") # noqa: RUF001 + + msg = f"The [[{func_string}]] syntax in need content is deprecated. Replace with :ndf:`{func_string}` instead." + log_warning(logger, msg, "deprecation", location=node) + func_return = execute_func(env.app, need, func_string, node) if isinstance(func_return, list): @@ -388,7 +395,7 @@ def check_and_get_content( :param location: source location of the function call :return: string """ - func_match = func_pattern.search(content) + func_match = FUNC_RE.search(content) if func_match is None: return content @@ -416,7 +423,7 @@ def _detect_and_execute_field( except UnicodeEncodeError: content = content.encode("utf-8") - func_match = func_pattern.search(content) + func_match = FUNC_RE.search(content) if func_match is None: return None, None diff --git a/sphinx_needs/needs.py b/sphinx_needs/needs.py index 399c44498..44051b063 100644 --- a/sphinx_needs/needs.py +++ b/sphinx_needs/needs.py @@ -254,7 +254,8 @@ def setup(app: Sphinx) -> dict[str, Any]: ), ) - app.add_role("need_func", NeedFuncRole()) + app.add_role("need_func", NeedFuncRole(with_brackets=True)) # deprecrated + app.add_role("ndf", NeedFuncRole(with_brackets=False)) ######################################################################## # EVENTS diff --git a/sphinx_needs/roles/need_func.py b/sphinx_needs/roles/need_func.py index 16c7b3646..309f883d3 100644 --- a/sphinx_needs/roles/need_func.py +++ b/sphinx_needs/roles/need_func.py @@ -1,5 +1,5 @@ """ -Provide the role ``need_func``, which executes a dynamic function. +Provide a role which executes a dynamic function. """ from __future__ import annotations @@ -10,30 +10,64 @@ from sphinx.util.docutils import SphinxRole from sphinx_needs.data import NeedsInfoType -from sphinx_needs.logging import get_logger +from sphinx_needs.logging import get_logger, log_warning from sphinx_needs.utils import add_doc -log = get_logger(__name__) +LOGGER = get_logger(__name__) class NeedFuncRole(SphinxRole): """Role for creating ``NeedFunc`` node.""" + def __init__(self, *, with_brackets: bool = False) -> None: + """Initialize the role. + + :param with_brackets: If True, the function is expected to be wrapped in brackets ``[[]]``. + """ + self.with_brackets = with_brackets + super().__init__() + def run(self) -> tuple[list[nodes.Node], list[nodes.system_message]]: add_doc(self.env, self.env.docname) node = NeedFunc( - self.rawtext, nodes.literal(self.rawtext, self.text), **self.options + self.rawtext, + nodes.literal(self.rawtext, self.text), + with_brackets=self.with_brackets, + **self.options, ) self.set_source_info(node) + if self.with_brackets: + from sphinx_needs.functions.functions import FUNC_RE + + msg = "The `need_func` role is deprecated. " + if func_match := FUNC_RE.search(node.astext()): + func_call = func_match.group(1) + msg += f"Replace with :ndf:`{func_call}` instead." + else: + msg += "Replace with ndf role instead." + log_warning(LOGGER, msg, "deprecation", location=node) return [node], [] class NeedFunc(nodes.Inline, nodes.Element): + @property + def with_brackets(self) -> bool: + """Return the function with brackets.""" + return self.get("with_brackets", False) # type: ignore[no-any-return] + def get_text(self, env: BuildEnvironment, need: NeedsInfoType | None) -> nodes.Text: """Execute function and return result.""" - from sphinx_needs.functions.functions import check_and_get_content + from sphinx_needs.functions.functions import check_and_get_content, execute_func + + if not self.with_brackets: + func_return = execute_func(env.app, need, self.astext(), self) + if isinstance(func_return, list): + func_return = ", ".join(str(el) for el in func_return) + + return nodes.Text("" if func_return is None else str(func_return)) result = check_and_get_content(self.astext(), need, env, self) + return nodes.Text(str(result)) diff --git a/tests/doc_test/doc_dynamic_functions/index.rst b/tests/doc_test/doc_dynamic_functions/index.rst index dd03f8815..85d90e2a2 100644 --- a/tests/doc_test/doc_dynamic_functions/index.rst +++ b/tests/doc_test/doc_dynamic_functions/index.rst @@ -10,6 +10,8 @@ DYNAMIC FUNCTIONS This is also id :need_func:`[[copy("id")]]` + This is the best id :ndf:`copy("id")` + .. spec:: TEST_2 :id: TEST_2 :tags: my_tag; [[copy("tags", "SP_TOO_001")]] @@ -37,4 +39,6 @@ DYNAMIC FUNCTIONS nested id also :need_func:`[[copy("id")]]` -This should warn since it has no associated need: :need_func:`[[copy("id")]]` + nested id best :ndf:`copy("id")` + +These should warn since they have no associated need: :need_func:`[[copy("id")]]`, :ndf:`copy("id")` diff --git a/tests/doc_test/needextract_with_nested_needs/index.rst b/tests/doc_test/needextract_with_nested_needs/index.rst index df3bcd86f..d98b7d875 100644 --- a/tests/doc_test/needextract_with_nested_needs/index.rst +++ b/tests/doc_test/needextract_with_nested_needs/index.rst @@ -10,7 +10,7 @@ Test Another, child spec - This is id [[copy("id")]] + This is id [[copy("id")]] :ndf:`copy("id")` .. spec:: Child spec :id: SPEC_1_1 @@ -30,6 +30,6 @@ Test awesome grandchild spec number 2. - This is grandchild id [[copy("id")]] + This is grandchild id [[copy("id")]] :ndf:`copy("id")` Some parent text \ No newline at end of file diff --git a/tests/test_dynamic_functions.py b/tests/test_dynamic_functions.py index 6de4265ca..caef56c76 100644 --- a/tests/test_dynamic_functions.py +++ b/tests/test_dynamic_functions.py @@ -26,12 +26,21 @@ def test_doc_dynamic_functions(test_app): app._warning.getvalue().replace(str(app.srcdir) + os.sep, "srcdir/") ).splitlines() assert warnings == [ - "srcdir/index.rst:40: WARNING: Error while executing function 'copy': Need not found [needs.dynamic_function]" + 'srcdir/index.rst:11: WARNING: The `need_func` role is deprecated. Replace with :ndf:`copy("id")` instead. [needs.deprecation]', + 'srcdir/index.rst:40: WARNING: The `need_func` role is deprecated. Replace with :ndf:`copy("id")` instead. [needs.deprecation]', + 'srcdir/index.rst:44: WARNING: The `need_func` role is deprecated. Replace with :ndf:`copy("id")` instead. [needs.deprecation]', + 'srcdir/index.rst:9: WARNING: The [[copy("id")]] syntax in need content is deprecated. Replace with :ndf:`copy("id")` instead. [needs.deprecation]', + 'srcdir/index.rst:27: WARNING: The [[copy("tags")]] syntax in need content is deprecated. Replace with :ndf:`copy("tags")` instead. [needs.deprecation]', + "srcdir/index.rst:33: WARNING: The [[copy('id')]] syntax in need content is deprecated. Replace with :ndf:`copy('id')` instead. [needs.deprecation]", + "srcdir/index.rst:38: WARNING: The [[copy('id')]] syntax in need content is deprecated. Replace with :ndf:`copy('id')` instead. [needs.deprecation]", + "srcdir/index.rst:44: WARNING: Error while executing function 'copy': Need not found [needs.dynamic_function]", + "srcdir/index.rst:44: WARNING: Error while executing function 'copy': Need not found [needs.dynamic_function]", ] html = Path(app.outdir, "index.html").read_text() assert "This is id SP_TOO_001" in html assert "This is also id SP_TOO_001" in html + assert "This is the best id SP_TOO_001" in html assert ( sum(1 for _ in re.finditer('test2', html)) == 2 @@ -59,7 +68,7 @@ def test_doc_dynamic_functions(test_app): sum(1 for _ in re.finditer('TEST_5', html)) == 2 ) - assert "Test output of need_func; need: TEST_3" in html + assert "Test output of dynamic function; need: TEST_3" in html assert "Test dynamic func in tags: test_4a, test_4b, TEST_4" in html @@ -67,6 +76,7 @@ def test_doc_dynamic_functions(test_app): assert "nested id TEST_6" in html assert "nested id also TEST_6" in html + assert "nested id best TEST_6" in html @pytest.mark.parametrize( @@ -118,8 +128,12 @@ def test_doc_df_user_functions(test_app): # print(warnings) expected = [ "srcdir/index.rst:10: WARNING: Return value of function 'bad_function' is of type . Allowed are str, int, float, list [needs.dynamic_function]", + "srcdir/index.rst:8: WARNING: The [[my_own_function()]] syntax in need content is deprecated. Replace with :ndf:`my_own_function()` instead. [needs.deprecation]", + "srcdir/index.rst:14: WARNING: The [[bad_function()]] syntax in need content is deprecated. Replace with :ndf:`bad_function()` instead. [needs.deprecation]", "srcdir/index.rst:14: WARNING: Return value of function 'bad_function' is of type . Allowed are str, int, float, list [needs.dynamic_function]", + "srcdir/index.rst:16: WARNING: The [[invalid]] syntax in need content is deprecated. Replace with :ndf:`invalid` instead. [needs.deprecation]", "srcdir/index.rst:16: WARNING: Function string 'invalid' could not be parsed: Given dynamic function string is not a valid python call. Got: invalid [needs.dynamic_function]", + "srcdir/index.rst:18: WARNING: The [[unknown()]] syntax in need content is deprecated. Replace with :ndf:`unknown()` instead. [needs.deprecation]", "srcdir/index.rst:18: WARNING: Unknown function 'unknown' [needs.dynamic_function]", ] if version_info >= (7, 3): diff --git a/tests/test_global_options.py b/tests/test_global_options.py index d5d1e19be..d3135a79c 100644 --- a/tests/test_global_options.py +++ b/tests/test_global_options.py @@ -21,7 +21,7 @@ def test_doc_global_option(test_app): assert "test_global" in html assert "1.27" in html - assert "Test output of need_func; need: GLOBAL_ID" in html + assert "Test output of dynamic function; need: GLOBAL_ID" in html assert "STATUS_IMPL" in html assert "STATUS_UNKNOWN" in html diff --git a/tests/test_needextract.py b/tests/test_needextract.py index d87edc62e..44ca3e889 100644 --- a/tests/test_needextract.py +++ b/tests/test_needextract.py @@ -3,6 +3,7 @@ import pytest from lxml import html as html_parser +from sphinx.util.console import strip_colors @pytest.mark.parametrize( @@ -69,7 +70,17 @@ def run_checks(checks, html_path): def test_needextract_with_nested_needs(test_app): app = test_app app.build() - assert not app._warning.getvalue() + warnings = strip_colors( + app._warning.getvalue().replace(str(app.srcdir) + os.sep, "srcdir/") + ).splitlines() + # print(warnings) + # note these warnings are emitted twice because they are resolved twice: once when first specified and once when copied with needextract + assert warnings == [ + 'srcdir/index.rst:13: WARNING: The [[copy("id")]] syntax in need content is deprecated. Replace with :ndf:`copy("id")` instead. [needs.deprecation]', + 'srcdir/index.rst:33: WARNING: The [[copy("id")]] syntax in need content is deprecated. Replace with :ndf:`copy("id")` instead. [needs.deprecation]', + 'srcdir/index.rst:13: WARNING: The [[copy("id")]] syntax in need content is deprecated. Replace with :ndf:`copy("id")` instead. [needs.deprecation]', + 'srcdir/index.rst:33: WARNING: The [[copy("id")]] syntax in need content is deprecated. Replace with :ndf:`copy("id")` instead. [needs.deprecation]', + ] needextract_html = Path(app.outdir, "needextract.html").read_text() @@ -93,5 +104,6 @@ def test_needextract_with_nested_needs(test_app): in needextract_html ) - assert "This is id SPEC_1" in needextract_html - assert "This is grandchild id SPEC_1_1_2" in needextract_html + # dynamic functions should be executed + assert "This is id SPEC_1 SPEC_1" in needextract_html + assert "This is grandchild id SPEC_1_1_2 SPEC_1_1_2" in needextract_html