Skip to content

Commit

Permalink
👌 Improve parsing of need option lists with dynamic functions (#1272)
Browse files Browse the repository at this point in the history 
For need directive options of the format e.g. `:tags: a,[[func()]],b`,
this commit consolidates parsing into a single conversion function and adds unit tests
  • Loading branch information
@chrisjsewell
chrisjsewell authored Sep 5, 2024
1 parent e5e370e commit c862e9d
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 146 deletions.
219 changes: 73 additions & 146 deletions sphinx_needs/api/need.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,9 @@ def run():
type_style = ""
found = False

# location is used to provide source mapped warnings
location = (docname, lineno) if docname else None

# Log messages for need elements that could not be imported.
configured_need_types = [ntype["directive"] for ntype in types]
if need_type not in configured_need_types:
Expand All @@ -172,7 +175,7 @@ def run():
f"Couldn't create need {id}. Reason: The need-type (i.e. `{need_type}`) is not set "
"in the project's 'need_types' configuration in conf.py.",
"add",
location=(docname, lineno) if docname else None,
location=location,
)

for ntype in types:
Expand Down Expand Up @@ -234,79 +237,26 @@ def run():
"by config value 'needs_statuses'."
)

if tags is None:
tags = []
if len(tags) > 0:
# tags should be a string, but it can also be already a list, which can be used.
if isinstance(tags, str):
tags = [tag.strip() for tag in re.split("[;,]", tags)]
new_tags = [] # Shall contain only valid tags
for i in range(len(tags)):
if len(tags[i]) == 0 or tags[i].isspace():
log_warning(
logger,
f"Scruffy tag definition found in need {need_id!r}. "
"Defined tag contains spaces only.",
"add",
location=(docname, lineno) if docname else None,
tags = _split_list_with_dyn_funcs(tags, location)
# Check if tag is in needs_tags. If not raise an error.
if needs_config.tags:
needs_tags = [t["name"] for t in needs_config.tags]
for tag in tags:
if tag not in needs_tags:
raise NeedsTagNotAllowed(
f"Tag {tag} of need id {need_id} is not allowed "
"by config value 'needs_tags'."
)
else:
new_tags.append(tags[i])

tags = new_tags
# Check if tag is in needs_tags. If not raise an error.
if needs_config.tags:
for tag in tags:
needs_tags = [tag["name"] for tag in needs_config.tags]
if tag not in needs_tags:
raise NeedsTagNotAllowed(
f"Tag {tag} of need id {need_id} is not allowed "
"by config value 'needs_tags'."
)
# This may have cut also dynamic function strings, as they can contain , as well.
# So let put them together again
# ToDo: There may be a smart regex for the splitting. This would avoid this mess of code...
else:
tags = []
tags = _fix_list_dyn_func(tags)

if constraints is None:
constraints = []
if len(constraints) > 0:
# tags should be a string, but it can also be already a list,which can be used.
if isinstance(constraints, str):
constraints = [
constraint.strip() for constraint in re.split("[;,]", constraints)
]

new_constraints = [] # Shall contain only valid constraints
for i in range(len(constraints)):
if len(constraints[i]) == 0 or constraints[i].isspace():
log_warning(
logger,
f"Scruffy constraint definition found in need {need_id!r}. "
"Defined constraint contains spaces only.",
"add",
location=(docname, lineno) if docname else None,

constraints = _split_list_with_dyn_funcs(constraints, location)
# Check if constraint is in needs_constraints. If not raise an error.
if needs_config.constraints:
for constraint in constraints:
if constraint not in needs_config.constraints:
raise NeedsConstraintNotAllowed(
f"Constraint {constraint} of need id {need_id} is not allowed "
"by config value 'needs_constraints'."
)
else:
new_constraints.append(constraints[i])

constraints = new_constraints
# Check if constraint is in needs_constraints. If not raise an error.
if needs_config.constraints:
for constraint in constraints:
if constraint not in needs_config.constraints.keys():
raise NeedsConstraintNotAllowed(
f"Constraint {constraint} of need id {need_id} is not allowed "
"by config value 'needs_constraints'."
)
# This may have cut also dynamic function strings, as they can contain , as well.
# So let put them together again
# ToDo: There may be a smart regex for the splitting. This would avoid this mess of code...
else:
constraints = []
constraints = _fix_list_dyn_func(constraints)

#############################################################################################
# Add need to global need list
Expand All @@ -325,12 +275,7 @@ def run():
"from the content, then ensure the first sentence of the "
"requirements are different."
)
log_warning(
logger,
message,
"duplicate_id",
location=(docname, lineno) if docname else None,
)
log_warning(logger, message, "duplicate_id", location=location)
return []

# Trim title if it is too long
Expand Down Expand Up @@ -424,10 +369,12 @@ def run():
or len(str(kwargs[link_type["option"]])) == 0
):
# If it is in global option, value got already set during prior handling of them
links = _read_in_links(needs_info[link_type["option"]])
links = _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 = _read_in_links(kwargs[link_type["option"]])
links = _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 @@ -697,34 +644,6 @@ def _prepare_template(app: Sphinx, needs_info: NeedsInfoType, template_key: str)
return new_content


def _read_in_links(links_string: None | str | list[str]) -> list[str]:
# Get links
links = []
if links_string:
# Check if links_string is really a string, otherwise it will be a list, which can be used
# without modifications
if isinstance(links_string, str):
link_list = re.split(";|,", links_string)
else:
link_list = links_string
for link in link_list:
if link.isspace():
log_warning(
logger,
f"Grubby link definition found in need {id}. "
"Defined link contains spaces only.",
None,
None,
)
else:
links.append(link.strip())

# This may have cut also dynamic function strings, as they can contain , as well.
# So let put them together again
# ToDo: There may be a smart regex for the splitting. This would avoid this mess of code...
return _fix_list_dyn_func(links)


def make_hashed_id(
app: Sphinx,
need_type: str,
Expand Down Expand Up @@ -770,48 +689,56 @@ def make_hashed_id(
return f"{type_prefix}{cal_hashed_id[:id_length]}"


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 splitted in list of 3 parts:
#. link_a
#. [[copy('links'
#. need_id)]]
def _split_list_with_dyn_funcs(
text: None | str | list[str], location: tuple[str, int | None] | None
) -> list[str]:
"""Split a ``;|,`` delimited string that may contain ``[[...]]`` dynamic functions.
This function fixes the above list to the following:
Remove any empty strings from the result.
#. link_a
#. [[copy('links', need_id)]]
:param list: list which may contain splitted function calls
:return: list of fixed elements
If the input is a list of strings, return the list unchanged,
or if the input is None, return an empty list.
"""
open_func_string = False
new_list = []
for element in list:
# If dyn_func got not cut, just add it
if "[[" in element and "]]" in element:
new_list.append(element)
# Other check if this is the starting element of dyn function
elif "[[" in element:
open_func_string = True
new_link = [element]
# Check if this is the ending element if dyn function
elif "]]" in element:
new_link.append(element)
open_func_string = False
element = ",".join(new_link)
new_list.append(element)
# Check it is a "middle" part of the dyn function
elif open_func_string:
new_link.append(element)
# Looks like it isn't a cut dyn_func, just add.
if text is None:
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

result: list[str] = []
_current_element = ""
while text:
if text.startswith("[["):
_current_element += text[:2]
text = text[2:]
while text and not text.startswith("]]"):
_current_element += text[0]
text = text[1:]
if _current_element.endswith("]"):
_current_element += "]"
else:
_current_element += "]]"
if not text.startswith("]]"):
log_warning(
logger,
f"Dynamic function not closed correctly: {text}",
None,
location=location,
)
text = text[2:]
elif text[0] in ";|,":
result.append(_current_element)
_current_element = ""
text = text[1:]
else:
new_list.append(element)
return new_list
_current_element += text[0]
text = text[1:]
result.append(_current_element)
result = [element.strip() for element in result if element.strip()]
return result


def _merge_extra_options(
Expand Down
32 changes: 32 additions & 0 deletions tests/test_utils.py
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
"""Test utility functions."""

import pytest

from sphinx_needs.api.need import _split_list_with_dyn_funcs


@pytest.mark.parametrize(
"input, expected",
[
(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]]"]),
],
)
def test_split_list_with_dyn_funcs(input, expected):
assert _split_list_with_dyn_funcs(input, None) == expected

0 comments on commit c862e9d

Please sign in to comment.