Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔧 Synchronize list splitting behaviour in need and needextend directives #1385

Merged
merged 1 commit into from
Jan 27, 2025
Merged

🔧 Synchronize list splitting behaviour in need and needextend directives #1385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 36 additions & 17 deletions sphinx_needs/api/need.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@
)

# 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}
):
Expand All @@ -171,7 +171,7 @@
)

# 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",
Expand Down Expand Up @@ -255,12 +255,20 @@
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 = [

Check warning on line 258 in sphinx_needs/api/need.py

View check run for this annotation

Codecov / codecov/patch

sphinx_needs/api/need.py#L258 

Added line #L258 was not covered by tests
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"])] = []
Expand Down Expand Up @@ -728,27 +736,34 @@

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("]]"):
Expand All @@ -767,15 +782,19 @@
)
text = text[2:]
elif text[0] in ";|,":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like there are multiple splitting characters. Maybe mention this in https://sphinx-needs.readthedocs.io/en/latest/filter.html#filter-options. The pages currently says For :status:, :tags: and :types: values are separated by “;”.. Have not found another mention of this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I would rather there were not these multiple characters, but that would be back breaking 🤷
will look at docs in separate PR

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(
Expand Down
37 changes: 0 additions & 37 deletions sphinx_needs/directives/need.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 #
#####################
Expand Down
55 changes: 18 additions & 37 deletions sphinx_needs/directives/needextend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -78,29 +79,6 @@
return [targetnode, node]


RE_ID_FUNC = re.compile(r"\s*((?P<function>\[\[[^\]]*\]\])|(?P<id>[^;,]+))\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],
Expand Down Expand Up @@ -164,26 +142,30 @@
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:
Expand All @@ -203,21 +185,20 @@
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):

Check warning on line 201 in sphinx_needs/directives/needextend.py

View check run for this annotation

Codecov / codecov/patch

sphinx_needs/directives/needextend.py#L201 

Added line #L201 was not covered by tests
need[option].append(item)
else:
need[option] = value
25 changes: 0 additions & 25 deletions tests/test_needextend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"}],
Expand Down
48 changes: 31 additions & 17 deletions tests/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading