🧪 Pin list2need behaviour and correct its documentation - #1788
Merged
Conversation
tests/test_list2need.py covers the happy path over one fixture project. This adds a suite over the inputs that project does not contain: id capture, inline options, the delimiter, continuation lines, level skips, the eleven inputs that abort the build, the Markdown host, and the line numbers recorded around the directive. The tests pin what the directive does today, including the behaviours that are surprising, so that any later change to the implementation shows up as a deliberate edit to an assertion rather than as a silent change in what users' documents produce.
- The "List with need-ids" example rendered a need titled "(FEATURE",
because the default delimiter splits "(FEATURE.3)" before the ID is
read. It now uses an ID without a dot, and the trap is spelled out
next to it. The code-block above it and the live directive beside it
also disagreed on :types:, and now agree.
- New "Need IDs" section: an ID is any bracketed group, found from the
first "(" to the last ")", not a prefix; what that means for a title
that carries a second parenthetical; and the generated-ID formula,
its title-only input and the resulting cross-document collision.
- The claim that IDs use the same mechanism as need_part is qualified:
need_part anchors its match and allows only word characters and "-".
- The indentation rule is stated exactly: 2 spaces per level, no tabs,
and a build error for anything that is not a multiple of 2.
- The meta-data rules replace "the position of the option-string inside
the line is not important" and the "," separator with what the parser
does: the first "((...))" region only, matched greedily, values
quoted, and unquoted values silently ignored.
- New "Markdown (MyST)" section recording that the directive works in
reStructuredText only, with the eval-rst workaround.
The two Markdown-hosted list2need cases name myst_parser in their project's extensions, so the application cannot be created without it and pytest.importorskip inside the test body runs too late. They are guarded by a skipif mark instead, and myst-parser is added to the test extra so the cases actually run rather than always skipping.
Six of the eleven build-aborting inputs raise a bare Python exception, not three.
myst-parser 5.x requires sphinx>=8, so pinning it at ~=5.1 made `pip install sphinx~=7.4 -e .[test]` fail with ResolutionImpossible, which is the oldest leg of the CI matrix. `>=4,<6` lets the resolver pick 4.x alongside sphinx 7.4 and 5.x alongside 8.2 and 9.1. The Markdown-hosted list2need cases were checked against all three: sphinx 7.4.7 / myst-parser 4.0.1, sphinx 8.2.3 / myst-parser 5.1.0 and sphinx 9.1.0 / myst-parser 5.1.0 all agree.
- Tabs: the claim that "a tab counts as a single character" was wrong. docutils expands tabs before the parser runs (string2lines, tab_width 8 by default), so a tab never reaches the directive as a tab and the indentation it produces depends on the column it sits in. Measured: with the content at indent 3 a tab-indented child aborts the build, but at indent 4 the same tab expands to a multiple of 2 and silently creates a level-2 need. The "ends the build as well" generalisation was wrong too; both are now stated as measured. - Continuation lines: "indented further than the *" was over-strict. Only a line starting in the first column loses its first word; one space is enough, at any level. - Generated IDs: the type does take part, by contributing its prefix. Only the hash input is title-only, which is why the same title at two levels yields two IDs rather than a collision.
- The unquoted-option row asserted only that no warning named the need, which no sphinx-needs option diagnostic does anyway. It moves to its own single-item project and asserts the warning stream is empty, so "silently ignored" is actually pinned. - Add the no-space-after-bullet case, which the grammar allows. - The collision test covered the cross-document case only. It now also covers two identical titles in one list, and the same title at two levels, where the differing type prefixes mean both needs are created.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1788 +/- ##
==========================================
+ Coverage 86.87% 91.04% +4.16%
==========================================
Files 56 77 +21
Lines 6532 11584 +5052
==========================================
+ Hits 5675 10547 +4872
- Misses 857 1037 +180
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Aug 26, 2026
chrisjsewell
added a commit
that referenced
this pull request
Aug 26, 2026
…dText (#1790) `list2need` renders every item into a need directive through a template and hands the result back to the parser with `state_machine.insert_input`. This replaces that step: the items are built with `add_need`, and a nested item is placed inside its parent. **The syntax and its parsing are untouched.** The same four regular expressions, the same level→type mapping, the same `(ID)` capture, the same `:delimiter:` split, the same `((option="value"))` extraction, the same `:tags:` merge, the same `:links-down:` walk, the same validation and the same eleven inputs that end the build. Only the generation mechanism changes. This is offered as the answer to the question in #1427 — *"can we save the syntax (mostly) but have a better implementation (without jinja)?"* ### What the step cost **Markdown.** myst-parser's mock state machine does not implement `insert_input`, so a `{list2need}` fence produced no needs at all — only an error naming a myst-parser internal. It now works: | | before | after | |---|---|---| | needs created | none | `MD-A`, `MD-B` | | nesting | — | `MD-B.parent_need == "MD-A"` | | diagnostics | `MockingError: MockStateMachine has not yet implemented attribute 'insert_input'` | none | **Line numbers (#1349, the `list2need` half).** `insert_input` advances the parser's flat line counter by the length of the generated text, for the rest of the file, and the error compounds with each list. For a document with two items on lines 10 and 11 and an ordinary `.. req::` on line 13: | need | written on | `lineno` before | after | |---|---|---|---| | `LN-BEFORE` | 4 | 4 | 4 | | `LN-A` | 10 | 14 | **10** | | `LN-B` | 11 | 20 | **11** | | `LN-AFTER` (an ordinary `.. req::`) | 13 | 27 | **13** | In the repository's own `doc_list2need` fixture, `NEED-Z` was recorded at line 45 of a 19-line file. The general case — a `lineno` shifted by `rst_prolog` or an `.. include::` — is a separate small PR, #1789. **Template artefacts.** An item written `()` with options lost them: suppressing the `:id:` left a blank line where it would have been, which ended the generated need's option block, so every option after it became body text. And a parent need's stored `content` was the generated reStructuredText of its children rather than its own text, which `needs.json` and any filter reading `content` saw. ### The one deliberate change to an input that worked Writing `()` used to suppress the `:id:` of the generated need, which sent it down the need directives' own id generator — a **different** function, which honours `needs_id_from_title` where `list2need`'s does not, hashes the *content* when the title is empty, and hashes the title *after* the `((option="value"))` area has been removed from it. One list could carry two id schemes at once, chosen by two characters of punctuation. The directive now derives every id itself, from the same input at the same point, so `()` means what no brackets at all means. Three kinds of item therefore get a different generated id: | item | before | after | |---|---|---| | `* ()Alpha title` with `needs_id_from_title = True` | `R_ALPHA` | `R_D1EC6`, the plain hash | | `* (). some content here` (no title) | `R_EBC29`, the hash of the **content** | `R_DA39A`, the hash of the empty title | | `* ()Title with empty parens ((status="open"))` | `R_6AFF7` | **`R_D7997`** — the id `* Title with empty parens ((status="open"))` already had | The third needs no configuration to reach and is the one to watch for; the changelog says so under Breaking changes, and the documentation carries a `versionchanged`. Everything else keeps the id it had, which the untouched id tests prove. ### Four inputs that were errors, and are now defined None of these can appear in a document that builds cleanly today, so nothing that works can regress; they are listed because each is a decision the template used to delegate to docutils and the directive now has to make. - **`((id="MY-ID"))`** names the need. The template wrote the derived id *and* the inline one as two `:id:` lines of one generated need, which docutils refused with `duplicate option "id"`, dropping the item. The id is applied before `:links-down:` is built, so the other items' links agree with it. An empty `((id=""))` is refused with a diagnostic rather than quietly replaced by the title hash. - **An inline option naming the same link field as `:links-down:`** — `((links="X"))` under `:links-down: links` — produced one corrupt link value (the template's `{%- for %}` glued the two field lines into `:links: C-2:links: X`) and an `unknown outgoing link` warning. The two sets are now merged. - **`((title_from_content="true"))`** is read as a need directive reads it. - **A child of a `((hide="true"))` item** is rendered at the level above it. A hidden need is taken out of the document once read, so nesting a child inside one would put its target where no page can reach it. ### Evidence - The rendered HTML of `tests/doc_test/doc_list2need/` is **byte-identical** before and after, all four pages, apart from the per-build random `SNCB-…` container ids. Its `needs.json` keeps all 16 ids, every title, status, tag, link and `parent_need`; only the linenos and the two parent `content` strings change. - The characterization suite added in #1788 is the safety net. The eleven build-aborting inputs and the `links-down` level-skip test pass **untouched** — this PR takes neither of those on. **Four** assertions are edited, each one pinning something the template did: the drifted linenos, the MyST failure (which inverts), the parent's generated-text content, and the second id function. A fifth prediction went the other way (below). - Two tests gain an assertion: the Markdown ones now pin the items' line numbers. Linenos inside an `{eval-rst}` fence were wrong too — that block was the only way to reach the directive from a Markdown document, and its needs were recorded at 10 and 16 for items written on 7 and 8. - `tests/__snapshots__/test_list2need.ambr` is rewritten: every `lineno` in it was wrong. - Six tests are added: the `()`-plus-options id, a directive written in an item's content, and one for each of the four inputs above. One assertion moved the other way. Removing the three-space indent the parser prepends to a `:`-continuation line looked like removing a workaround for the template — it is not: it is the only thing standing in for the indentation `lstrip` removes, and without it the options of a directive written in an item's content (the `rst-directives in lists` example in the documentation) land in the directive's own column and are dropped. It is kept, and now has a test. Refs #1427, Refs #1349.
This was referenced Aug 26, 2026
Merged
chrisjsewell
added a commit
that referenced
this pull request
Aug 27, 2026
## Release v8.4.0 Minor release. Headline changes: the `needflow` portable vocabulary (`:direction:`, named legends, `:show_link_names:` values), the new `needs_card_layouts` configuration, `list2need` rebuilt to create its needs directly, and `max_items` on every view. ### Changes - Bump version to `8.4.0` in `sphinx_needs/__init__.py` - Bump `NEEDS_VERSION` to `8.4.0` in the Docker workflow - Stamp the changelog's "Unreleased" section as `8.4.0` — release date `27.08.2026`, the `v8.3.1...v8.4.0` compare link, and a release summary - Move the `needtable :style_col:` deprecation out of "Improvements" into a "Deprecations" heading, matching 8.2.0 - Add the one missing entry: 📚 `list2need` documentation corrections (#1788) `docs/_static/tutorial_needs.json` is **not** regenerated this time — #1755 stopped tracking it, so unlike 8.2.0 and 8.3.0 there is nothing to commit for it. There are no `.. versionadded:: unreleased` markers to stamp either; every one in `docs/` was already written as `8.4.0`. ### Coverage All 24 commits since 8.3.1 were reviewed against the changelog. 19 user-facing changes all have entries; 5 are internal or convention-exempt (#1747, #1764 — `docs/ubproject.toml` only; #1763 — README only; #1781 — tests only; #1769 — refactor, filed under "Internal changes"). One gap was found and filled: #1788's `list2need` documentation corrections. ### Release steps after merge 1. Merge this PR 2. Create and push tag `8.4.0` — the release pipeline (`.github/workflows/release.yaml`) publishes to PyPI on tag push
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
list2needhas one test — a happy-path build over one fixture project — and its documentation has drifted from what the code does. This PR adds tests that describe the current behaviour, and corrects the documentation to match it. It changes no behaviour: nothing undersphinx_needs/is touched.Tests. A new
tests/test_list2need_behaviour.pycovers what the existing fixture project does not: how(...)becomes an id, the inline((option="value"))area, the:delimiter:split, continuation lines,presentationandlinks-down(including a skipped indentation level), the eleven inputs that currently end the build, the directive in a Markdown document, and the line numbers recorded around the directive. Each case builds its own inline project, so the input sits next to the behaviour it pins. The existing test and its snapshot are unchanged. To let the two Markdown-host tests run in CI,myst-parseris added to thetestextra (>=4,<6, so the sphinx 7.4 matrix leg still resolves); without it those two tests skip.Documentation. The "List with need-ids" example rendered a need titled
(FEATURE, because the default delimiter splits(FEATURE.3)before the id is read; it now uses an id without a dot, and says why. A new "Need IDs" section states how ids are actually recognised (any bracketed group, first(to last), greedy) and documents the generated-id formula, its title-only hash input and the resulting cross-document collision. The claim that this is "the same mechanism asneed_part" is corrected. The indentation rule, the((...))rules, and the fact that the directive does not work in MyST documents are all now stated.Behaviours pinned as-is that a future change may wish to revisit. These are recorded as they are, not endorsed; each carries a
# NOTE: current behaviourcomment.test_id_capture), so the need is refused and losttest_id_capture()selects a different auto-id function, visible underneeds_id_from_titletest_empty_parentheses_select_a_second_auto_id_function((...))region is greedy, so prose between two regions is deletedtest_inline_optionstest_an_unquoted_option_value_is_dropped_without_a_diagnostic.delimiter truncates any title containing a full stop, with no escapetest_delimitertest_continuation_lineslinks-downinto a different subtreetest_a_skipped_level_redirects_links_down_into_another_subtreetest_malformed_input_aborts_the_buildtest_the_directive_does_not_run_in_a_markdown_documentlinenois shifted for the generated needs and for everything after the directive (#1349)test_a_list2need_shifts_the_line_numbers_after_itThis is groundwork. With the behaviour written down, a change to the implementation becomes reviewable: whatever it alters shows up as a deliberate edit to an assertion here. It is offered with the question in #1427 in mind — whether the syntax can be kept while the implementation improves — without presupposing the answer.