Generic alter <type> X { set / insert / replace / drop }: grammar, target resolver, page family (#712, part 1) - #722
Merged
Merged
Conversation
…or the page family (#712) One grammar rule, ALTER alterDocumentType qualifiedName { alterOperation+ }, replaces the separate ALTER PAGE / SNIPPET / LAYOUT alternatives, with a shared target address (name, name.member, 'caption', @n) and fragment. Targets resolve through a per-document-type backend.AlterTargetResolver, which PageMutator now embeds: the modelsdk page mutator resolves widgets, grid columns and scroll-container regions with the finders the operations use; the MCP mutator resolves widgets and refuses dotted targets explicitly. The executor resolves every operation's targets before running it. The old spellings (set K = v, set K: v without parentheses, drop widget) parse to the identical operation and warn as MDL-DEPR101..103, from a table shaped like the #709 registry and pinned to `// alias:` grammar markers. A caption or @n target on a page is refused at check time (MDL-ALTER01). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…layouts (#712) mxcli syntax page/snippet/layout.alter, the alter-page skill, the quick reference and the docs-site alter-page pages now show set (Key: value) and drop <target>, and name the MDL-DEPR101..103 aliases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ADR-0011 requires an alias to parse to exactly what its canonical form parses to. Twelve pairs (set with =, unparenthesised set, page-level set, quoted key, action, named action slot, datasource, visible, expression, column target, drop widget) are compared with reflect.DeepEqual after clearing the Legacy marker. Control: stubbing the visitor to drop a target on the alias path fails the column_target and drop_widget cases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…an ambiguous column (#712) The resolver ran columnMatchCount for every bare name, so a real widget whose name is also a derived column name in two grids was refused as an ambiguous column, although set had always gone to the widget (SetWidgetProperty raises the ambiguity only when the name resolved to a column). That is a new rejection, which ADR-0011 allows only behind the language header. The ambiguity is now checked only when the name resolved to a column; drop/replace/insert keep their own check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… form (#712) Two refusals told the user to write `set Action = ... on w`, which now warns MDL-DEPR101. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e script (#712) AP14 exercises set (K: v) at page level and on a widget, insert, replace and drop without the widget keyword on a page and a snippet, so the canonical path is covered by the doctype mx-check run, next to the old spellings above it (which still run and warn). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ey: value) form (#712) The skill's property table and column examples still taught `set Key = value on w`, which now warns MDL-DEPR101, so an agent following the skill produced a deprecation warning on every alter. All 29 set examples in the skill now parse through check without a warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…...) (#712) Follows the hint change: the refusal on a built-in widget now names `set (Action: …)`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
Independent review: findings and fixes pushedWhat I verified by running it
Fixes pushed on this branch (tests first)
Needs the maintainer's judgement
|
11 tasks
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
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.
Part of #712 (partial; the issue stays open for the remainder)
Plan item 4.1 of
PROPOSAL_mdl_beta_syntax_freeze.md§8.3 / §9, implementing ADR-0012 decision 2. First PR of two: the generic grammar, the per-document-type target resolver interface, and the page family (alter page/alter snippet/alter layout) ported onto it.alter workflowis the follow-up (see below).What changes
One grammar rule.
alter page|snippet|layout Module.Name { … }now all parse throughwith the shared operations
alterSet/alterInsert/alterReplace/alterDrop, the shared addressalterTarget(name,name.member,'caption', each with optional@n) andalterFragment(thecreatebody of the document type —pageBodyV3today). The three per-typeALTER PAGE/LAYOUT/SNIPPET … alterPageOperation+alternatives andwidgetRefare gone; every existing alter page/snippet/layout test now runs through the generic rule, unchanged.Canonical form:
Target resolver interface —
backend.AlterTargetResolver(mdl/backend/alter_target.go):ResolveAlterTarget(AlterTarget) (AlterTargetMatch, error), plusAlterTargetError(a miss, or an ambiguity listing the matches as@1 … @n— never a guess) andPickAlterTargetMatch, the shared@nsemantics the microflow resolver (#713) can reuse.PageMutatorembeds it, so both backends implement it:pagemutator.Mutator): widget by name,grid.Column, scroll-container regionlayoutContainer.top, using the same finders the operations use, so resolution cannot accept what an operation then misses, and a miss reads as before (widget "x" not found, the column list, the region refusals).--mcp: widget by name; a dotted target gets an explicit "not yet supported by the MCP backend" (no MCP page operation supports columns/regions today).The executor resolves every operation's targets before running it (
resolveAlterPageTargets); a refusal stops the statement before the operation and beforeSave.Old forms are aliases, parse to the identical operation, and warn from
checkandexec:set K = v [on w],set (K = v, …) [on w]set (K: v, …) [on w]set K: v [on w](no parentheses)set (K: v) [on w]drop widget a, bdrop a, bcheckalso refuses, with no project, a caption /@n/ 3-part target on a page (MDL-ALTER01) — the grammar accepts every address form some document type needs; the rule is the page resolver's ownbackend.CheckPageAlterTarget, not a copy.Decisions the ADRs did not settle
mdl/executor/alter_aliases.go) with exactly Deprecation registry for MDL aliases (MDL-DEPRnnn), generalising MDL065 #709's entry shape{code, old, canonical, rewrite, removed in}, and the grammar marks each alias alternative// alias: MDL-DEPRnnn.TestAlterAliasGrammarMarkersMatchTablepins marker ↔ table both ways — the check Deprecation registry for MDL aliases (MDL-DEPRnnn), generalising MDL065 #709 generalises. Codes are provisional, numbered from 101 to stay clear of Deprecation registry for MDL aliases (MDL-DEPRnnn), generalising MDL065 #709's seed entries; Deprecation registry for MDL aliases (MDL-DEPRnnn), generalising MDL065 #709 should absorb or renumber them.set layout = X [map (…)],drop template for E in lv,add variables,drop variables. (Aset (Layout: …)spelling needs a home formap; left for the R2/R3 alias work.)@non a page is refused rather than ignored: widget names are unique within a page, so neither has anything to mean (ADR-0010 R11 strictness).AlterPageStmt/WidgetRef(now withCaption/Ordinal) because every page validator and ~25 test files speak them; the generic-ness lives in the grammar and the resolver.fmt --upgrade's job (Phase 3).Follow-ups (not in this PR)
alter workflow(wfmutator) onto the generic grammar:alterFragmentgainsworkflowBody, the workflow mutator implementsAlterTargetResolver(activity by name or caption,@n), and the currentset activity … / insert outcome … on …forms become registered aliases.fmt --upgraderewrites) once it lands.alter pagespellings in other skills/docs.Test plan (what I actually ran)
mdl/visitor/visitor_alter_generic_test.go(canonical set/drop, target forms incl.'caption'@2andlayoutContainer.top, aliases flagged, doc-specific ops,@0refused),mdl/backend/alter_target_test.go,mdl/backend/pagemutator/alter_target_test.go,mdl/backend/mcp/page_mutator_alter_target_test.go,mdl/executor/cmd_alter_generic_test.go(every target goes through the resolver; a refusal stops before the op and Save),mdl/executor/alter_aliases_test.go(warnings with a canonical control that must not warn; grammar-marker ↔ table; MDL-ALTER01 with a name-target control).execAlterPage→resolved targets: got [], want [a btnSave ctn dg.Total hdr lv]andwant the resolver's AlterTargetError, got <nil>;codes: got [], want [MDL-DEPR101 MDL-DEPR101 MDL-DEPR102 MDL-DEPR103];caption: want a by-name refusal, got widget "" not found;want 2 MDL-ALTER01 errors, got [];no viable alternative at input 'Caption:') and accepts the old one; the new binary accepts both and warns MDL-DEPR101 on the old one.testdata/expr-checker/minimal.mpr(Mendix 11.6.6): canonicalset (Caption: …) on actionButton11,insert after,drop→Altered page,describeshows the change;drop 'Learn more'→ refused by the resolver;drop nosuchwidget→widget "nosuchwidget" not found. No new BSON is written by this change (the operations are the existing ones), so no Studio Pro /mx checkrun was needed for it.make build: pass.make lint: pass.make test: every package passes exceptmdl/backend/modelsdk, which hit the default 10-minutego testtimeout under the shared machine's load (fixture copies; untouched by this change) — re-run with-timeout 60mtogether withmdl/executor,mdl/visitorandcmd/mxcli/...: all pass (modelsdk 670s)./tmpwas full from other sessions, so tests ran withTMPDIRon the workspace disk.🤖 Generated with Claude Code