✨ sphinx-needs: choose / when / otherwise directives, one of several branches by variant data - #2020
chrisjsewell wants to merge 27 commits into
Conversation
The evaluation of an `if` condition moves out of `IfDirective.run` into a module-level function, `evaluate_variant_condition`, which takes the directive name and the warning subtype as parameters. `if` calls it with `"if"` / `"if"`, so its three messages, its warning subtype and its behaviour are unchanged: unconfigured variant data and an expression that raises skip the body, and a result that is not a bool is warned about and then used as its truth value. This is preparation for the `case` directive of `match` (#2011), whose conditions must mean exactly what the same text means in `if`. One function for both makes that true by construction rather than by keeping two copies in step. The `if` warnings were compared byte for byte against the previous code over nine expressions, with and without variant data configured: identical.
A `match` holds `case` directives and comments. The first `case` whose condition holds is included, and a `case` with no condition is the default, which must be the last. Only the content of that case is parsed, so the needs in every other case are never created, as for a false `if`. Why a container rather than `elif` / `else` siblings: a directive cannot see its source siblings, so sibling branches have to leave state behind for the next one, and every mechanism that needs (a marker node after every branch, a backward scan of the parent, a stripping transform, a second strip for the need-node cache) exists only for that. A `match` sees all its cases at once, decides once, and returns only the winner, so none of it is needed and `if` stays as it is. How it works: - `case` does not parse its content. Inside a `match` body it returns a private placeholder carrying its condition, its raw content and its location; outside one it warns and is skipped. - `match` parses its body into a detached node it never returns, accepts only placeholders and comments there, checks that there is at most one default and that it is last, and evaluates the conditions in order with `evaluate_variant_condition`, the evaluator `if` uses, so a condition means the same in both. Later conditions are not evaluated once a case is taken. - Whether a `case` is in a `match` body is a depth counter in `env.temp_data`, raised only around the body parse; the winner is parsed at depth 0, so a `case` loose in a case's content is caught too. - Under MyST a directive's `content_offset` is relative to its own line, so the winner's offset is re-based onto the `match`'s line there. - Content written directly in the body runs its directives when the body is parsed, so any need the body parse created is removed again: case content is deferred, so such a need can only come from that mistake. Every mistake warns once under the new `needs.match` subtype and skips the whole `match`: an argument on `match`, content other than cases and comments, no case, two defaults or a default before another case, variant data that is not configured (even for a default-only `match`), and a condition that cannot be evaluated, which also keeps the default from being taken, so a typo in the intended case never renders the fallback in its place. A result that is not a bool is warned about and used, as in `if`. A `system_message` in the body was already reported by docutils or MyST, so the `match` is skipped without a second warning. The placeholder and the body node are not registered with `add_node`, there is no transform, and `api/need.py` is not touched.
A doc project, `doc_match_directive`, holds the happy paths in reStructuredText: the first true case wins while a later condition that is not Python, and one naming an unknown key, are never evaluated (the build has no warning at all); the default is taken, and nothing at all when no case holds and there is none; the needs of the untaken cases are absent from the needs view and the taken one records its source line; a heading in the taken case is a section nested where it stands; a nested match; an include that supplies the cases and one inside the taken case; and a match in a need's content that `needextract` renders on another page. No placeholder or body node reaches any pickled doctree or the need-node cache. One parametrised table holds every mistake of the contract (#2011), each asserted to warn exactly once, with its text, `[needs.match]`, and the `<srcdir>/index.rst:N:` line of the offending directive or child, and to render nothing of the match; the cases of #1999's warning table map onto it one to one. A docutils error in the body is reported once, by docutils. The MyST twin covers colon and backtick fences, `%` comments and `+++`, needs in the taken and untaken case (with the true line, which the content-offset re-basing gives), nesting, a default with trailing spaces, needextract, and the MyST warnings; lines are asserted for the backtick spellings only, since MyST reports a directive nested in a colon fence one line late. One table of expressions runs through both `.. if::` and `.. case::` and asserts the same verdict and the same warning text for each, so the two directives cannot drift into two condition languages. `test_if_directive.py` and its projects are unchanged.
`docs/directives/match.rst` documents the directive pair after `if` in the directives index: the RST and MyST spellings (colon fences first, with the rule that an outer fence must be longer than the fences inside it, and `%` as the MyST comment), the rules (the first true case wins and later conditions are not evaluated, the default comes last, only cases and comments, the included case is ordinary content), that a condition is exactly an `if` condition, the one-line-condition pitfall, every warning under `needs.match`, and the one limitation: a directive that produces no node, written directly in a `match`, is not detected and still runs. `if.rst` gains a one-line pointer to `match`, a label on its expression context for that page to link to, and the warning `if` emits for a condition whose result is not a bool, which it has always emitted and never listed. The changelog gains an `Unreleased` section with the entry; its pull request number is a placeholder until the pull request exists.
Every `case` of a `match` must now be written in the body of that `match`.
A `case` that an `.. include::` (or a MyST `{include}`) supplies
warns once under `needs.match`, at the `case` in the included file,
and the whole `match` is skipped, before any condition is evaluated.
An include inside the content of a case stays fine,
and so does a whole `match` written in an included file.
Why: one choice should be one directive in one place.
Splitting a choice across files is exactly what the sibling `elif` / `else`
design allowed and this one was chosen to avoid,
and ubCode, which implements the same contract, sees an include in a `match` body
as an invalid child (it splices includes as a separate tree, so it cannot see
the cases one supplies). Refusing it here keeps both engines on one rule.
How: a `case` records the source docutils or MyST attributes its line to
(`get_source_info()`), and the `match` compares it with its own.
Measured in both parsers, on Sphinx 9.1 and 7.4:
docutils reports the included file and its exact line;
MyST reports the included file too, with the line one late, as it does for
every line of an included file.
The doc project's include-supplied cases become two negative cases in the
warnings table, with a MyST twin, and a whole `match` in an included file is
added to the RST and MyST happy paths. `match.rst` states the rule and lists
the warning.
…quiet ones
Two ways to get a `match` wrong were not reported, which broke its promise
that every mistake warns once.
A line of one to three punctuation characters in the body, such as `---`
between two cases, makes docutils append an INFO message, which is below the
report level and never shown, BEFORE the paragraph it then makes of the line.
The match took every `system_message` child for one docutils had already
reported, and skipped itself in silence: under `-W` the build was green with
the content of the match gone. A message below the reporter's report level is
now passed over, so the paragraph after it is judged as any other content and
warns once, at its own line. A message at or above that level keeps the
silent skip, since docutils or MyST did show it.
A `case` inside a directive that returns the nodes of its own content
(a true `if`, a `rst-class` with content, a MyST `{eval-rst}` block) was
adopted as a case of the match, though it is not written in the match at all;
under `{eval-rst}` its RST content was then parsed as Markdown. A `case` now
records the node it is appended to (`state_machine.node`, which is docutils'
`RSTState.parent` and MyST's current node alike: measured identical in both
parsers on Sphinx 9.1 and 7.4), and the match refuses, with one warning at the
case, any case whose owner is not its own body. This is also the rule ubCode
applies to the same contract: anything but a case or a comment as a child of
a `match` is an invalid child. The depth counter still decides whether a
`case` is outside every `match`, so a case inside a `note` in a match still
gives exactly one warning, the match's `got <note>`.
Tests: a `---` and a `...` line, cases in a true `if` and in `rst-class`, and
a case in `{eval-rst}` in both MyST spellings each warn once and render nothing.
`match.rst` says that a case belongs directly in its match, lists both
mistakes, and states plainly that a FALSE `if` around cases returns nothing,
so those cases vanish without a warning unless the match is left with none.
The changelog entry's list of mistakes gains the case inside another directive
and the include refusal.
…er of `match` Three behaviours of `match` were correct but unguarded: a reviewer's mutations of each passed every test, and each mutant crashes or misleads a real build. - The rollback removes the NEWEST needs, the ones the body parse added. A new test writes a need before the stray-need `match` and one after it on the same page, and one in a document read earlier, and asserts all three survive while the stray one goes. Removing the oldest needs instead deletes a legitimate need and crashes `analyse_need_locations`. - The depth counter is restored in a `finally`. A new test has a project directive catch an exception raised from inside a `match` body, and asserts that the loose `case` after it is still reported; without the `finally` it becomes a placeholder that reaches the writer. - The structure is checked before the configuration. A new row has no variant data and a misplaced default, and expects the one structural warning at the case; checking the configuration first would report the other warning at the match and never parse the body. Also a row for an argument-less `case` outside every match (the counterpart of #1999's orphan `else`), in RST and MyST. The warning about content in the body now names the first node the author wrote: a need directive emits a target without a line before the need, so a stray need was reported as `got <target>`; it now says `got <Need>`, at the same line. A label written in the body is still reported as a target. Prose: the one-line-condition sentence said a wrapped condition is always a syntax error, but a break inside brackets evaluates; and "a typo in a condition never renders the default" was not true for a typo that leaves a valid, false condition. Both now say what holds: an unevaluable condition, such as a misspelt key or a syntax error, never renders a later case or the default in its place. Two test comments no longer claim to guard the whitespace strips the parsers already perform.
…h` mistake
The match passed over a `system_message` in its body only when it was below
the reporter's report level, taking any other for an error docutils had shown.
A project whose `docutils.conf` sets `report_level: 1` shows INFO messages,
but as information rather than warnings: the INFO docutils puts before the
paragraph of a `---` line then made the match skip itself in silence again,
and `-W` stayed green with its content gone. A message is now taken as
reported only at WARNING or above (and at or above the report level), so the
paragraph is reported whatever the report level is. A new test sets the level
through the project's `docutils.conf`, as `sphinx-build` reads it.
The include check now runs before the direct-child check, so a case that
arrives through an include standing inside another directive (such as
`.. include:: … :parser: …`) gets the include wording.
`match.rst`: a wrapped condition also evaluates when the break is escaped with
a backslash, and the limitation note names MyST substitutions too: a
`{{ sub }}` in a `match` whose definition holds cases is expanded in place,
and those cases are taken without a warning.
The `match` / `case` entry under `Unreleased` was stamped with a placeholder until the pull request existed; it is #2020.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2020 +/- ##
==========================================
+ Coverage 91.87% 91.93% +0.06%
==========================================
Files 129 130 +1
Lines 18084 18323 +239
==========================================
+ Hits 16614 16845 +231
- Misses 1470 1478 +8
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:
|
`match` and `case` name a construct with a subject, whose cases are patterns tested against it (Python, Rust); the container built here has no subject and holds conditions, which is XSLT's, JSTL's and MSBuild's `choose` / `when`. The rename leaves the names `match` and `case` free for a subject-based form later (#2011). This commit only renames; behaviour is unchanged, and the default is still a `when` with no condition until the next commit gives it a directive of its own: - `needmatch.py` becomes `needchoose.py`, with `ChooseDirective`, `WhenDirective`, `_BranchPlaceholder` and `_ChooseBody`, registered as `choose` and `when` next to `if`; the `env.temp_data` key becomes `sphinx_needs_choose_depth`. - The warning type `needs.match` becomes `needs.choose`, sorted into `WarningSubTypes`, and every message names `choose` and `when`. - The test module, its doc project and the docs page are renamed with `git mv` (`test_choose_directive.py`, `doc_choose_directive/`, `docs/directives/choose.rst`, label `choose`), as are the toctree entry, the pointer in `if.rst` and the changelog entry. `if` is untouched: only the evaluator's docstring names `when` now.
…dition A `when` with no condition was the default of its `choose`, so a `when` whose condition had been forgotten silently became the catch-all for every other variant. The default now has a directive of its own, `otherwise`, and every `when` must have a condition. These are the only behaviour changes; every other rule of `choose` is as before. - `when` declares its argument optional, not required, so that the `choose` refuses a missing or blank condition itself, once, at the `when`: "'when' directive has no condition (use 'otherwise' for the default); the whole choose is skipped". A required argument would have made docutils reject the directive with an error of its own. - `otherwise` declares an argument only to refuse one: "'otherwise' directive takes no condition; the whole choose is skipped". It must be the last branch, at most one per `choose` (the former default-position warnings, reworded for `otherwise`). - A branch outside a `choose`, a branch inside another directive and a branch supplied through an include are warned about under the name of the directive it is written with; a `choose` with no branch says it has no 'when' or 'otherwise'. - The `choose` argument is refused as before, but no longer described as reserved: `match` and `case` stay free for a form with a subject. Both new refusals are structural, so they are checked after the body's contents and before the configuration. `when` and `otherwise` share one private base directive, and the placeholder carries the kind of its branch, so that the `choose` can tell a `when` without a condition from an `otherwise`. Tests: every default in the module and its doc project is an `otherwise` now; new rows for a `when` without a condition (RST, MyST backticks, and a blank condition in MyST colons), an `otherwise` with a condition (RST and MyST), an `otherwise` outside a `choose` (full text), inside a true `if` and supplied through an include, and the check order of a `when` without a condition when variant data is not configured.
`choose.rst` now describes the directives as built: `choose` runs its
`when` tests in order and the first true one is included; `otherwise` is
the optional default and comes last; when no test holds, nothing is
rendered. It names the precedent (XSLT, JSTL, MSBuild), says that a
`choose` has no subject, and lists the two new mistakes: a `when` without
a condition, and an `otherwise` with one. The MyST examples use
`{otherwise}`.
The content of the taken branch is parsed into a detached container, as
the body of a true `if` is, so a sphinx-design `tab-item` in it warns that
its parent should be a `tab-set`, even when the directive stands in one.
`choose.rst` says so in a note, with the remedy (put the `choose` inside
the `tab-item`), and `if.rst` gains a one-line bullet. A new test pins
the warning for both directives, at each `tab-item`, with the same text,
and that a `choose` inside a `tab-item` does not warn.
The changelog entry names the three directives and lists the two new
mistakes.
…docs nits
The order between the condition faults and the `otherwise` count and
position was what the code did but nothing pinned: moving the count and
position checks before the condition faults passed every test. New rows,
in reStructuredText and as MyST twins for the first two:
- two `otherwise` and then a `when` without a condition warn about the
`when` (the condition faults, in document order, come first);
- an `otherwise` with a condition, a `when` and a bare `otherwise` warn
about the condition, at the first `otherwise`;
- two bare `otherwise` and nothing else warn "more than one 'otherwise'"
at the second.
An `otherwise` given a condition now names it ("'otherwise' directive
takes no condition, got '…'"), since the text may be content docutils
took for the argument because no blank line followed the directive;
`choose.rst` says so in its Warnings list.
The `tab-item` test no longer skips when sphinx-design is missing: it
imports it, so that the test fails if sphinx-design ever leaves the shared
`test` group, rather than silently no longer pinning the documented
limitation.
The two whitespace-only-argument guards are kept as the contract's, with
a comment saying that docutils and MyST already drop such an argument.
The `ChooseDirective` docstring and the changelog entry list "no branch at
all" among the mistakes, and `choose.rst` no longer claims that the names
mean exactly what they mean in XSLT, JSTL and MSBuild, which require a
`when`: it says that those run their tests the same way.
…ation With no argument declared, docutils does not reject the directive: like MyST, it moves the text into the content, which the `choose` would then report only as a stray paragraph. The declaration stays, since it gives the clearer message; only the comment is corrected.
The `_BranchDirective` docstring claimed docutils would reject an `otherwise` with an error of its own if no argument were declared; measured, docutils and MyST both fold the text into the content instead, where it would be taken silently. The docstring now states the measured reason for each directive.
Two rows (and a MyST twin) pin that a choose with two mistakes reports the first in document order: an otherwise with a condition before a when without one reports the otherwise, and a when without a condition before a stray paragraph reports the paragraph (the children are checked before the condition faults). Both orders are the contract ubCode implements too.
match / case directives, one of several branches by variant datachoose / when / otherwise directives, one of several branches by variant data
docutils writes the path of an included file with "/" on every platform (utils.relative_path), so on Windows a warning located in such a file starts with C:/... while the source directory is spelt C:\...; the srcdir rewrite to <srcdir>/ missed it and the three choose rows that locate a warning in an included file failed every Windows cell of CI. build_warnings now rewrites the POSIX spelling too (a no-op elsewhere), with a unit test that gives it a Windows source directory by hand.
The Codecov upload steps required github.event.pull_request.head.repo to equal the repository, a guard against fork pull requests that has no value on a push event, so since #2009 every upload on master was skipped (measured on the run for cc0ffe0: all five "upload to Codecov" steps skipped). Codecov then compared every pull request against the newest ancestor with a report, d68d10d, 35 commits back and from before that PR split sphinx-test-reports, so the reports flag read -0.74% on pull requests that never touched the package. The guard now applies to pull request events only; a push to master uploads under its own secrets.
|
Reviewed at I also built against the head. A What is left are a few places where a mistake still lets the 1. [Medium] A branch written with one colon silently hands the choice to sphinx-needs/packages/sphinx-needs/src/sphinx_needs/directives/needchoose.py Lines 344 to 345 in dadd8c4
2. [Medium] With sphinx-needs/packages/sphinx-needs/src/sphinx_needs/directives/needchoose.py Lines 347 to 354 in dadd8c4 A system message below
3. [Medium] The include rows of the suite depend on where the checkout lives (tests) docutils records an included file with
Locating the warning with an absolute path, as Sphinx does for node locations, fixes both the tests and the warning. 4. [Low] The body rollback is partial (durability) sphinx-needs/packages/sphinx-needs/src/sphinx_needs/directives/needchoose.py Lines 290 to 297 in dadd8c4
5. [Low] Invalid escape sequences in two new docstrings (correctness)
6. [Low] Land the Codecov change on its own, and fix three details (CI) sphinx-needs/.github/workflows/test-package.yaml Lines 137 to 144 in dadd8c4 It is unrelated to
|
Reverts dadd8c4 on this branch: the change lands separately, with the review corrections (fail_ci_if_error limited to pull requests, the token descriptions, the history of the guard), so that a squash merge of this branch does not bury a workspace-level CI change in a feature commit.
Documentation build overview
|
Every Codecov upload step is guarded by `github.event.pull_request.head.repo.full_name == github.repository`, a guard against fork pull requests (#1229, August 2024) that has no value on a push event. So no push to master has uploaded a report since then; measured on the CI run for `cc0ffe0`, all five "upload to Codecov" steps are skipped. Codecov compares a pull request against the newest ancestor that has a report. That ancestor is now `d68d10d`, 35 commits back and from before the sphinx-test-reports split (#2009), which moved statements out of the `reports` flag, so `codecov/project/reports` reads as a coverage drop (−0.74%) on every pull request, including ones that never touch the package (#2020 is the current example). A looser threshold in `codecov.yml` would hide this one delta while the base kept drifting; the cause is the missing upload. ### What changes - The fork guard applies to pull-request events only: `(github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository)`. A push to master uploads under the repository's own secrets; the dependabot and repository checks are unchanged. - `fail_ci_if_error` is limited to pull requests, so a Codecov outage cannot turn master red. - The two `CODECOV_TOKEN` secret descriptions say when the steps read it. - The reason is recorded in the step comment, with the history (#1229 wrote the guard, #2009 copied it onto the ub-test-reports step). ### What to expect The first push to master after this merges uploads a fresh report; pull requests opened or rebased after that compare against it. Pull requests whose base is older, #2020 included, keep their current `reports` status until they rebase or merge; it is not a finding about them. Split out of #2020 at review's request; that branch reverts its copy of the change.
`.. when: var.arch == "arm"` (one colon) is a comment in reStructuredText, and so is `.. when::var.arch == "arm"` (no space after `::`): the comment swallows the indented branch under it, comments are accepted in a `choose`, and so for `arm` the `choose` rendered its `otherwise` with no warning, which is exactly the mistake the explicit `otherwise` exists to catch (review of #2020, point 1). A comment in the body whose text, after leading whitespace, begins with `when` or `otherwise` and a colon (case-insensitive) is now refused with one warning at the comment: "'choose' directive has a comment that begins with 'when:' (a branch written with one colon? write '.. when:: <condition>'); the whole choose is skipped" (for `otherwise`, "write '.. otherwise::'"). It is a fault of a child, found in document order with the others, before the no-branch check and the condition faults. A comment that merely begins with the word ("when we migrate, drop this") is still accepted. A MyST `%` comment follows the same rule. A comment carries no line of its own (docutils gives it the line after it, MyST the `choose`'s), so the warning's line is found in the content of the `choose`: the first body-level comment line the rule matches, skipping the lines docutils reads as `when` / `otherwise` directives and, under MyST, the lines inside the fences of the body's directives. Tests: `[when with one colon]`, `[otherwise with one colon]`, `[when without the space after ::]`, the control `[a comment that starts with the word when]` (no warning, its branch taken), the order pair `[a when without a condition, then a when with one colon]`, and MyST `[when with one colon, % comment]` with its control (a row without a text is now a control in the MyST table). `choose.rst` and the changelog entry list the mistake.
Two rows pin that a comment beginning with `When:` or `when :` is refused like `when:`, since ubCode mirrors the rule; and the docs bullet says what a one-colon `otherwise` does (the default vanishes) rather than claiming it hands the choice to the `otherwise`.
The warning about a comment that begins with `when:` or `otherwise:` told
a MyST author to write `.. when:: <condition>`, which is reStructuredText.
Under MyST it now says "write a '{when} <condition>' fence" (or "an
'{otherwise}' fence"); the reStructuredText spelling is unchanged. ubCode
gives the same hint by syntax.
The MyST row for `% when:` expects the fence hint, and a new MyST row pins
the `otherwise` form.
docutils records an included file with `utils.relative_path(None, path)`, which is relative to the working directory whenever the two share their first two path components: a checkout under /tmp building under /tmp, or, on Windows, a checkout and %TEMP% both under C:\Users. A warning about a branch in an included file (or about anything in a `choose` that stands in one) then read `../…/branches.txt:1`, and the suite's rows that locate such a warning failed wherever the checkout lived (review of #2020, point 3). Every location the `choose` directives report now goes through one helper that makes the source absolute, as Sphinx does for a node's location: the `choose`'s own warnings, the stray-branch warning, the condition warnings the shared evaluator gives for a `when`, and the line the one-colon rule finds. A node location is left to Sphinx, which makes it absolute itself. The include check compares the two sources absolute as well. Measured from a worktree under /tmp, with this code: the three include rows failed before the change, located at `../../../../../../../../../sn_test_build_data/…/branches.txt:1`, and pass after it. A unit test pins the helper with a relative source.
…dows" This reverts commit 48d293d.
docutils records an included file relative to the working directory only when the two share their first two path components, so from a CI runner the absolute-location fix was never exercised. The warning rows now build from the source directory, where every include is recorded relative, and two rows cover the other exits that can report a location in an included file: an unevaluable condition inside an included choose, and a stray branch. Each exit now has a row that fails when it loses the absolute source.
New
choose,whenandotherwisedirectives select one of several branches of content by variant data.A
chooseholdswhenandotherwisedirectives (its branches) and comments, written in its own body:it runs its
whentests in order and the first true one is included;otherwiseis the optional default and comes last; when no test holds and there is nootherwise, nothing is rendered.The other branches are never parsed, so their needs are never created, exactly as for a false
if.The names are those of
choose/when/otherwisein XSLT, JSTL and MSBuild, which run their tests the same way.MyST:
::::{choose}/:::{when} <condition>/:::{otherwise}(the outer fence longer than the inner ones), or backtick fences.Closes #2011.
Why a container rather than
elif/elsesiblingsFor authors, a
choosereads the way a choice is thought about: every alternative for one piece of content stands in one block,in order, with the fallback visibly last. What is inside the block is the whole story.
A paragraph or a comment between two alternatives, or an
includeboundary, cannot detach a branch or turn a fallback into anorphan, and a branch written inside a wrapper directive that passes its content through is refused with a warning, because every
whenbelongs to itschooseand to nothing else. Mistakes are reported once, where they are made, and a mistake that makes a condition unevaluable never showsthe wrong variant: it skips the whole choice rather than falling through to the default, and a
whenthat lost its condition isreported rather than silently becoming the catch-all. A branch can hold anything a document can (sections, needs, includes, another
choose), and the branches not takenare never parsed, so no stray need or ID from another variant reaches the build. The same block builds the same way in
reStructuredText, in MyST and in ubCode (what still differs is registered in ubCode's divergence register), and in ubCode the editor fades the
branches that do not apply to the variant being built, so an author always sees which content is live.
ifis unchanged, so existing documents keep working as they are.This is also why a container is sound where sibling
elif/elseare not. A docutils directive cannot see its source siblings,so chained branches have to leave state behind for the next one, and every mechanism that needs (a marker node after every
branch including every plain
if, a backward scan of the parent, a MyST fallback through the inliner's parent, a strippingtransform, a second strip for the need-node cache) exists only for that. A
choosesees all its branches at once, decides once,and returns only the winner, so none of it is needed. This supersedes the sibling-chain design of #1999; its applicable test
cases and its
if.rstfix are salvaged here. ubCode implements the same contract (useblocks/ubcode#3783, companion issueuseblocks/ubcode#3763), so a document builds the same way in both tools.
Why
choose/when/otherwise, and an explicit defaultThis PR first named the directives
match/case, with acasewithout a condition as the default.In Python and Rust a
matchtakes a subject and everycaseis a pattern tested against it;this construct has no subject and holds conditions, which is XSLT's, JSTL's and MSBuild's
choose/when/otherwise,so the names were changed (#2011, comment by @ubmarco), leaving
match/casefree for a subject-based form later.The default is now a directive of its own: a
whenmust have a condition,so a condition forgotten on the last branch warns instead of silently making it the catch-all for every other variant.
How
whenandotherwisedo not parse their content: inside achoosebody each returns a private placeholder(kind, condition, raw
StringList, content offset, line, location, source file, and the node it is appended to);outside one it warns and is skipped. Both declare an optional argument,
whenso that a missing condition is reported by thechoosewith one warning (a required argument would make docutils refuse the directive with an error of its own),otherwiseso that a condition on it can be refused.chooseparses its body into a detached node it never returns, accepts only placeholders and comments there,refuses a branch that is not a direct child of its body (one inside another directive, even a true
if,a
rst-classor a MyST{eval-rst}that hands its content's nodes through)and a branch supplied through an include (its source file differs from the
choose's),checks the branches (every
whenhas a condition,otherwisehas none, is last and occurs at most once),and then evaluates the conditions in order;
only the winner is parsed (
nested_parse_with_titles), so headings in it are real sections.Later conditions are never evaluated.
evaluate_variant_condition, the evaluator moved out ofIfDirective.run(
if's messages and behaviour are byte-identical), so a condition means the same inifandwhen.env.temp_data(raised only around the body parse, restored in afinally;the winner is parsed at 0) detects a
whenorotherwiseoutside achoose, including one loose in a branch's content.content_offsetis relative to its own line, so the winner's offset is re-based there.---line, whatever the project's report level)is not taken for a reported error, so the content after it warns; a message docutils or MyST did report skips the
choosewithout a second warning.
add_node, no change toapi/need.py, andifis untouched apart from the extraction.get_node_locationdoes: docutils records anincluded file relative to the working directory whenever the two share their first two path components, which printed
../…locations and made the include rows of the suite depend on where the checkout lives (every Windows cell, and anycheckout under
/tmp).Every mistake warns once under the new
needs.choosetype and skips the wholechoose:an argument on
choose; content other than branches and comments (a line of punctuation such as---included);a branch inside another directive or supplied through an include; a comment that begins with
when:orotherwise:(a branch written with one colon, or without the space after
::, is a comment, and would hand the choice to theotherwiseor make the default vanish; the hint names the spelling of the syntax in use); no branch at all; a
whenwithout a condition;an
otherwisewith one, a second one, or one that is not last;unconfigured variant data (even for an
otherwise-onlychoose); and a condition that cannot be evaluated,so a mistake that makes a condition unevaluable, such as a misspelt key or a syntax error,
never renders a later branch or the default in its place.
A non-bool result warns and is used, as in
if.The documented limitations: a directive that produces no node (such as
default-role, or a FALSEif, whose branches thenvanish) written directly in a
chooseis not detected, and neither is content a MyST substitution supplies; the non-need effectsof stray body content (a label, a
needextend) stay. One inherited fromifand documented on both pages: a sphinx-designtab-itemdirectly inside the winner warns that itsparent should be a
tab-set, because the winner is parsed into a detached container, exactly as a trueifdoes.Tests
tests/test_choose_directive.py(94 tests) andtests/doc_test/doc_choose_directive/:the happy paths (first true wins with later invalid conditions never evaluated,
otherwise, nootherwise,needs and their line numbers, sections, nesting, an include inside a branch, a whole
choosein an included file,a
choosein a need extracted byneedextract);a parametrised table of every mistake (one warning each, its text,
[needs.choose]and its line,in the included file for an include-supplied branch), including the check order
(no variant data and a misplaced
otherwisegive the one structural warning; two branch faults report the first in document order);the rollback keeping the needs written before and after the
chooseand in an earlier document;the depth counter restored when a body raises;
the MyST twin (colon and backtick fences,
%and+++, nesting, includes,{eval-rst}, needextract, the MyST warnings);the one-colon rows (
whenandotherwise, capitalised, with a space before the colon, without the space after::, the MyST%form, and the control comment that merely starts with the word);the warning rows built from the source directory, where docutils records an included file relative to the working directory,
with a row for each exit that can locate a warning in an included file;
a check that no placeholder reaches any pickled doctree or the need-node cache;
the
tab-itemwarning pinned inside a winningwhenand inside a trueif;and one expression table run through both
.. if::and.. when::asserting identical verdicts and warnings.Every row of #1999's warning table that has a counterpart in a container design is in that table.
tests/test_if_directive.pyis unchanged and green; the two files also pass on the sphinx-7 cell (Sphinx 7.4, myst-parser 4.0.1),and the full suite is green (1959 passed).
Docs
New
docs/directives/choose.rst(afterifin the directives index);if.rstgains a pointer tochoose,a label on its expression context, the non-bool warning it always emitted but never listed (salvaged from #1999),
and the
tab-itemnote.Changelog
docs/changelog.rstgains anUnreleasedsection with the entry (the:pr:reference is this PR's number).Review
Two independent review passes on the
match/casedraft (one refuting fourteen named claims with its own probes andmutations, one re-running every gate and mutation proof by hand) found no code defect; their findings (a silent skip on a
below-report-level docutils message, cases adopted through a transparent wrapper, four test gaps, three prose overclaims) were
fixed and validated. The rename and the explicit
otherwisewere then validated by two further review passes of the same shape on the renamed tree,whose findings (the check order between the branch faults pinned, the refused
otherwisetext named, two comments and docs nits) are fixed in the commits after the docs commit.The one-colon rule from the review below was then added in both engines, each validated by one of those reviewers.
Related
reportsflag read as a drop) landed as 🧪 CI: upload coverage on pushes to master too #2023, split out at review's request; this branch carries a revert of its own copy, sothe squash nets to nothing for the workflows. This PR's
reportsstatus refreshes once it rebases or merges.Merging
When squashing, strip the
Co-authored-bytrailer GitHub proposes (the commits carry the sandbox's git identity).Follow-ups (not in this PR)
__bool__raising) aborts the build inwhenexactly as inif(bool(result)isoutside the evaluator's
try); inherited, tiny.choosebody (a FALSEif,default-role), or content a MySTsubstitution supplies, is undetectable from the doctree; documented, and registered as a divergence from ubCode, which
diagnoses any non-branch child.
match/case(patterns, Python semantics) may follow; the names are left free for it... when cond) is a comment thechoosecannot tell from one; the one-colon rulecovers the likely slip (
.. when: cond,.. when::cond) and leaves that one documented.ifwarnings located in an included file still carry docutils' relative spelling, soifandwhencan name the same filedifferently; the same absolute-source treatment belongs in
needif.py, outside this PR's rule not to touchif.