From 33d41bec6c47e089cb7cc354bfbe4abbbe52f2f3 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Sun, 26 Jan 2025 13:57:09 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A7=20de-duplicate=20`=5Fsplit=5Flist?= =?UTF-8?q?=5Fwith=5Fdyn=5Ffuncs`=20&=20`=5Fsplit=5Fvalue`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `NeedextendDirective` had a different function than `NeedDirective`, for splitting lists from directive options. This PR combines the two, to make the behaviour consistent. --- sphinx_needs/api/need.py | 53 +++++++++++++++++--------- sphinx_needs/directives/need.py | 37 ------------------ sphinx_needs/directives/needextend.py | 55 +++++++++------------------ tests/test_needextend.py | 25 ------------ tests/test_utils.py | 48 ++++++++++++++--------- 5 files changed, 85 insertions(+), 133 deletions(-) diff --git a/sphinx_needs/api/need.py b/sphinx_needs/api/need.py index 726d01916..b9c9f08ed 100644 --- a/sphinx_needs/api/need.py +++ b/sphinx_needs/api/need.py @@ -162,7 +162,7 @@ def generate_need( ) # validate tags - tags = _split_list_with_dyn_funcs(tags, location) + tags = [v for v, _ in _split_list_with_dyn_funcs(tags, location)] if needs_config.tags and ( unknown_tags := set(tags) - {t["name"] for t in needs_config.tags} ): @@ -171,7 +171,7 @@ def generate_need( ) # validate constraints - constraints = _split_list_with_dyn_funcs(constraints, location) + constraints = [v for v, _ in _split_list_with_dyn_funcs(constraints, location)] if unknown_constraints := set(constraints) - set(needs_config.constraints): raise InvalidNeedException( "invalid_constraints", @@ -255,12 +255,20 @@ def generate_need( or len(str(kwargs[link_type["option"]])) == 0 ): # If it is in global option, value got already set during prior handling of them - links = _split_list_with_dyn_funcs( - needs_info[link_type["option"]], location - ) + links = [ + v + for v, _ in _split_list_with_dyn_funcs( + needs_info[link_type["option"]], location + ) + ] else: # if it is set in kwargs, take this value and maybe override set value from global_options - links = _split_list_with_dyn_funcs(kwargs[link_type["option"]], location) + links = [ + v + for v, _ in _split_list_with_dyn_funcs( + kwargs[link_type["option"]], location + ) + ] needs_info[link_type["option"]] = links needs_info["{}_back".format(link_type["option"])] = [] @@ -728,27 +736,34 @@ def _make_hashed_id( def _split_list_with_dyn_funcs( text: None | str | list[str], location: tuple[str, int | None] | None -) -> list[str]: +) -> Iterable[tuple[str, bool]]: """Split a ``;|,`` delimited string that may contain ``[[...]]`` dynamic functions. - Remove any empty strings from the result. + :param text: The string to split. + If the input is a list of strings, yield the list unchanged, + or if the input is None, yield nothing. + :param location: A location to use for emitting warnings about badly formatted strings. + + :yields: A tuple of the string and a boolean indicating if the string contains one or more dynamic function. + Each string is stripped of leading and trailing whitespace, + and only yielded if it is not empty. - If the input is a list of strings, return the list unchanged, - or if the input is None, return an empty list. """ if text is None: - return [] + return if not isinstance(text, str): assert isinstance(text, list) and all( isinstance(x, str) for x in text ), "text must be a string or a list of strings" - return text + yield from ((x, False) for x in text) + return - result: list[str] = [] _current_element = "" + _has_dynamic_function = False while text: if text.startswith("[["): + _has_dynamic_function = True _current_element += text[:2] text = text[2:] while text and not text.startswith("]]"): @@ -767,15 +782,19 @@ def _split_list_with_dyn_funcs( ) text = text[2:] elif text[0] in ";|,": - result.append(_current_element) + _current_element = _current_element.strip() + if _current_element: + yield _current_element, _has_dynamic_function _current_element = "" + _has_dynamic_function = False text = text[1:] else: _current_element += text[0] text = text[1:] - result.append(_current_element) - result = [element.strip() for element in result if element.strip()] - return result + + _current_element = _current_element.strip() + if _current_element: + yield _current_element, _has_dynamic_function def _merge_extra_options( diff --git a/sphinx_needs/directives/need.py b/sphinx_needs/directives/need.py index e5e0fe458..d7d41b0de 100644 --- a/sphinx_needs/directives/need.py +++ b/sphinx_needs/directives/need.py @@ -437,43 +437,6 @@ def create_back_links(needs: NeedsMutable, config: NeedsSphinxConfig) -> None: ) -def _fix_list_dyn_func(list: list[str]) -> list[str]: - """ - This searches a list for dynamic function fragments, which may have been cut by generic searches for ",|;". - - Example: - `link_a, [[copy('links', need_id)]]` this will be split in list of 3 parts: - - #. link_a - #. [[copy('links' - #. need_id)]] - - This function fixes the above list to the following: - - #. link_a - #. [[copy('links', need_id)]] - - :param list: list which may contain split function calls - :return: list of fixed elements - """ - open_func_string = False - new_list = [] - for element in list: - if "[[" in element: - open_func_string = True - new_link = [element] - elif "]]" in element: - new_link.append(element) - open_func_string = False - element = ",".join(new_link) - new_list.append(element) - elif open_func_string: - new_link.append(element) - else: - new_list.append(element) - return new_list - - ##################### # Visitor functions # ##################### diff --git a/sphinx_needs/directives/needextend.py b/sphinx_needs/directives/needextend.py index 8bfc621cc..9db1e9d08 100644 --- a/sphinx_needs/directives/needextend.py +++ b/sphinx_needs/directives/needextend.py @@ -9,6 +9,7 @@ from sphinx.util.docutils import SphinxDirective 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, @@ -78,29 +79,6 @@ def run(self) -> Sequence[nodes.Node]: return [targetnode, node] -RE_ID_FUNC = re.compile(r"\s*((?P\[\[[^\]]*\]\])|(?P[^;,]+))\s*([;,]|$)") -"""Regex to find IDs or functions, delimited by one of `;|,`.""" - - -def _split_value(value: str) -> Sequence[tuple[str, bool]]: - """Split a string into a list of values. - - The string is split on `;`/`,` and whitespace is removed from the start and end of each value. - If a value starts with `[[` and ends with `]]`, it is considered a function. - - :return: A list of tuples, where the first item is the value and the second is True if the value is a function. - """ - if "[[" in value: - # may contain dynamic functions - return [ - (m.group("function"), True) - if m.group("function") - else (m.group("id").strip(), False) - for m in RE_ID_FUNC.finditer(value) - ] - return [(i.strip(), False) for i in re.split(";|,", value) if i.strip()] - - def extend_needs_data( all_needs: NeedsMutable, extends: dict[str, NeedsExtendType], @@ -164,26 +142,30 @@ def extend_needs_data( need["is_modified"] = True need["modifications"] += 1 + location = ( + current_needextend["docname"], + current_needextend["lineno"], + ) + for option, value in current_needextend["modifications"].items(): if option.startswith("+"): option_name = option[1:] if option_name in link_names: - for item, is_function in _split_value(value): - if (not is_function) and (item not in all_needs): + for item, has_function in _split_list_with_dyn_funcs( + value, location + ): + if (not has_function) and (item not in all_needs): log_warning( logger, f"Provided link id {item} for needextend does not exist.", "needextend", - location=( - current_needextend["docname"], - current_needextend["lineno"], - ), + location=location, ) continue if item not in need[option_name]: need[option_name].append(item) elif option_name in list_values: - for item, _is_function in _split_value(value): + for item, _ in _split_list_with_dyn_funcs(value, location): if item not in need[option_name]: need[option_name].append(item) else: @@ -203,21 +185,20 @@ def extend_needs_data( else: if option in link_names: need[option] = [] - for item, is_function in _split_value(value): - if (not is_function) and (item not in all_needs): + for item, has_function in _split_list_with_dyn_funcs( + value, location + ): + if (not has_function) and (item not in all_needs): log_warning( logger, f"Provided link id {item} for needextend does not exist.", "needextend", - location=( - current_needextend["docname"], - current_needextend["lineno"], - ), + location=location, ) continue need[option].append(item) elif option in list_values: - for item, _is_function in _split_value(value): + for item, _ in _split_list_with_dyn_funcs(value, location): need[option].append(item) else: need[option] = value diff --git a/tests/test_needextend.py b/tests/test_needextend.py index 80f1b6ed0..788391c04 100644 --- a/tests/test_needextend.py +++ b/tests/test_needextend.py @@ -6,8 +6,6 @@ from sphinx.util.console import strip_colors from syrupy.filters import props -from sphinx_needs.directives.needextend import _split_value - @pytest.mark.parametrize( "test_app", @@ -66,29 +64,6 @@ def test_doc_needextend_unknown_id(test_app: Sphinx): ] -@pytest.mark.parametrize( - "value,expected", - [ - ("a", [("a", False)]), - ("a,", [("a", False)]), - ("[[a]]", [("[[a]]", True)]), - ("[[a]]b", [("[[a]]b", False)]), - ("[[a;]],", [("[[a;]]", True)]), - ("a,b;c", [("a", False), ("b", False), ("c", False)]), - ("[[a]],[[b]];[[c]]", [("[[a]]", True), ("[[b]]", True), ("[[c]]", True)]), - (" a ,, b; c ", [("a", False), ("b", False), ("c", False)]), - ( - " [[a]] ,, [[b]] ; [[c]] ", - [("[[a]]", True), ("[[b]]", True), ("[[c]]", True)], - ), - ("a,[[b]];c", [("a", False), ("[[b]]", True), ("c", False)]), - (" a ,, [[b;]] ; c ", [("a", False), ("[[b;]]", True), ("c", False)]), - ], -) -def test_split_value(value, expected): - assert _split_value(value) == expected - - @pytest.mark.parametrize( "test_app", [{"buildername": "html", "srcdir": "doc_test/doc_needextend_dynamic"}], diff --git a/tests/test_utils.py b/tests/test_utils.py index d25c13f8f..cb598e75f 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -10,23 +10,37 @@ [ (None, []), ([], []), - (["a"], ["a"]), - ("a", ["a"]), - ("a,b", ["a", "b"]), - ("a, b", ["a", "b"]), - ("a,b,", ["a", "b"]), - ("a|b", ["a", "b"]), - ("a| b", ["a", "b"]), - ("a|b,", ["a", "b"]), - ("a;b", ["a", "b"]), - ("a; b", ["a", "b"]), - ("a;b,", ["a", "b"]), - ("a,b|c;d,", ["a", "b", "c", "d"]), - ("[[x,y]],b", ["[[x,y]]", "b"]), - ("a,[[x,y]],b", ["a", "[[x,y]]", "b"]), - ("a,[[x,y", ["a", "[[x,y]]"]), - ("a,[[x,y]", ["a", "[[x,y]]"]), + (["a"], [("a", False)]), + ("a", [("a", False)]), + ("a,", [("a", False)]), + ("[[a]]", [("[[a]]", True)]), + ("a,b", [("a", False), ("b", False)]), + ("a, b", [("a", False), ("b", False)]), + ("a,b,", [("a", False), ("b", False)]), + ("a|b", [("a", False), ("b", False)]), + ("a| b", [("a", False), ("b", False)]), + ("a|b,", [("a", False), ("b", False)]), + ("a;b", [("a", False), ("b", False)]), + ("a; b", [("a", False), ("b", False)]), + ("a;b,", [("a", False), ("b", False)]), + ("a,b|c;d,", [("a", False), ("b", False), ("c", False), ("d", False)]), + ("[[x,y]],b", [("[[x,y]]", True), ("b", False)]), + ("a,[[x,y]],b", [("a", False), ("[[x,y]]", True), ("b", False)]), + ("a,[[x,y", [("a", False), ("[[x,y]]", True)]), + ("a,[[x,y]", [("a", False), ("[[x,y]]", True)]), + # previously in from _split_value in needextend.py + ("[[a]]b", [("[[a]]b", True)]), + ("[[a;]],", [("[[a;]]", True)]), + ("a,b;c", [("a", False), ("b", False), ("c", False)]), + ("[[a]],[[b]];[[c]]", [("[[a]]", True), ("[[b]]", True), ("[[c]]", True)]), + (" a ,, b; c ", [("a", False), ("b", False), ("c", False)]), + ( + " [[a]] ,, [[b]] ; [[c]] ", + [("[[a]]", True), ("[[b]]", True), ("[[c]]", True)], + ), + ("a,[[b]];c", [("a", False), ("[[b]]", True), ("c", False)]), + (" a ,, [[b;]] ; c ", [("a", False), ("[[b;]]", True), ("c", False)]), ], ) def test_split_list_with_dyn_funcs(input, expected): - assert _split_list_with_dyn_funcs(input, None) == expected + assert list(_split_list_with_dyn_funcs(input, None)) == expected