diff --git a/.claude/skills/mendix/alter-page/SKILL.md b/.claude/skills/mendix/alter-page/SKILL.md index d19707e4a..001d8d60e 100644 --- a/.claude/skills/mendix/alter-page/SKILL.md +++ b/.claude/skills/mendix/alter-page/SKILL.md @@ -43,16 +43,34 @@ alter snippet Module.SnippetName { }; ``` +This is the **generic ALTER** — the same four verbs for every document type +(`alter layout` too): + +```sql +alter page Module.PageName { + set (Key: value, …) on ; -- no `on`: the page itself + insert before|after|into { } + replace with { } + drop , ; +}; +``` + +A `` on a page is a widget name, `grid.Column`, or a layout region +`layoutContainer.top`. Properties go in parentheses with `:`, exactly as in +`create page`. The older spellings `set Key = value on w`, `set Key: value` +(no parentheses) and `drop widget w` still run, and warn with MDL-DEPR101, +MDL-DEPR102 and MDL-DEPR103 — write the form above. + Multiple operations can be combined in a single ALTER statement. They are applied sequentially; later operations see the page state produced by earlier ones, so you can `set` on a widget you just `insert`ed. ```sql -- Rename a column, add a sibling, drop an obsolete one — all in one block. alter page MyMod.Product_Overview { - set caption = 'Product Name' on dgProducts.Name; + set (caption: 'Product Name') on dgProducts.Name; insert after dgProducts.Lifecycle { column NewCol (attribute: Sku, caption: 'SKU') }; - drop widget dgProducts.OldCol + drop dgProducts.OldCol }; ``` @@ -82,7 +100,7 @@ Naming the list view in the `drop` is required, not optional: one page can hold two list views with a template for the same entity. Most template edits need none of this. The widgets **inside** a template are -ordinary named widgets, so `set content = '…' on busLabel` and +ordinary named widgets, so `set (content: '…') on busLabel` and `insert after busLabel { … }` already work and land in the right template. To replace a whole template, `drop` it and `insert` the new one in the same block — operations apply in order. @@ -104,35 +122,35 @@ Refused, each naming the problem: ```sql -- Single property -set caption = 'New Caption' on widgetName +set (caption: 'New Caption') on widgetName -- Multiple properties -set (caption = 'Save & Close', buttonstyle = success) on btnSave +set (caption: 'Save & Close', buttonstyle: success) on btnSave -- Page-level property (no ON clause). Page-level property names are -- case-sensitive and must match the Mendix property exactly. -set Title = 'New Page Title' +set (Title: 'New Page Title') -- Pop-up dimensions (apply when the page is opened in a pop-up) -set PopupWidth = 800 -set PopupHeight = 480 -set PopupResizable = true -set Documentation = 'What this page is for.' +set (PopupWidth: 800) +set (PopupHeight: 480) +set (PopupResizable: true) +set (Documentation: 'What this page is for.') -- Retarget a button's on-click action. Any form `create page` accepts works -- here, including the combined ones. -set Action = microflow Module.ACT_Other on btnSave -set Action = SAVE_CHANGES CLOSE_PAGE on btnSave -set Action = SHOW_PAGE Module.DetailPage on btnEdit +set (Action: microflow Module.ACT_Other) on btnSave +set (Action: SAVE_CHANGES CLOSE_PAGE) on btnSave +set (Action: SHOW_PAGE Module.DetailPage) on btnEdit -- Retarget ONE named action slot of a pluggable widget, by the widget's own -- property key (the same key `create page` takes: `createFileAction: …`). -set 'createFileAction' = microflow Module.ACT_CreateFile on fileUploader1 -set 'onSelectionChange' = show_page Module.Detail on dgOrders +set ('createFileAction': microflow Module.ACT_CreateFile) on fileUploader1 +set ('onSelectionChange': show_page Module.Detail) on dgOrders -- Rebind a data-bound widget -set DataSource = $OrderParam on dvOrder -set DataSource = microflow Module.MF_Get on dvOrder +set (DataSource: $OrderParam) on dvOrder +set (DataSource: microflow Module.MF_Get) on dvOrder ``` **Prefer `set Action` over `replace` when only the action changes.** `replace` @@ -149,30 +167,30 @@ so a silent write would build cleanly and then fail to open. | Property | Widget Types | Value Type | Example | |----------|-------------|------------|---------| -| `Action` | Widgets with an on-click action (ACTIONBUTTON, LINKBUTTON, clickable containers) | Any `create page` action expression | `set Action = microflow M.ACT_Go on btnSave` | -| `''` | Pluggable widgets — any **action-typed** property (File Uploader `createFileAction`, DataGrid 2 `onSelectionChange`, …) | Any `create page` action expression | `set 'createFileAction' = microflow M.ACT_Create on fileUploader1` — refused, naming the widget's action slots, if the key is not action-typed | -| `caption` | ACTIONBUTTON, LINKBUTTON | String | `set caption = 'Submit' on btnSave` | -| `content` | DYNAMICTEXT | String | `set content = 'New Heading' on txtTitle` | -| `label` | TEXTBOX, TEXTAREA, DATEPICKER, COMBOBOX, CHECKBOX, RADIOBUTTONS | String | `set label = 'full Name' on txtName` | -| `buttonstyle` | ACTIONBUTTON, LINKBUTTON | Primary, Default, Success, Danger, Warning, Info | `set buttonstyle = danger on btnDelete` | -| `class` | Any widget | CSS class string | `set class = 'card mx-2' on container1` | -| `style` | Any widget (see warning below) | Inline CSS string | `set style = 'padding: 16px;' on container1` | -| `editable` | Input widgets | String | `set editable = 'Never' on txtReadOnly` | -| `visible` | Any widget | String or Boolean | `set visible = false on txtHidden` | -| `Name` | Any widget | String | `set Name = 'newName' on oldName` | -| `Title` | Page-level only (case-sensitive) | String | `set Title = 'Edit Customer'` | -| `Documentation` | Page-level only (case-sensitive) | String (`''` clears) | `set Documentation = 'Coordinator triage step.'` | +| `Action` | Widgets with an on-click action (ACTIONBUTTON, LINKBUTTON, clickable containers) | Any `create page` action expression | `set (Action: microflow M.ACT_Go) on btnSave` | +| `''` | Pluggable widgets — any **action-typed** property (File Uploader `createFileAction`, DataGrid 2 `onSelectionChange`, …) | Any `create page` action expression | `set ('createFileAction': microflow M.ACT_Create) on fileUploader1` — refused, naming the widget's action slots, if the key is not action-typed | +| `caption` | ACTIONBUTTON, LINKBUTTON | String | `set (caption: 'Submit') on btnSave` | +| `content` | DYNAMICTEXT | String | `set (content: 'New Heading') on txtTitle` | +| `label` | TEXTBOX, TEXTAREA, DATEPICKER, COMBOBOX, CHECKBOX, RADIOBUTTONS | String | `set (label: 'full Name') on txtName` | +| `buttonstyle` | ACTIONBUTTON, LINKBUTTON | Primary, Default, Success, Danger, Warning, Info | `set (buttonstyle: danger) on btnDelete` | +| `class` | Any widget | CSS class string | `set (class: 'card mx-2') on container1` | +| `style` | Any widget (see warning below) | Inline CSS string | `set (style: 'padding: 16px;') on container1` | +| `editable` | Input widgets | String | `set (editable: 'Never') on txtReadOnly` | +| `visible` | Any widget | String or Boolean | `set (visible: false) on txtHidden` | +| `Name` | Any widget | String | `set (Name: 'newName') on oldName` | +| `Title` | Page-level only (case-sensitive) | String | `set (Title: 'Edit Customer')` | +| `Documentation` | Page-level only (case-sensitive) | String (`''` clears) | `set (Documentation: 'Coordinator triage step.')` | | `layout` | Page-level only | Qualified name | `set layout = Atlas_Core.Atlas_Default` | -| `PopupWidth` | Page-level only (case-sensitive) | Positive integer (pixels) | `set PopupWidth = 800` | -| `PopupHeight` | Page-level only (case-sensitive) | Positive integer (pixels) | `set PopupHeight = 480` | -| `PopupResizable` | Page-level only (case-sensitive) | Boolean | `set PopupResizable = true` | -| `Class` | Page-level (case-sensitive, no ON) | CSS class string | `set Class = 'container-fluid bg-light'` | -| `Style` | Page-level (case-sensitive, no ON) | Inline CSS string | `set Style = 'min-height: 100vh'` | -| `Visible` (conditional) | Any widget | `[expression]` | `set Visible = [Name != ''] on ctnDetails` | -| `Editable` (conditional) | Input widgets | `[expression]` | `set Editable = [Active] on txtName` | -| `'quotedProp'` | Pluggable widgets | String, Boolean, Number | `set 'showLabel' = false on cbStatus` | - -> **Conditional visibility/editability** — `set Visible = [expr] on widget` (and +| `PopupWidth` | Page-level only (case-sensitive) | Positive integer (pixels) | `set (PopupWidth: 800)` | +| `PopupHeight` | Page-level only (case-sensitive) | Positive integer (pixels) | `set (PopupHeight: 480)` | +| `PopupResizable` | Page-level only (case-sensitive) | Boolean | `set (PopupResizable: true)` | +| `Class` | Page-level (case-sensitive, no ON) | CSS class string | `set (Class: 'container-fluid bg-light')` | +| `Style` | Page-level (case-sensitive, no ON) | Inline CSS string | `set (Style: 'min-height: 100vh')` | +| `Visible` (conditional) | Any widget | `[expression]` | `set (Visible: [Name != '']) on ctnDetails` | +| `Editable` (conditional) | Input widgets | `[expression]` | `set (Editable: [Active]) on txtName` | +| `'quotedProp'` | Pluggable widgets | String, Boolean, Number | `set ('showLabel': false) on cbStatus` | + +> **Conditional visibility/editability** — `set (Visible: [expr]) on widget` (and > `Editable`) attach a per-object expression. Bare attributes are rooted in the > widget data context automatically: `[Name != '']` becomes > `$currentObject/Name != ''` (paths you write with `$currentObject/…`/`$Param/…` @@ -181,12 +199,12 @@ so a silent write would build cleanly and then fail to open. **Pluggable widget properties** use quoted names to set values in the widget's `Object.Properties[]`. Boolean values are stored as `"yes"`/`"no"` in BSON. -**Column property names are case-insensitive** in MDL — `set caption = …` and `set Caption = …` both work. The internal BSON keys are dictated by the widget schema and stay case-sensitive on the storage side. +**Column property names are case-insensitive** in MDL — `set (caption: …)` and `set (Caption: …)` both work. The internal BSON keys are dictated by the widget schema and stay case-sensitive on the storage side. > **Warning: Style on DYNAMICTEXT** — Setting `style` directly on a DYNAMICTEXT widget crashes MxBuild with a NullReferenceException. Wrap the DYNAMICTEXT in a CONTAINER and apply styling to the container instead: > ```sql > -- Wrong: crashes MxBuild -> SET Style = 'color: red;' ON txtHeading +> SET (Style: 'color: red;') ON txtHeading > > -- Correct: style the container > REPLACE txtHeading WITH { @@ -203,10 +221,10 @@ from a microflow to a page parameter: ```sql ALTER PAGE MyModule.OrderPage { - SET DataSource = $Order ON dvOrder; -- page/snippet parameter - SET DataSource = microflow MyModule.MF_Get ON dvOrder; -- microflow - SET DataSource = nanoflow MyModule.NF_Get ON dvOrder; -- nanoflow - SET DataSource = selection dgOrders ON dvDetail; -- listen to widget + SET (DataSource: $Order) ON dvOrder; -- page/snippet parameter + SET (DataSource: microflow MyModule.MF_Get) ON dvOrder; -- microflow + SET (DataSource: nanoflow MyModule.NF_Get) ON dvOrder; -- nanoflow + SET (DataSource: selection dgOrders) ON dvDetail; -- listen to widget } ``` @@ -274,10 +292,10 @@ moves data-bound widgets. ```sql -- Drop a single widget -drop widget txtUnused +drop txtUnused -- Drop multiple widgets -drop widget txtOldField, lblOldLabel, container2 +drop txtOldField, lblOldLabel, container2 ``` Removes widgets and their entire subtree from the page. @@ -302,10 +320,10 @@ DataGrid2 columns are addressable using dotted notation: `gridName.columnName`. ```sql -- SET a column property -set caption = 'Product SKU' on dgProducts.Code +set (caption: 'Product SKU') on dgProducts.Code -- DROP a column -drop widget dgProducts.OldColumn +drop dgProducts.OldColumn -- INSERT a column after an existing one insert after dgProducts.Price { @@ -330,7 +348,7 @@ If the column name you copied from DESCRIBE still doesn't work, check whether th **The authored `column colFoo (...)` name is NOT how you address it.** A column carries no stored name in the Mendix model, so the name you wrote in `create page` is dropped on write — always address a column by its *derived* name (the one `describe page` shows). Using the authored name now fails with an error that lists the available column names, rather than a bare "not found". -**Duplicate captions are ambiguous and rejected.** Two dynamic-text (or custom-content) columns with the same caption derive the same name, so `ON "Amount"` can't tell them apart. mxcli now refuses the operation with an ambiguity error instead of silently mutating the first and leaving the second unreachable. Give such columns distinct captions to address them individually. (Non-attribute column handles are the caption, so `set Caption = ...` also *renames* the handle — plan multi-step caption edits accordingly.) +**Duplicate captions are ambiguous and rejected.** Two dynamic-text (or custom-content) columns with the same caption derive the same name, so `ON "Amount"` can't tell them apart. mxcli now refuses the operation with an ambiguity error instead of silently mutating the first and leaving the second unreachable. Give such columns distinct captions to address them individually. (Non-attribute column handles are the caption, so `set (Caption: ...)` also *renames* the handle — plan multi-step caption edits accordingly.) ### ADD Variables - Add a Page Variable @@ -368,7 +386,7 @@ When placeholders have the same names in both layouts (e.g., both have `Main`), ```sql alter page MyModule.Customer_Edit { - set (caption = 'Save & Close', buttonstyle = success) on btnSave + set (caption: 'Save & Close', buttonstyle: success) on btnSave }; ``` @@ -394,9 +412,9 @@ alter page MyModule.ProductOverview { ```sql alter page MyModule.Customer_Edit { - set title = 'Edit Customer Details'; - drop widget txtLegacyField, lblOldNote; - set label = 'Email Address' on txtEmail + set (title: 'Edit Customer Details'); + drop txtLegacyField, lblOldNote; + set (label: 'Email Address') on txtEmail }; ``` @@ -418,7 +436,7 @@ alter page MyModule.Customer_Edit { ```sql alter snippet MyModule.NavigationMenu { - set caption = 'Dashboard' on btnHome; + set (caption: 'Dashboard') on btnHome; insert after btnHome { actionbutton btnReports (caption: 'Reports', action: show_page MyModule.Reports_Overview) } @@ -429,8 +447,8 @@ alter snippet MyModule.NavigationMenu { ```sql alter page MyModule.Customer_Edit { - set 'showLabel' = false on cbStatus; - set 'labelWidth' = 4 on cbCategory + set ('showLabel': false) on cbStatus; + set ('labelWidth': 4) on cbCategory }; ``` @@ -452,8 +470,8 @@ create or replace page Mod.P (...) { answers to: ```mdl -alter page Mod.P { SET Caption = 'Renamed' ON dg1.colLabel } -- WRONG: column not found -alter page Mod.P { SET Caption = 'Renamed' ON dg1.Label } -- correct +alter page Mod.P { SET (Caption: 'Renamed') ON dg1.colLabel } -- WRONG: column not found +alter page Mod.P { SET (Caption: 'Renamed') ON dg1.Label } -- correct ``` The derived name is, in order: **the bound attribute's short name**, else the @@ -476,13 +494,13 @@ the quoted class name, and a computed one is the expression itself: ```mdl -- a literal class: the string 'highlight' -alter page Mod.P { SET DynamicCellClass = 'highlight' ON dg1.Label } +alter page Mod.P { SET (DynamicCellClass: 'highlight') ON dg1.Label } -- a computed class -alter page Mod.P { SET DynamicCellClass = if $currentObject/Price > 100 then 'highlight' else '' ON dg1.Label } +alter page Mod.P { SET (DynamicCellClass: if $currentObject/Price > 100 then 'highlight' else '') ON dg1.Label } -- WRONG: a bare name is an identifier, not a string — mxbuild reports CE0117 -alter page Mod.P { SET DynamicCellClass = highlight ON dg1.Label } +alter page Mod.P { SET (DynamicCellClass: highlight) ON dg1.Label } ``` The old spelling — the expression's text in quotes, `'if … then ''a'' else '''''` @@ -499,7 +517,7 @@ reference. **Widget property names are matched case-insensitively**, pluggable ones included, so a spelling `CREATE PAGE` accepts is a spelling `ALTER PAGE` accepts -— `set PageSize = 10 on dgProducts` and `set pageSize = 10 on dgProducts` are the +— `set (PageSize: 10) on dgProducts` and `set (pageSize: 10) on dgProducts` are the same statement. This is what makes DESCRIBE output re-executable: `describe page` prints the capitalised `PageSize:`, while the widget template stores `pageSize` (mendixlabs/mxcli#1069). A property the widget does not declare is still an @@ -520,7 +538,7 @@ adds. Both still fail at exec if they are genuinely wrong. |---------|-----| | Missing `on widgetName` for widget SET | Add `on widgetName` (only page-level properties — `Title`, `Documentation`, `PopupWidth`, `PopupHeight`, `PopupResizable`, `Class`, `Style` — omit ON) | | `unsupported page-level property: title` | Page-level property names are case-sensitive — use `Title`, `PopupWidth`, `PopupHeight`, `PopupResizable`, `Class`, `Style` | -| Using unquoted pluggable property names | Quote pluggable props: `set 'showLabel' = false on cb` | +| Using unquoted pluggable property names | Quote pluggable props: `set ('showLabel': false) on cb` | | `pluggable property "X" not found` | The widget does not declare it — casing is not the problem (any casing resolves). The error lists the keys it does declare; `describe widget ` or `describe page` shows them in context. Run `mxcli check … --references` to get this before the script runs | | Wrong widget name | Use `describe page Module.Name` to see widget names | | SET on non-existent widget | Widget names are case-sensitive; check with DESCRIBE | @@ -545,8 +563,8 @@ page and bind the buttons at creation time instead of rewiring afterwards: 3. **A footer is not addressable by its author-given name.** A `footer myName { … }` is a *marker*: its children are hoisted into the data view's footer and the - footer itself is serialized as `footer1`, so `drop widget myName` (and even - `drop widget footer1`) report "not found". To change footer contents, edit the + footer itself is serialized as `footer1`, so `drop myName` (and even + `drop footer1`) report "not found". To change footer contents, edit the children by their own names, or `create or replace page`. **Recommended pattern**: put save/reset microflows in a file that runs *before* the diff --git a/CHANGELOG.md b/CHANGELOG.md index 520d9cc6b..6c7f46a67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Changed +- **`alter page`, `alter snippet` and `alter layout` are the first document types on the generic ALTER** (ako/mxcli#712, ADR-0012) — one grammar, `alter Module.Name { set (Key: value) on ; insert before|after|into { … } replace with { … } drop ; }`, whose target is resolved by the document type (a new `backend.AlterTargetResolver`, implemented by the page mutator on the modelsdk and `--mcp` backends). Properties are written as in `create`: `set (Caption: 'Save') on btnSave`, `set (Title: 'Edit')` for the page itself. A target is a widget name, `grid.Column` or `layoutContainer.top`; a quoted-caption target or `@n` is refused on a page, whose elements have names — by `check` too, as **MDL-ALTER01**. **Migrating a script:** nothing breaks — `set Key = value`, `set Key: value` without parentheses and `drop widget a` still run and build the identical change, and `check` / `exec` warn with **MDL-DEPR101**, **MDL-DEPR102** and **MDL-DEPR103** naming the rewrite. + - **`DynamicClasses` and a column's `DynamicCellClass` are written as Mendix expressions** (mendixlabs/mxcli#750) — the expression is written as-is, so the doubled-quote spelling is gone: `dynamicclasses: if $currentObject/Featured then 'is-featured' else ''`, and `dynamicclasses: 'is-featured'` is the string — the class `is-featured`. The same rule as the OData client's credentials. `create page`, `alter page … set` and `describe` all use it, and a describe → exec round trip stores identical values (measured on a Mendix 11.14.0 project). **Migrating a script:** the old spelling, the expression's text in quotes (`'if … then ''a'' else '''''`), still parses but would now store that text as a class name, so `check` and `exec` refuse it as **MDL-WIDGET33** and give the unquoted expression. An expression in any other widget property is an error rather than an empty value; a pluggable property whose schema kind is Expression (a column's `Visible`, for one) keeps the quoted form until a following change. - **An OData client's credentials and header values are written as Mendix expressions** (mendixlabs/mxcli#750) — `HttpUsername`, `HttpPassword`, `ClientCertificate` and every `headers (…)` value hold an expression, and MDL now writes it as-is: `HttpUsername: 'admin'` is the string `'admin'`, `@Module.Const` reads a constant, and `'Bearer ' + @Module.Token` concatenates. Before, a quoted value was the expression's *text*, so `'admin'` stored the identifier `admin` and a string needed `'''admin'''`. `describe` prints the stored expression as-is, so Studio Pro's `'abc'` now reads `HttpUsername: 'abc'`; measured against a Studio Pro-authored client, and a describe → exec round trip stores identical values. **Migrating a script:** `'''admin'''` becomes `'admin'`, and a quoted constant `'@Module.Const'` becomes `@Module.Const` — both old forms still parse but would now store something else, so `check` and `exec` refuse them as **MDL-ODATA07**. A compound expression in any other OData property (`Path: 'a' + 'b'`) is an error rather than an empty value. `ServiceUrl` is a constant reference, not an expression — see the next entry. - **An OData client's `ServiceUrl` names a constant, like `ProxyHost`** (mendixlabs/mxcli#750) — Studio Pro picks the service URL as a constant and stores it as `@Module.Name`. `ServiceUrl: Module.Location` is now accepted alongside `@Module.Location` and `'@Module.Location'` (the bare name used to be refused as "not a constant reference"); all three store the same value, and `describe` prints the bare name, as it does for the proxy references. A literal URL is still refused (CE6825). diff --git a/cmd/mxcli/syntax/features_page.go b/cmd/mxcli/syntax/features_page.go index f8f1e46fc..bae54b5ed 100644 --- a/cmd/mxcli/syntax/features_page.go +++ b/cmd/mxcli/syntax/features_page.go @@ -284,8 +284,8 @@ CREATE PAGE Sales.Detail (Title: 'Detail', Layout: Atlas_Core.Atlas_Default) { "popup width", "popup height", "popup resizable", "drop template", "insert template", "list view template", }, - Syntax: "ALTER PAGE Module.Name {\n SET property = value ON widgetName; -- widget property names: any casing\n SET 'Row size' = 'Small' ON lvOrders; -- an Atlas DESIGN property of that widget's\n -- type; quoted and case-sensitive.\n -- `show design properties for ` lists\n -- them. ON/OFF for a toggle, where OFF\n -- REMOVES the entry.\n -- A multi-select ('Hide on') or compound\n -- ('Spacing') one needs the inline\n -- DesignProperties: [...] form, because a\n -- SET assignment carries one value.\n SET Action = MICROFLOW Module.MF ON btnSave; -- any CREATE PAGE action form\n SET 'createFileAction' = MICROFLOW Module.MF ON fileUploader1;\n -- a pluggable widget's NAMED action slot,\n -- by the widget's own key; refused on a\n -- key that is not action-typed\n SET DataSource = $Param ON dvOrder; -- parameter/microflow/nanoflow/selection;\n -- DATABASE and association are REPLACE-only,\n -- and a data view takes no database source\n SET (prop1 = val1, prop2 = val2) ON widgetName;\n SET Title = 'New Title'; -- page-level (case-sensitive)\n SET Documentation = 'What this page is for.';\n SET Class = 'css-class'; -- page-level CSS class / style\n SET Style = 'css: rule';\n SET PopupWidth = 800; -- page-level pop-up dimensions\n SET PopupHeight = 480;\n SET PopupResizable = true;\n INSERT AFTER widgetName { };\n INSERT BEFORE widgetName { };\n INSERT INTO containerName { };\n DROP WIDGET name1, name2;\n DROP TEMPLATE FOR Module.Specialization IN listViewName;\n REPLACE widgetName WITH { };\n};\n\n-- The BULK form: one design property on every widget of a TYPE.\nALTER PAGES [IN Module]\n SET 'Compact' = ON, 'Striped' = ON\n WHERE WIDGETTYPE = datagrid -- the MDL keyword, which resolves to\n -- exactly one widget id. A full id in\n -- quotes works too. NOT a name: a widget\n -- name is unique only within its page.\n [DRY RUN]; -- run this FIRST. It reports the matches\n -- against a discardable copy and writes\n -- nothing.", - Example: "ALTER PAGE Module.EditPage {\n SET (Caption = 'Save & Close', ButtonStyle = Success) ON btnSave;\n INSERT AFTER txtName {\n TEXTBOX txtMiddleName (Label: 'Middle Name', Attribute: MiddleName)\n };\n DROP WIDGET txtUnused;\n};", + Syntax: "ALTER PAGE Module.Name { -- the generic ALTER: set / insert / replace / drop\n SET (property: value) ON widgetName; -- widget property names: any casing\n SET ('Row size': 'Small') ON lvOrders; -- an Atlas DESIGN property of that widget's\n -- type; quoted and case-sensitive.\n -- `show design properties for ` lists\n -- them. ON/OFF for a toggle, where OFF\n -- REMOVES the entry.\n -- A multi-select ('Hide on') or compound\n -- ('Spacing') one needs the inline\n -- DesignProperties: [...] form, because a\n -- SET assignment carries one value.\n SET (Action: MICROFLOW Module.MF) ON btnSave; -- any CREATE PAGE action form\n SET ('createFileAction': MICROFLOW Module.MF) ON fileUploader1;\n -- a pluggable widget's NAMED action slot,\n -- by the widget's own key; refused on a\n -- key that is not action-typed\n SET (DataSource: $Param) ON dvOrder; -- parameter/microflow/nanoflow/selection;\n -- DATABASE and association are REPLACE-only,\n -- and a data view takes no database source\n SET (prop1: val1, prop2: val2) ON widgetName;\n SET (Title: 'New Title'); -- page-level (case-sensitive): no ON\n SET (Documentation: 'What this page is for.');\n SET (Class: 'css-class'); -- page-level CSS class / style\n SET (Style: 'css: rule');\n SET (PopupWidth: 800, PopupHeight: 480, PopupResizable: true); -- page-level pop-up\n INSERT AFTER widgetName { };\n INSERT BEFORE widgetName { };\n INSERT INTO containerName { };\n DROP name1, name2;\n DROP TEMPLATE FOR Module.Specialization IN listViewName;\n REPLACE widgetName WITH { };\n};\n-- A target is a widget name, or grid.Column. The old spellings `SET p = v`,\n-- `SET p: v` (no parentheses) and `DROP WIDGET a` still run and warn\n-- (MDL-DEPR101..103).\n\n-- The BULK form: one design property on every widget of a TYPE.\nALTER PAGES [IN Module]\n SET 'Compact' = ON, 'Striped' = ON\n WHERE WIDGETTYPE = datagrid -- the MDL keyword, which resolves to\n -- exactly one widget id. A full id in\n -- quotes works too. NOT a name: a widget\n -- name is unique only within its page.\n [DRY RUN]; -- run this FIRST. It reports the matches\n -- against a discardable copy and writes\n -- nothing.", + Example: "ALTER PAGE Module.EditPage {\n SET (Caption: 'Save & Close', ButtonStyle: Success) ON btnSave;\n INSERT AFTER txtName {\n TEXTBOX txtMiddleName (Label: 'Middle Name', Attribute: MiddleName)\n };\n DROP txtUnused;\n};", SeeAlso: []string{"page.create", "page.show", "snippet.alter"}, }) @@ -423,8 +423,8 @@ CREATE PAGE Sales.Detail (Title: 'Detail', Layout: Atlas_Core.Atlas_Default) { Keywords: []string{ "alter snippet", "modify snippet", "update snippet", }, - Syntax: "ALTER SNIPPET Module.Name {\n SET property = value ON widgetName;\n INSERT AFTER widgetName { };\n INSERT BEFORE widgetName { };\n INSERT INTO containerName { };\n DROP WIDGET name1, name2;\n REPLACE widgetName WITH { };\n};", - Example: "ALTER SNIPPET Module.NavSnippet {\n REPLACE navItem1 WITH {\n ACTIONBUTTON btnHome (Caption: 'Home', Action: SHOW_PAGE Module.HomePage)\n };\n DROP WIDGET txtOldField;\n INSERT AFTER txtName {\n TEXTBOX txtNewField (Label: 'New Field', Attribute: NewAttr)\n };\n};", + Syntax: "ALTER SNIPPET Module.Name {\n SET (property: value) ON widgetName;\n INSERT AFTER widgetName { };\n INSERT BEFORE widgetName { };\n INSERT INTO containerName { };\n DROP name1, name2;\n REPLACE widgetName WITH { };\n};", + Example: "ALTER SNIPPET Module.NavSnippet {\n REPLACE navItem1 WITH {\n ACTIONBUTTON btnHome (Caption: 'Home', Action: SHOW_PAGE Module.HomePage)\n };\n DROP txtOldField;\n INSERT AFTER txtName {\n TEXTBOX txtNewField (Label: 'New Field', Attribute: NewAttr)\n };\n};", SeeAlso: []string{"snippet", "page.alter"}, }) @@ -559,8 +559,8 @@ CREATE PAGE Sales.Detail (Title: 'Detail', Layout: Atlas_Core.Atlas_Default) { "ALTER LAYOUT Module.Name {\n" + " INSERT INTO . { };\n" + " INSERT BEFORE|AFTER { };\n" + - " SET = ON ;\n" + - " DROP WIDGET , ;\n" + + " SET (: ) ON ;\n" + + " DROP , ;\n" + " REPLACE WITH { };\n" + "};\n\n" + "-- Point one page at a different layout:\n" + diff --git a/docs-site/src/language/alter-page.md b/docs-site/src/language/alter-page.md index 8e4898af7..35efbb8ea 100644 --- a/docs-site/src/language/alter-page.md +++ b/docs-site/src/language/alter-page.md @@ -25,12 +25,12 @@ Change one or more properties on a widget identified by name: ```sql -- Single property ALTER PAGE Module.EditPage { - SET Caption = 'Save & Close' ON btnSave + SET (Caption: 'Save & Close') ON btnSave }; -- Multiple properties at once ALTER PAGE Module.EditPage { - SET (Caption = 'Save & Close', ButtonStyle = Success) ON btnSave + SET (Caption: 'Save & Close', ButtonStyle: Success) ON btnSave }; ``` @@ -38,15 +38,15 @@ ALTER PAGE Module.EditPage { | Property | Description | Example | |----------|-------------|---------| -| `Caption` | Button/link caption | `SET Caption = 'Submit' ON btnSave` | -| `Label` | Input field label | `SET Label = 'Full Name' ON txtName` | -| `ButtonStyle` | Button visual style | `SET ButtonStyle = Danger ON btnDelete` | -| `Class` | CSS class names | `SET Class = 'card p-3' ON cMain` | -| `Style` | Inline CSS | `SET Style = 'margin: 8px;' ON cBox` | -| `DynamicClasses` | Runtime-computed CSS classes | `SET DynamicClasses = if $currentObject/IsActive then 'is-active' else '' ON cMain` | -| `Editable` | Editability mode | `SET Editable = ReadOnly ON txtEmail` | -| `Visible` | Visibility expression | `SET Visible = '$showField' ON txtPhone` | -| `Name` | Widget name | `SET Name = 'txtFullName' ON txtName` | +| `Caption` | Button/link caption | `SET (Caption: 'Submit') ON btnSave` | +| `Label` | Input field label | `SET (Label: 'Full Name') ON txtName` | +| `ButtonStyle` | Button visual style | `SET (ButtonStyle: Danger) ON btnDelete` | +| `Class` | CSS class names | `SET (Class: 'card p-3') ON cMain` | +| `Style` | Inline CSS | `SET (Style: 'margin: 8px;') ON cBox` | +| `DynamicClasses` | Runtime-computed CSS classes | `SET (DynamicClasses: if $currentObject/IsActive then 'is-active' else '') ON cMain` | +| `Editable` | Editability mode | `SET (Editable: ReadOnly) ON txtEmail` | +| `Visible` | Visibility expression | `SET (Visible: '$showField') ON txtPhone` | +| `Name` | Widget name | `SET (Name: 'txtFullName') ON txtName` | ### SET -- Page-Level Properties @@ -61,9 +61,9 @@ it to `''` clears it. ```sql ALTER PAGE Module.EditPage { - SET Title = 'Customer Details'; - SET Class = 'container-fluid bg-light'; -- page CSS class (Forms$Appearance) - SET Style = 'min-height: 100vh' -- page inline style + SET (Title: 'Customer Details'); + SET (Class: 'container-fluid bg-light'); -- page CSS class (Forms$Appearance) + SET (Style: 'min-height: 100vh') -- page inline style }; ``` @@ -73,7 +73,7 @@ Use quoted property names to set properties on pluggable widgets (ComboBox, Data ```sql ALTER PAGE Module.EditPage { - SET 'showLabel' = false ON cbStatus + SET ('showLabel': false) ON cbStatus }; ``` @@ -129,18 +129,18 @@ ALTER PAGE Module.EditPage { The inserted widgets use the same syntax as in `CREATE PAGE`. Multiple widgets can be inserted in a single block. -### DROP WIDGET -- Remove Widgets +### DROP -- Remove Widgets Remove one or more widgets by name: ```sql ALTER PAGE Module.EditPage { - DROP WIDGET txtUnused + DROP txtUnused }; -- Multiple widgets ALTER PAGE Module.EditPage { - DROP WIDGET txtFax, txtPager, btnObsolete + DROP txtFax, txtPager, btnObsolete }; ``` @@ -181,10 +181,10 @@ Multiple operations can be combined in a single ALTER statement. They are applie ```sql ALTER PAGE Module.Customer_Edit { -- Change button appearance - SET (Caption = 'Save & Close', ButtonStyle = Success) ON btnSave; + SET (Caption: 'Save & Close', ButtonStyle: Success) ON btnSave; -- Remove unused fields - DROP WIDGET txtFax; + DROP txtFax; -- Add new fields after email INSERT AFTER txtEmail { @@ -227,8 +227,8 @@ ALTER PAGE MyModule.Customer_Edit { ```sql ALTER PAGE MyModule.Order_Edit { - SET (Caption = 'Submit Order', ButtonStyle = Success) ON btnSave; - SET Caption = 'Discard' ON btnCancel + SET (Caption: 'Submit Order', ButtonStyle: Success) ON btnSave; + SET (Caption: 'Discard') ON btnCancel }; ``` @@ -246,12 +246,12 @@ ALTER PAGE MyModule.Customer_Overview { -- Remove a column ALTER PAGE MyModule.Customer_Overview { - DROP WIDGET dgCustomers.OldColumn + DROP dgCustomers.OldColumn }; -- Change a column's caption ALTER PAGE MyModule.Customer_Overview { - SET Caption = 'E-mail Address' ON dgCustomers.Email + SET (Caption: 'E-mail Address') ON dgCustomers.Email }; -- Replace a column diff --git a/docs-site/src/reference/page/alter-page.md b/docs-site/src/reference/page/alter-page.md index 2e5f86c2f..f048296de 100644 --- a/docs-site/src/reference/page/alter-page.md +++ b/docs-site/src/reference/page/alter-page.md @@ -12,18 +12,31 @@ ALTER SNIPPET module.Name { } ``` +`ALTER LAYOUT module.Name { … }` takes the same operations. This is the +generic ALTER — `set` / `insert` / `replace` / `drop` against a target — which +every document type shares; a page's targets are widget names, `grid.Column`, +or a layout region `container.top`. + +The older spellings still run and warn with a deprecation code: + +| Old spelling | Write instead | Code | +|---|---|---| +| `SET Caption = 'x' ON w`, `SET (A = 1, B = 2) ON w` | `SET (Caption: 'x') ON w`, `SET (A: 1, B: 2) ON w` | MDL-DEPR101 | +| `SET Caption: 'x' ON w` (no parentheses) | `SET (Caption: 'x') ON w` | MDL-DEPR102 | +| `DROP WIDGET a, b` | `DROP a, b` | MDL-DEPR103 | + Where each operation is one of: ```sql -- Set a property on a widget -SET property = value ON widgetName; -SET ( property1 = value1, property2 = value2 ) ON widgetName; +SET (property: value) ON widgetName; +SET ( property1: value1, property2: value2 ) ON widgetName; -- Set a page-level property (no ON clause) -SET Title = 'New Title'; +SET (Title: 'New Title'); -- Set a pluggable widget property (quoted name) -SET 'propertyName' = value ON widgetName; +SET ('propertyName': value) ON widgetName; -- Insert widgets before or after a target INSERT BEFORE widgetName { widget_definitions }; @@ -33,7 +46,7 @@ INSERT AFTER widgetName { widget_definitions }; INSERT INTO containerName { widget_definitions }; -- Remove widgets -DROP WIDGET widgetName1, widgetName2; +DROP widgetName1, widgetName2; -- Replace a widget with new widgets REPLACE widgetName WITH { widget_definitions }; @@ -77,7 +90,7 @@ Appends new widgets as the **last children** of a named container. This is the o Supported on simple containers (container/DivContainer, data view, group box, scroll-container region). A layout grid (rows/columns) and tab container have no single child list — insert relative to a widget inside the target column or tab instead. -### DROP WIDGET +### DROP Removes one or more widgets by name. The widget and all its children are removed from the tree. @@ -92,8 +105,8 @@ DataGrid2 columns are addressable using dotted notation: `gridName.columnName`. All four operations (SET, INSERT, DROP, REPLACE) support dotted column references: ```sql -SET Caption = 'Product SKU' ON dgProducts.Code -DROP WIDGET dgProducts.OldColumn +SET (Caption: 'Product SKU') ON dgProducts.Code +DROP dgProducts.OldColumn INSERT AFTER dgProducts.Price { COLUMN Margin (Attribute: Margin) } REPLACE dgProducts.Description WITH { COLUMN Notes (Attribute: Notes) } ``` @@ -130,7 +143,7 @@ Change button caption and style: ```sql ALTER PAGE Sales.Order_Edit { - SET (Caption = 'Save & Close', ButtonStyle = Success) ON btnSave; + SET (Caption: 'Save & Close', ButtonStyle: Success) ON btnSave; }; ``` @@ -138,7 +151,7 @@ Remove an unused widget and add a new field: ```sql ALTER PAGE Sales.Order_Edit { - DROP WIDGET txtUnused; + DROP txtUnused; INSERT AFTER txtEmail { TEXTBOX txtPhone (Label: 'Phone', Attribute: Phone) } @@ -160,7 +173,7 @@ Set a page-level property: ```sql ALTER PAGE Sales.Order_Edit { - SET Title = 'Edit Order Details'; + SET (Title: 'Edit Order Details'); }; ``` @@ -168,7 +181,7 @@ Set a pluggable widget property: ```sql ALTER PAGE Sales.Order_Edit { - SET 'showLabel' = false ON cbStatus; + SET ('showLabel': false) ON cbStatus; }; ``` @@ -204,7 +217,7 @@ Modify a snippet: ```sql ALTER SNIPPET MyModule.NavMenu { - SET Caption = 'Dashboard' ON btnHome; + SET (Caption: 'Dashboard') ON btnHome; INSERT AFTER btnHome { ACTIONBUTTON btnReports (Caption: 'Reports', Action: PAGE MyModule.Reports) } @@ -215,9 +228,9 @@ Combined operations in a single ALTER: ```sql ALTER PAGE MyModule.Customer_Edit { - SET Title = 'Customer Details'; - SET (Caption = 'Update', ButtonStyle = Primary) ON btnSave; - DROP WIDGET txtObsolete; + SET (Title: 'Customer Details'); + SET (Caption: 'Update', ButtonStyle: Primary) ON btnSave; + DROP txtObsolete; INSERT BEFORE txtEmail { TEXTBOX txtPhone (Label: 'Phone', Attribute: Phone) } diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index af27e8df2..b30d0acdb 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -1474,7 +1474,7 @@ MDL uses explicit property declarations for pages: | Drop layout | `drop layout [if exists] Module.Name;` | Pages still bound to it are named in a warning and the drop proceeds; left dropped they fail **CE1613**, which names the *page* | | Declare a placeholder | `placeholder Main` | **No body.** Exactly one must be named `Main` — mxbuild enforces it (**CE0848**/**CE0849**), and names must be unique (**CE0495**). `placeholder X { … }` is the page-side form and declares nothing (MDL083) | | Alter layout | `alter layout Module.Name { };` | Edits the stored document, so widgets MDL cannot spell survive. Refused for a Marketplace target | -| Set a design property | `alter page Module.Page { set 'Row size' = 'Small' on lvOrders; };` | An Atlas design property of that widget's **type** — quoted, case-sensitive; `show design properties for ` lists them. `on`/`off` for a toggle, where `off` removes the entry. Same document `alter styling` writes. A **multi-select** (`Hide on`) or **compound** (`Spacing`) property needs the inline `DesignProperties: [...]` form, since a `set` assignment carries one value | +| Set a design property | `alter page Module.Page { set ('Row size': 'Small') on lvOrders; };` | An Atlas design property of that widget's **type** — quoted, case-sensitive; `show design properties for ` lists them. `on`/`off` for a toggle, where `off` removes the entry. Same document `alter styling` writes. A **multi-select** (`Hide on`) or **compound** (`Spacing`) property needs the inline `DesignProperties: [...]` form, since a `set` assignment carries one value | | Repoint one page | `alter page Module.Page { set Layout = Module.Layout [map (Old as New, …)]; };` | Rewrites the layout reference **and** every placeholder binding | | Set a design property on every widget of a type | `alter pages [in ] set 'Compact' = on, 'Striped' = on where widgettype = datagrid [dry run];` | The house-style sweep. `widgettype` takes the **MDL keyword**, which resolves to exactly one widget id — a `like '%datagrid%'` predicate also matches the data grid's *filter* widgets. Never a widget **name**: a name is unique only within its page. `dry run` previews against a discardable copy. A sweep that matches widgets and writes none of them exits non-zero | | Repoint many pages | `alter pages [in ] set layout = Module.Layout [map (…)] [where layout = Module.Old];` | The migration form. Marketplace pages are skipped and named. A `where layout` that names no real layout is an error, not a 0-page success | @@ -1637,24 +1637,26 @@ Keys: `decimalPrecision` (int), `groupDigits` (bool), `dateFormat` (`Date`|`Date Modify an existing page or snippet's widget tree in-place without full `create or replace`. Works directly on the raw BSON tree, preserving unsupported widget types. +This is the generic ALTER — `alter Module.Name { set (Key: value) on ; insert before|after|into { … } replace with { … } drop ; }` — shared by pages, snippets and layouts. The old spellings `set Key = value`, `set Key: value` (no parentheses) and `drop widget` still run and warn (MDL-DEPR101..103). + | Operation | Syntax | Notes | |-----------|--------|-------| -| Set property | `set caption = 'New' on widgetName` | Single property on a widget | -| Set multiple | `set (caption = 'Save', buttonstyle = success) on btn` | Multiple properties at once | -| Page-level set | `set Title = 'New title'` | No ON clause; page-level names are case-sensitive | -| Documentation | `set Documentation = 'What this page is for.'` | Page-level. Same property the `/** … */` doc comment on `CREATE PAGE` writes, so an existing page can be documented without restating it. `''` clears it | -| Pop-up dimensions | `set PopupWidth = 800` / `set PopupHeight = 480` / `set PopupResizable = true` | Page-level; apply when the page opens in a pop-up | -| Page CSS class / style | `set Class = 'css-class'` / `set Style = 'css: rule'` | Page-level (no ON clause); sets the page's Appearance | -| Widget dynamic classes | `set DynamicClasses = 'expr' on widgetName` | Runtime-computed classes on a widget — the surgical alternative to a bulk `update widgets` | +| Set property | `set (caption: 'New') on widgetName` | Single property on a widget | +| Set multiple | `set (caption: 'Save', buttonstyle: success) on btn` | Multiple properties at once | +| Page-level set | `set (Title: 'New title')` | No ON clause; page-level names are case-sensitive | +| Documentation | `set (Documentation: 'What this page is for.')` | Page-level. Same property the `/** … */` doc comment on `CREATE PAGE` writes, so an existing page can be documented without restating it. `''` clears it | +| Pop-up dimensions | `set (PopupWidth: 800, PopupHeight: 480, PopupResizable: true)` | Page-level; apply when the page opens in a pop-up | +| Page CSS class / style | `set (Class: 'css-class')` / `set (Style: 'css: rule')` | Page-level (no ON clause); sets the page's Appearance | +| Widget dynamic classes | `set (DynamicClasses: 'expr') on widgetName` | Runtime-computed classes on a widget — the surgical alternative to a bulk `update widgets` | | Insert after | `insert after widgetName { widgets }` | Add widgets after target | | Insert before | `insert before widgetName { widgets }` | Add widgets before target | | Insert into | `insert into containerName { widgets }` | Append as the container's last child (fills an empty container; dataview children take its entity) | -| Drop widgets | `drop widget name1, name2` | Remove widgets by name | +| Drop widgets | `drop name1, name2` | Remove widgets by name | | Replace widget | `replace widgetName with { widgets }` | Replace widget subtree | -| Pluggable prop | `set 'showLabel' = false on cbStatus` | Quoted name for pluggable widgets | -| Named action slot | `set 'createFileAction' = microflow M.ACT_Create on fileUploader1` | A pluggable widget's action-typed property, by its own key; any `create page` action form. Refused on a key that is not action-typed | -| Set column prop | `set caption = 'New' on dgGrid.colName` | Dotted ref targets DataGrid column | -| Drop column | `drop widget dgGrid.colName` | Remove a DataGrid column | +| Pluggable prop | `set ('showLabel': false) on cbStatus` | Quoted name for pluggable widgets | +| Named action slot | `set ('createFileAction': microflow M.ACT_Create) on fileUploader1` | A pluggable widget's action-typed property, by its own key; any `create page` action form. Refused on a key that is not action-typed | +| Set column prop | `set (caption: 'New') on dgGrid.colName` | Dotted ref targets DataGrid column | +| Drop column | `drop dgGrid.colName` | Remove a DataGrid column | | Insert column | `insert after dgGrid.colName { column ... }` | Add column to DataGrid | | Add variable | `add variables $name: type = 'expr'` | Add a page variable | | Drop variable | `drop variables $name` | Remove a page variable | @@ -1666,15 +1668,15 @@ Modify an existing page or snippet's widget tree in-place without full `create o **Example:** ```sql alter page Module.EditPage { - set (caption = 'Save & Close', buttonstyle = success) on btnSave; - drop widget txtUnused; + set (caption: 'Save & Close', buttonstyle: success) on btnSave; + drop txtUnused; insert after txtEmail { textbox txtPhone (label: 'Phone', attribute: Phone) } }; alter snippet Module.NavMenu { - set caption = 'Dashboard' on btnHome + set (caption: 'Dashboard') on btnHome }; ``` diff --git a/mdl-examples/doctype-tests/33-alter-page-examples.mdl b/mdl-examples/doctype-tests/33-alter-page-examples.mdl index 1a4c00d0f..16a8a749c 100644 --- a/mdl-examples/doctype-tests/33-alter-page-examples.mdl +++ b/mdl-examples/doctype-tests/33-alter-page-examples.mdl @@ -426,6 +426,35 @@ alter snippet AlterPg.Product_Card_Snippet { drop widget btnViewDetails }; +-- MARK: AP14 — The canonical generic ALTER form + +-- ============================================================================ +-- AP14: `alter page|snippet|layout` share one patch grammar (ADR-0012): +-- properties are a parenthesised `(Key: value, ...)` list, and `drop` names its +-- targets without a `widget` keyword. The older spellings used above +-- (`set Key = value`, `drop widget a`) still run, and warn MDL-DEPR101/103. +-- ============================================================================ + +alter page AlterPg.Product_Overview { + set (Title: 'Product catalog'); + set (Class: 'product-overview-heading') on heading; + insert after heading { + dynamictext canonicalNote (Content: 'Edited with the generic alter form', RenderMode: paragraph) + } + drop canonicalNote; +}; + +alter snippet AlterPg.Product_Card_Snippet { + set (Class: 'mx-card-body', Style: 'padding: 4px') on cardBody; + insert after cardBody { + dynamictext cardFooter (Content: 'More details inside') + } + replace cardFooter with { + dynamictext cardFooter2 (Content: 'See details') + } + drop cardFooter2; +}; + -- ============================================================================ -- End of file. The Product_Overview page is now a fully functional CRUD -- overview with a heading, a DataGrid2 showing Name, Lifecycle, Price (renamed diff --git a/mdl/ast/ast_alter_page.go b/mdl/ast/ast_alter_page.go index cdbb19005..bf2aa89ad 100644 --- a/mdl/ast/ast_alter_page.go +++ b/mdl/ast/ast_alter_page.go @@ -2,6 +2,8 @@ package ast +import "strconv" + // ============================================================================ // ALTER PAGE / ALTER SNIPPET — in-place widget tree modification // ============================================================================ @@ -20,21 +22,58 @@ type AlterPageOperation interface { isAlterPageOperation() } -// WidgetRef represents a widget reference, optionally with a sub-element path. -// Plain: "btnSave" (Widget="btnSave", Column="") -// Dotted: "dgProducts.Name" (Widget="dgProducts", Column="Name") +// WidgetRef is the of a generic ALTER operation as written: the +// element `set … on`, `insert before|after|into`, `replace … with` and `drop` +// address (ADR-0012 decision 2). +// +// One address syntax serves every document type, and the document type's +// resolver (backend.AlterTargetResolver) decides which forms it accepts: +// +// btnSave Widget="btnSave" +// dgProducts.Name Widget="dgProducts", Column="Name" (a grid column, a scroll-container region) +// 'Approve order' Caption="Approve order" (content addressing, for elements with no name) +// hdr@2 / 'x'@2 Ordinal=2 — picks one of several matches; never a guess +// +// The name stays WidgetRef because the page family is the first document type +// on the generic path and every page operation already speaks it. type WidgetRef struct { - Widget string // widget name (always set) - Column string // column name within widget (empty for plain widget refs) + Widget string // name (empty when the target is addressed by caption) + Column string // sub-element name within Widget (empty for a plain name) + Caption string // quoted content address; empty when addressed by name + Ordinal int // @n, 1-based; 0 when absent } -// Name returns the full reference string for error messages. +// Name returns the full reference string for error messages, as it was written. func (r WidgetRef) Name() string { - if r.Column != "" { - return r.Widget + "." + r.Column + var s string + switch { + case r.Caption != "": + s = "'" + r.Caption + "'" + case r.Column != "": + s = r.Widget + "." + r.Column + default: + s = r.Widget } - return r.Widget -} + if r.Ordinal > 0 { + s += "@" + strconv.Itoa(r.Ordinal) + } + return s +} + +// Spellings of the generic ALTER that are aliases of its canonical form +// (ADR-0011: an old form warns, and is rewritten mechanically). The visitor +// records which one a statement used; the executor maps it to a deprecation +// code. Nothing downstream of the validator may branch on these: both spellings +// build the identical operation. +const ( + // `set Key = value …` / `set (Key = value, …) …` — R3 puts `:` between a + // property and its value; `=` is comparison. + AlterAliasSetEquals = "set-equals" + // `set Key: value …` — properties are a parenthesised list (R2), even one. + AlterAliasSetUnparenthesised = "set-unparenthesised" + // `drop widget a, b` — the target names the element; the kind is its own. + AlterAliasDropWidget = "drop-widget" +) // IsColumn returns true if this is a column reference (dotted path). func (r WidgetRef) IsColumn() bool { @@ -46,6 +85,7 @@ func (r WidgetRef) IsColumn() bool { type SetPropertyOp struct { Target WidgetRef // empty Widget for page-level SET Properties map[string]interface{} // property name -> value + Legacy string // AlterAlias* when an old spelling was used, else "" } func (s *SetPropertyOp) isAlterPageOperation() {} @@ -62,6 +102,7 @@ func (s *InsertWidgetOp) isAlterPageOperation() {} // DropWidgetOp represents: DROP WIDGET ref1, ref2, ... type DropWidgetOp struct { Targets []WidgetRef + Legacy string // AlterAliasDropWidget when written `drop widget …`, else "" } func (s *DropWidgetOp) isAlterPageOperation() {} diff --git a/mdl/backend/alter_target.go b/mdl/backend/alter_target.go new file mode 100644 index 000000000..9da0d287e --- /dev/null +++ b/mdl/backend/alter_target.go @@ -0,0 +1,144 @@ +// SPDX-License-Identifier: Apache-2.0 + +package backend + +import ( + "fmt" + "strconv" + "strings" +) + +// The generic ALTER (ADR-0012 decision 2) is one patch grammar for every +// document type: +// +// alter Module.Name { +// set ( Key: value ) on ; +// insert before|after|into { } +// replace with { } +// drop ; +// } +// +// The grammar and the operations are shared; what differs per document type is +// what a means. A page addresses widgets by name, a microflow has no +// names and addresses activities by content. That is the resolver's job, and +// the only per-type knowledge the generic path needs: one AlterTargetResolver +// per document type, implemented by that type's mutator on every backend. + +// AlterTarget is the document-independent address an ALTER operation names, as +// it was written. At most one of Path and Caption is set. +type AlterTarget struct { + // Path is a name, or a name and one member: ["btnSave"], + // ["dgOrders", "Total"], ["layoutContainer", "top"]. + Path []string + // Caption is a quoted content address: 'Approve order'. + Caption string + // Ordinal is @n (1-based), choosing one of several matches; 0 when absent. + Ordinal int +} + +// String renders the target as MDL writes it, for messages. +func (t AlterTarget) String() string { + var s string + if t.Caption != "" { + s = "'" + strings.ReplaceAll(t.Caption, "'", "''") + "'" + } else { + s = strings.Join(t.Path, ".") + } + if t.Ordinal > 0 { + s += "@" + strconv.Itoa(t.Ordinal) + } + return s +} + +// AlterTargetMatch is one element an AlterTarget resolved to. +type AlterTargetMatch struct { + // Kind names what the element is, in the document's own terms: "widget", + // "column", "region", "activity". + Kind string + // Name is how describe names it — the handle to write to address it alone. + Name string +} + +// AlterTargetResolver is the per-document-type half of the generic ALTER. +// +// ResolveAlterTarget returns the one element the target addresses. It never +// guesses: an address that matches nothing is an error, and so is one that +// matches more than one element with no @n to choose — that error lists the +// matches (AlterTargetError), so the author can write the address that picks +// one. A form the document type does not support (a caption on a page, whose +// elements have names) is refused with a message saying which form it takes. +// +// Resolving has no side effects; the operation that follows does the change. +type AlterTargetResolver interface { + ResolveAlterTarget(t AlterTarget) (AlterTargetMatch, error) +} + +// AlterTargetError is a target that did not resolve to exactly one element. +// Matches is empty for a miss and holds every candidate for an ambiguity. +type AlterTargetError struct { + Target AlterTarget + Matches []AlterTargetMatch + // Detail replaces the generic message when the resolver has a better one + // (a not-found listing what IS there, say). Optional. + Detail string +} + +func (e *AlterTargetError) Error() string { + if e.Detail != "" { + return e.Detail + } + if len(e.Matches) == 0 { + return fmt.Sprintf("alter target %s not found", e.Target) + } + parts := make([]string, len(e.Matches)) + for i, m := range e.Matches { + parts[i] = fmt.Sprintf("@%d %s %s", i+1, m.Kind, m.Name) + } + return fmt.Sprintf("alter target %s is ambiguous: it matches %d elements (%s) — add @n to choose one", + e.Target, len(e.Matches), strings.Join(parts, ", ")) +} + +// PickAlterTargetMatch applies a target's @n to the candidates a resolver +// found: exactly one candidate and no @n resolves; @n within range picks that +// one; anything else is an AlterTargetError. Resolvers share it so an ordinal +// means the same thing in every document type. +func PickAlterTargetMatch(t AlterTarget, candidates []AlterTargetMatch) (AlterTargetMatch, error) { + switch { + case len(candidates) == 0: + return AlterTargetMatch{}, &AlterTargetError{Target: t} + case t.Ordinal > len(candidates): + return AlterTargetMatch{}, &AlterTargetError{Target: t, Matches: candidates, + Detail: fmt.Sprintf("alter target %s: there are only %d matches", t, len(candidates))} + case t.Ordinal > 0: + return candidates[t.Ordinal-1], nil + case len(candidates) == 1: + return candidates[0], nil + default: + return AlterTargetMatch{}, &AlterTargetError{Target: t, Matches: candidates} + } +} + +// CheckPageAlterTarget refuses the address forms of the generic ALTER that a +// widget tree has no use for. Every page element that can be a target has a +// Name, and names are unique within the document, so a caption would have to +// guess among widgets that share one, and an @n would pick among matches that +// cannot occur. Both are errors rather than being quietly ignored. Shared by +// every backend's page mutator, so the page address syntax has one definition. +func CheckPageAlterTarget(t AlterTarget) error { + switch { + case t.Caption != "": + return &AlterTargetError{Target: t, Detail: fmt.Sprintf( + "alter target %s: a page, snippet or layout element is addressed by name "+ + "(`btnSave`, `grid.Column`, `layoutContainer.top`), not by caption — "+ + "`describe` prints every widget's name", t)} + case t.Ordinal > 0: + return &AlterTargetError{Target: t, Detail: fmt.Sprintf( + "alter target %s: @%d chooses among several matches, and a widget name is unique "+ + "within its page — drop the @%d; for a column that two grids share, qualify it as `grid.Column`", + t, t.Ordinal, t.Ordinal)} + case len(t.Path) == 0 || len(t.Path) > 2: + return &AlterTargetError{Target: t, Detail: fmt.Sprintf( + "alter target %q: want a widget name or `widget.member`", t.String())} + } + return nil +} diff --git a/mdl/backend/alter_target_test.go b/mdl/backend/alter_target_test.go new file mode 100644 index 000000000..e46f02fa3 --- /dev/null +++ b/mdl/backend/alter_target_test.go @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: Apache-2.0 + +package backend + +import ( + "errors" + "strings" + "testing" +) + +func TestPickAlterTargetMatch(t *testing.T) { + one := []AlterTargetMatch{{Kind: "activity", Name: "Approve"}} + two := []AlterTargetMatch{{Kind: "activity", Name: "Approve"}, {Kind: "activity", Name: "Approve2"}} + + if m, err := PickAlterTargetMatch(AlterTarget{Caption: "Approve"}, one); err != nil || m.Name != "Approve" { + t.Errorf("single match: %v %v", m, err) + } + if m, err := PickAlterTargetMatch(AlterTarget{Caption: "Approve", Ordinal: 2}, two); err != nil || m.Name != "Approve2" { + t.Errorf("@2: %v %v", m, err) + } + + // Ambiguity is an error that lists the matches with their ordinals: never a guess. + _, err := PickAlterTargetMatch(AlterTarget{Caption: "Approve"}, two) + var te *AlterTargetError + if !errors.As(err, &te) || len(te.Matches) != 2 { + t.Fatalf("ambiguous: want AlterTargetError with 2 matches, got %v", err) + } + if msg := err.Error(); !strings.Contains(msg, "@1 activity Approve") || !strings.Contains(msg, "@2 activity Approve2") { + t.Errorf("ambiguity message must list the matches with ordinals: %s", msg) + } + + if _, err := PickAlterTargetMatch(AlterTarget{Path: []string{"x"}}, nil); err == nil || + !strings.Contains(err.Error(), "x not found") { + t.Errorf("miss: %v", err) + } + if _, err := PickAlterTargetMatch(AlterTarget{Path: []string{"x"}, Ordinal: 3}, two); err == nil { + t.Error("an ordinal past the matches must be refused") + } +} + +func TestAlterTargetString(t *testing.T) { + for _, c := range []struct { + t AlterTarget + want string + }{ + {AlterTarget{Path: []string{"btnSave"}}, "btnSave"}, + {AlterTarget{Path: []string{"dg", "Total"}, Ordinal: 2}, "dg.Total@2"}, + {AlterTarget{Caption: "it's"}, "'it''s'"}, + } { + if got := c.t.String(); got != c.want { + t.Errorf("got %q want %q", got, c.want) + } + } +} diff --git a/mdl/backend/mcp/page_mutator.go b/mdl/backend/mcp/page_mutator.go index 323e4142c..a14015609 100644 --- a/mdl/backend/mcp/page_mutator.go +++ b/mdl/backend/mcp/page_mutator.go @@ -591,3 +591,22 @@ func (m *mcpPageMutator) AddVariable(name, _, _ string) error { func (m *mcpPageMutator) DropVariable(name string) error { return fmt.Errorf("dropping page variable %q is not yet supported by the MCP backend", name) } + +// ResolveAlterTarget is the MCP page mutator's backend.AlterTargetResolver. +// It addresses what this backend can edit — a widget by name — and says so +// explicitly for a dotted `grid.Column` / `container.region` address, which no +// operation over MCP supports yet, rather than letting each operation fail +// differently. +func (m *mcpPageMutator) ResolveAlterTarget(t backend.AlterTarget) (backend.AlterTargetMatch, error) { + if err := backend.CheckPageAlterTarget(t); err != nil { + return backend.AlterTargetMatch{}, err + } + if len(t.Path) == 2 { + return backend.AlterTargetMatch{}, fmt.Errorf( + "alter target %s: addressing a column or region is not yet supported by the MCP backend", t) + } + if _, _, _, _, ok := findWidget(m.content, t.Path[0]); !ok { + return backend.AlterTargetMatch{}, fmt.Errorf("widget %q not found", t.Path[0]) + } + return backend.AlterTargetMatch{Kind: "widget", Name: t.Path[0]}, nil +} diff --git a/mdl/backend/mcp/page_mutator_alter_target_test.go b/mdl/backend/mcp/page_mutator_alter_target_test.go new file mode 100644 index 000000000..e7c9d98c6 --- /dev/null +++ b/mdl/backend/mcp/page_mutator_alter_target_test.go @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: Apache-2.0 + +package mcp + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend" +) + +// The generic ALTER resolves every target on every backend (ako/mxcli#712): +// over MCP a widget resolves by name, and what this backend cannot address yet +// is refused explicitly instead of surfacing as some operation's own failure. +func TestPageMutator_ResolveAlterTarget(t *testing.T) { + m := newTestMutator() + + if got, err := m.ResolveAlterTarget(backend.AlterTarget{Path: []string{"t1"}}); err != nil || got.Kind != "widget" || got.Name != "t1" { + t.Errorf("widget by name: %+v %v", got, err) + } + cases := []struct { + target backend.AlterTarget + want string + }{ + {backend.AlterTarget{Path: []string{"nope"}}, `widget "nope" not found`}, + {backend.AlterTarget{Path: []string{"dg", "Total"}}, "not yet supported by the MCP backend"}, + {backend.AlterTarget{Caption: "Save"}, "by name"}, + {backend.AlterTarget{Path: []string{"t1"}, Ordinal: 2}, "@2"}, + } + for _, c := range cases { + _, err := m.ResolveAlterTarget(c.target) + if err == nil || !strings.Contains(err.Error(), c.want) { + t.Errorf("%s: want error containing %q, got %v", c.target, c.want, err) + } + } +} diff --git a/mdl/backend/mock/mock_page_mutator.go b/mdl/backend/mock/mock_page_mutator.go index b0f44ab84..df1b79e41 100644 --- a/mdl/backend/mock/mock_page_mutator.go +++ b/mdl/backend/mock/mock_page_mutator.go @@ -18,6 +18,7 @@ var _ backend.PageMutator = (*MockPageMutator)(nil) // all other methods return zero values. type MockPageMutator struct { ContainerTypeFunc func() backend.ContainerKind + ResolveAlterTargetFunc func(t backend.AlterTarget) (backend.AlterTargetMatch, error) SetWidgetPropertyFunc func(widgetRef string, prop string, value any) error SetWidgetDataSourceFunc func(widgetRef string, ds pages.DataSource) error SetWidgetActionFunc func(widgetRef string, action pages.ClientAction) error @@ -245,3 +246,13 @@ func (m *MockPageMutator) BoundPlaceholders() []string { } return nil } + +// ResolveAlterTarget resolves every target unless ResolveAlterTargetFunc says +// otherwise, so a test that does not care about resolution exercises the +// operation behind it. +func (m *MockPageMutator) ResolveAlterTarget(t backend.AlterTarget) (backend.AlterTargetMatch, error) { + if m.ResolveAlterTargetFunc != nil { + return m.ResolveAlterTargetFunc(t) + } + return backend.AlterTargetMatch{Kind: "widget", Name: t.String()}, nil +} diff --git a/mdl/backend/mutation.go b/mdl/backend/mutation.go index 3c9db9400..ba92e5da4 100644 --- a/mdl/backend/mutation.go +++ b/mdl/backend/mutation.go @@ -70,6 +70,13 @@ type PageMutator interface { // ContainerType returns the kind of container (page, layout, or snippet). ContainerType() ContainerKind + // AlterTargetResolver resolves a generic ALTER target against this widget + // tree: the page family's half of `alter X { … }` (ADR-0012). The + // executor resolves every operation's target through it before applying + // the operation, so each document type answers "what does this address + // mean" in one place. + AlterTargetResolver + // --- Widget property operations --- // SetWidgetProperty sets a simple property on the named widget. diff --git a/mdl/backend/pagemutator/alter_target.go b/mdl/backend/pagemutator/alter_target.go new file mode 100644 index 000000000..b9e9b74d3 --- /dev/null +++ b/mdl/backend/pagemutator/alter_target.go @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pagemutator + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/bsonnav" +) + +// ResolveAlterTarget is the page family's backend.AlterTargetResolver (page, +// snippet and layout alike — one widget tree, one address syntax). +// +// A page element is addressed by name: `btnSave`, `grid.Column`, or a scroll +// container's region, `layoutContainer.top`. The lookups are the ones the +// operations themselves make, so resolution cannot accept a target the +// operation then misses, and a miss reports what the operation would have +// reported. +func (m *Mutator) ResolveAlterTarget(t backend.AlterTarget) (backend.AlterTargetMatch, error) { + if err := backend.CheckPageAlterTarget(t); err != nil { + return backend.AlterTargetMatch{}, err + } + name := strings.Join(t.Path, ".") + if len(t.Path) == 2 { + container, member := t.Path[0], t.Path[1] + if kind, ok, err := m.resolveScrollRegion(container, member); ok { + return backend.AlterTargetMatch{Kind: kind, Name: name}, err + } + if _, err := findBsonColumn(m.rawData, container, member, m.widgetFinder); err != nil { + return backend.AlterTargetMatch{}, err + } + return backend.AlterTargetMatch{Kind: "column", Name: name}, nil + } + + result := m.widgetFinder(m.rawData, name) + if result == nil { + return backend.AlterTargetMatch{}, m.widgetNotFoundError(name) + } + if bsonnav.DGetString(result.widget, "$Type") != objectListItemType && len(result.colPropKeys) == 0 { + // A real widget. Whether a same-named column elsewhere on the page makes + // the address ambiguous is each operation's call, as it was before the + // resolver existed: drop/replace/insert refuse it, set goes to the + // widget. Refusing here would be a new rejection (ADR-0011). + return backend.AlterTargetMatch{Kind: "widget", Name: name}, nil + } + if n := m.columnMatchCount(name); n > 1 { + return backend.AlterTargetMatch{}, columnAmbiguityError(name, n) + } + return backend.AlterTargetMatch{Kind: "column", Name: name}, nil +} + +// resolveScrollRegion answers a `container.slot` address when the container is a +// scroll container, with the same refusals insertIntoScrollRegion makes. ok is +// false when the container is not one, so the caller tries a grid column — the +// dotted form serves both, and what the named widget IS decides which. +func (m *Mutator) resolveScrollRegion(container, slot string) (kind string, ok bool, err error) { + result := m.widgetFinder(m.rawData, container) + if result == nil { + return "", false, nil + } + if t := bsonnav.DGetString(result.widget, "$Type"); t != "Forms$ScrollContainer" && t != "Pages$ScrollContainer" { + return "", false, nil + } + key, known := scrollRegionKey(slot) + if !known { + return "", true, fmt.Errorf("scroll container %q has no region %q (want top, right, bottom, left or center)", container, slot) + } + if bsonnav.DGetDoc(result.widget, key) == nil { + return "", true, fmt.Errorf("scroll container %q has no %s region; "+ + "add one with `create or replace layout` — an empty slot has no stored document to insert into", + container, strings.ToLower(slot)) + } + return "region", true, nil +} diff --git a/mdl/backend/pagemutator/alter_target_test.go b/mdl/backend/pagemutator/alter_target_test.go new file mode 100644 index 000000000..325bcf968 --- /dev/null +++ b/mdl/backend/pagemutator/alter_target_test.go @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pagemutator + +import ( + "errors" + "strings" + "testing" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/mdl/backend" +) + +// The page family's half of the generic ALTER (ako/mxcli#712): a page element +// is addressed by name, so the resolver answers from the same finders the +// operations use, and refuses the address forms a page has no use for. + +func namedFinder(widgets map[string]bson.D) widgetFinder { + return func(_ bson.D, name string) *bsonWidgetResult { + if w, ok := widgets[name]; ok { + return &bsonWidgetResult{widget: w} + } + return nil + } +} + +func resolverMutator() *Mutator { + grid := buildGridWithColumns([]map[string]string{{"attr": "M.E.Merchant"}, {"caption": "Amount"}}) + scroll := bson.D{ + {Key: "$Type", Value: "Forms$ScrollContainer"}, + {Key: "Name", Value: "layoutContainer"}, + {Key: "Top", Value: bson.D{{Key: "$Type", Value: "Forms$ScrollContainerRegion"}}}, + } + return &Mutator{rawData: bson.D{}, widgetFinder: namedFinder(map[string]bson.D{ + "btnSave": {{Key: "$Type", Value: "Forms$ActionButton"}, {Key: "Name", Value: "btnSave"}}, + "dg": grid, + "layoutContainer": scroll, + })} +} + +func TestResolveAlterTarget_ByName(t *testing.T) { + m := resolverMutator() + cases := []struct { + path []string + kind string + }{ + {[]string{"btnSave"}, "widget"}, + {[]string{"dg", "Merchant"}, "column"}, + {[]string{"layoutContainer", "top"}, "region"}, + } + for _, c := range cases { + got, err := m.ResolveAlterTarget(backend.AlterTarget{Path: c.path}) + if err != nil { + t.Errorf("%v: %v", c.path, err) + continue + } + if got.Kind != c.kind || got.Name != strings.Join(c.path, ".") { + t.Errorf("%v: got %+v", c.path, got) + } + } +} + +func TestResolveAlterTarget_Misses(t *testing.T) { + m := resolverMutator() + cases := []struct { + path []string + want string + }{ + {[]string{"btnMissing"}, `widget "btnMissing" not found`}, + {[]string{"dg", "Nope"}, "Nope"}, + {[]string{"layoutContainer", "middle"}, `has no region "middle"`}, + {[]string{"layoutContainer", "left"}, "has no left region"}, + } + for _, c := range cases { + _, err := m.ResolveAlterTarget(backend.AlterTarget{Path: c.path}) + if err == nil || !strings.Contains(err.Error(), c.want) { + t.Errorf("%v: want error containing %q, got %v", c.path, c.want, err) + } + } +} + +// A page element has a name; content addressing and @n belong to documents +// whose elements do not. Accepting either would have to mean something, and a +// page has nothing for it to mean — so each is refused, naming the form to use. +func TestResolveAlterTarget_RefusesFormsAPageDoesNotUse(t *testing.T) { + m := resolverMutator() + _, err := m.ResolveAlterTarget(backend.AlterTarget{Caption: "Save"}) + if err == nil || !strings.Contains(err.Error(), "by name") { + t.Errorf("caption: want a by-name refusal, got %v", err) + } + _, err = m.ResolveAlterTarget(backend.AlterTarget{Path: []string{"btnSave"}, Ordinal: 1}) + if err == nil || !strings.Contains(err.Error(), "@1") { + t.Errorf("ordinal: want a refusal naming @1, got %v", err) + } + var te *backend.AlterTargetError + if !errors.As(err, &te) { + t.Errorf("refusal should be an AlterTargetError, got %T", err) + } +} + +// A real widget whose name is also a derived column name in two grids. `set` +// on it has always gone to the widget (SetWidgetProperty only raises the +// column ambiguity when the name resolved to a column), so the resolver must +// not refuse it: a new rejection is a change of meaning ADR-0011 only allows +// behind the language header. The operations that did refuse it (drop, +// replace, insert) still do, on their own. The column case is the control: a +// bare name that resolves to a column still reports the ambiguity. +func TestResolveAlterTarget_WidgetNamedLikeAnAmbiguousColumn(t *testing.T) { + grid1 := buildGridWithColumns([]map[string]string{{"attr": "M.E.Merchant"}}) + grid2 := buildGridWithColumns([]map[string]string{{"attr": "M.E.Merchant"}}) + page := bson.D{{Key: "Widgets", Value: bson.A{int32(2), grid1, grid2}}} + txt := bson.D{{Key: "$Type", Value: "Forms$TextBox"}, {Key: "Name", Value: "Merchant"}, + {Key: "Appearance", Value: bson.D{{Key: "Class", Value: ""}}}} + m := &Mutator{rawData: page, widgetFinder: namedFinder(map[string]bson.D{"Merchant": txt})} + + if err := m.SetWidgetProperty("Merchant", "Class", "x"); err != nil { + t.Fatalf("control: the operation itself accepts the widget: %v", err) + } + got, err := m.ResolveAlterTarget(backend.AlterTarget{Path: []string{"Merchant"}}) + if err != nil { + t.Fatalf("resolver refuses a target the operation accepts: %v", err) + } + if got.Kind != "widget" { + t.Errorf("kind: got %q, want widget", got.Kind) + } + + col := bson.D{{Key: "$Type", Value: objectListItemType}} + m.widgetFinder = namedFinder(map[string]bson.D{"Merchant": col}) + if _, err := m.ResolveAlterTarget(backend.AlterTarget{Path: []string{"Merchant"}}); err == nil || + !strings.Contains(err.Error(), "Merchant") { + t.Errorf("a bare name resolving to one of two columns must stay ambiguous, got %v", err) + } +} diff --git a/mdl/backend/pagemutator/mutator.go b/mdl/backend/pagemutator/mutator.go index a458d783f..aeb7cbe64 100644 --- a/mdl/backend/pagemutator/mutator.go +++ b/mdl/backend/pagemutator/mutator.go @@ -239,7 +239,7 @@ func (m *Mutator) SetWidgetNamedAction(widgetRef, propertyKey string, action pag obj := bsonnav.DGetDoc(result.widget, "Object") if obj == nil { return fmt.Errorf("widget %q (%s) is not a pluggable widget and has no named action slots — "+ - "a built-in widget's click action is set with `set Action = … on %s`", + "a built-in widget's click action is set with `set (Action: …) on %s`", widgetRef, widgetTypeName(result.widget), widgetRef) } diff --git a/mdl/backend/pagemutator/mutator_named_action_test.go b/mdl/backend/pagemutator/mutator_named_action_test.go index 1f59d0c19..a941e8b3f 100644 --- a/mdl/backend/pagemutator/mutator_named_action_test.go +++ b/mdl/backend/pagemutator/mutator_named_action_test.go @@ -147,12 +147,12 @@ func TestSetWidgetNamedAction_UnknownKeyAndWidget(t *testing.T) { } // TestSetWidgetNamedAction_RefusesBuiltInWidget: a Forms$ActionButton has no -// pluggable slots; its click action is `set Action = …`. +// pluggable slots; its click action is `set (Action: …)`. func TestSetWidgetNamedAction_RefusesBuiltInWidget(t *testing.T) { btn := bson.D{{Key: "$Type", Value: "Forms$ActionButton"}, {Key: "Name", Value: "btnGo"}, {Key: "Action", Value: noAction()}} m := New(makeRawPage(btn), model.ID("u"), &stubActionDeps{serialized: microflowMarker}) err := m.SetWidgetNamedAction("btnGo", "onClick", &pages.MicroflowClientAction{MicroflowName: "M.F"}) - if err == nil || !strings.Contains(err.Error(), "set Action") { - t.Errorf("got %v, want a refusal pointing at `set Action`", err) + if err == nil || !strings.Contains(err.Error(), "set (Action:") { + t.Errorf("got %v, want a refusal pointing at `set (Action: …)`", err) } } diff --git a/mdl/executor/alter_aliases.go b/mdl/executor/alter_aliases.go new file mode 100644 index 000000000..647668eef --- /dev/null +++ b/mdl/executor/alter_aliases.go @@ -0,0 +1,149 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// The old spellings of ALTER PAGE / SNIPPET / LAYOUT are aliases of the generic +// ALTER (ADR-0012 decision 2): they parse to the identical operation and warn +// (ADR-0011: an old form keeps working through the alias window, and says what +// replaces it). +// +// Each entry has the shape the deprecation registry (ako/mxcli#709) takes — +// code, old form, canonical form, mechanical rewrite, language version it is +// removed in — so the registry absorbs this table rather than re-deriving it. +// Until it lands the codes are provisional and numbered from 101, clear of the +// registry's seed entries. The grammar marks each alias alternative with +// `// alias: `; TestAlterAliasGrammarMarkersMatchTable pins the two +// together, in both directions. +type alterAlias struct { + Code string // MDL-DEPRnnn + Spelling string // ast.AlterAlias* + Old string // the old form, as written + Canonical string // what replaces it + Rewrite string // the mechanical rewrite, for `fmt --upgrade` + RemovedIn string // the MDL language version that drops the alias +} + +var alterAliases = []alterAlias{ + { + Code: "MDL-DEPR101", + Spelling: ast.AlterAliasSetEquals, + Old: "set Key = value [on target] / set (Key = value, …) [on target]", + Canonical: "set (Key: value, …) [on target]", + Rewrite: "put the assignments in parentheses and write each `=` as `:`", + RemovedIn: "mdl 2", + }, + { + Code: "MDL-DEPR102", + Spelling: ast.AlterAliasSetUnparenthesised, + Old: "set Key: value [on target]", + Canonical: "set (Key: value) [on target]", + Rewrite: "put the assignment in parentheses", + RemovedIn: "mdl 2", + }, + { + Code: "MDL-DEPR103", + Spelling: ast.AlterAliasDropWidget, + Old: "drop widget a, b", + Canonical: "drop a, b", + Rewrite: "remove the word `widget`", + RemovedIn: "mdl 2", + }, +} + +func alterAliasFor(spelling string) (alterAlias, bool) { + for _, a := range alterAliases { + if a.Spelling == spelling { + return a, true + } + } + return alterAlias{}, false +} + +// validateAlterAliases warns on every old ALTER spelling a statement uses, once +// per operation. A warning, never an error: both spellings build the identical +// operation, and scripts in the wild use the old ones. +func validateAlterAliases(stmt ast.Statement) []linter.Violation { + s, ok := stmt.(*ast.AlterPageStmt) + if !ok { + return nil + } + kind := strings.ToLower(s.ContainerType) + if kind == "" { + kind = "page" + } + var out []linter.Violation + for _, op := range s.Operations { + var spelling string + switch o := op.(type) { + case *ast.SetPropertyOp: + spelling = o.Legacy + case *ast.DropWidgetOp: + spelling = o.Legacy + } + if spelling == "" { + continue + } + a, known := alterAliasFor(spelling) + if !known { + continue + } + out = append(out, linter.Violation{ + RuleID: a.Code, + Severity: linter.SeverityWarning, + Message: fmt.Sprintf("alter %s %s: `%s` is the old spelling of `%s`", + kind, s.PageName.String(), a.Old, a.Canonical), + Location: linter.Location{ + Module: s.PageName.Module, + DocumentType: kind, + DocumentName: s.PageName.Name, + }, + Suggestion: fmt.Sprintf("Write `%s` (%s). Both build the identical change; the old form is removed in %s.", + a.Canonical, a.Rewrite, a.RemovedIn), + }) + } + return out +} + +// validateAlterPageAddresses refuses, with no project needed, a target whose +// address FORM a page cannot use: a quoted caption, an @n, a path of more than +// two names. The generic grammar accepts every address form any document type +// uses, and the page family's resolver refuses these whatever the page holds — +// so `check` can say so up front instead of `exec` stopping mid-script. The +// rule is the resolver's own (backend.CheckPageAlterTarget), not a copy. +func validateAlterPageAddresses(stmt ast.Statement) []linter.Violation { + s, ok := stmt.(*ast.AlterPageStmt) + if !ok { + return nil + } + kind := strings.ToLower(s.ContainerType) + if kind == "" { + kind = "page" + } + var out []linter.Violation + for _, op := range s.Operations { + for _, ref := range alterPageOperationTargets(op) { + if err := backend.CheckPageAlterTarget(alterTargetOf(ref)); err != nil { + out = append(out, linter.Violation{ + RuleID: "MDL-ALTER01", + Severity: linter.SeverityError, + Message: fmt.Sprintf("alter %s %s: %v", kind, s.PageName.String(), err), + Location: linter.Location{ + Module: s.PageName.Module, + DocumentType: kind, + DocumentName: s.PageName.Name, + }, + }) + } + } + } + return out +} diff --git a/mdl/executor/alter_aliases_test.go b/mdl/executor/alter_aliases_test.go new file mode 100644 index 000000000..2ef32eae8 --- /dev/null +++ b/mdl/executor/alter_aliases_test.go @@ -0,0 +1,140 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "os" + "path/filepath" + "regexp" + "sort" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/linter" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +func aliasWarnings(t *testing.T, src string) []linter.Violation { + t.Helper() + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + var out []linter.Violation + for _, v := range ValidateProgram(prog, "") { + if strings.HasPrefix(v.RuleID, "MDL-DEPR") { + out = append(out, v) + } + } + return out +} + +// The old ALTER PAGE spellings still run, and warn with the code that names +// their rewrite (ako/mxcli#712). The canonical script is the control: it must +// produce no deprecation warning at all, or a warning on every ALTER would pass. +func TestAlterAliases_OldSpellingsWarn(t *testing.T) { + old := aliasWarnings(t, `alter page M.P { + set Caption = 'Save' on btnSave; + set (Caption = 'x', ButtonStyle = Success) on btn2; + set Title: 'T'; + drop widget txtOld; + };`) + var codes []string + for _, v := range old { + if v.Severity != linter.SeverityWarning { + t.Errorf("%s must be a warning, got %v", v.RuleID, v.Severity) + } + codes = append(codes, v.RuleID) + } + want := []string{"MDL-DEPR101", "MDL-DEPR101", "MDL-DEPR102", "MDL-DEPR103"} + if strings.Join(codes, ",") != strings.Join(want, ",") { + t.Errorf("codes: got %v, want %v", codes, want) + } + if len(old) > 0 && !strings.Contains(old[0].Suggestion, "set (Key: value") { + t.Errorf("suggestion should name the canonical form: %q", old[0].Suggestion) + } + + canonical := aliasWarnings(t, `alter page M.P { + set (Caption: 'Save') on btnSave; + set (Caption: 'x', ButtonStyle: Success) on btn2; + set (Title: 'T'); + drop txtOld; + set layout = Atlas_Core.TopBar; + };`) + if len(canonical) != 0 { + t.Errorf("canonical form must not warn, got %v", canonical) + } +} + +// Every grammar alternative marked `// alias: ` has an entry in the +// alias table, and every entry is marked somewhere in the grammar — so an alias +// cannot be added to one without the other. The deprecation registry +// (ako/mxcli#709) generalises this check. +func TestAlterAliasGrammarMarkersMatchTable(t *testing.T) { + marker := regexp.MustCompile(`//\s*alias:\s*(MDL-DEPR\d+)`) + files, _ := filepath.Glob("../grammar/*.g4") + more, _ := filepath.Glob("../grammar/domains/*.g4") + files = append(files, more...) + if len(files) == 0 { + t.Fatal("no grammar files found") + } + marked := map[string]bool{} + for _, f := range files { + b, err := os.ReadFile(f) + if err != nil { + t.Fatal(err) + } + for _, m := range marker.FindAllStringSubmatch(string(b), -1) { + marked[m[1]] = true + } + } + table := map[string]bool{} + for _, a := range alterAliases { + table[a.Code] = true + if !marked[a.Code] { + t.Errorf("%s is in the alias table but no grammar alternative is marked `// alias: %s`", a.Code, a.Code) + } + } + var missing []string + for code := range marked { + if !table[code] { + missing = append(missing, code) + } + } + sort.Strings(missing) + for _, code := range missing { + t.Errorf("grammar marks an alias %s with no entry in alterAliases", code) + } +} + +// A caption or @n target is a form a page does not use. The generic grammar +// parses it (other document types need it), so check must refuse it — with no +// project — rather than leave exec to stop partway through a script. A name +// target is the control. +func TestAlterPageAddresses_RefusedAtCheck(t *testing.T) { + errorsOf := func(src string) []linter.Violation { + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + var out []linter.Violation + for _, v := range ValidateProgram(prog, "") { + if v.RuleID == "MDL-ALTER01" { + out = append(out, v) + } + } + return out + } + bad := errorsOf(`alter page M.P { drop 'Learn more'; replace hdr@2 with { container c1 } };`) + if len(bad) != 2 { + t.Fatalf("want 2 MDL-ALTER01 errors, got %v", bad) + } + for _, v := range bad { + if v.Severity != linter.SeverityError { + t.Errorf("want an error, got %v", v.Severity) + } + } + if ok := errorsOf(`alter page M.P { drop txtOld, dg.Total; replace hdr with { container c1 } };`); len(ok) != 0 { + t.Errorf("name targets must pass, got %v", ok) + } +} diff --git a/mdl/executor/cmd_alter_generic_test.go b/mdl/executor/cmd_alter_generic_test.go new file mode 100644 index 000000000..4e58a9cbd --- /dev/null +++ b/mdl/executor/cmd_alter_generic_test.go @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "errors" + "sort" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// The generic ALTER resolves every operation's target through the document +// type's backend.AlterTargetResolver before the operation runs (ako/mxcli#712). + +func alterPageCtxWith(t *testing.T, mut *mock.MockPageMutator) *ExecContext { + t.Helper() + mod := mkModule("MyModule") + pg := mkPage(mod.ID, "TestPage") + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListFoldersFunc: func() ([]*types.FolderInfo, error) { return nil, nil }, + ListPagesFunc: func() ([]*pages.Page, error) { return []*pages.Page{pg}, nil }, + OpenPageForMutationFunc: func(model.ID) (backend.PageMutator, error) { + return mut, nil + }, + } + h := mkHierarchy(mod) + withContainer(h, pg.ContainerID, mod.ID) + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h)) + return ctx +} + +func TestAlterPage_ResolvesEveryTargetThroughTheResolver(t *testing.T) { + var resolved []string + mut := &mock.MockPageMutator{ + ResolveAlterTargetFunc: func(tg backend.AlterTarget) (backend.AlterTargetMatch, error) { + resolved = append(resolved, tg.String()) + return backend.AlterTargetMatch{Kind: "widget", Name: tg.String()}, nil + }, + DropListViewTemplateFunc: func(string, string) error { return nil }, + } + ctx := alterPageCtxWith(t, mut) + assertNoError(t, execAlterPage(ctx, &ast.AlterPageStmt{ + PageName: ast.QualifiedName{Module: "MyModule", Name: "TestPage"}, + Operations: []ast.AlterPageOperation{ + &ast.SetPropertyOp{Target: ast.WidgetRef{Widget: "btnSave"}, Properties: map[string]any{"Caption": "Save"}}, + &ast.SetPropertyOp{Properties: map[string]any{"Title": "T"}}, // the document itself: nothing to resolve + &ast.DropWidgetOp{Targets: []ast.WidgetRef{{Widget: "a"}, {Widget: "dg", Column: "Total"}}}, + &ast.ReplaceWidgetOp{Target: ast.WidgetRef{Widget: "hdr"}}, + &ast.InsertWidgetOp{Position: "INTO", Target: ast.WidgetRef{Widget: "ctn"}}, + &ast.DropListViewTemplateOp{ListView: "lv", Specialization: "M.E"}, + }, + })) + sort.Strings(resolved) + want := []string{"a", "btnSave", "ctn", "dg.Total", "hdr", "lv"} + if strings.Join(resolved, ",") != strings.Join(want, ",") { + t.Errorf("resolved targets: got %v, want %v", resolved, want) + } +} + +// A refusal from the resolver stops the statement before the operation runs +// and before anything is saved. +func TestAlterPage_ResolverRefusalStopsBeforeTheOperation(t *testing.T) { + saved, dropped := false, false + mut := &mock.MockPageMutator{ + ResolveAlterTargetFunc: func(tg backend.AlterTarget) (backend.AlterTargetMatch, error) { + return backend.AlterTargetMatch{}, &backend.AlterTargetError{Target: tg, + Matches: []backend.AlterTargetMatch{{Kind: "widget", Name: "x"}, {Kind: "widget", Name: "y"}}} + }, + DropWidgetFunc: func([]backend.WidgetRef) error { dropped = true; return nil }, + SaveFunc: func() error { saved = true; return nil }, + } + ctx := alterPageCtxWith(t, mut) + err := execAlterPage(ctx, &ast.AlterPageStmt{ + PageName: ast.QualifiedName{Module: "MyModule", Name: "TestPage"}, + Operations: []ast.AlterPageOperation{&ast.DropWidgetOp{Targets: []ast.WidgetRef{{Caption: "Save"}}}}, + }) + var te *backend.AlterTargetError + if !errors.As(err, &te) { + t.Fatalf("want the resolver's AlterTargetError, got %v", err) + } + if !strings.Contains(err.Error(), "@2 widget y") { + t.Errorf("error should list the matches: %v", err) + } + if dropped || saved { + t.Errorf("operation ran (%v) or document saved (%v) after a refused target", dropped, saved) + } +} diff --git a/mdl/executor/cmd_alter_page.go b/mdl/executor/cmd_alter_page.go index 6266ec845..370cd9682 100644 --- a/mdl/executor/cmd_alter_page.go +++ b/mdl/executor/cmd_alter_page.go @@ -57,6 +57,13 @@ func execAlterPage(ctx *ExecContext, s *ast.AlterPageStmt) error { modName := h.GetModuleName(containerID) for _, op := range s.Operations { + // Every target resolves through the document type's resolver before the + // operation runs (ADR-0012): what an address means is answered once, + // per document type, and a miss or an ambiguity stops the statement + // before anything changes. + if err := resolveAlterPageTargets(mutator, op); err != nil { + return mdlerrors.NewBackend("resolve "+strings.ToLower(containerType)+" target", err) + } switch o := op.(type) { case *ast.SetPropertyOp: if err := applySetPropertyMutator(ctx, mutator, o, modName, containerID); err != nil { @@ -111,6 +118,53 @@ func execAlterPage(ctx *ExecContext, s *ast.AlterPageStmt) error { return nil } +// alterTargetOf converts an operation's target, as the visitor recorded it, +// into the backend's document-independent address. +func alterTargetOf(r ast.WidgetRef) backend.AlterTarget { + t := backend.AlterTarget{Caption: r.Caption, Ordinal: r.Ordinal} + if r.Caption == "" { + t.Path = []string{r.Widget} + if r.Column != "" { + t.Path = append(t.Path, r.Column) + } + } + return t +} + +// alterPageOperationTargets lists the targets an operation addresses, in the +// order it names them. A page-level SET and the variable and layout operations +// address the document itself and have none. +func alterPageOperationTargets(op ast.AlterPageOperation) []ast.WidgetRef { + switch o := op.(type) { + case *ast.SetPropertyOp: + if o.Target.Widget == "" && o.Target.Caption == "" { + return nil + } + return []ast.WidgetRef{o.Target} + case *ast.InsertWidgetOp: + return []ast.WidgetRef{o.Target} + case *ast.ReplaceWidgetOp: + return []ast.WidgetRef{o.Target} + case *ast.DropWidgetOp: + return o.Targets + case *ast.DropListViewTemplateOp: + return []ast.WidgetRef{{Widget: o.ListView}} + } + return nil +} + +// resolveAlterPageTargets resolves every target of one operation. A drop's +// targets are resolved together up front: an unresolvable second target then +// refuses the whole drop instead of leaving the first one dropped. +func resolveAlterPageTargets(resolver backend.AlterTargetResolver, op ast.AlterPageOperation) error { + for _, ref := range alterPageOperationTargets(op) { + if _, err := resolver.ResolveAlterTarget(alterTargetOf(ref)); err != nil { + return err + } + } + return nil +} + // resolveAlterPageUnit resolves an ALTER PAGE / SNIPPET / LAYOUT target to the // storage unit it edits and the module holding it. One statement type, three // document kinds — the visitor sets ContainerType from the keyword, and an empty @@ -272,7 +326,7 @@ func convertASTAction(ctx *ExecContext, value any, moduleName string, moduleID m action, ok := value.(*ast.ActionV3) if !ok { return nil, mdlerrors.NewValidation("Action value must be an action expression, " + - "for example `set Action = microflow Module.MF on btnSave`") + "for example `set (Action: microflow Module.MF) on btnSave`") } pb := &pageBuilder{ ctx: ctx, diff --git a/mdl/executor/validate_program.go b/mdl/executor/validate_program.go index 3c9871861..b1f7cc8c5 100644 --- a/mdl/executor/validate_program.go +++ b/mdl/executor/validate_program.go @@ -105,6 +105,12 @@ func ValidateProgram(prog *ast.Program, projectPath string) []linter.Violation { if awfStmt, ok := stmt.(*ast.AlterWorkflowStmt); ok { violations = append(violations, ValidateAlterWorkflow(awfStmt)...) } + // The old ALTER PAGE / SNIPPET / LAYOUT spellings are aliases of the + // generic ALTER and warn with their deprecation code (MDL-DEPR101..103). + violations = append(violations, validateAlterAliases(stmt)...) + // A page element is addressed by name; a caption or @n target is + // refused before exec would stop on it (MDL-ALTER01). + violations = append(violations, validateAlterPageAddresses(stmt)...) // Check GRANT for member rights Mendix cannot store if grantStmt, ok := stmt.(*ast.GrantEntityAccessStmt); ok { violations = append(violations, ValidateGrantEntityAccess(grantStmt)...) diff --git a/mdl/grammar/MDLParser.g4 b/mdl/grammar/MDLParser.g4 index fc2f79d57..4afc00cf5 100644 --- a/mdl/grammar/MDLParser.g4 +++ b/mdl/grammar/MDLParser.g4 @@ -146,16 +146,17 @@ alterStatement | ALTER ODATA SERVICE qualifiedName SET odataAlterAssignment (COMMA odataAlterAssignment)* | ALTER STYLING ON (PAGE | SNIPPET) qualifiedName WIDGET IDENTIFIER alterStylingAction+ | ALTER SETTINGS alterSettingsClause - | ALTER PAGE qualifiedName LBRACE alterPageOperation+ RBRACE + // The generic ALTER (ADR-0012 decision 2): one patch grammar for every + // document type — `alter Module.Name { set / insert / replace / drop }`. + // The document type chooses how a target is resolved (a per-type + // backend.AlterTargetResolver) and what a fragment is written in (exactly the + // `create` syntax of that type). A layout's widget tree is a page's with four + // extra element types, so SET/INSERT/DROP/REPLACE mean exactly the same thing + // there; a scroll-container region is addressed as `layoutContainer.top`, + // because a region has no Name of its own. + | ALTER alterDocumentType qualifiedName LBRACE alterOperation+ RBRACE | alterPagesLayoutStatement | alterPagesStylingStatement - // ALTER LAYOUT reuses alterPageOperation wholesale: a layout's widget tree is - // a page's widget tree with four extra element types, so SET/INSERT/DROP/ - // REPLACE mean exactly the same thing. A scroll-container region is addressed - // through the dotted widgetRef the grammar already has — `layoutContainer.top` - // — because a region has no Name of its own. - | ALTER LAYOUT qualifiedName LBRACE alterPageOperation+ RBRACE - | ALTER SNIPPET qualifiedName LBRACE alterPageOperation+ RBRACE | ALTER WORKFLOW qualifiedName alterWorkflowAction+ SEMICOLON? | alterMessageDefinitionCollectionStatement | alterMessageDefinitionStatement @@ -212,57 +213,87 @@ alterStylingAssignment ; /** - * ALTER PAGE operations for modifying widget trees in-place. + * The generic ALTER's operations (ADR-0012 decision 2, ako/mxcli#712). * - * @example Set property on widget - * ```mdl - * ALTER PAGE Module.Page { - * SET Caption = 'Save' ON btnSave - * } - * ``` + * Canonical form, the same for every document type: * - * @example Insert widget after another * ```mdl - * ALTER PAGE Module.Page { - * INSERT AFTER txtName { TEXTBOX txtNew (Label: 'New', Binds: Attr) } + * alter page Module.Page { + * set (Caption: 'Save', ButtonStyle: Success) on btnSave; + * set (Title: 'Edit order'); -- the document itself + * insert after txtName { textbox txtNew (Label: 'New', Attribute: Attr) } + * insert into ctnMain { … } + * replace footer1 with { footer f1 { … } } + * drop txtOld, dgOrders.Total; * } * ``` * - * @example Drop widgets - * ```mdl - * ALTER PAGE Module.Page { - * DROP WIDGET txtOld, txtUnused - * } - * ``` + * Alternatives marked `alias:` are the old page spellings. They still parse to + * the identical operation and warn with the named deprecation code; the table + * that maps each code to its rewrite is mdl/executor/alter_aliases.go, and a + * test fails when a marker here has no entry there (or the reverse). * - * @example Replace widget subtree - * ```mdl - * ALTER PAGE Module.Page { - * REPLACE footer1 WITH { FOOTER f1 { ACTIONBUTTON btn1 (Caption: 'OK', Action: SAVE_CHANGES) } } - * } - * ``` + * Page-family operations that have no generic spelling yet (`set layout = … map`, + * `drop template for … in …`, `add variables`, `drop variables`) keep their own + * form inside the generic block; they are not aliases. */ -alterPageOperation - : alterPageSet SEMICOLON? - | alterPageInsert SEMICOLON? - | alterPageDrop SEMICOLON? +alterDocumentType + : PAGE + | SNIPPET + | LAYOUT + ; + +alterOperation + : alterSet SEMICOLON? + | alterInsert SEMICOLON? + | alterReplace SEMICOLON? + | alterDrop SEMICOLON? | alterPageDropTemplate SEMICOLON? - | alterPageReplace SEMICOLON? | alterPageAddVariable SEMICOLON? | alterPageDropVariable SEMICOLON? ; -alterPageSet +alterSet : SET LAYOUT EQUALS qualifiedName (MAP LPAREN alterLayoutMapping (COMMA alterLayoutMapping)* RPAREN)? // SET Layout = Atlas_Core.TopBar MAP (Main AS Content) - | SET alterPageAssignment ON widgetRef // SET Caption = 'Save' ON btnSave | ON dgProducts.Name - | SET LPAREN alterPageAssignment (COMMA alterPageAssignment)* RPAREN ON widgetRef // SET (Caption = 'Save', ButtonStyle = Success) ON btnSave - | SET alterPageAssignment // SET Title = 'Edit' (page-level) + | SET LPAREN alterPageAssignment (COMMA alterPageAssignment)* RPAREN (ON alterTarget)? // set (Caption: 'Save', ButtonStyle: Success) on btnSave + | SET alterPageAssignment (ON alterTarget)? // alias: MDL-DEPR102 — set Caption: 'Save' on btnSave ; alterLayoutMapping : identifierOrKeyword AS identifierOrKeyword // OldPlaceholder AS NewPlaceholder ; +alterInsert + : INSERT (AFTER | BEFORE | INTO) alterTarget alterFragment // INTO appends as children of a container + ; + +alterReplace + : REPLACE alterTarget WITH alterFragment + ; + +alterDrop + : DROP alterTarget (COMMA alterTarget)* + | DROP WIDGET alterTarget (COMMA alterTarget)* // alias: MDL-DEPR103 — drop widget a, b + ; + +// A fragment is written exactly as `create` writes the same content. Only the +// page family is on the generic path so far; a workflow's body joins here when +// ALTER WORKFLOW is ported. +alterFragment + : LBRACE pageBodyV3 RBRACE + ; + +// The one address syntax every document type shares. Which forms a type +// accepts is its resolver's call, not the grammar's: a page element is +// addressed by name (`btnSave`, `dgProducts.Name`, `layoutContainer.top`); +// elements with no name are addressed by content (`'Approve order'`). `@n` +// picks one of several matches — an ambiguous address is an error that lists +// them, never a guess. +alterTarget + : identifierOrKeyword (DOT identifierOrKeyword)? (AT NUMBER_LITERAL)? + | STRING_LITERAL (AT NUMBER_LITERAL)? + ; + // ALTER PAGES [IN ] SET LAYOUT = Module.Layout [MAP (...)] [WHERE LAYOUT = Module.Old] // // The bulk form is the real one: an app has one layout and many pages, so @@ -309,11 +340,18 @@ alterPagesStylingAssignment | STRING_LITERAL EQUALS OFF // 'Striped' = OFF ; +// `Key: value` is canonical (R3: `:` binds a property, `=` compares). `=` is +// the old spelling, still accepted. +alterAssignOp + : COLON + | EQUALS // alias: MDL-DEPR101 — set (Caption = 'Save') / set Caption = 'Save' + ; + alterPageAssignment - : DATASOURCE EQUALS dataSourceExprV3 // DataSource = SELECTION widgetName - | ACTION EQUALS actionExprV3 // Action = MICROFLOW Module.MF | SHOW_PAGE Module.Page | SAVE_CHANGES CLOSE_PAGE - | VISIBLE EQUALS xpathConstraint // Visible = [Name != ''] (conditional visibility) - | EDITABLE EQUALS xpathConstraint // Editable = [Status = 'Open'] (conditional editability) + : DATASOURCE alterAssignOp dataSourceExprV3 // DataSource: selection widgetName + | ACTION alterAssignOp actionExprV3 // Action: MICROFLOW Module.MF | SHOW_PAGE Module.Page | SAVE_CHANGES CLOSE_PAGE + | VISIBLE alterAssignOp xpathConstraint // Visible: [Name != ''] (conditional visibility) + | EDITABLE alterAssignOp xpathConstraint // Editable: [Status = 'Open'] (conditional editability) // A pluggable widget's NAMED action slot, addressed by the widget's own key: // `set 'createFileAction' = microflow M.F on fileUploader1`. The ALTER-level // twin of widgetPropertyV3's `key: actionExprV3` (#956); without it the value @@ -324,27 +362,17 @@ alterPageAssignment // no datasource overlap to yield to, since DataSource is its own alternative. // Whether the key IS an action slot is the stored widget's call, not the // grammar's — the mutator refuses one that is not. - | STRING_LITERAL EQUALS actionExprV3 // 'createFileAction' = MICROFLOW Module.MF - | identifierOrKeyword EQUALS actionExprV3 // createFileAction = MICROFLOW Module.MF - | identifierOrKeyword EQUALS propertyValueV3 // Caption = 'Save' - | STRING_LITERAL EQUALS propertyValueV3 // 'showLabel' = false - | identifierOrKeyword EQUALS expression // DynamicClasses = if $x/F then 'a' else '' (see widgetPropertyV3) - ; - -alterPageInsert - : INSERT AFTER widgetRef LBRACE pageBodyV3 RBRACE - | INSERT BEFORE widgetRef LBRACE pageBodyV3 RBRACE - | INSERT INTO widgetRef LBRACE pageBodyV3 RBRACE // append as children of a container - ; - -alterPageDrop - : DROP WIDGET widgetRef (COMMA widgetRef)* + | STRING_LITERAL alterAssignOp actionExprV3 // 'createFileAction': microflow Module.MF + | identifierOrKeyword alterAssignOp actionExprV3 // createFileAction: microflow Module.MF + | identifierOrKeyword alterAssignOp propertyValueV3 // Caption: 'Save' + | STRING_LITERAL alterAssignOp propertyValueV3 // 'showLabel': false + | identifierOrKeyword alterAssignOp expression // DynamicClasses: if $x/F then 'a' else '' (see widgetPropertyV3) ; // DROP TEMPLATE FOR Module.Specialization IN listViewName // // A List View specialization template has no name — the entity it renders is -// what identifies it — so it cannot be reached through widgetRef like every +// what identifies it — so it cannot be reached through alterTarget like every // other DROP target. Naming the list view is required, not optional: one page // can hold two list views with a template for the same entity. // @@ -352,17 +380,7 @@ alterPageDrop // `INSERT INTO { template for Module.Entity { ... } }`, which reuses // the same block as CREATE PAGE, so a template has one spelling everywhere. alterPageDropTemplate - : DROP TEMPLATE FOR qualifiedName IN widgetRef - ; - -alterPageReplace - : REPLACE widgetRef WITH LBRACE pageBodyV3 RBRACE - ; - -// Widget reference: plain name (btnSave) or dotted path (dgProducts.Name) -widgetRef - : identifierOrKeyword DOT identifierOrKeyword // dgProducts.Name (column ref) - | identifierOrKeyword // btnSave (widget ref) + : DROP TEMPLATE FOR qualifiedName IN alterTarget ; alterPageAddVariable diff --git a/mdl/visitor/visitor_alter.go b/mdl/visitor/visitor_alter.go index e69ffc2ea..c878412ef 100644 --- a/mdl/visitor/visitor_alter.go +++ b/mdl/visitor/visitor_alter.go @@ -11,9 +11,10 @@ import ( // Sub-types (PAGE, SNIPPET, STYLING, WORKFLOW) are handled by dedicated visitor files; // OData ALTER is handled inline below. func (b *Builder) ExitAlterStatement(ctx *parser.AlterStatementContext) { - // Handle ALTER PAGE / ALTER SNIPPET - if (ctx.PAGE() != nil || ctx.SNIPPET() != nil || ctx.LAYOUT() != nil) && len(ctx.AllAlterPageOperation()) > 0 { - b.exitAlterPageStatement(ctx) + // The generic ALTER Module.Name { … } (ADR-0012). Only the page + // family is on it so far. + if ctx.AlterDocumentType() != nil { + b.exitAlterDocumentStatement(ctx) return } diff --git a/mdl/visitor/visitor_alter_alias_equivalence_test.go b/mdl/visitor/visitor_alter_alias_equivalence_test.go new file mode 100644 index 000000000..d48d43d73 --- /dev/null +++ b/mdl/visitor/visitor_alter_alias_equivalence_test.go @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "reflect" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// ADR-0011: an alias parses to the IDENTICAL operation as its canonical form — +// the only difference allowed is the Legacy marker that drives the +// deprecation warning. Without this, an alias could silently build a different +// change (a value parsed through another rule, a target dropped) and the +// warning would tell the user the two are interchangeable when they are not. +// `fmt --upgrade` relies on the same equivalence (ADR-0011, Negative). +func TestGenericAlter_AliasesBuildTheIdenticalOperation(t *testing.T) { + pairs := []struct{ name, old, canonical string }{ + {"set property on widget", `set Caption = 'Save' on btnSave`, `set (Caption: 'Save') on btnSave`}, + {"set list with =", `set (Caption = 'x', ButtonStyle = Success) on btn2`, `set (Caption: 'x', ButtonStyle: Success) on btn2`}, + {"set unparenthesised colon", `set Caption: 'Save' on btnSave`, `set (Caption: 'Save') on btnSave`}, + {"page-level set", `set Title = 'Edit'`, `set (Title: 'Edit')`}, + {"quoted key", `set 'showLabel' = false on w1`, `set ('showLabel': false) on w1`}, + {"action", `set Action = microflow M.ACT on btnGo`, `set (Action: microflow M.ACT) on btnGo`}, + {"named action slot", `set 'createFileAction' = microflow M.F on up1`, `set ('createFileAction': microflow M.F) on up1`}, + {"datasource", `set DataSource = $Param on dv1`, `set (DataSource: $Param) on dv1`}, + {"visible", `set Visible = [Name != ''] on txt1`, `set (Visible: [Name != '']) on txt1`}, + {"expression", `set DynamicClasses = if $x/F then 'a' else '' on c1`, `set (DynamicClasses: if $x/F then 'a' else '') on c1`}, + {"column target", `set Caption = 'Total' on dg.Total`, `set (Caption: 'Total') on dg.Total`}, + {"drop widget", `drop widget a, dg.Total`, `drop a, dg.Total`}, + } + for _, p := range pairs { + t.Run(p.name, func(t *testing.T) { + old := buildAlterPage(t, "alter page M.P { "+p.old+"; };").Operations + canon := buildAlterPage(t, "alter page M.P { "+p.canonical+"; };").Operations + if len(old) != 1 || len(canon) != 1 { + t.Fatalf("want one operation each, got %d and %d", len(old), len(canon)) + } + if legacyOf(old[0]) == "" { + t.Fatalf("old spelling %q not flagged as an alias", p.old) + } + if legacyOf(canon[0]) != "" { + t.Fatalf("canonical spelling %q flagged as alias %q", p.canonical, legacyOf(canon[0])) + } + clearLegacy(old[0]) + if !reflect.DeepEqual(old[0], canon[0]) { + t.Errorf("alias builds a different operation:\n old: %#v\n canonical: %#v", old[0], canon[0]) + } + }) + } +} + +func legacyOf(op ast.AlterPageOperation) string { + switch o := op.(type) { + case *ast.SetPropertyOp: + return o.Legacy + case *ast.DropWidgetOp: + return o.Legacy + } + return "" +} + +func clearLegacy(op ast.AlterPageOperation) { + switch o := op.(type) { + case *ast.SetPropertyOp: + o.Legacy = "" + case *ast.DropWidgetOp: + o.Legacy = "" + } +} diff --git a/mdl/visitor/visitor_alter_generic_test.go b/mdl/visitor/visitor_alter_generic_test.go new file mode 100644 index 000000000..81de33731 --- /dev/null +++ b/mdl/visitor/visitor_alter_generic_test.go @@ -0,0 +1,174 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// The generic ALTER (ADR-0012 decision 2, ako/mxcli#712): one grammar rule +// `alter Module.Name { set / insert / replace / drop }` for every +// document type, with the target written in one address syntax and resolved by +// the document type. These tests pin the CANONICAL spelling and that the old +// page spellings still parse to the same AST, flagged as the alias they are. + +func buildAlterPage(t *testing.T, input string) *ast.AlterPageStmt { + t.Helper() + prog, errs := Build(input) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + if len(prog.Statements) != 1 { + t.Fatalf("want 1 statement, got %d", len(prog.Statements)) + } + stmt, ok := prog.Statements[0].(*ast.AlterPageStmt) + if !ok { + t.Fatalf("want *ast.AlterPageStmt, got %T", prog.Statements[0]) + } + return stmt +} + +func TestGenericAlter_CanonicalSetIsParenthesisedAndColon(t *testing.T) { + stmt := buildAlterPage(t, `alter page Module.Page { + set (Caption: 'Save', ButtonStyle: Success) on btnSave; + set (Title: 'Edit order'); + };`) + if len(stmt.Operations) != 2 { + t.Fatalf("want 2 operations, got %d", len(stmt.Operations)) + } + onWidget := stmt.Operations[0].(*ast.SetPropertyOp) + if onWidget.Target.Widget != "btnSave" { + t.Errorf("target: got %q", onWidget.Target.Widget) + } + if onWidget.Properties["Caption"] != "Save" || onWidget.Properties["ButtonStyle"] != "Success" { + t.Errorf("properties: got %v", onWidget.Properties) + } + if onWidget.Legacy != "" { + t.Errorf("canonical set must not be flagged as an alias, got %q", onWidget.Legacy) + } + pageLevel := stmt.Operations[1].(*ast.SetPropertyOp) + if pageLevel.Target.Widget != "" || pageLevel.Properties["Title"] != "Edit order" { + t.Errorf("page-level set: target %q, properties %v", pageLevel.Target.Widget, pageLevel.Properties) + } + if pageLevel.Legacy != "" { + t.Errorf("canonical page-level set flagged as alias: %q", pageLevel.Legacy) + } +} + +func TestGenericAlter_CanonicalDropNamesTargetsWithoutKeyword(t *testing.T) { + stmt := buildAlterPage(t, `alter snippet Module.Snip { + drop txtOld, dgOrders.Total; + };`) + if stmt.ContainerType != "SNIPPET" { + t.Errorf("container type: got %q", stmt.ContainerType) + } + drop := stmt.Operations[0].(*ast.DropWidgetOp) + if len(drop.Targets) != 2 || drop.Targets[0].Widget != "txtOld" || + drop.Targets[1].Widget != "dgOrders" || drop.Targets[1].Column != "Total" { + t.Errorf("targets: got %+v", drop.Targets) + } + if drop.Legacy != "" { + t.Errorf("canonical drop flagged as alias: %q", drop.Legacy) + } +} + +// A widget may be NAMED like a keyword the old forms use; the canonical drop +// of it must still parse as a drop of that name. +func TestGenericAlter_DropOfWidgetNamedLikeAKeyword(t *testing.T) { + stmt := buildAlterPage(t, `alter page Module.Page { drop widget; };`) + drop := stmt.Operations[0].(*ast.DropWidgetOp) + if len(drop.Targets) != 1 || drop.Targets[0].Widget != "widget" || drop.Legacy != "" { + t.Errorf("got %+v legacy=%q", drop.Targets, drop.Legacy) + } +} + +func TestGenericAlter_TargetAddressForms(t *testing.T) { + stmt := buildAlterPage(t, `alter layout Module.Lay { + insert into layoutContainer.top { snippetcall bar (Snippet: Module.Bar) } + insert after 'Approve order'@2 { textbox t1 (Label: 'x') } + replace hdr@1 with { container c1 } + };`) + if stmt.ContainerType != "LAYOUT" { + t.Errorf("container type: got %q", stmt.ContainerType) + } + into := stmt.Operations[0].(*ast.InsertWidgetOp) + if into.Position != "INTO" || into.Target.Widget != "layoutContainer" || into.Target.Column != "top" { + t.Errorf("into: %+v", into) + } + byCaption := stmt.Operations[1].(*ast.InsertWidgetOp) + if byCaption.Target.Caption != "Approve order" || byCaption.Target.Ordinal != 2 || byCaption.Target.Widget != "" { + t.Errorf("caption target: %+v", byCaption.Target) + } + repl := stmt.Operations[2].(*ast.ReplaceWidgetOp) + if repl.Target.Widget != "hdr" || repl.Target.Ordinal != 1 { + t.Errorf("ordinal target: %+v", repl.Target) + } +} + +// The old spellings are aliases: they must parse to the same operations as the +// canonical form, and record which alias was used so check/exec can warn. +func TestGenericAlter_OldSpellingsAreFlaggedAliases(t *testing.T) { + cases := []struct { + name, op string + legacy string + }{ + {"set without parentheses", `set Caption = 'Save' on btnSave`, ast.AlterAliasSetEquals}, + {"page-level set without parentheses", `set Title = 'Edit'`, ast.AlterAliasSetEquals}, + {"parenthesised set with =", `set (Caption = 'Save', ButtonStyle = Success) on btnSave`, ast.AlterAliasSetEquals}, + {"set without parentheses, with colon", `set Caption: 'Save' on btnSave`, ast.AlterAliasSetUnparenthesised}, + {"drop widget", `drop widget txtOld, txtUnused`, ast.AlterAliasDropWidget}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + stmt := buildAlterPage(t, "alter page Module.Page { "+c.op+"; };") + var got string + switch o := stmt.Operations[0].(type) { + case *ast.SetPropertyOp: + got = o.Legacy + case *ast.DropWidgetOp: + got = o.Legacy + default: + t.Fatalf("unexpected op %T", o) + } + if got != c.legacy { + t.Errorf("legacy spelling: got %q, want %q", got, c.legacy) + } + }) + } +} + +// The document-specific operations with no generic spelling yet keep their own +// form inside the generic block and are NOT aliases. +func TestGenericAlter_DocumentSpecificOperationsStillParse(t *testing.T) { + stmt := buildAlterPage(t, `alter page Module.Page { + set layout = Atlas_Core.TopBar map (Main as Content); + drop template for Module.Special in lvItems; + add variables $show: Boolean = 'true'; + drop variables $show; + };`) + if len(stmt.Operations) != 4 { + t.Fatalf("want 4 operations, got %d", len(stmt.Operations)) + } + if _, ok := stmt.Operations[0].(*ast.SetLayoutOp); !ok { + t.Errorf("op 0: %T", stmt.Operations[0]) + } + if _, ok := stmt.Operations[1].(*ast.DropListViewTemplateOp); !ok { + t.Errorf("op 1: %T", stmt.Operations[1]) + } + if _, ok := stmt.Operations[2].(*ast.AddVariableOp); !ok { + t.Errorf("op 2: %T", stmt.Operations[2]) + } + if _, ok := stmt.Operations[3].(*ast.DropVariableOp); !ok { + t.Errorf("op 3: %T", stmt.Operations[3]) + } +} + +// @0 would read as "no ordinal" and address whatever a bare name addresses. +func TestGenericAlter_OrdinalZeroIsRefused(t *testing.T) { + _, errs := Build(`alter page Module.Page { drop txtName@0; };`) + if len(errs) == 0 { + t.Fatal("want an error for @0, got none") + } +} diff --git a/mdl/visitor/visitor_alter_page.go b/mdl/visitor/visitor_alter_page.go index f8d801584..cbc79b2dc 100644 --- a/mdl/visitor/visitor_alter_page.go +++ b/mdl/visitor/visitor_alter_page.go @@ -3,45 +3,49 @@ package visitor import ( + "fmt" + "strconv" "strings" "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/grammar/parser" ) -// exitAlterPageStatement handles ALTER PAGE/SNIPPET Module.Name { operations } -func (b *Builder) exitAlterPageStatement(ctx *parser.AlterStatementContext) { +// exitAlterDocumentStatement handles the generic +// ALTER Module.Name { set / insert / replace / drop } (ADR-0012 +// decision 2). The page family — page, snippet, layout — is the first set of +// document types on it, and builds the AlterPageStmt every page validator and +// the executor already speak. +func (b *Builder) exitAlterDocumentStatement(ctx *parser.AlterStatementContext) { stmt := &ast.AlterPageStmt{} - // Container type + docType := ctx.AlterDocumentType().(*parser.AlterDocumentTypeContext) switch { - case ctx.SNIPPET() != nil: + case docType.SNIPPET() != nil: stmt.ContainerType = "SNIPPET" - case ctx.LAYOUT() != nil: + case docType.LAYOUT() != nil: stmt.ContainerType = "LAYOUT" default: stmt.ContainerType = "PAGE" } - // Page/snippet name if qn := ctx.QualifiedName(); qn != nil { stmt.PageName = buildQualifiedName(qn) } - // Parse operations - for _, opCtx := range ctx.AllAlterPageOperation() { - op := opCtx.(*parser.AlterPageOperationContext) + for _, opCtx := range ctx.AllAlterOperation() { + op := opCtx.(*parser.AlterOperationContext) - if setCtx := op.AlterPageSet(); setCtx != nil { - stmt.Operations = append(stmt.Operations, b.buildAlterPageSet(setCtx.(*parser.AlterPageSetContext))) - } else if insertCtx := op.AlterPageInsert(); insertCtx != nil { - stmt.Operations = append(stmt.Operations, b.buildAlterPageInsert(insertCtx.(*parser.AlterPageInsertContext))) - } else if dropCtx := op.AlterPageDrop(); dropCtx != nil { - stmt.Operations = append(stmt.Operations, b.buildAlterPageDrop(dropCtx.(*parser.AlterPageDropContext))) + if setCtx := op.AlterSet(); setCtx != nil { + stmt.Operations = append(stmt.Operations, b.buildAlterSet(setCtx.(*parser.AlterSetContext))) + } else if insertCtx := op.AlterInsert(); insertCtx != nil { + stmt.Operations = append(stmt.Operations, b.buildAlterInsert(insertCtx.(*parser.AlterInsertContext))) + } else if dropCtx := op.AlterDrop(); dropCtx != nil { + stmt.Operations = append(stmt.Operations, b.buildAlterDrop(dropCtx.(*parser.AlterDropContext))) } else if dropTplCtx := op.AlterPageDropTemplate(); dropTplCtx != nil { stmt.Operations = append(stmt.Operations, b.buildAlterPageDropTemplate(dropTplCtx.(*parser.AlterPageDropTemplateContext))) - } else if replaceCtx := op.AlterPageReplace(); replaceCtx != nil { - stmt.Operations = append(stmt.Operations, b.buildAlterPageReplace(replaceCtx.(*parser.AlterPageReplaceContext))) + } else if replaceCtx := op.AlterReplace(); replaceCtx != nil { + stmt.Operations = append(stmt.Operations, b.buildAlterReplace(replaceCtx.(*parser.AlterReplaceContext))) } else if addVarCtx := op.AlterPageAddVariable(); addVarCtx != nil { stmt.Operations = append(stmt.Operations, b.buildAlterPageAddVariable(addVarCtx.(*parser.AlterPageAddVariableContext))) } else if dropVarCtx := op.AlterPageDropVariable(); dropVarCtx != nil { @@ -59,44 +63,55 @@ func (b *Builder) buildAlterPageDropTemplate(ctx *parser.AlterPageDropTemplateCo if qn := ctx.QualifiedName(); qn != nil { op.Specialization = qn.GetText() } - if wr := ctx.WidgetRef(); wr != nil { - op.ListView = buildWidgetRef(wr).Widget + if tr := ctx.AlterTarget(); tr != nil { + op.ListView = b.buildAlterTarget(tr).Widget } return op } -// buildAlterPageSet builds a SetPropertyOp or SetLayoutOp from the parse tree. -func (b *Builder) buildAlterPageSet(ctx *parser.AlterPageSetContext) ast.AlterPageOperation { +// buildAlterSet builds a SetPropertyOp or SetLayoutOp from the parse tree. +func (b *Builder) buildAlterSet(ctx *parser.AlterSetContext) ast.AlterPageOperation { // SET Layout = Module.LayoutName [MAP (...)] if ctx.LAYOUT() != nil { - return b.buildAlterPageSetLayout(ctx) + return b.buildAlterSetLayout(ctx) } op := &ast.SetPropertyOp{ Properties: make(map[string]interface{}), } - // Widget ref (if ON widgetRef is present) if ctx.ON() != nil { - if wr := ctx.WidgetRef(); wr != nil { - op.Target = buildWidgetRef(wr) + if tr := ctx.AlterTarget(); tr != nil { + op.Target = b.buildAlterTarget(tr) } } - // Parse assignments + usedEquals := false for _, assignCtx := range ctx.AllAlterPageAssignment() { assign := assignCtx.(*parser.AlterPageAssignmentContext) + if ao := assign.AlterAssignOp(); ao != nil && ao.(*parser.AlterAssignOpContext).EQUALS() != nil { + usedEquals = true + } name, value := b.buildAlterPageAssignment(assign) if name != "" { op.Properties[name] = value } } + // Which alias, if any. `=` is reported first: its rewrite — the + // parenthesised, colon form — also fixes a missing parenthesis. + switch { + case usedEquals: + op.Legacy = ast.AlterAliasSetEquals + case ctx.LPAREN() == nil: + op.Legacy = ast.AlterAliasSetUnparenthesised + } + return op } -// buildAlterPageSetLayout builds a SetLayoutOp from: SET Layout = QN [MAP (old -> new, ...)] -func (b *Builder) buildAlterPageSetLayout(ctx *parser.AlterPageSetContext) *ast.SetLayoutOp { +// buildAlterSetLayout builds a SetLayoutOp from: SET Layout = QN [MAP (old -> new, ...)] +func (b *Builder) buildAlterSetLayout(ctx *parser.AlterSetContext) *ast.SetLayoutOp { op := &ast.SetLayoutOp{} // Layout qualified name @@ -187,8 +202,8 @@ func (b *Builder) buildAlterPageAssignment(ctx *parser.AlterPageAssignmentContex return name, value } -// buildAlterPageInsert builds an InsertWidgetOp from the parse tree. -func (b *Builder) buildAlterPageInsert(ctx *parser.AlterPageInsertContext) *ast.InsertWidgetOp { +// buildAlterInsert builds an InsertWidgetOp from the parse tree. +func (b *Builder) buildAlterInsert(ctx *parser.AlterInsertContext) *ast.InsertWidgetOp { op := &ast.InsertWidgetOp{} if ctx.AFTER() != nil { @@ -199,58 +214,78 @@ func (b *Builder) buildAlterPageInsert(ctx *parser.AlterPageInsertContext) *ast. op.Position = "INTO" } - if wr := ctx.WidgetRef(); wr != nil { - op.Target = buildWidgetRef(wr) - } - - if body := ctx.PageBodyV3(); body != nil { - op.Widgets = buildPageBodyV3(body, b) + if tr := ctx.AlterTarget(); tr != nil { + op.Target = b.buildAlterTarget(tr) } + op.Widgets = b.buildAlterFragment(ctx.AlterFragment()) return op } -// buildAlterPageDrop builds a DropWidgetOp from the parse tree. -func (b *Builder) buildAlterPageDrop(ctx *parser.AlterPageDropContext) *ast.DropWidgetOp { +// buildAlterDrop builds a DropWidgetOp from the parse tree. +func (b *Builder) buildAlterDrop(ctx *parser.AlterDropContext) *ast.DropWidgetOp { op := &ast.DropWidgetOp{} - - for _, wr := range ctx.AllWidgetRef() { - op.Targets = append(op.Targets, buildWidgetRef(wr)) + if ctx.WIDGET() != nil { + op.Legacy = ast.AlterAliasDropWidget + } + for _, tr := range ctx.AllAlterTarget() { + op.Targets = append(op.Targets, b.buildAlterTarget(tr)) } - return op } -// buildAlterPageReplace builds a ReplaceWidgetOp from the parse tree. -func (b *Builder) buildAlterPageReplace(ctx *parser.AlterPageReplaceContext) *ast.ReplaceWidgetOp { +// buildAlterReplace builds a ReplaceWidgetOp from the parse tree. +func (b *Builder) buildAlterReplace(ctx *parser.AlterReplaceContext) *ast.ReplaceWidgetOp { op := &ast.ReplaceWidgetOp{} - if wr := ctx.WidgetRef(); wr != nil { - op.Target = buildWidgetRef(wr) - } - - if body := ctx.PageBodyV3(); body != nil { - op.NewWidgets = buildPageBodyV3(body, b) + if tr := ctx.AlterTarget(); tr != nil { + op.Target = b.buildAlterTarget(tr) } + op.NewWidgets = b.buildAlterFragment(ctx.AlterFragment()) return op } -// buildWidgetRef extracts a WidgetRef from a widgetRef grammar context. -// Supports both plain "btnSave" and dotted "dgProducts.Name" references. -func buildWidgetRef(ctx parser.IWidgetRefContext) ast.WidgetRef { - wrCtx := ctx.(*parser.WidgetRefContext) - ids := wrCtx.AllIdentifierOrKeyword() - if len(ids) == 2 { - return ast.WidgetRef{ - Widget: identifierOrKeywordText(ids[0]), - Column: identifierOrKeywordText(ids[1]), +// buildAlterFragment builds the widgets of an INSERT / REPLACE fragment, which +// is written exactly as CREATE PAGE writes its body. +func (b *Builder) buildAlterFragment(ctx parser.IAlterFragmentContext) []*ast.WidgetV3 { + if ctx == nil { + return nil + } + if body := ctx.(*parser.AlterFragmentContext).PageBodyV3(); body != nil { + return buildPageBodyV3(body, b) + } + return nil +} + +// buildAlterTarget extracts the generic ALTER address — a name, a dotted +// name.member, or a quoted caption, each with an optional @n. What an address +// means is the document type's call (backend.AlterTargetResolver); this only +// records what was written. +func (b *Builder) buildAlterTarget(ctx parser.IAlterTargetContext) ast.WidgetRef { + tc := ctx.(*parser.AlterTargetContext) + var ref ast.WidgetRef + if n := tc.NUMBER_LITERAL(); n != nil { + // @n counts matches from 1, as the ambiguity error lists them. + if v, err := strconv.Atoi(n.GetText()); err == nil && v >= 1 { + ref.Ordinal = v + } else { + b.addError(fmt.Errorf("alter target %s: @%s is not a match number — matches are counted from @1", + tc.GetText(), n.GetText())) } } - if len(ids) == 1 { - return ast.WidgetRef{Widget: identifierOrKeywordText(ids[0])} + if sl := tc.STRING_LITERAL(); sl != nil { + ref.Caption = unquoteString(sl.GetText()) + return ref + } + ids := tc.AllIdentifierOrKeyword() + if len(ids) >= 1 { + ref.Widget = identifierOrKeywordText(ids[0]) + } + if len(ids) == 2 { + ref.Column = identifierOrKeywordText(ids[1]) } - return ast.WidgetRef{} + return ref } // buildAlterPageAddVariable builds an AddVariableOp from the parse tree.