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.

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

♻️ Don't process dynamic functions on internal need fields #1387

Merged
merged 5 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions sphinx_needs/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ class CoreFieldParameters(TypedDict):
"""JSON schema for the field."""
allow_extend: NotRequired[bool]
"""Whether field can be modified by needextend (False if not present)."""
allow_df: NotRequired[bool]
"""Whether dynamic functions are allowed for this field (False if not present)."""
show_in_layout: NotRequired[bool]
"""Whether to show the field in the rendered layout of the need by default (False if not present)."""
exclude_external: NotRequired[bool]
Expand Down Expand Up @@ -96,21 +98,25 @@ class CoreFieldParameters(TypedDict):
"full_title": {
"description": "Title of the need, of unlimited length.",
"schema": {"type": "string", "default": ""},
"allow_df": True,
},
"title": {
"description": "Title of the need, trimmed to a maximum length.",
"schema": {"type": "string"},
"allow_df": True,
},
"status": {
"description": "Status of the need.",
"schema": {"type": ["string", "null"], "default": None},
"show_in_layout": True,
"allow_df": True,
"allow_extend": True,
},
"tags": {
"description": "List of tags.",
"schema": {"type": "array", "items": {"type": "string"}, "default": []},
"show_in_layout": True,
"allow_df": True,
"allow_extend": True,
},
"collapse": {
Expand Down Expand Up @@ -144,6 +150,7 @@ class CoreFieldParameters(TypedDict):
"schema": {"type": ["string", "null"], "default": None},
"show_in_layout": True,
"exclude_external": True,
"allow_df": True,
"allow_extend": True,
},
"arch": {
Expand Down Expand Up @@ -174,33 +181,38 @@ class CoreFieldParameters(TypedDict):
"type": {
"description": "Type of the need.",
"schema": {"type": "string", "default": ""},
"allow_df": True,
Copy link
Member

Choose a reason for hiding this comment

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

How can the type and below type specific values be set by a df?

Copy link
Member Author

@chrisjsewell chrisjsewell Jan 27, 2025

Choose a reason for hiding this comment

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

well thats a discussion to be had; I wouldn't expect them to, but they technically can already, so removing them would be a breaking change.
Currently, in this PR I have basically just allowed all str/list[str] fields, so as to be approximately back-compatible

Copy link
Member

Choose a reason for hiding this comment

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

Let's deprecate this, only usecase are 'type fields' coming from needimport or external needs. Users are in control of the JSONs, so they can set primary data there.

},
"type_name": {
"description": "Name of the type.",
"schema": {"type": "string", "default": ""},
"exclude_external": True,
"exclude_import": True,
"allow_df": True,
},
"type_prefix": {
"description": "Prefix of the type.",
"schema": {"type": "string", "default": ""},
"exclude_json": True,
"exclude_external": True,
"exclude_import": True,
"allow_df": True,
},
"type_color": {
"description": "Hexadecimal color code of the type.",
"schema": {"type": "string", "default": ""},
"exclude_json": True,
"exclude_external": True,
"exclude_import": True,
"allow_df": True,
},
"type_style": {
"description": "Style of the type.",
"schema": {"type": "string", "default": ""},
"exclude_json": True,
"exclude_external": True,
"exclude_import": True,
"allow_df": True,
},
"is_modified": {
"description": "Whether the need was modified by needextend.",
Expand Down Expand Up @@ -299,6 +311,7 @@ class CoreFieldParameters(TypedDict):
"constraints": {
"description": "List of constraint names, which are defined for this need.",
"schema": {"type": "array", "items": {"type": "string"}, "default": []},
"allow_df": True,
"allow_extend": True,
},
"constraints_results": {
Expand Down
38 changes: 20 additions & 18 deletions sphinx_needs/functions/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@
from sphinx.util.tags import Tags

from sphinx_needs.config import NeedsSphinxConfig
from sphinx_needs.data import NeedsInfoType, NeedsMutable, SphinxNeedsData
from sphinx_needs.data import (
NeedsCoreFields,
NeedsInfoType,
NeedsMutable,
SphinxNeedsData,
)
from sphinx_needs.debug import measure_time_func
from sphinx_needs.logging import get_logger, log_warning
from sphinx_needs.nodes import Need
Expand Down Expand Up @@ -228,14 +233,19 @@ def resolve_dynamic_values(needs: NeedsMutable, app: Sphinx) -> None:
- ``*args``: optional arguments (specified in the function string)
- ``**kwargs``: optional keyword arguments (specified in the function string)
"""

config = NeedsSphinxConfig(app.config)

allowed_fields: set[str] = {
*(k for k, v in NeedsCoreFields.items() if v.get("allow_df", False)),
*config.extra_options,
*(link["option"] for link in config.extra_links),
*config.global_options,
}

for need in needs.values():
for need_option in need:
if need_option in [
"docname",
"lineno",
"content",
]:
# dynamic values in this data are not allowed.
if need_option not in allowed_fields:
continue
if not isinstance(need[need_option], (list, set)):
func_call: str | None = "init"
Expand Down Expand Up @@ -288,18 +298,10 @@ def resolve_dynamic_values(needs: NeedsMutable, app: Sphinx) -> None:
)
if func_call is None:
new_values.append(element)
elif isinstance(func_return, (list, set)):
new_values += func_return
else:
# Replace original function string with return value of function call
if isinstance(need[need_option], (str, int, float)):
new_values.append(
element.replace(f"[[{func_call}]]", str(func_return))
)
else:
if isinstance(need[need_option], (list, set)):
if isinstance(func_return, (list, set)):
new_values += func_return
else:
new_values += [func_return]
new_values += [func_return]

need[need_option] = new_values

Expand Down
Loading