Skip to content

Commit

Permalink
♻️ Output content in needs.json not description (#1312)
Browse files Browse the repository at this point in the history
I'm not sure if there was a legacy reason for this, but it is an unnecessary complication/confusion,
to convert between `content` and `description` and breaks the `needs_schema` and defaults.

Note, `needs.json` importing logic has been adapted to handle reading of both new and legacy formats
  • Loading branch information
chrisjsewell authored Oct 2, 2024
1 parent ede3b06 commit 02b2494
Show file tree
Hide file tree
Showing 16 changed files with 172 additions and 140 deletions.
1 change: 0 additions & 1 deletion sphinx_needs/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,6 @@ class CoreFieldParameters(TypedDict):
"content": {
"description": "Content of the need.",
"schema": {"type": "string", "default": ""},
"exclude_json": True,
},
"pre_content": {
"description": "Pre-content of the need.",
Expand Down
11 changes: 7 additions & 4 deletions sphinx_needs/directives/needimport.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,9 @@ def run(self) -> Sequence[nodes.Node]:
else:
filter_context = need.copy()

# Support both ways of addressing the description, as "description" is used in json file, but
# "content" is the sphinx internal name for this kind of information
filter_context["content"] = need["description"] # type: ignore[typeddict-item]
if "description" in need and not need.get("content"):
# legacy versions of sphinx-needs changed "description" to "content" when outputting to json
filter_context["content"] = need["description"] # type: ignore[typeddict-item]
try:
if filter_single_need(filter_context, needs_config, filter_string):
needs_list_filtered[key] = need
Expand Down Expand Up @@ -243,7 +243,10 @@ def run(self) -> Sequence[nodes.Node]:
# Replace id, to get unique ids
need_params["id"] = id_prefix + need_params["id"]

need_params["content"] = need_params["description"] # type: ignore[typeddict-item]
if "description" in need_params and not need_params.get("content"):
# legacy versions of sphinx-needs changed "description" to "content" when outputting to json
need_params["content"] = need_params["description"] # type: ignore[typeddict-item]
del need_params["description"] # type: ignore[typeddict-item]

# Remove unknown options, as they may be defined in source system, but not in this sphinx project
for option in list(need_params):
Expand Down
5 changes: 4 additions & 1 deletion sphinx_needs/external_needs.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,10 @@ def load_external_needs(app: Sphinx, env: BuildEnvironment, docname: str) -> Non
f'{source["base_url"]}/{need.get("docname", "__error__")}.html#{need["id"]}'
)

need_params["content"] = need["description"]
if "description" in need_params and not need_params.get("content"):
# legacy versions of sphinx-needs changed "description" to "content" when outputting to json
need_params["content"] = need_params["description"]
del need_params["description"]

# Remove unknown options, as they may be defined in source system, but not in this sphinx project
for option in list(need_params):
Expand Down
2 changes: 1 addition & 1 deletion sphinx_needs/needsfile.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"properties": {
"title": {"type": "string"},
"id": {"type": "string"},
"description": {"type": "string"}
"content": {"type": "string"}
}
}
}
Expand Down
1 change: 0 additions & 1 deletion sphinx_needs/needsfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ def add_need(self, version: str, need_info: NeedsInfoType) -> None:
key in self._need_defaults and value == self._need_defaults[key]
)
}
writable_needs["description"] = need_info["content"] # TODO why this?
self.needs_list["versions"][version]["needs"][need_info["id"]] = writable_needs
self.needs_list["versions"][version]["needs_amount"] = len(
self.needs_list["versions"][version]["needs"]
Expand Down
6 changes: 3 additions & 3 deletions sphinx_needs/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,9 @@ def import_prefix_link_edit(
need[extra_link["option"]][n] = f"{id_prefix}{id}"
# Manipulate descriptions
# ToDo: Use regex for better matches.
need["description"] = need["description"].replace(
id, "".join([id_prefix, id])
)
for key in ("content", "description"):
if key in need:
need[key] = need[key].replace(id, "".join([id_prefix, id]))


FuncT = TypeVar("FuncT")
Expand Down
5 changes: 2 additions & 3 deletions tests/__snapshots__/test_add_sections_sigs.ambr
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# name: test_section_is_usable_in_filters[test_app0]
dict({
'R_12345': dict({
'description': 'The Tool **shall** have a command line interface.',
'content': 'The Tool **shall** have a command line interface.',
'docname': 'index',
'external_css': 'external_link',
'full_title': 'Command line interface',
Expand All @@ -25,7 +25,7 @@
'updated': '1.5.1',
}),
'R_12346': dict({
'description': 'The Tool **shall** have a command line interface.',
'content': 'The Tool **shall** have a command line interface.',
'docname': 'index',
'external_css': 'external_link',
'full_title': 'Another Requirement',
Expand All @@ -49,7 +49,6 @@
'updated': '1.4.0',
}),
'T_001': dict({
'description': '',
'docname': 'index',
'external_css': 'external_link',
'full_title': 'test method',
Expand Down
10 changes: 8 additions & 2 deletions tests/__snapshots__/test_basic_doc.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
'constraints_passed': True,
'constraints_results': dict({
}),
'content': '',
'created_at': '',
'delete': False,
'description': '',
'docname': 'index',
'doctype': '.rst',
'duration': '',
Expand Down Expand Up @@ -85,9 +85,9 @@
'constraints_passed': True,
'constraints_results': dict({
}),
'content': '',
'created_at': '',
'delete': False,
'description': '',
'docname': 'index',
'doctype': '.rst',
'duration': '',
Expand Down Expand Up @@ -209,6 +209,12 @@
'field_type': 'core',
'type': 'object',
}),
'content': dict({
'default': '',
'description': 'Content of the need.',
'field_type': 'core',
'type': 'string',
}),
'created_at': dict({
'default': '',
'description': 'Added by service github-issues',
Expand Down
24 changes: 17 additions & 7 deletions tests/__snapshots__/test_external.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
'1.3': dict({
'needs': dict({
'EXT_REQ_01': dict({
'description': '',
'doctype': '',
'external_css': 'external_link',
'external_url': 'http://my_company.com/docs/v1/index.html#REQ_01',
Expand All @@ -22,7 +21,6 @@
'type_name': 'Requirement',
}),
'IMP_REQ_01': dict({
'description': '',
'docname': 'index',
'external_css': 'external_link',
'full_title': 'REQ_01',
Expand Down Expand Up @@ -103,6 +101,12 @@
'field_type': 'core',
'type': 'object',
}),
'content': dict({
'default': '',
'description': 'Content of the need.',
'field_type': 'core',
'type': 'string',
}),
'created_at': dict({
'default': '',
'description': 'Added by service github-issues',
Expand Down Expand Up @@ -482,9 +486,9 @@
'constraints_passed': True,
'constraints_results': dict({
}),
'content': 'EXT_TEST_01',
'created_at': '',
'delete': False,
'description': 'EXT_TEST_01',
'docname': None,
'doctype': '',
'duration': '',
Expand Down Expand Up @@ -555,9 +559,9 @@
'constraints_passed': True,
'constraints_results': dict({
}),
'content': 'EXT_TEST_02',
'created_at': '',
'delete': False,
'description': 'EXT_TEST_02',
'docname': None,
'doctype': '',
'duration': '',
Expand Down Expand Up @@ -631,9 +635,9 @@
'constraints_passed': True,
'constraints_results': dict({
}),
'content': '',
'created_at': '',
'delete': False,
'description': '',
'docname': 'index',
'doctype': '.rst',
'duration': '',
Expand Down Expand Up @@ -703,9 +707,9 @@
'constraints_passed': True,
'constraints_results': dict({
}),
'content': '',
'created_at': '',
'delete': False,
'description': '',
'docname': 'index',
'doctype': '.rst',
'duration': '',
Expand Down Expand Up @@ -776,9 +780,9 @@
'constraints_passed': True,
'constraints_results': dict({
}),
'content': '',
'created_at': '',
'delete': False,
'description': '',
'docname': 'subfolder_a/subfolder_b/subpage',
'doctype': '.rst',
'duration': '',
Expand Down Expand Up @@ -900,6 +904,12 @@
'field_type': 'core',
'type': 'object',
}),
'content': dict({
'default': '',
'description': 'Content of the need.',
'field_type': 'core',
'type': 'string',
}),
'created_at': dict({
'default': '',
'description': 'Added by service github-issues',
Expand Down
22 changes: 14 additions & 8 deletions tests/__snapshots__/test_need_constraints.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
'constraints_passed': True,
'constraints_results': dict({
}),
'content': 'This is a requirement describing OPSEC processes.',
'created_at': '',
'delete': False,
'description': 'This is a requirement describing OPSEC processes.',
'docname': 'index',
'doctype': '.rst',
'duration': '',
Expand Down Expand Up @@ -91,9 +91,9 @@
'check_0': True,
}),
}),
'content': 'Example of a successful constraint.',
'created_at': '',
'delete': False,
'description': 'Example of a successful constraint.',
'docname': 'index',
'doctype': '.rst',
'duration': '',
Expand Down Expand Up @@ -168,9 +168,9 @@
'check_0': False,
}),
}),
'content': 'Example of a failed constraint.',
'created_at': '',
'delete': False,
'description': 'Example of a failed constraint.',
'docname': 'index',
'doctype': '.rst',
'duration': '',
Expand Down Expand Up @@ -243,9 +243,9 @@
'check_0': False,
}),
}),
'content': 'Example of a failed constraint with medium severity. Note the style from :ref:`needs_constraint_failed_options`',
'created_at': '',
'delete': False,
'description': 'Example of a failed constraint with medium severity. Note the style from :ref:`needs_constraint_failed_options`',
'docname': 'index',
'doctype': '.rst',
'duration': '',
Expand Down Expand Up @@ -313,9 +313,9 @@
'constraints_passed': True,
'constraints_results': dict({
}),
'content': 'The Tool awesome shall have a command line interface.',
'created_at': '',
'delete': False,
'description': 'The Tool awesome shall have a command line interface.',
'docname': 'index',
'doctype': '.rst',
'duration': '',
Expand Down Expand Up @@ -390,9 +390,9 @@
'check_0': False,
}),
}),
'content': 'asdf',
'created_at': '',
'delete': False,
'description': 'asdf',
'docname': 'index',
'doctype': '.rst',
'duration': '',
Expand Down Expand Up @@ -467,9 +467,9 @@
'check_0': False,
}),
}),
'content': '',
'created_at': '',
'delete': False,
'description': '',
'docname': 'style_test',
'doctype': '.rst',
'duration': '',
Expand Down Expand Up @@ -541,9 +541,9 @@
'check_0': False,
}),
}),
'content': '',
'created_at': '',
'delete': False,
'description': '',
'docname': 'style_test',
'doctype': '.rst',
'duration': '',
Expand Down Expand Up @@ -665,6 +665,12 @@
'field_type': 'core',
'type': 'object',
}),
'content': dict({
'default': '',
'description': 'Content of the need.',
'field_type': 'core',
'type': 'string',
}),
'created_at': dict({
'default': '',
'description': 'Added by service github-issues',
Expand Down
4 changes: 2 additions & 2 deletions tests/__snapshots__/test_need_parts.ambr
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# name: test_doc_need_parts[test_app0]
dict({
'SP_TOO_001': dict({
'description': '''
'content': '''
The Tool awesome shall have a command line interface with following commands:

* :need_part:`(1)exit()`
Expand Down Expand Up @@ -95,7 +95,7 @@
'type_name': 'Specification',
}),
'TEST_2': dict({
'description': 'Part in nested need: :need_part:`(nested_id)something`',
'content': 'Part in nested need: :need_part:`(nested_id)something`',
'docname': 'index',
'external_css': 'external_link',
'full_title': 'TEST_2',
Expand Down
Loading

0 comments on commit 02b2494

Please sign in to comment.