fix: support log node expressions and harden microflow describe roundtrip#264
Conversation
# Conflicts: # mdl/grammar/parser/MDLParser.interp
Code Review — fix: support log node expressions and harden microflow describe roundtripOverviewThree fixes bundled together: (1) legacy Scope concernCLAUDE.md requires each PR to cover a single concern. This is three separate fixes that happen to touch overlapping files. The description acknowledges this ("Three microflow round-trip fixes that surfaced together"). Practically speaking the overlap in Fix 1:
|
- CHANGELOG [Unreleased] Changed: document mdlQuote/unquoteString escape-
sequence behaviour change (now treats \n \r \t \\ as escapes instead of
literal pairs). Compatibility break flagged for users who intentionally
embedded raw backslash-letter sequences in MDL string literals.
- Add comment in expressionToString noting the dual use of mdlQuote for
MDL source output vs Mendix expression strings, and the describe→re-execute
trade-off it implies.
- Extract defaultLogNodeExpression constant ('Application') so the builder,
the formatter, and cmd_diff_mdl share a single source of truth for the
LOG node fallback.
- Add mdl-examples/bug-tests/264-log-node-expression-roundtrip.mdl
reproducer per CLAUDE.md checklist.
hjotha
force-pushed
the
submit/microflow-roundtrip-log-node
branch
from
497867f to
494900a
Compare
|
Thanks @ako — addressed in 494900a:
Re: the scope concern — fair flag. The three fixes did genuinely surface from the same end-to-end exercise and splitting them after the fact would have required grammar regeneration in a specific order; agree on the principle for future PRs in this umbrella. |
- CHANGELOG [Unreleased] Changed: document mdlQuote/unquoteString escape-
sequence behaviour change (now treats \n \r \t \\ as escapes instead of
literal pairs). Compatibility break flagged for users who intentionally
embedded raw backslash-letter sequences in MDL string literals.
- Add comment in expressionToString noting the dual use of mdlQuote for
MDL source output vs Mendix expression strings, and the describe→re-execute
trade-off it implies.
- Extract defaultLogNodeExpression constant ('Application') so the builder,
the formatter, and cmd_diff_mdl share a single source of truth for the
LOG node fallback.
- Add mdl-examples/bug-tests/264-log-node-expression-roundtrip.mdl
reproducer per CLAUDE.md checklist.
hjotha
force-pushed
the
submit/microflow-roundtrip-log-node
branch
from
494900a to
6ac56fa
Compare
|
CI was red on 6ac56fa because Fixed in the same commit — changed the assertion to the lowercase form, matching the other |
Older Mendix project versions store a sequence flow's branch value in a single inline `NewCaseValue` document, while newer versions use the `CaseValues` array ([marker, case_object]). `parseSequenceFlow` only handled the new form, so opening a legacy MPR lost all true/false labels on decisions and every subsequent describe -> exec cycle rewrote them as default-case flows. Fall back to `parseCaseValue` on `NewCaseValue` when `CaseValues` is absent. Regression tests (parser_microflow_test.go) construct both shapes and assert the CaseValue makes it through parsing.
MDL text produced by `describe microflow` was failing to re-parse on several shapes: multiline captions/messages lost their newlines, tabs were silently dropped when unquoting strings, and loop positions anchored with `@position` drifted by half a loop width per roundtrip. Centralise string escaping in an mdlQuote helper that doubles single quotes and encodes `\n`, `\r`, `\t`, and `\\`. Switch every call site (`cmd_microflows_show.go`, `cmd_microflows_show_helpers.go`, `cmd_microflows_format_action.go`, `cmd_microflows_helpers.go`, `cmd_pages_describe_output.go`) to use it. Mirror the encoding in the visitor by rewriting `unquoteString` to walk the input and decode each escape explicitly instead of doing naive sequential `ReplaceAll` calls, which mangled `\\n` and similar sequences. Loops now compute `loopLeftX`/`loopCenterX` up front and use the annotated `@position` when set, then advance `fb.posX` past the actual left edge of the loop instead of the stale pre-annotation position. `addWhileStatement` gets the same treatment. Regression tests cover: - `emitObjectAnnotations` emitting escaped `@caption` / `@annotation` - `formatAction` for `ShowMessage` and `LogMessage` with multiline text - `unquoteString` decoding `\n`, `\t`, `\\`, and doubled quotes - `addLoopStatement` / `addWhileStatement` honouring `@position` and leaving `fb.posX` past the loop's right edge
The LOG statement's optional NODE clause only accepted a bare
`STRING_LITERAL` (e.g. `LOG INFO NODE 'MyModule' 'msg'`), but Studio
Pro persists projects where the node is any expression — a constant
reference like `@MyModule.SecurityLogNode`, a variable, or a function
call. Describing those microflows emitted MDL the parser rejected with
`mismatched input '$' expecting STRING_LITERAL`, and
`addLogMessageAction` was wrapping the already-quoted node in an extra
pair of quotes whenever the input was a plain string.
Widen the grammar and AST:
logStatement
: LOG logLevel? (NODE expression)? expression logTemplateParams?
;
LogStmt.Node: Expression // was string
The visitor now collects all child expressions and assigns the first
to `Node` / second to `Message` when `NODE` is present; the builder
formats `s.Node` through `exprToString` and defaults to `'Application'`
when absent. Describer and diff emitter pass the node through the same
expression formatter, and the format casing is lowercased to match the
rest of the MDL output.
`formatAction` for `LogMessageAction` keeps raw template text on the
wire so Mendix-style `{1}` placeholders survive when WITH params are
present; without that, the literal got re-quoted into `'''Order {1}'''`
on every roundtrip.
Regression tests cover:
- `log info node @Module.Constant 'msg'` roundtrip through
create -> describe -> re-exec
- template LOG statements: `log info node 'App' 'Order {1}' with ...`
- visitor builds `ConstantRefExpr` for `@Module.Constant` node
- `formatAction` multiline escaping matches the new lowercased form
- CHANGELOG [Unreleased] Changed: document mdlQuote/unquoteString escape-
sequence behaviour change (now treats \n \r \t \\ as escapes instead of
literal pairs). Compatibility break flagged for users who intentionally
embedded raw backslash-letter sequences in MDL string literals.
- Add comment in expressionToString noting the dual use of mdlQuote for
MDL source output vs Mendix expression strings, and the describe→re-execute
trade-off it implies.
- Extract defaultLogNodeExpression constant ('Application') so the builder,
the formatter, and cmd_diff_mdl share a single source of truth for the
LOG node fallback.
- Add mdl-examples/bug-tests/264-log-node-expression-roundtrip.mdl
reproducer per CLAUDE.md checklist.
hjotha
force-pushed
the
submit/microflow-roundtrip-log-node
branch
from
6ac56fa to
16515e1
Compare
AI Code ReviewWhat Looks Good
RecommendationApprove the PR. The changes are well-scoped, thoroughly tested, and maintain full-stack consistency for the MDL LOG statement enhancement while fixing critical roundtrip regressions. The MDL syntax modification is minimal, justified, and follows established patterns. Automated review via OpenRouter (Nemotron Super 120B) — workflow source |
3 participants
Summary
Three microflow round-trip fixes that surfaced together while exercising real
.mprfiles:parse legacy NewCaseValue sequence flows— older.mprfiles (Mendix 9.x) emitNewCaseValuefor the true/false labels on exclusive split outgoing sequence flows. The parser dropped them, so the round-tripped describer lost the case value and Studio Pro couldn't reconstruct the split.harden microflow describe roundtrip— the describer bailed or emitted malformed MDL when it hit certain activity shapes it only partially understood. Fix makes the describer skip-and-preserve unknown subfields so the BSON survives write-back unchanged.support log node expressions in microflow roundtrip— the MDL grammar didn't accept thelogactivity's expression/category shape, so round-trips that included a log node failed to re-parse. Grammar + visitor + describer updated end-to-end.Part of umbrella #257.
Test plan
go build ./... && go test ./...pass.