Repository: useblocks/sphinx-needs
Pull request #1456 (2025-05-15 11:38:51)

Allow `collapse` and `hide` in `needs_global_options`
`needs_global_options`, a.k.a need field defaults, should only be used if the user does not specifically set their value,
i.e. user specified values should take precedence over defaults.

The problem with the existing logic, was that the only check for if a value had been set, was if it was "truthy".
This does not work if the user wants to explicitly set a `False` value, e.g.

```restructuredtext
.. spec:: Specification 3
   :id: SPEC_3
   :collapse: false
```

For `collapse` and `hide`, this is now accounted for, and so they can be allowed as defaults.
(In future this logic maybe should be extended to all fields, accounting for explicitly set empty strings and lists)


Patch:
diff --git a/docs/directives/need.rst b/docs/directives/need.rst
index 5de4a0d4..81a6899a 100644
--- a/docs/directives/need.rst
+++ b/docs/directives/need.rst
@@ -299,17 +299,16 @@ By using :ref:`needs_extra_links <needs_extra_links>`, you can use the configure
 
 delete
 ~~~~~~
-There is a **:delete:** option. If the value of the option is set to ``true``, the need will be deleted completely
+
+There is a **:delete:** option. If the value of the option is set to **True**, the need will be deleted completely
 from any NeedLists or NeedDicts including the ``needs.json`` file.
 
 This option allows a user to have multiple need-objects with the same id, but only one is shown in the documentation.
 
-If set to ``false``, the need is not removed.
-
-Allowed values:
+Allowed values (case-insensitive):
 
-* ``true`` or ``yes`` or ``1``
-* ``false`` or ``no`` or ``0``
+:True: empty, ``true`` or ``yes``
+:False: ``false`` or ``no``
 
 Default: False
 
@@ -343,8 +342,15 @@ Default: False
 
 hide
 ~~~~
-There is a **:hide:** option. If this is set (no value is needed), the need will not be printed in the
-documentation. But you can use it with **need filters**.
+There is a **:hide:** option. If this is set to **True**, the need will not be printed in the documentation.
+But you can use it with **need filters**.
+
+Allowed values (case-insensitive):
+
+:True: empty, ``true`` or ``yes``
+:False: ``false`` or ``no``
+
+Default: False
 
 .. _need_collapse:
 
@@ -355,10 +361,10 @@ You can view the details by clicking on the forward arrow symbol near the need t
 
 If set to **False**, the need shows the details section.
 
-Allowed values:
+Allowed values (case-insensitive):
 
- * true; yes; 1
- * false; no; 0
+:True: empty, ``true`` or ``yes``
+:False: ``false`` or ``no``
 
 Default: False
 
@@ -389,10 +395,10 @@ and the data in :ref:`needs_filter_data`.
 
 If you set the option to **False**, you deactivate jinja-parsing for the need's content.
 
-Allowed values:
+Allowed values (case-insensitive):
 
-* empty, ``true`` or ``yes``
-* ``false`` or ``no``
+:True: empty, ``true`` or ``yes``
+:False: ``false`` or ``no``
 
 Default: False
 
diff --git a/sphinx_needs/api/need.py b/sphinx_needs/api/need.py
index c08d4ccd..2b16c205 100644
--- a/sphinx_needs/api/need.py
+++ b/sphinx_needs/api/need.py
@@ -57,7 +57,7 @@ def generate_need(
     signature: str = "",
     sections: list[str] | None = None,
     jinja_content: None | bool = False,
-    hide: bool = False,
+    hide: None | bool = None,
     collapse: None | bool = None,
     style: None | str = None,
     layout: None | str = None,
@@ -113,8 +113,10 @@ def generate_need(
     :param tags: A list of tags, or a comma separated string.
     :param constraints: Constraints as single, comma separated, string.
     :param constraints_passed: Contains bool describing if all constraints have passed
-    :param hide: boolean value.
-    :param collapse: boolean value.
+    :param hide: If True then the need is not rendered.
+        ``None`` means that the value can be overriden by global defaults, else it is set to False.
+    :param collapse: If True, then hide the meta-data information of the need.
+        ``None`` means that the value can be overriden by global defaults, else it is set to False.
     :param style: String value of class attribute of node.
     :param layout: String value of layout definition to use
     :param template_root: Root path for template files, only required if the template_path config is relative.
@@ -217,7 +219,7 @@ def generate_need(
         "template": template,
         "pre_template": pre_template,
         "post_template": post_template,
-        "hide": hide,
+        "hide": hide or False,
         "jinja_content": jinja_content or False,
         "parts": parts or {},
         "is_part": False,
@@ -240,7 +242,12 @@ def generate_need(
 
     _add_extra_fields(needs_info, kwargs, needs_config)
     _add_link_fields(needs_info, kwargs, needs_config, location)
-    _set_field_defaults(needs_info, needs_config)
+    user_set = {
+        key
+        for key, value in (("hide", hide), ("collapse", collapse))
+        if value is not None
+    }
+    _set_field_defaults(needs_info, needs_config, user_set)
     _copy_links(needs_info, needs_config)
 
     if parent_needs := needs_info.get("parent_needs"):
@@ -299,7 +306,7 @@ def add_need(
     signature: str = "",
     sections: list[str] | None = None,
     jinja_content: None | bool = False,
-    hide: bool = False,
+    hide: None | bool = None,
     collapse: None | bool = None,
     style: None | str = None,
     layout: None | str = None,
@@ -357,8 +364,10 @@ def add_need(
     :param tags: A list of tags, or a comma separated string.
     :param constraints: Constraints as single, comma separated, string.
     :param constraints_passed: Contains bool describing if all constraints have passed
-    :param hide: boolean value.
-    :param collapse: boolean value.
+    :param hide: If True then the need is not rendered.
+        ``None`` means that the value can be overriden by global defaults, else it is set to False.
+    :param collapse: If True, then hide the meta-data information of the need.
+        ``None`` means that the value can be overriden by global defaults, else it is set to False.
     :param style: String value of class attribute of node.
     :param layout: String value of layout definition to use
     :param template: Template name to use for the content of this need
@@ -808,12 +817,21 @@ def _copy_links(needs_info: NeedsInfoType, config: NeedsSphinxConfig) -> None:
     needs_info["links"] += copy_links  # Set copied links to main-links
 
 
-def _set_field_defaults(needs_info: NeedsInfoType, config: NeedsSphinxConfig) -> None:
-    """Set default values."""
+def _set_field_defaults(
+    needs_info: NeedsInfoType, config: NeedsSphinxConfig, user_set: set[str]
+) -> None:
+    """Set default values.
+
+    This is used to set default values for fields that are not set in the need info.
+
+    :param needs_info: The need info dictionary.
+    :param config: The Sphinx configuration object.
+    :param user_set: The set of fields that were specifically set by the user.
+    """
     # TODO should defaults be applied to external/import needs?
     # currently any "falsy" value will be replaced by the default, even if it was explicitly set
     for key, defaults in config.field_defaults.items():
-        if key not in needs_info or needs_info[key]:
+        if key in user_set or key not in needs_info or needs_info[key]:
             continue
         for predicate, v in defaults.get("predicates", []):
             # use the first predicate that is satisfied
diff --git a/sphinx_needs/data.py b/sphinx_needs/data.py
index ea410f52..a7a7f5ce 100644
--- a/sphinx_needs/data.py
+++ b/sphinx_needs/data.py
@@ -60,7 +60,7 @@ class CoreFieldParameters(TypedDict):
     """Description of the field."""
     schema: dict[str, Any]
     """JSON schema for the field."""
-    allow_default: NotRequired[Literal["str", "str_list"]]
+    allow_default: NotRequired[Literal["str", "str_list", "bool"]]
     """Whether the field allows custom default values to be set, and their type,
     via needs_global_options (False if not present).
     """
@@ -129,6 +129,7 @@ NeedsCoreFields: Final[Mapping[str, CoreFieldParameters]] = {
     "collapse": {
         "description": "Hide the meta-data information of the need.",
         "schema": {"type": "boolean", "default": False},
+        "allow_default": "bool",
         "exclude_json": True,
         "exclude_external": True,
         "allow_extend": True,
@@ -136,6 +137,7 @@ NeedsCoreFields: Final[Mapping[str, CoreFieldParameters]] = {
     "hide": {
         "description": "If true, the need is not rendered.",
         "schema": {"type": "boolean", "default": False},
+        "allow_default": "bool",
         "exclude_json": True,
         "exclude_external": True,
         "allow_extend": True,
diff --git a/sphinx_needs/defaults.py b/sphinx_needs/defaults.py
index 1ca17897..3e8a8bd7 100644
--- a/sphinx_needs/defaults.py
+++ b/sphinx_needs/defaults.py
@@ -261,7 +261,7 @@ NEED_DEFAULT_OPTIONS: dict[str, Any] = {
     "collapse": string_to_boolean,
     "delete": string_to_boolean,
     "jinja_content": string_to_boolean,
-    "hide": directives.flag,
+    "hide": string_to_boolean,
     "title_from_content": directives.flag,
     "style": directives.unchanged_required,
     "layout": directives.unchanged_required,
diff --git a/sphinx_needs/directives/need.py b/sphinx_needs/directives/need.py
index 3bef759d..8b47b9ec 100644
--- a/sphinx_needs/directives/need.py
+++ b/sphinx_needs/directives/need.py
@@ -63,7 +63,7 @@ class NeedDirective(SphinxDirective):
 
         collapse = self.options.get("collapse")
         jinja_content = self.options.get("jinja_content")
-        hide = "hide" in self.options
+        hide = self.options.get("hide")
 
         id = self.options.get("id")
         status = self.options.get("status")
@@ -100,10 +100,10 @@ class NeedDirective(SphinxDirective):
                 lineno_content=self.content_offset + 1,
                 status=status,
                 tags=tags,
-                hide=hide,
                 template=template,
                 pre_template=pre_template,
                 post_template=post_template,
+                hide=hide,
                 collapse=collapse,
                 style=style,
                 layout=layout,
diff --git a/sphinx_needs/needs.py b/sphinx_needs/needs.py
index d8873777..f1ade065 100644
--- a/sphinx_needs/needs.py
+++ b/sphinx_needs/needs.py
@@ -755,7 +755,7 @@ def _gather_field_defaults(
     needs_config: NeedsSphinxConfig, link_types: set[str]
 ) -> None:
     """gather defaults from needs_global_options and set on config"""
-    allowed_internal_defaults: dict[str, Literal["str", "str_list"]] = {
+    allowed_internal_defaults: dict[str, Literal["str", "str_list", "bool"]] = {
         k: v["allow_default"]
         for k, v in NeedsCoreFields.items()
         if "allow_default" in v
@@ -883,11 +883,13 @@ def _gather_field_defaults(
 def _check_type(
     key: str,
     default: FieldDefault,
-    type_name: Literal["str", "str_list"],
+    type_name: Literal["str", "str_list", "bool"],
 ) -> bool:
     """Check the values in a FieldDefault are the given type."""
-    assert type_name in ("str", "str_list")
-    type_ = str if type_name == "str" else (str, list)
+    assert type_name in ("str", "str_list", "bool")
+    type_ = (
+        str if type_name == "str" else (bool if type_name == "bool" else (str, list))
+    )
     if "default" in default:
         if not isinstance(default["default"], type_):
             log_warning(


Test patch:
diff --git a/tests/__snapshots__/test_global_options.ambr b/tests/__snapshots__/test_global_options.ambr
index b220d57b..f22465e9 100644
--- a/tests/__snapshots__/test_global_options.ambr
+++ b/tests/__snapshots__/test_global_options.ambr
@@ -1,6 +1,12 @@
 # serializer version: 1
 # name: test_doc_global_option[test_app0]
   dict({
+    'collapse': dict({
+      'default': True,
+    }),
+    'hide': dict({
+      'default': False,
+    }),
     'layout': dict({
       'default': 'clean_l',
     }),
@@ -94,6 +100,7 @@
       '': dict({
         'needs': dict({
           'SPEC_1': dict({
+            'collapse': True,
             'docname': 'index',
             'external_css': 'external_link',
             'id': 'SPEC_1',
@@ -134,6 +141,7 @@
             'type_name': 'Specification',
           }),
           'SPEC_2': dict({
+            'collapse': True,
             'docname': 'index',
             'external_css': 'external_link',
             'id': 'SPEC_2',
@@ -152,6 +160,9 @@
             'option_2': 'SPEC_2',
             'option_4': 'STATUS_CLOSED',
             'option_5': 'STATUS_CLOSED',
+            'parent_needs_back': list([
+              'SPEC_3',
+            ]),
             'section_name': 'GLOBAL OPTIONS',
             'sections': list([
               'GLOBAL OPTIONS',
@@ -167,6 +178,7 @@
           'SPEC_3': dict({
             'docname': 'index',
             'external_css': 'external_link',
+            'hide': True,
             'id': 'SPEC_3',
             'layout': 'clean_l',
             'lineno': 13,
@@ -183,6 +195,10 @@
             'option_2': 'SPEC_3',
             'option_4': 'STATUS_UNKNOWN',
             'option_5': 'final',
+            'parent_need': 'SPEC_2',
+            'parent_needs': list([
+              'SPEC_2',
+            ]),
             'section_name': 'GLOBAL OPTIONS',
             'sections': list([
               'GLOBAL OPTIONS',
diff --git a/tests/doc_test/doc_global_options/conf.py b/tests/doc_test/doc_global_options/conf.py
index 82bb12f5..dd0d9248 100644
--- a/tests/doc_test/doc_global_options/conf.py
+++ b/tests/doc_test/doc_global_options/conf.py
@@ -37,6 +37,8 @@ needs_extra_options = [
 ]
 
 needs_global_options = {
+    "collapse": {"default": True},
+    "hide": {"default": False},
     "layout": {"default": "clean_l"},
     "option_1": {"default": "test_global"},
     "option_2": {"default": "[[copy('id')]]"},
@@ -75,3 +77,11 @@ needs_global_options = {
 
 needs_build_json = True
 needs_json_remove_defaults = True
+needs_json_exclude_fields = [
+    "id_complete",
+    "id_parent",
+    "lineno_content",
+    "type_color",
+    "type_prefix",
+    "type_style",
+]
diff --git a/tests/doc_test/doc_global_options/index.rst b/tests/doc_test/doc_global_options/index.rst
index 2a271468..95ce7c4f 100644
--- a/tests/doc_test/doc_global_options/index.rst
+++ b/tests/doc_test/doc_global_options/index.rst
@@ -13,3 +13,5 @@ GLOBAL OPTIONS
 .. spec:: Specification 3
     :id: SPEC_3
     :status: other
+    :collapse: false
+    :hide: true
\ No newline at end of file
diff --git a/tests/test_global_options.py b/tests/test_global_options.py
index d1731c9e..bd613d55 100644
--- a/tests/test_global_options.py
+++ b/tests/test_global_options.py
@@ -31,7 +31,7 @@ def test_doc_global_option_old(test_app, snapshot):
         "WARNING: needs_global_options 'link3' has a default value that is not of type 'str_list' [needs.config]",
         "WARNING: needs_global_options 'bad_value_type' has a default value that is not of type 'str' [needs.config]",
         "WARNING: needs_global_options 'too_many_params' has an unknown value format [needs.config]",
-        "WARNING: needs_global_options 'unknown' must also exist in needs_extra_options, needs_extra_links, or ['constraints', 'layout', 'post_template', 'pre_template', 'status', 'style', 'tags', 'template'] [needs.config]",
+        "WARNING: needs_global_options 'unknown' must also exist in needs_extra_options, needs_extra_links, or ['collapse', 'constraints', 'hide', 'layout', 'post_template', 'pre_template', 'status', 'style', 'tags', 'template'] [needs.config]",
         "WARNING: needs_global_options uses old, non-dict, format. please update to new format: {'layout': {'default': 'clean_l'}, 'option_1': {'default': 'test_global'}, 'option_2': {'default': \"[[copy('id')]]\"}, 'option_3': {'predicates': [('status == \"implemented\"', 'STATUS_IMPL')]}, 'option_4': {'predicates': [('status == \"closed\"', 'STATUS_CLOSED')], 'default': 'STATUS_UNKNOWN'}, 'option_5': {'predicates': [('status == \"implemented\"', 'STATUS_IMPL'), ('status == \"closed\"', 'STATUS_CLOSED')], 'default': 'final'}, 'link1': {'default': ['SPEC_1']}, 'link2': {'predicates': [('status == \"implemented\"', ['SPEC_2', \"[[copy('link1')]]\"]), ('status == \"closed\"', ['SPEC_3'])], 'default': ['SPEC_1']}, 'tags': {'predicates': [('status == \"implemented\"', ['a', 'b']), ('status == \"closed\"', ['c'])], 'default': ['d']}} [needs.deprecated]",
     ]
 
@@ -65,7 +65,7 @@ def test_doc_global_option(test_app, snapshot):
         "WARNING: needs_global_options 'link3' has a default value that is not of type 'str_list' [needs.config]",
         "WARNING: needs_global_options 'bad_value_type' has a default value that is not of type 'str' [needs.config]",
         "WARNING: needs_global_options 'too_many_params', 'predicates', must be a list of (filter string, value) pairs [needs.config]",
-        "WARNING: needs_global_options 'unknown' must also exist in needs_extra_options, needs_extra_links, or ['constraints', 'layout', 'post_template', 'pre_template', 'status', 'style', 'tags', 'template'] [needs.config]",
+        "WARNING: needs_global_options 'unknown' must also exist in needs_extra_options, needs_extra_links, or ['collapse', 'constraints', 'hide', 'layout', 'post_template', 'pre_template', 'status', 'style', 'tags', 'template'] [needs.config]",
     ]
 
     needs_config = NeedsSphinxConfig(test_app.config)