Skip to content

refactor(parser): promote instead-override SpecialClause variants to ClauseDisposition::ReplaceMeaning (U5-M2)#5569

Merged
matthewevans merged 1 commit into
mainfrom
ship/u5-m2-replacemeaning
Jul 11, 2026
Merged

refactor(parser): promote instead-override SpecialClause variants to ClauseDisposition::ReplaceMeaning (U5-M2)#5569
matthewevans merged 1 commit into
mainfrom
ship/u5-m2-replacemeaning

Conversation

@matthewevans

Copy link
Copy Markdown
Member

Fifth U5-M2 increment. DigInsteadAlt + InsteadClause + KeywordInsteadOverride
(CR 608.2c / CR 614.1a / CR 608.2e — all intrinsic: None) share the structural
role "replace/override the meaning of the prior emitted def(s), emit no sibling"
and fold into ClauseDisposition::ReplaceMeaning { kind: ReplaceMeaningKind },
where kind (DigAlt | Instead | KeywordOverride) carries each variant's payload
and keeps the distinct rules concept typed. AdditionalCostInsteadSearch
(intrinsic-carrier) and DrawnThisTurnPayOrTopdeck are deliberately left for inc6.

The three lower.rs handler bodies — including InsteadClause's intricate multi-clause
Cow-swap (mem::take the def chain, build the base, stash the tail in the override's
else_ability) — move byte-for-byte under match kind; only the arm pattern names
change (the else-if block evaluates to the match's bool, so the per-arm true
bodies are untouched). All three SpecialClause variants are deleted.

Faithful-by-construction (ClauseDisposition never reaches a serialized snapshot).
All three kinds have real fixture cards (DigAlt 2, Instead 24, KeywordOverride 1);
parity: Follow the Lumarets (DigAlt — else_ability + Dig), Evil's Thrall (Instead —
ConditionInstead override + Untap/Haste tail in else_ability), Conformer Shuriken
(KeywordOverride — sub_ability + counters). Zero snapshot drift.

…ClauseDisposition::ReplaceMeaning (U5-M2)

Fifth U5-M2 increment. DigInsteadAlt + InsteadClause + KeywordInsteadOverride
(CR 608.2c / CR 614.1a / CR 608.2e — all `intrinsic: None`) share the structural
role "replace/override the meaning of the prior emitted def(s), emit no sibling"
and fold into `ClauseDisposition::ReplaceMeaning { kind: ReplaceMeaningKind }`,
where `kind` (DigAlt | Instead | KeywordOverride) carries each variant's payload
and keeps the distinct rules concept typed. AdditionalCostInsteadSearch
(intrinsic-carrier) and DrawnThisTurnPayOrTopdeck are deliberately left for inc6.

The three lower.rs handler bodies — including InsteadClause's intricate multi-clause
Cow-swap (mem::take the def chain, build the base, stash the tail in the override's
else_ability) — move byte-for-byte under `match kind`; only the arm pattern names
change (the else-if block evaluates to the match's bool, so the per-arm `true`
bodies are untouched). All three SpecialClause variants are deleted.

Faithful-by-construction (ClauseDisposition never reaches a serialized snapshot).
All three kinds have real fixture cards (DigAlt 2, Instead 24, KeywordOverride 1);
parity: Follow the Lumarets (DigAlt — else_ability + Dig), Evil's Thrall (Instead —
ConditionInstead override + Untap/Haste tail in else_ability), Conformer Shuriken
(KeywordOverride — sub_ability + counters). Zero snapshot drift.
@matthewevans
matthewevans enabled auto-merge July 11, 2026 10:05

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the representation of meaning-replacement overrides in the parser by introducing the ReplaceMeaningKind enum and ClauseDisposition::ReplaceMeaning variant, replacing several older SpecialClause variants. It also adds corresponding snapshot tests. The reviewer pointed out a violation of Rule R6 in lower.rs, where the KeywordOverride match arm is missing a mandatory CR 608.2e annotation comment.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +1601 to 1602
ReplaceMeaningKind::KeywordOverride => {
// Build the def for this clause and attach to previous as sub_ability

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

According to the repository style guide (Rule R6), every rules-touching line of engine code must carry a verified CR <number> annotation. Please add the CR 608.2e annotation to the KeywordOverride arm comment.

Suggested change
ReplaceMeaningKind::KeywordOverride => {
// Build the def for this clause and attach to previous as sub_ability
ReplaceMeaningKind::KeywordOverride => {
// CR 608.2e: Build the def for this clause and attach to previous as sub_ability
References
  1. Every rules-touching line of engine code must carry a comment of the form CR <number>: <description> (regex CR \d{3}(\.\d+[a-z]?)?). (link)

@matthewevans
matthewevans added this pull request to the merge queue Jul 11, 2026
@github-actions

Copy link
Copy Markdown

Parse changes introduced by this PR

✓ No card-parse changes detected.

Merged via the queue into main with commit a99fdf6 Jul 11, 2026
13 checks passed
@matthewevans
matthewevans deleted the ship/u5-m2-replacemeaning branch July 11, 2026 10:27
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