Skip to content

refactor: compose the shared BOOLEAN combinator instead of inlining i… - #521

Merged
SJrX merged 1 commit into
242.xfrom
issue-509
Aug 29, 2026
Merged

refactor: compose the shared BOOLEAN combinator instead of inlining i…#521
SJrX merged 1 commit into
242.xfrom
issue-509

Conversation

@SJrX

@SJrX SJrX commented Aug 29, 2026

Copy link
Copy Markdown
Owner

…ts spellings

Review follow-up. Three of the new validators wrote parse_boolean()'s twelve spellings out by hand, two of them folded into a larger choice set so the boolean was no longer distinguishable from the enum it sat next to. To systemd these are separate branches, and keeping them separate in the grammar means a later pass — a formatter normalising yes/Yes, completion offering only the sensible half — can tell which is which from the grammar rather than by re-sniffing the text.

ConditionVirtualization= AlternativeCombinator(names, BOOLEAN)
DuplicateAddressDetection= AlternativeCombinator(families, deprecatedBoolean(...))
PreferredSource= BOOLEAN_FALSE, a named home for the false-only spellings
config_parse_preferred_src accepts

Ordering matters in both alternations and is now commented at each site: BOOLEAN matches a prefix, so on none — a real virtualization_table entry, and a DAD family name — it would take the leading no and strand ne, which the classic first-full-match engine cannot back out of.

deprecatedBoolean() returns a FRESH terminal each call. FlexibleLiteralChoiceTerminal .deprecating() mutates in place, so reusing the shared BOOLEAN for DuplicateAddressDetection= would have attached its "For historical reasons" note to every boolean-valued setting in the plugin. DeprecationsTest now pins that isolation.

New tests for three things raised in review that turned out to already hold, so they stay holding: ConditionArchitecture=ppc64-le and =arm64-be are not truncated to their shorter prefixes, ConditionControlGroupController=cpuacct io parses as two controllers rather than stopping at cpu, and ConditionFirstBoot=/ConditionACPower= still take every boolean spelling with the [|] [!] markers. FlexibleLiteralChoiceTerminal sorts its choices longest-first in its init block, so declaration order in these files is cosmetic — the tests say so explicitly to stop someone "fixing" it later.

…ts spellings

Review follow-up. Three of the new validators wrote parse_boolean()'s twelve spellings out
by hand, two of them folded into a larger choice set so the boolean was no longer
distinguishable from the enum it sat next to. To systemd these are separate branches, and
keeping them separate in the grammar means a later pass — a formatter normalising yes/Yes,
completion offering only the sensible half — can tell which is which from the grammar
rather than by re-sniffing the text.

  ConditionVirtualization=   AlternativeCombinator(names, BOOLEAN)
  DuplicateAddressDetection= AlternativeCombinator(families, deprecatedBoolean(...))
  PreferredSource=           BOOLEAN_FALSE, a named home for the false-only spellings
                             config_parse_preferred_src accepts

Ordering matters in both alternations and is now commented at each site: BOOLEAN matches a
*prefix*, so on `none` — a real virtualization_table entry, and a DAD family name — it
would take the leading `no` and strand `ne`, which the classic first-full-match engine
cannot back out of.

deprecatedBoolean() returns a FRESH terminal each call. FlexibleLiteralChoiceTerminal
.deprecating() mutates in place, so reusing the shared BOOLEAN for DuplicateAddressDetection=
would have attached its "For historical reasons" note to every boolean-valued setting in the
plugin. DeprecationsTest now pins that isolation.

New tests for three things raised in review that turned out to already hold, so they stay
holding: `ConditionArchitecture=ppc64-le` and `=arm64-be` are not truncated to their shorter
prefixes, `ConditionControlGroupController=cpuacct io` parses as two controllers rather than
stopping at `cpu`, and ConditionFirstBoot=/ConditionACPower= still take every boolean
spelling with the [|] [!] markers. FlexibleLiteralChoiceTerminal sorts its choices
longest-first in its init block, so declaration order in these files is cosmetic — the tests
say so explicitly to stop someone "fixing" it later.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Unit Test Results (grammar engine false)

1 263 tests   1 263 ✅  52s ⏱️
  316 suites      0 💤
  316 files        0 ❌

Results for commit 56ef309.

@github-actions

Copy link
Copy Markdown

Unit Test Results (grammar engine true)

1 263 tests   1 263 ✅  1m 1s ⏱️
  316 suites      0 💤
  316 files        0 ❌

Results for commit 56ef309.

@SJrX
SJrX merged commit 9b8dd4d into 242.x Aug 29, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant