diff --git a/.claude/skills/fix-issue/findings/mdl-backend/2026-10-21-note-only-splice-shifts-the-row.json b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-21-note-only-splice-shifts-the-row.json new file mode 100644 index 0000000000..97a91eb533 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-backend/2026-10-21-note-only-splice-shifts-the-row.json @@ -0,0 +1 @@ +{"area":"mdl/backend","date":"2026-10-21","symptom":"Adding one `@annotation(text: 'note D', position: (1500, 600), size: (200, 80))` to a described flow and re-running it as create or modify stored the note at x 3390 instead of 1500 and shifted the three later activities by +1180px; with size-only notes every activity after the first noted one shifted by +140..+280px. mx check: 0 errors. With @position on every statement nothing moved.","cause":"Plain describe omits positions, so the re-noted statement is not declaredMatches and becomes a 1:1 Replace whose fragment is unplaced. fragmentGeometry boxed every fragment object, notes included: activity at 300..420 and note at 1400..1600 gave a 1300px box, Replace's `extra = length - size` shifted everything past the cut by 1180, and placeCentred translated the note with the box (1500 + 1890 = 3390). A size-only note is centred on its activity, so a 260/400px note made extra 140/280.","file":"mdl/backend/mfmutator/splice.go (fragmentGeometry, placeCentred), mdl/backend/microflow_mutation.go (MicroflowFragment.Pinned), mdl/executor/cmd_microflows_builder_annotations.go (recordStatedNote), cmd_alter_flow.go (buildFragment), cmd_microflows_builder_flows.go (handler merge-back)","insight":"The agent's arithmetic from source reproduced every reported number (1180, 3390), and the unit test then reproduced them in the mutator (b 420 -> 1600) before the fix — so trust the arithmetic, but run it. A note sits off its line and takes no room on it: leave notes out of the box the splice makes room for, translate an unplaced note with its activity, and leave a note with a stated (absolute) position where the script put it. Loop-body notes need no pin: they are children of the loop and move with its box. Still open: a note-only change is a Replace at all, so the activity gets a new $ID; an AddNotes op would keep it.","refs":["ako/mxcli#1508","mendixlabs/mxcli#1598"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-case-outcome-sides-not-authorable.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-case-outcome-sides-not-authorable.json new file mode 100644 index 0000000000..5c90d4d7a4 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-case-outcome-sides-not-authorable.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-21","symptom":"A CASE stored each outcome flow's OriginConnectionIndex by WHEN order only (1st 0/top, 2nd 1/right, 3rd 2/bottom; 4+ grouped thirds). `@anchor(A: (from: right))` was refused with MDL092, `@anchor(from: right)` on the CASE and `true:`/`false:` were accepted and silently ignored, and describe never emitted a CASE's outcome sides, so a Studio Pro-drawn CASE lost them on describe -> exec.","cause":"parseAnchorAnnotation knew only from/to/true/false/iterator/tail keys and annotationParamName could not spell `(empty)` or a keyword value; addEnumSplit consumed the CASE's annotations without reading any anchor and never set nextFlowAnchor; emitSplitAnchorAnnotation only looked for true/false flows and mergeExitAnchor returned \"\" for a non-Boolean split","file":"mdl/grammar/domains/MDLSettings.g4 (annotationParamName), mdl/visitor/visitor_microflow_statements.go (parseAnchorAnnotation), mdl/ast/ast_microflow.go (CaseAnchors), mdl/executor/cmd_microflows_builder_actions.go (splitCaseSlot.applyCaseAnchor, addEnumSplit), validate_microflow.go (checkCaseAnchors), cmd_microflows_show_helpers.go (enumCaseAnchorFragments, mergeExitAnchor)","insight":"Do NOT make the default follow geometry for up to three cases, tempting as the data is (Studio Pro: straight below -> bottom 467/467, level right -> right 269/286): describe reads the WHEN order back from those sides (splitCaseOrder), and TestEnumSplitRoundtripPreservesEmptyCaseBeforeNamedCases fails the moment a branch placed out of order changes side. The per-outcome form is the escape hatch, and describe writes an entry only where the stored side differs from what the builder computes for that branch (same slot.apply, same position in describe order), so an ordinary CASE stays annotation-free. `to:` goes in the CASE entry only for a flow into a merge; into an activity it is that activity's own @anchor(to:). A bare `name: (…)` on an IF now parses as an outcome key and is refused by validation instead of the visitor. Over --mcp the create path sends no connection sides for ANY flow, so Studio Pro shows its right->left default there; verify sides on the modelsdk engine.","refs":["ako/mxcli#1492","mendixlabs/mxcli#1587"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-error-handler-row-inside-output-label.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-error-handler-row-inside-output-label.json new file mode 100644 index 0000000000..1fcb61bacc --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-error-handler-row-inside-output-label.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-21","symptom":"An error handler's row was centred 90px under its activity (retrieve at 360;200, handler activity at 360;290): a 30px gap, inside the output variable and type Studio Pro prints under the box. A handler in a THEN branch also overlapped the ELSE row (+90 against +100). mx check: 0 errors.","cause":"addErrorHandlerFlow (and the web-service call and the pending-handler terminal end event) used VerticalSpacing, a constant shared with type-split branches and RowGap; measureStatement gave an activity with a handler body the height of a bare activity, so the IF placed its ELSE row as if the handler row did not exist","file":"mdl/executor/layout.go (ErrorHandlerPitch, measureStatement), cmd_microflows_builder_flows.go (addErrorHandlerFlow), cmd_microflows_builder_calls.go, cmd_microflows_builder_graph.go","insight":"The issue's ~140px was an estimate; Studio Pro's own documents settle it: 160 distinct handler rows straight under their activity (ako/TestApp, PedApp, seven .mpk apps) have a median offset of 130px, quartiles 125..150, minimum 90. IF branch rows measured the same way sit a median 134px under the split, but mxcli's +100 else row already leaves 40px, clear of the ~35px label, so only the handler pitch changed; give it its own constant rather than raising VerticalSpacing, which also drives RowGap and type-split branches. Write the measured number into the test literally: a test comparing against the builder's own constant passed at 90 and proved nothing.","refs":["ako/mxcli#1508","mendixlabs/mxcli#1598"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-loop-tail-if-gets-dead-end-merge.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-loop-tail-if-gets-dead-end-merge.json new file mode 100644 index 0000000000..5b0b5e1254 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-loop-tail-if-gets-dead-end-merge.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-21","symptom":"An IF/ELSE as the last statement of a loop or while body got an ExclusiveMerge with no outgoing flow: both branches end the iteration, so it joined nothing. Studio Pro authors delete it by hand (the reporter's hand-polished flow had 2 merges, mxcli's rebuild of the same MDL 4), and since describe printed the same MDL with and without it, every re-exec drew it again.","cause":"addIfStatement decided needMerge from returns alone; the loop-body loops in addLoopStatement/addWhileStatement never told a statement it was last, so nothing knew the IF's continuation was the end of the iteration","file":"mdl/executor/cmd_microflows_builder_control.go (atBodyTail, addIfStatement tailWithoutMerge, both loop-body loops), cmd_microflows_builder.go (flowBuilder.atBodyTail), cmd_microflows_show_helpers.go (emitMergeAnnotation)","insight":"The claim the fix rests on — Mendix accepts two dead-end branches at a loop tail — was run, not assumed: PedApp 11.13 with the shape built gives the identical mx check error set to the pristine copy, and Studio Pro 11.15's ped_check_errors reports none. Keep the merge when either branch is empty (a split needs a flow per case: CE0079), when a branch ends in a continuing custom error handler, and when @merge is written; pass the tail on to the last statement of each branch so a nested IF/ELSE at the tail loses its merge too. The read side is the trap: a stored tail merge (an earlier mxcli's) sits at the OLD derived position, so the canonical describe treated its @merge as derivable and dropped it, and the rebuild then removed the node — emitMergeAnnotation now always states @merge for a split merge with no way out. assertDescriptionRebuildsGraph compares graphs modulo merges and cannot see any of this; count merges.","refs":["ako/mxcli#1273","mendixlabs/mxcli#884"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-merge-after-loop-in-branch-inside-loop-box.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-merge-after-loop-in-branch-inside-loop-box.json new file mode 100644 index 0000000000..b7c4b5aae6 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-merge-after-loop-in-branch-inside-loop-box.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-21","symptom":"The merge after an IF whose branch holds a foreach LOOP landed inside the loop's box: LoopedActivity 950;200 320x160 spans x 790..1110, ExclusiveMerge 1070;200 spans 1050..1090, so the loop's outgoing line ran right to left into it. mx check: 0 errors.","cause":"measureLoopStatement sized a loop as body + 2*LoopPadding while addLoopStatement builds body + 2*LoopPadding + iteratorSpace (a local 100); the IF's mergeX comes from the measured branch width, so it fell 100px short for every foreach loop at every nesting level. WHILE has no iterator column and no mismatch.","file":"mdl/executor/layout.go (LoopIteratorSpace, measureLoopStatement), mdl/executor/cmd_microflows_builder_control.go (addLoopStatement)","insight":"The issue's cause was right in effect but not in mechanism: the builder's `+iteratorSpace` width is overwritten by fitContainerSize after the body is built, so the column reaches the final width through innerStartX instead. The measurer still has to include it, because the IF reads the measured width before the loop exists. A centre-to-centre layout invariant (merge.X >= loop.X) passed throughout, so the test compares box EDGES (merge.X - MergeSize/2 >= loop.X + Width/2); TestLayoutInvariants_NoBackwardMergeAndEndRightmost treats a LoopedActivity as ActivityWidth wide and could not see it.","refs":["ako/mxcli#1508","mendixlabs/mxcli#1598"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-note-line-always-leaves-note-bottom.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-note-line-always-leaves-note-bottom.json new file mode 100644 index 0000000000..5738963ba8 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-note-line-always-leaves-note-bottom.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-21","symptom":"Every Microflows$AnnotationFlow was written OriginConnectionIndex 2 -> DestinationConnectionIndex 0 (note bottom -> element top) wherever the note sat, so a note placed BELOW its activity drew its line out of its own bottom edge and round both boxes. The reader dropped the stored sides too, so a rewrite of a Studio Pro-drawn note below its activity flipped it to 2 -> 0.","cause":"annotationFlowToGen hardcoded SetOriginConnectionIndex(2)/SetDestinationConnectionIndex(0), and microflows.AnnotationFlow had no field for the sides, so neither the builder nor the reader could say otherwise","file":"mdl/executor/cmd_microflows_builder_annotations.go (orientAnnotationFlows, facingSides), mdl/backend/modelsdk/microflow_write.go (annotationFlowToGen), mdl/backend/modelsdk/microflow.go (annotationFlowsFromGen), sdk/microflows/microflows.go (AnnotationFlow.Sides)","insight":"The issue proposed a separate note numbering (top 0, bottom 1, left 2, right 3); measuring Studio Pro's own documents refuted it in one pass: 891 AnnotationFlows from ako/TestApp, PedApp and the mx-test-projects .mpk apps (523 distinct). A note level with its activity on the left stores 1 -> 3 and on the right 3 -> 1, which only the activity numbering (0 top, 1 right, 2 bottom, 3 left) explains; a note below stores 0 -> 2 (58 of 67). The element side follows 'vertically clear -> top/bottom, else left/right' in 98%; the NOTE side is not predictable from geometry (Studio Pro often stores 1 -> 0 for a note above, presumably from where the author dragged), so do not chase the origin past 'the facing side'. Sides are chosen after separateRows, because notes are placed before rows and containers settle, and loop-body positions are relative to the loop's top-left, so the comparison is done in absolute coordinates.","refs":["ako/mxcli#1508","mendixlabs/mxcli#1598"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-on-error-rejoin-merge-lands-on-end-event.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-on-error-rejoin-merge-lands-on-end-event.json new file mode 100644 index 0000000000..a6770af673 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-on-error-rejoin-merge-lands-on-end-event.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-21","symptom":"The merge an ON ERROR handler falls through to was stored on top of the next node: `@position(240,200) call … on error without rollback {…}; @position(560,200) return;` gave ExclusiveMerge 560;200 40x40 and EndEvent 560;200 20x20. With an activity after the guarded one, the merge landed RIGHT of that activity and the edge into it ran backwards. Moving the RETURN moved both.","cause":"addErrorHandlerRejoinFlowForState (and the empty-handler variant, and mergeOverConnectedEndEvents) placed the merge at fb.posX - HorizontalSpacing/2, Y fb.baseY. The rejoin is wired after addStatement(next), when the cursor has already moved past the successor (an end event advances it by spacing/2, an activity by a full spacing), and baseY is never updated inside an IF branch","file":"mdl/executor/cmd_microflows_builder_flows.go (mergeOnEdge, objectByID, the two rejoin sites, mergeOverConnectedEndEvents)","insight":"The issue's 'make @merge work on ON ERROR' half was already done (#750: finishCustomErrorHandler reads @merge into rejoinMergeAt); the real defect was the default. Place the merge from the two nodes it sits between — midway between the origin's right edge and the destination's left edge, on the destination's line — and keep the cursor formula only for an edge that does not run left to right (a row wrap). With default spacing the 40px merge exactly fills the 40px gap between two activities: no overlap, no daylight; widening the gap would shift every downstream node and needs the measurer to agree. CALL WEB SERVICE still bypasses finishCustomErrorHandler, so @merge is ignored there. Verified on PedApp 11.13.0: main 560;200 (on the end event), fixed 425;200.","refs":["ako/mxcli#1273","mendixlabs/mxcli#884"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-single-inbound-merge-not-represented-by-describe.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-single-inbound-merge-not-represented-by-describe.json new file mode 100644 index 0000000000..109271cd9d --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-21-single-inbound-merge-not-represented-by-describe.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-21","symptom":"`join finish; merge finish;` (a merge only one path enters) was written by exec and then described as nothing, with '-- WARNING: the merge at (1330, 350) is not represented in this description ... re-executing this MDL DELETES it' (mendixlabs/mxcli#923 MF_RT_Chain). The warning had also stopped being true: `create or modify` from the description reports Unchanged and keeps the merge.","cause":"traverseFlow walks through any unlabelled merge that no split pairs with, so a one-input merge left no trace in the text; droppedMergeWarnings then flagged it with wording from before modify-in-place compared descriptions instead of rebuilding","file":"mdl/executor/cmd_microflows_show_helpers.go (traverseFlow pass-through), mdl/executor/cmd_microflows_show_merge.go (inlineMerges, droppedMergeWarningsIn), mdl/executor/cmd_microflows_derived_layout.go (memo mergeLabels)","insight":"Only the MAIN LINE (indent 0) can spell it back: there a `merge` after a `join` has no open path, so the builder draws exactly one edge into it. In a branch body the branch loop would add its own fall-through edge (no pathOpen guard), and MDL-FLOW04 bans labels in loop bodies, so those keep the (reworded) warning. Labels are minted lazily in walk order (junction1, …) so unused candidates leave no gaps. Two traps: assertDescriptionRebuildsGraph decided 'is this a whole statement?' with strings.Contains(src, \"create \"), which a warning mentioning `create or modify` defeats (fixed to a line check); and the graph comparison is merge-blind, so a merge-count assertion is needed to see the fix at all. Verified on PedApp 11.13.0: main binary 1 warning, fixed 0; describe -> exec Unchanged as modify AND as replace; 3 merges before and after.","refs":["ako/mxcli#1277","mendixlabs/mxcli#923"]} diff --git a/.claude/skills/fix-issue/findings/mdl-other/2026-10-21-while-true-early-returns-flagged-recombinable.json b/.claude/skills/fix-issue/findings/mdl-other/2026-10-21-while-true-early-returns-flagged-recombinable.json new file mode 100644 index 0000000000..e392e3db7a --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-other/2026-10-21-while-true-early-returns-flagged-recombinable.json @@ -0,0 +1 @@ +{"area":"mdl/microflowgraph","date":"2026-10-21","symptom":"`while true begin if $C = 'a' then return 'A'; end if; … set $S = 'x'; end while;` was described with one '-- WARNING: … NOT equivalent to the microflow and must not be re-executed over it (mxcli #923, recombinable)' line per decision, and lint raised MDL-FLOW01 as often ('overlap 1 node(s)'), although describe -> exec reported Unchanged and mx check gave 0 errors.","cause":"classify measured each branch body with reachableBefore(branch, join), stopping only at the join and the virtual exit. Each early return is its own end event, so the join is the exit; the false branch walked set -> header merge -> the decision itself -> its true branch's end event, which then counted as overlap.","file":"mdl/microflowgraph/structure.go (classify, reachableBefore)","insight":"The reporter's diagnosis (back edge, structure.go) was exact; the minimal fix is to stop each branch walk at its own split — a branch that re-enters its split has gone round a loop. Stripping back edges via dominators would also work but changes the post-dominator input for every consumer (describe warnings, lint, labelCrossedMerges, normalizeCollection). Keep the crossing-diamond-inside-a-loop control: it must stay Interleaved, and it is the case a cruder 'ignore nodes that reach the split' filter would hide, since inside a loop every body node reaches the split.","refs":["ako/mxcli#1277","mendixlabs/mxcli#923"]} diff --git a/.claude/skills/fix-issue/findings/sdk/2026-10-21-parameter-row-fixed-pitch-overlaps-long-names.json b/.claude/skills/fix-issue/findings/sdk/2026-10-21-parameter-row-fixed-pitch-overlaps-long-names.json new file mode 100644 index 0000000000..91f9b9bed5 --- /dev/null +++ b/.claude/skills/fix-issue/findings/sdk/2026-10-21-parameter-row-fixed-pitch-overlaps-long-names.json @@ -0,0 +1 @@ +{"area":"sdk/microflows","date":"2026-10-21","symptom":"Parameters without @position were written at 200;53, 300;53, 400;53 whatever their names, so LongParameterNameOne/Two/Three (20-22 characters) overlapped on the canvas. describe prints no parameter positions, so nothing headless showed it.","cause":"DerivedParameterPosition(idx) returned 200 + idx*100; the MCP backend repeated the same arithmetic inline","file":"sdk/microflows/microflows.go (ParameterPitch, DerivedParameterPosition, AuthoredParameterPosition, legacyParameterPosition), mdl/backend/modelsdk/microflow.go (splitFlowObjects, now two-pass), microflow_write.go, nanoflow_write.go, rule_write.go, mdl/backend/mcp/microflow.go, mdl/executor parameter sites","insight":"The pitch now depends on every name, so the reader must read all parameters before deciding which positions are authored (splitFlowObjects is two-pass). The trap the reporter flagged is real: AuthoredParameterPosition compares against the derived point, so without also accepting the OLD 100px row every long-named parameter an earlier mxcli wrote would read as hand-placed and stay overlapped. Fitted to 95 Studio Pro-authored parameter rows (median pitch 95/106/125/160px for names of 5-9/10-14/15-19/20-24 characters): ~7px a character, rounded up to 10, floor 100 — names under 15 characters keep the old row, so ordinary flows do not move. 73% of 1,109 multi-parameter Studio Pro flows stack parameters in a COLUMN (median pitch 95px) instead; switching to that would move every generated flow's parameters and was not done.","refs":["ako/mxcli#1508","mendixlabs/mxcli#1598"]} diff --git a/.claude/skills/mendix/write-microflows/reference/control-flow.md b/.claude/skills/mendix/write-microflows/reference/control-flow.md index 88be47f331..fa85345570 100644 --- a/.claude/skills/mendix/write-microflows/reference/control-flow.md +++ b/.claude/skills/mendix/write-microflows/reference/control-flow.md @@ -330,6 +330,8 @@ commit $Product; - **Leave `@position` out unless you are reproducing a hand-made diagram.** Without it the builder lays the flow out itself: the main line wraps onto rows past two canvas widths, a guard's branch drops into the lane below while the main line carries on above it, and a `case` of four or more branches leaves the decision in three groups so its lines do not cross. A statement with `@position` is never moved and is not measured against what is placed around it, so a few hand-placed statements in an otherwise automatic flow are what produces overlaps (mendixlabs/mxcli#1154) - `@position` always appears in DESCRIBE output; `@caption` only when custom; `@color` only when not Default - DESCRIBE MICROFLOW shows `@` annotations before their activities +- `@anchor(from: X, to: Y)` names two different flows: `from:` is the side the flow **leaving** the statement starts from, `to:` the side the flow **arriving** at it enters by. An annotation never reaches the far end of its own outgoing flow — that end is the next statement's `to:`. A straight vertical edge from A down to B is `@anchor(from: bottom)` on A **and** `@anchor(to: top)` on B; `@anchor(from: bottom, to: top)` on A alone bends both of A's edges and leaves B entered on the left (ako/mxcli#1279). A `return`'s `from:` does nothing (an end event has no outgoing flow). +- On a `case`, `@anchor(Open: (from: right), (empty): (from: bottom, to: top))` names each outcome's edge by its `when` value; an unnamed outcome keeps its default (up to three outcomes: top, right, bottom in `when` order). `true:`/`false:` on a `case`, or a value key on anything else, is refused (MDL092) (ako/mxcli#1492). - `@anchor(from: X)` is the side the flow **leaving** a statement starts from. On an `if` that is the flow out of its closing merge (the merge has no statement of its own, just as its position rides on the if as `@merge`); the if's `to:` is its incoming flow and `true:` / `false:` its branches. `@anchor(from: bottom, to: top, …)` on an if is how DESCRIBE writes a decision whose merge drops onto the next row (#767) - `@start(x, y)` positions the **start event** and goes on the first statement, because the start has no statement of its own. Omit it and the start is derived — one spacing unit (160) left of the first activity, on its centre line — and a rewrite re-derives it so the start follows the activities when they move. A start that is not at the derived spot was placed by hand (in Studio Pro or with `@start`): it survives a rewrite that does not mention it, and DESCRIBE emits `@start` for it. An explicit `@start` overrides both (#951) - `@position(x, y)` on a **parameter** goes inside the parameter list, ahead of the parameter it places — a parameter is a stored node with its own coordinates, and this is the only annotation it takes. Omit it and the parameters form a row along the top of the canvas (200;53, 300;53, …). The `@start` rule above applies unchanged: a parameter on that derived row is re-derived on a rewrite, one anywhere else was placed by hand, survives, and is emitted by DESCRIBE (#993). Before this, a hand-aligned parameter block was moved back onto the row by any rewrite — including a describe → exec of mxcli's own output: diff --git a/cmd/mxcli/syntax/features_microflow.go b/cmd/mxcli/syntax/features_microflow.go index cc3153bf53..d1a1775406 100644 --- a/cmd/mxcli/syntax/features_microflow.go +++ b/cmd/mxcli/syntax/features_microflow.go @@ -820,16 +820,26 @@ func init() { Syntax: "@position(x, y) -- the activity's centre point\n" + "@position(x, y) -- also on a PARAMETER, in the ( … ) list\n" + "@start(x, y) -- the start event, on the FIRST statement\n" + - "@anchor(from: right, to: left) -- which SIDE each end of the outgoing flow attaches to\n" + + "@anchor(from: bottom, to: top) -- from = the side the flow LEAVING this statement\n" + + " -- leaves by; to = the side the flow ARRIVING at\n" + + " -- this statement enters by (not its far end)\n" + "@curve(from: (40, -90), to: (-40, 90)) -- the flow's bezier control vectors\n" + "@merge(x, y) -- the implicit merge that closes a split\n" + "@anchor(from: bottom, to: top, true: (…), false: (…)) -- on an IF: to = its incoming flow,\n" + " -- from = the flow leaving its closing merge\n" + "@anchor(true: (to: top)) -- one branch's edge; either side may be left out\n" + + "@anchor(Open: (from: right), (empty): (from: bottom)) -- on a CASE: one outcome's edge,\n" + + " -- keyed by its WHEN value; from = its merge's exit\n" + "@caption 'text'\n@color Green\n@annotation 'a note'\n@excluded\n" + "@applyentityaccess | @applyentityaccess(false) -- DOCUMENT-level, before CREATE MICROFLOW/RULE\n" + "@annotation(id: n1, text: 'a note', position: (x, y), size: (w, h))\n" + "@annotation(id: n1) -- attaches THAT note to another activity\n\n" + + "An @anchor never reaches the far end of its own outgoing flow: that end is\n" + + "the NEXT statement's `to:`. A straight vertical edge from A down to B is\n" + + "@anchor(from: bottom) on A and @anchor(to: top) on B; `to: top` on A alone\n" + + "turns the edge INTO A, and the edge into B keeps entering on the left. A\n" + + "RETURN has no outgoing flow, so its `from:` does nothing — the edge into the\n" + + "end event leaves by the previous statement's `from:`. (#990)\n\n" + "Inside ON ERROR ... BEGIN ... END ERROR the annotations mean what they mean\n" + "outside it: @anchor(to:) on the handler's first statement is the side the\n" + "error edge enters, @anchor(from:) and @curve shape the edge leaving a\n" + @@ -870,7 +880,8 @@ func init() { "A PARAMETER is a stored node with its own coordinates, so it takes\n" + "@position too — written inside the parameter list, ahead of the parameter\n" + "it places. It is the only annotation a parameter takes. Omit it and the\n" + - "parameters form a row along the top of the canvas at 200;53, 300;53, … ;\n" + + "parameters form a row along the top of the canvas from 200;53, spaced 100px\n" + + "apart or wider when a name needs it (about 7px a character, #1598);\n" + "the same derived/authored rule as @start then applies, so a parameter on\n" + "that row is re-derived and one anywhere else survives a rewrite and is\n" + "emitted by DESCRIBE.\n\n" + @@ -885,9 +896,9 @@ func init() { "what a rewrite re-derives (100px above the activity, stacked 60px per extra\n" + "note, 200x50), so an ordinary note keeps the short form. (#1077)", Example: "create microflow MyModule.ACT_Flow (\n @position(145, 0)\n $In: String\n)\nreturns String as $Out\nbegin\n" + - " @start(145, 100)\n @position(200, 100)\n @anchor(from: bottom, to: top)\n" + + " @start(145, 100)\n @position(200, 100)\n @anchor(from: bottom)\n" + " @curve(from: (40, -90), to: (-40, 90))\n declare $Tmp String = $In;\n" + - " @position(200, 300)\n declare $Out String = $Tmp;\n return $Out;\nend;", + " @position(200, 300)\n @anchor(to: top)\n declare $Out String = $Tmp;\n return $Out;\nend;", SeeAlso: []string{"microflow", "microflow.create"}, }) diff --git a/docs-site/src/language/microflow-structure.md b/docs-site/src/language/microflow-structure.md index a4aa0d481b..489db14228 100644 --- a/docs-site/src/language/microflow-structure.md +++ b/docs-site/src/language/microflow-structure.md @@ -202,6 +202,32 @@ end; This is how `DESCRIBE` writes a decision whose merge drops onto the next row. +An annotation never reaches the far end of its own outgoing flow: that end is +the next statement's `to:`. A straight vertical edge from A down to B is +`@anchor(from: bottom)` on A **and** `@anchor(to: top)` on B. + +A `case` names each outcome's edge by its `when` value, `(empty)` included, and +its own `from:` is the flow leaving its merge, as on an `if`: + +```sql +create microflow Sales.ACT_RouteOrder ($Order: Sales.Order) +begin + @anchor(Open: (from: right), (empty): (from: bottom, to: top)) + case $Order/Status + when Open then + log info node 'Order' 'open'; + when Closed then + log info node 'Order' 'closed'; + when (empty) then + log warning node 'Order' 'no status'; + end case; +end; +``` + +An outcome not named keeps its default side (with up to three outcomes: the +first leaves the top corner, the second the right, the third the bottom). +`DESCRIBE` writes an outcome only where its stored side differs from that. + ### Layout in `DESCRIBE` output `DESCRIBE MICROFLOW` and `DESCRIBE NANOFLOW` print a layout annotation — diff --git a/docs-wiki/bug-patterns/flow-graph-geometry.md b/docs-wiki/bug-patterns/flow-graph-geometry.md index 633a675568..5303e9f98c 100644 --- a/docs-wiki/bug-patterns/flow-graph-geometry.md +++ b/docs-wiki/bug-patterns/flow-graph-geometry.md @@ -1,13 +1,14 @@ --- title: Generated Microflow Geometry and Wiring category: bug-pattern -last-synced: ced830e0 +last-synced: 40d189164 covers: - mdl/executor sources: - .claude/skills/fix-issue/findings/mdl-executor/ - mdl/executor/layout.go - mdl/executor/cmd_microflows_builder_control.go + - mdl/backend/mfmutator/splice.go --- > **Do not duplicate**: the measurement formulas and per-construct fixes live in @@ -66,6 +67,33 @@ coordinate fix against the *whole* coordinate set — dump every point before an after and diff — or a change that pins one element while shifting another reads as success. +**The convention is measurable; measure it.** Most geometry findings are a +side, a pitch or a placement where mxcli chose a constant and Studio Pro does +something else. Studio Pro's own documents settle it in one pass — the +`mx-test-projects` apps, ako/TestApp and PedApp together hold hundreds of +note lines, handler rows, parameter rows and CASE outcomes. A guess written +into an issue is not evidence: a proposed note numbering was refuted by notes +level with their activity, which only the activity numbering explains, and a +"~140px" estimate turned out to be a 130px median. Write the measured number +into the test literally; a test that compares against the builder's own +constant passes at any value. + +**A side can carry meaning, not just drawing.** Up to three CASE outcomes keep +their WHEN-order sides because DESCRIBE reads the case order back from them; +following the geometry there reorders a CASE whose branches were placed out of +order. Before making a default "look right", find every reader of the value. + +**Removing a node needs a way to keep it.** When the builder stops drawing a +node (a merge at a loop tail), a stored one survives a rebuild only if the +description states it — and the canonical describe drops what it thinks is +derivable, so the statement has to be unconditional. Graph comparisons in the +tests are merge-blind; count merges. + +**A splice has its own geometry.** A fragment is laid out alone and then +translated into the gap; anything that is not on the line (a note) must not +size the gap, and anything the script placed absolutely must not be +translated. + ## See also - [fix-issue findings](../../.claude/skills/fix-issue/findings/) — the formulas, diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index e6031de7b8..1f72217340 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -608,7 +608,7 @@ it is for pages. | Unknown annotation | — | **MDL059**. An annotation that parses and does nothing loses whatever it was meant to express, so a name the target does not read is refused — on a statement *and* before a `create`. Covers a typo (`@applyentityacces`), an annotation on a document kind that reads none (`@excluded` on a queue), and an activity annotation written at document level. The message names what that document does accept | | Parameter position | `@position(x, y)` before a parameter, **inside** the `( … )` list | The only annotation a parameter takes. Omit it and parameters form a row at 200;53, 300;53, …; a parameter off that row is treated as hand-placed, survives a rewrite, and is emitted by DESCRIBE (#993) | | Start event | `@start(x, y)` | Canvas position of the start, on the **first** statement. Omit it and the start is placed one spacing unit left of the first activity and MOVES with it on a rewrite; a start that is not at that derived spot is treated as hand-placed, survives a rewrite, and is emitted by DESCRIBE (#951) | -| Flow anchors | `@anchor(from: bottom, to: top)`; on an `if` also `true: (from: …, to: …)`, `false: (…)` | The side each end of a flow attaches to: `to` = the flow arriving, `from` = the flow leaving. On an `if`, `from` is the flow out of its closing **merge**, which has no statement of its own (#767) | +| Flow anchors | `@anchor(from: bottom, to: top)`; on an `if` also `true: (from: …, to: …)`, `false: (…)`; on a `case` one entry per outcome, `Open: (from: right)`, `(empty): (…)` | `to` = the flow arriving at the statement, `from` = the flow leaving it — never the far end of its own outgoing flow, which is the next statement's `to:`. On an `if` or a `case`, `from` is the flow out of its closing **merge**, which has no statement of its own (#767) | | Caption | `@caption 'text'` | Custom caption (before activity). A decision with no `@caption` is captioned with its condition, so `describe` prints none for one whose caption is its condition | | Color | `@color Green` | Background color (before activity) | | Annotation | `@annotation 'text'` | Visual note attached to next activity. **Repeatable** — an activity can carry several, and each is its own note | diff --git a/mdl-examples/bug-tests/flow-geometry-1273-loop-tail-merge.mdl b/mdl-examples/bug-tests/flow-geometry-1273-loop-tail-merge.mdl new file mode 100644 index 0000000000..00c3eea7fe --- /dev/null +++ b/mdl-examples/bug-tests/flow-geometry-1273-loop-tail-merge.mdl @@ -0,0 +1,51 @@ +mdl 1; +-- ============================================================================ +-- Bug: an IF/ELSE ending a loop body got a merge with no way out +-- (ako/mxcli#1273; mendixlabs/mxcli#884) +-- ============================================================================ +-- +-- Both branches end the iteration, so there is nothing to join. Studio Pro +-- authors delete the merge by hand; every re-run drew it again (describe +-- printed the same MDL with and without it). +-- +-- Fixed: no merge at a loop or while tail, including an IF/ELSE ending a +-- branch of that IF. An explicit @merge still draws one, and DESCRIBE writes +-- @merge for a stored one, so it survives a rebuild. +-- +-- Verify: exec, then +-- mxcli bson dump -p app.mpr --type microflow --object MyFirstModule.ACT_LoopTail +-- Expected: no ExclusiveMerge inside the LoopedActivity or the while loop; +-- mx check reports nothing for these flows. +-- ============================================================================ + +create persistent entity MyFirstModule.TailItem (Code: string(50), Done: boolean); + +create or modify microflow MyFirstModule.ACT_LoopTail ($Flag: boolean) +begin + retrieve $Items from MyFirstModule.TailItem; + loop $Item in $Items begin + log info node 'Tail' 'item'; + if $Item/Done then + log info node 'Tail' 'done'; + else + if $Flag then + log info node 'Tail' 'flagged'; + else + log info node 'Tail' 'open'; + end if; + end if; + end loop; +end; + +create or modify microflow MyFirstModule.ACT_WhileTail ($Count: integer) +begin + declare $I integer = 0; + while $I < $Count begin + set $I = $I + 1; + if $I > 3 then + log info node 'Tail' 'big'; + else + log info node 'Tail' 'small'; + end if; + end while; +end; diff --git a/mdl-examples/bug-tests/flow-geometry-1273-on-error-rejoin-merge.mdl b/mdl-examples/bug-tests/flow-geometry-1273-on-error-rejoin-merge.mdl new file mode 100644 index 0000000000..514f8f0e28 --- /dev/null +++ b/mdl-examples/bug-tests/flow-geometry-1273-on-error-rejoin-merge.mdl @@ -0,0 +1,40 @@ +mdl 1; +-- ============================================================================ +-- Bug: the merge an ON ERROR handler rejoins at landed on the End event +-- (ako/mxcli#1273; mendixlabs/mxcli#884, TDSC0706's repro) +-- ============================================================================ +-- +-- Stored before the fix: +-- ExclusiveMerge @(560, 200) 40x40 +-- EndEvent @(560, 200) 20x20 <- same centre, the end inside the merge +-- and with an activity after the guarded call, the merge sat RIGHT of that +-- activity, so the edge into it ran backwards. +-- +-- Fixed: the merge sits on the edge it splits, midway between the guarded +-- activity's right edge and its successor's left edge. `@merge(x, y)` on the +-- guarded statement still places it explicitly. +-- +-- Verify: exec, then +-- mxcli bson dump -p app.mpr --type microflow --object MyFirstModule.ACT_ErrorBranch +-- Expected: ExclusiveMerge at (425, 200), clear of the call (x <= 300) and of +-- the End event (x >= 550). +-- ============================================================================ + +create or modify microflow MyFirstModule.SUB_ErrorCallee () +begin + @position(200, 200) + log info node 'Repro' 'callee'; + @position(400, 200) + return; +end; + +create or modify microflow MyFirstModule.ACT_ErrorBranch () +begin + @position(240, 200) + call microflow MyFirstModule.SUB_ErrorCallee() on error without rollback begin + @position(240, 380) + log error node 'Repro' 'handled'; + end error; + @position(560, 200) + return; +end; diff --git a/mdl-examples/bug-tests/flow-geometry-1277-single-inbound-merge.mdl b/mdl-examples/bug-tests/flow-geometry-1277-single-inbound-merge.mdl new file mode 100644 index 0000000000..e752edc74b --- /dev/null +++ b/mdl-examples/bug-tests/flow-geometry-1277-single-inbound-merge.mdl @@ -0,0 +1,51 @@ +mdl 1; +-- ============================================================================ +-- Bug: a merge with one incoming path was not represented by describe +-- (ako/mxcli#1277; mendixlabs/mxcli#923, MF_RT_Chain) +-- ============================================================================ +-- +-- `join finish; merge finish;` builds a merge only one path enters. DESCRIBE +-- walked through it and printed "WARNING: the merge at (1330, 350) is not +-- represented in this description ... re-executing this MDL DELETES it" - +-- for a node exec had just written from ordinary MDL. +-- +-- Fixed: on the main line the merge is described as written +-- (`join junction1; merge junction1;`), so the description rebuilds it and has +-- nothing to warn about. In a branch or loop body it is still walked through, +-- and the warning now says what is true: `create or modify` from the +-- description leaves the merge in place, a rebuild removes it. +-- +-- Verify: exec, then +-- mxcli -p app.mpr -c "describe microflow MyFirstModule.MF_RT_Chain" +-- Expected: no WARNING line; `join junction1;` / `merge junction1;` before the +-- return. Executing the description reports `Unchanged microflow`. +-- ============================================================================ + +create or modify microflow MyFirstModule.MF_RT_Chain () returns string as $Out +begin + declare $Out string = 'a'; + + if 1 = 1 then + if 2 = 2 then + set $Out = 'healthy'; + join emit; + else + set $Out = 'watch'; + join review; + end if; + else + set $Out = 'low'; + join review; + end if; + + merge review; + log info node 'T' 'review'; + join emit; + + merge emit; + log info node 'T' 'emit'; + join finish; + + merge finish; + return $Out; +end; diff --git a/mdl-examples/bug-tests/flow-geometry-1277-while-true-early-returns.mdl b/mdl-examples/bug-tests/flow-geometry-1277-while-true-early-returns.mdl new file mode 100644 index 0000000000..0a0a45d436 --- /dev/null +++ b/mdl-examples/bug-tests/flow-geometry-1277-while-true-early-returns.mdl @@ -0,0 +1,28 @@ +mdl 1; +-- ============================================================================ +-- Bug: `while true` with early returns was described with three "NOT +-- equivalent ... must not be re-executed" warnings and three MDL-FLOW01 lint +-- findings, although describe -> exec wrote nothing (ako/mxcli#1277; +-- mendixlabs/mxcli#923, saingge's re-test) +-- ============================================================================ +-- +-- The graph analysis walked each decision's false branch round the loop's back +-- edge into its own true branch and counted that branch's return as shared. +-- +-- Verify: exec, then +-- mxcli -p app.mpr -c "describe microflow MyFirstModule.ACT_LoopReturns" +-- mxcli lint -p app.mpr +-- Expected: no WARNING lines in the description, no MDL-FLOW01 for this flow. +-- ============================================================================ + +create or modify microflow MyFirstModule.ACT_LoopReturns ($Code: string) returns string as $R +begin + declare $Status string = 'new'; + while true + begin + if $Code = 'a' then return 'A'; end if; + if $Code = 'b' then return 'B'; end if; + if $Code = 'c' then return 'C'; end if; + set $Status = 'x'; + end while; +end; diff --git a/mdl-examples/bug-tests/flow-geometry-1492-case-outcome-anchors.mdl b/mdl-examples/bug-tests/flow-geometry-1492-case-outcome-anchors.mdl new file mode 100644 index 0000000000..a7dfb5ec87 --- /dev/null +++ b/mdl-examples/bug-tests/flow-geometry-1492-case-outcome-anchors.mdl @@ -0,0 +1,36 @@ +mdl 1; +-- ============================================================================ +-- Bug: a CASE's outcome sides could not be chosen (ako/mxcli#1492; +-- mendixlabs/mxcli#1587) +-- ============================================================================ +-- +-- Each outcome flow's OriginConnectionIndex came from WHEN order alone (1st +-- top, 2nd right, 3rd bottom). `@anchor(A: (from: right))` was refused with +-- MDL092 and `@anchor(from: right)` on the CASE was accepted and ignored. +-- +-- Fixed: `@anchor(: (from: …, to: …), (empty): (…))` on a CASE sets one +-- outcome's edge; the CASE's own `from:` is the flow leaving its merge, as on +-- an IF. DESCRIBE writes an outcome only where it differs from the default, so +-- describe -> exec keeps Studio Pro-drawn sides. +-- +-- Verify: exec, then +-- mxcli bson dump -p app.mpr --type microflow --object MyFirstModule.ACT_CaseSides +-- Expected: the Open flow leaves side 1 (right), the (empty) flow side 2 -> 0, +-- and the flow out of the merge leaves side 2 (bottom). +-- ============================================================================ + +create enumeration MyFirstModule.CaseState (Open 'Open', Closed 'Closed'); + +create or modify microflow MyFirstModule.ACT_CaseSides ($State: MyFirstModule.CaseState) +begin + @anchor(from: bottom, Open: (from: right), (empty): (from: bottom, to: top)) + case $State + when Open then + log info node 'T' 'open'; + when Closed then + log info node 'T' 'closed'; + when (empty) then + log info node 'T' 'empty'; + end case; + log info node 'T' 'after'; +end; diff --git a/mdl-examples/bug-tests/flow-geometry-1508-handler-row-pitch.mdl b/mdl-examples/bug-tests/flow-geometry-1508-handler-row-pitch.mdl new file mode 100644 index 0000000000..fe02f66fb8 --- /dev/null +++ b/mdl-examples/bug-tests/flow-geometry-1508-handler-row-pitch.mdl @@ -0,0 +1,42 @@ +mdl 1; +-- ============================================================================ +-- Bug: an error handler's row sat 90px under its activity, inside the output +-- variable label Studio Pro prints under the box (ako/mxcli#1508, case 3; +-- mendixlabs/mxcli#1598) +-- ============================================================================ +-- +-- Before: retrieve at (360, 200), handler activity at (360, 290). A handler in +-- a THEN branch also overlapped the ELSE row (+90 against +100), because the +-- layout measured the THEN activity without its handler row. +-- +-- Fixed: handler rows sit 130px under their activity (Studio Pro's median over +-- 160 measured handler rows), and the measurer counts the row, so an ELSE row +-- goes below it. +-- +-- Verify: exec, then +-- mxcli bson dump -p app.mpr --type microflow --object MyFirstModule.ACT_HandlerRow +-- Expected: the handler's log activity 130px below the retrieve; in +-- ACT_HandlerInThen no two activity boxes overlap. +-- ============================================================================ + +create persistent entity MyFirstModule.RowItem (Code: string(50)); + +create or modify microflow MyFirstModule.ACT_HandlerRow () +begin + retrieve $List from MyFirstModule.RowItem on error begin + log error node 'x' 'failed'; + end error; + log info node 'x' 'done'; +end; + +create or modify microflow MyFirstModule.ACT_HandlerInThen ($Flag: boolean) +begin + if $Flag then + retrieve $List from MyFirstModule.RowItem on error begin + log error node 'x' 'failed'; + end error; + log info node 'x' 'then'; + else + log info node 'x' 'else'; + end if; +end; diff --git a/mdl-examples/bug-tests/flow-geometry-1508-merge-after-loop-in-branch.mdl b/mdl-examples/bug-tests/flow-geometry-1508-merge-after-loop-in-branch.mdl new file mode 100644 index 0000000000..dd5d9e9edf --- /dev/null +++ b/mdl-examples/bug-tests/flow-geometry-1508-merge-after-loop-in-branch.mdl @@ -0,0 +1,30 @@ +mdl 1; +-- ============================================================================ +-- Bug: the merge after an IF whose branch holds a LOOP landed inside the loop +-- box (ako/mxcli#1508, case 4; mendixlabs/mxcli#1598) +-- ============================================================================ +-- +-- Stored before the fix: LoopedActivity 950;200 320x160 (x 790..1110) and the +-- ExclusiveMerge at 1070;200 (x 1050..1090), so the loop's outgoing line ran +-- right to left. The layout measurer sized a foreach loop without the 100px +-- iterator column the builder reserves on its left. +-- +-- Verify: exec, then +-- mxcli bson dump -p app.mpr --type microflow --object MyFirstModule.ACT_MergeAfterLoop +-- Expected: the merge's left edge (x - 20) is at or right of the loop's right +-- edge (x + width/2). +-- ============================================================================ + +create persistent entity MyFirstModule.LoopItem (Code: string(50), Amount: integer); + +create or modify microflow MyFirstModule.ACT_MergeAfterLoop ($Flag: boolean) returns boolean as $r +begin + declare $r boolean = false; + retrieve $L from MyFirstModule.LoopItem; + if $Flag then + loop $i in $L begin set $r = true; end loop; + else + set $r = false; + end if; + return $r; +end; diff --git a/mdl-examples/bug-tests/flow-geometry-1508-note-line-sides.mdl b/mdl-examples/bug-tests/flow-geometry-1508-note-line-sides.mdl new file mode 100644 index 0000000000..f7490af0a3 --- /dev/null +++ b/mdl-examples/bug-tests/flow-geometry-1508-note-line-sides.mdl @@ -0,0 +1,37 @@ +mdl 1; +-- ============================================================================ +-- Bug: a note's line always left the note's bottom edge (ako/mxcli#1508, case 2; +-- mendixlabs/mxcli#1598) +-- ============================================================================ +-- +-- Every Microflows$AnnotationFlow was written OriginConnectionIndex 2 -> +-- DestinationConnectionIndex 0 (note bottom -> element top), wherever the note +-- sat. A note BELOW its activity then drew its line out of its own bottom edge +-- and round the outside of both boxes. +-- +-- Fixed: the line leaves the side of the note that faces its element and enters +-- the element's facing side. Studio Pro numbers a note's sides like an +-- activity's (0 top, 1 right, 2 bottom, 3 left), measured on 523 distinct +-- Studio Pro-drawn note lines. +-- +-- Verify: +-- mxcli exec mdl-examples/bug-tests/flow-geometry-1508-note-line-sides.mdl -p app.mpr +-- mxcli bson dump -p app.mpr --type microflow --object MyFirstModule.ACT_NoteSides \ +-- | grep -B2 -A2 AnnotationFlow +-- Expected: 'Note above' 2 -> 0, 'Note below' 0 -> 2, 'Note left' 1 -> 3. +-- ============================================================================ + +create or modify microflow MyFirstModule.ACT_NoteSides () returns Boolean as $r +begin + @position(400, 200) + @annotation(text: 'Note above', position: (400, 60), size: (200, 60)) + log info node 'A' 'hello'; + @position(800, 200) + @annotation(text: 'Note below', position: (800, 400), size: (200, 60)) + log info node 'B' 'world'; + @position(1300, 200) + @annotation(text: 'Note left', position: (1050, 200), size: (180, 60)) + log info node 'C' 'again'; + @position(1500, 200) + return true; +end; diff --git a/mdl-examples/bug-tests/flow-geometry-1508-note-only-splice.mdl b/mdl-examples/bug-tests/flow-geometry-1508-note-only-splice.mdl new file mode 100644 index 0000000000..e8b9d32dbb --- /dev/null +++ b/mdl-examples/bug-tests/flow-geometry-1508-note-only-splice.mdl @@ -0,0 +1,54 @@ +mdl 1; +-- ============================================================================ +-- Bug: adding a note to one statement re-laid out the row after it +-- (ako/mxcli#1508, case 1; mendixlabs/mxcli#1598) +-- ============================================================================ +-- +-- Run this file once, then edit the second ACT_NoteSplice below: it is the +-- same flow with one note added before `declare $D`. Before the fix the +-- `create or modify` spliced the noted statement in as a replacement whose box +-- ran from the activity to the note, so the note was stored at x 3390 instead +-- of 1500 and every later activity shifted by +1180px (a size-only note: +-- +140..+280px). +-- +-- Fixed: a note takes no room on the line it documents, and a note with a +-- stated position stays where the script puts it. +-- +-- Verify: exec, then +-- mxcli -p app.mpr -c "describe microflow MyFirstModule.ACT_NoteSplice with handles" +-- Expected: the second create or modify leaves every activity where the first +-- put it, and the note is stored at (1500, 600). +-- ============================================================================ + +create persistent entity MyFirstModule.SpliceItem (Code: String(50), Status: String(50), Amount: Integer); + +create or modify microflow MyFirstModule.ACT_NoteSplice ($Amt: Integer) returns Boolean as $Ok +begin + declare $Ok Boolean = false; + declare $Total Integer = 0; + retrieve $Items from MyFirstModule.SpliceItem; + loop $I in $Items begin + set $Total = $Total + 1; + log info node 'Probe' 'step ' + toString($Total); + end loop; + if $Total > 3 then set $Ok = true; else set $Ok = false; end if; + declare $A Integer = 1; declare $B Integer = 2; declare $C Integer = 3; + declare $D Integer = 4; declare $E Integer = 5; declare $F Integer = 6; + return $Ok; +end; + +create or modify microflow MyFirstModule.ACT_NoteSplice ($Amt: Integer) returns Boolean as $Ok +begin + declare $Ok Boolean = false; + declare $Total Integer = 0; + retrieve $Items from MyFirstModule.SpliceItem; + loop $I in $Items begin + set $Total = $Total + 1; + log info node 'Probe' 'step ' + toString($Total); + end loop; + if $Total > 3 then set $Ok = true; else set $Ok = false; end if; + declare $A Integer = 1; declare $B Integer = 2; declare $C Integer = 3; + @annotation(text: 'note D', position: (1500, 600), size: (200, 80)) + declare $D Integer = 4; declare $E Integer = 5; declare $F Integer = 6; + return $Ok; +end; diff --git a/mdl-examples/bug-tests/flow-geometry-1508-parameter-row-pitch.mdl b/mdl-examples/bug-tests/flow-geometry-1508-parameter-row-pitch.mdl new file mode 100644 index 0000000000..04ad077199 --- /dev/null +++ b/mdl-examples/bug-tests/flow-geometry-1508-parameter-row-pitch.mdl @@ -0,0 +1,29 @@ +mdl 1; +-- ============================================================================ +-- Bug: parameters with long names were written 100px apart and their labels +-- overlapped (ako/mxcli#1508, case 5; mendixlabs/mxcli#1598) +-- ============================================================================ +-- +-- Before: LongParameterNameOne / Two / Three at 200;53, 300;53, 400;53. +-- +-- Fixed: the derived row is spaced by the widest name (about 7px a character, +-- rounded up to 10px, never under 100px). Names under 15 characters keep the +-- old 100px row, and the old row still reads as derived, so a flow an earlier +-- mxcli wrote is re-spaced rather than pinned. +-- +-- Verify: exec, then +-- mxcli bson dump -p app.mpr --type microflow --object MyFirstModule.ACT_LongParams +-- Expected: parameters at 200;53, 360;53, 520;53. +-- ============================================================================ + +create entity MyFirstModule.ParamItem (Code: string(20)); + +create or modify microflow MyFirstModule.ACT_LongParams ( + $LongParameterNameOne: MyFirstModule.ParamItem, + $LongParameterNameTwo: MyFirstModule.ParamItem, + $LongParameterNameThree: MyFirstModule.ParamItem +) returns Boolean as $Ok +begin + declare $Ok Boolean = true; + return $Ok; +end; diff --git a/mdl-examples/doctype-tests/02c-complex-layout-examples.mdl b/mdl-examples/doctype-tests/02c-complex-layout-examples.mdl index 4f2c6587e3..2e36ba693e 100644 --- a/mdl-examples/doctype-tests/02c-complex-layout-examples.mdl +++ b/mdl-examples/doctype-tests/02c-complex-layout-examples.mdl @@ -803,3 +803,32 @@ begin + ' with outcome ' + $Outcome; return $Outcome; end; + +-- ============================================================================ +-- 7. CX_ROUTE_ByStatus — choosing each CASE outcome's side +-- ============================================================================ +-- +-- @anchor on a CASE names each outcome's edge by its WHEN value, (empty) +-- included; an outcome it does not name keeps its default side. The CASE's +-- own `from:` is the flow leaving its merge, as on an IF. DESCRIBE writes an +-- outcome only where its stored side differs from the default. +-- (ako/mxcli#1492, mendixlabs/mxcli#1587) +-- ============================================================================ + +create microflow CxLayout.CX_ROUTE_ByStatus ( + $Factory: CxLayout.Factory +) +begin + @anchor(from: bottom, Active: (from: right), (empty): (from: bottom, to: top)) + case $Factory/Status + when Active then + log info node 'Route' 'active'; + when Maintenance then + log info node 'Route' 'maintenance'; + when Offline, Decommissioned then + log warning node 'Route' 'not running'; + when (empty) then + log error node 'Route' 'no status'; + end case; + log info node 'Route' 'routed'; +end; diff --git a/mdl/ast/ast_microflow.go b/mdl/ast/ast_microflow.go index 577a5e9951..5b5111632f 100644 --- a/mdl/ast/ast_microflow.go +++ b/mdl/ast/ast_microflow.go @@ -340,12 +340,24 @@ const ( AnchorSideLeft AnchorSide = 3 ) -// FlowAnchors captures the origin/destination anchors for a single -// SequenceFlow. Each side is independently optional: Unset means the builder -// should derive the anchor from the visual direction. +// FlowAnchors captures the sides a statement's flows attach at. Each side is +// independently optional: Unset means the builder should derive the anchor +// from the visual direction. +// +// The two halves name two DIFFERENT flows when written on a statement: From is +// the side the flow leaving it leaves by, To the side the flow arriving at it +// enters by. Nested in a branch form (`true: (from: …, to: …)`) both describe +// that one branch's edge. type FlowAnchors struct { From AnchorSide // OriginConnectionIndex on the outgoing SequenceFlow - To AnchorSide // DestinationConnectionIndex on the outgoing SequenceFlow + To AnchorSide // DestinationConnectionIndex on the INCOMING SequenceFlow +} + +// CaseAnchor is one outcome's entry in a CASE's @anchor: the WHEN value, as +// written ("(empty)" for the empty value), and the sides of its flow. +type CaseAnchor struct { + Value string + Anchors *FlowAnchors } // HasCaption reports whether the statement states a caption, empty or not. @@ -372,7 +384,7 @@ type ActivityAnnotations struct { // canvas wired to nothing. FreeNotes []MicroflowAnnotation Excluded bool // @excluded - Anchor *FlowAnchors // @anchor(from: X, to: Y) — anchors of the flow leaving this statement + Anchor *FlowAnchors // @anchor(from: X, to: Y) — From for the flow leaving, To for the flow arriving // Split-specific anchors for IF statements. When the statement is not an // IF these remain nil. The grammar accepts them on IfStmt only: @@ -387,6 +399,13 @@ type ActivityAnnotations struct { IteratorAnchor *FlowAnchors BodyTailAnchor *FlowAnchors + // CaseAnchors are a CASE's per-outcome anchors, keyed by the WHEN value, + // in source order: + // @anchor(A: (from: right), (empty): (from: bottom, to: top)) + // Each names the sides of the one flow that leaves the split for that + // value. Only a CASE takes them (mendixlabs/mxcli#1587). + CaseAnchors []CaseAnchor + // Curve is the bezier geometry of the flow LEAVING this statement: // @curve(from: (40, -90), to: (-40, 90)). // diff --git a/mdl/backend/mcp/microflow.go b/mdl/backend/mcp/microflow.go index f42dc66514..6fcbf8a31c 100644 --- a/mdl/backend/mcp/microflow.go +++ b/mdl/backend/mcp/microflow.go @@ -256,7 +256,7 @@ func (b *Backend) buildFlowDocContent(kind, name string, params []*microflows.Mi "$Type": "Microflows$MicroflowParameterObject", "name": p.Name, "type": typeName, - "relativeMiddlePoint": map[string]int{"x": 200 + i*100, "y": 53}, + "relativeMiddlePoint": map[string]int{"x": microflows.DerivedParameterPosition(i, params).X, "y": microflows.DerivedParameterPosition(i, params).Y}, } if entity != "" { po["entity"] = entity diff --git a/mdl/backend/mfmutator/splice.go b/mdl/backend/mfmutator/splice.go index e2046cb5d3..b00376e009 100644 --- a/mdl/backend/mfmutator/splice.go +++ b/mdl/backend/mfmutator/splice.go @@ -1028,6 +1028,13 @@ func fragmentGeometry(frag *backend.MicroflowFragment) (*fragmentBox, error) { fb := &fragmentBox{frag: frag} first := true for _, obj := range frag.Objects { + // A note sits off the line it documents, so it takes no room on it. + // Counted, a note at a stated position 1300px away made the box that + // wide and shifted the rest of the row by 1180px; a note wider than its + // activity shifted it by the difference (mendixlabs/mxcli#1598). + if _, isNote := obj.(*microflows.Annotation); isNote { + continue + } p, sz := obj.GetPosition(), objectSize(obj) lo := point{p.X - sz.X/2, p.Y - sz.Y/2} hi := point{p.X + sz.X/2, p.Y + sz.Y/2} @@ -1056,6 +1063,11 @@ func (fb *fragmentBox) placeCentred(ax axis, c point) { d = d.set(ax, c.get(ax)-(fb.min.get(ax)+fb.max.get(ax))/2) d = d.set(ax.other(), c.get(ax.other())-entry.get(ax.other())) for _, obj := range fb.frag.Objects { + // A stated canvas position is where the script wants the object, not a + // place relative to the fragment. + if fb.frag.Pinned[obj.GetID()] { + continue + } p := obj.GetPosition() obj.SetPosition(model.Point{X: p.X + d.X, Y: p.Y + d.Y}) } diff --git a/mdl/backend/mfmutator/splice_notes_geometry_test.go b/mdl/backend/mfmutator/splice_notes_geometry_test.go new file mode 100644 index 0000000000..aeb4f57c7a --- /dev/null +++ b/mdl/backend/mfmutator/splice_notes_geometry_test.go @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: Apache-2.0 + +package mfmutator + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// A note is not part of the room a spliced activity needs on its line +// (mendixlabs/mxcli#1598, case 1). Adding `@annotation(text: 'note D', +// position: (1500, 600))` to one statement made a Replace of it whose box ran +// from the activity to the note — 1300px — so every node after it shifted by +// 1180px and the note itself, translated with the box, landed at x 3390. A +// size-only note wider than its activity shifted the row by the difference. +func TestSplice_ReplaceWithANoteKeepsTheRow(t *testing.T) { + fragment := func(notePos model.Point, noteSize model.Size, stated bool) (*backend.MicroflowFragment, model.ID, model.ID) { + frag := oneActivity() + note := µflows.Annotation{BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, + Position: notePos, + Size: noteSize, + }} + frag.Objects = append(frag.Objects, note) + frag.AnnotationFlows = []*microflows.AnnotationFlow{{ + BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, + OriginID: note.ID, DestinationID: frag.Entry, + }} + if stated { + frag.Pinned = map[model.ID]bool{note.ID: true} + } + return frag, frag.Entry, note.ID + } + positions := func(t *testing.T, m *Mutator) map[string]point { + t.Helper() + out := map[string]point{} + for _, n := range m.graph().order { + out[n.id] = n.pos + } + return out + } + + t.Run("a note at a stated position", func(t *testing.T) { + m, _ := newMutator(t, unit(line(), lineFlows())) + frag, act, note := fragment(model.Point{X: 1500, Y: 600}, model.Size{Width: 200, Height: 80}, true) + if err := m.Replace(model.ID(uid("a")), frag); err != nil { + t.Fatalf("replace: %v", err) + } + got := positions(t, m) + if p := got[uid("b")]; p.X != 420 { + t.Errorf("b moved to x=%d, want it left at 420", p.X) + } + if p := got[uid("end")]; p.X != 600 { + t.Errorf("the end event moved to x=%d, want it left at 600", p.X) + } + if p := got[string(act)]; p.X != 250 || p.Y != 200 { + t.Errorf("the replacement is at (%d, %d), want the replaced activity's (250, 200)", p.X, p.Y) + } + if p := got[string(note)]; p.X != 1500 || p.Y != 600 { + t.Errorf("the note is at (%d, %d), want where the script put it, (1500, 600)", p.X, p.Y) + } + }) + + t.Run("a size-only note wider than its activity", func(t *testing.T) { + m, _ := newMutator(t, unit(line(), lineFlows())) + // Where the builder draws an unplaced note: above its activity. + frag, act, note := fragment(model.Point{X: 360, Y: 100}, model.Size{Width: 400, Height: 50}, false) + if err := m.Replace(model.ID(uid("a")), frag); err != nil { + t.Fatalf("replace: %v", err) + } + got := positions(t, m) + if p := got[uid("b")]; p.X != 420 { + t.Errorf("b moved to x=%d, want it left at 420", p.X) + } + // The note travels with its activity. + a, n := got[string(act)], got[string(note)] + if n.X-a.X != 0 || a.Y-n.Y != 100 { + t.Errorf("the note is at (%d, %d) for an activity at (%d, %d); want it 100px above it, as built", + n.X, n.Y, a.X, a.Y) + } + }) +} diff --git a/mdl/backend/microflow_mutation.go b/mdl/backend/microflow_mutation.go index 445d60727f..4217ff5d09 100644 --- a/mdl/backend/microflow_mutation.go +++ b/mdl/backend/microflow_mutation.go @@ -30,6 +30,11 @@ type MicroflowFragment struct { // nothing around it is moved to make room (ako/mxcli#818). Unset, the // mutator translates it into the gap it is spliced into. Placed bool + // Pinned are the objects whose position the script states on the canvas — + // a note's `position:` — rather than relative to the fragment. They stay + // where they are when the fragment is translated into place, and take no + // part in the room it needs (mendixlabs/mxcli#1598, case 1). + Pinned map[model.ID]bool } // MicroflowMutator splices into one stored microflow or nanoflow (ADR-0012 diff --git a/mdl/backend/modelsdk/microflow.go b/mdl/backend/modelsdk/microflow.go index 7bb63efa17..2541020ec9 100644 --- a/mdl/backend/modelsdk/microflow.go +++ b/mdl/backend/modelsdk/microflow.go @@ -358,9 +358,15 @@ func annotationFlowsFromGen(items []element.Element) []*microflows.AnnotationFlo if !ok { continue } + // The sides are carried, so a rewrite of a Studio Pro-drawn note keeps + // the line where its author put it. f := µflows.AnnotationFlow{ OriginID: model.ID(g.OriginRefID()), DestinationID: model.ID(g.DestinationRefID()), + Sides: µflows.ConnectionSides{ + Origin: int(g.OriginConnectionIndex()), + Destination: int(g.DestinationConnectionIndex()), + }, } f.ID = model.ID(g.ID()) flows = append(flows, f) @@ -463,6 +469,7 @@ func splitFlowObjects(coll element.Element) ([]*microflows.MicroflowParameter, [ return nil, nil } var params []*microflows.MicroflowParameter + var stored []model.Point var objs []microflows.MicroflowObject for _, el := range mc.ObjectsItems() { if po, ok := el.(*genMf.MicroflowParameter); ok { @@ -474,7 +481,9 @@ func splitFlowObjects(coll element.Element) ([]*microflows.MicroflowParameter, [ // others the moment a parameter is inserted (#993, and #951 before // it). Without this the position was never read at all, so a rewrite // moved every hand-placed parameter back onto the grid. - p.Position = microflows.AuthoredParameterPosition(pointFromGen(el), len(params)) + // The derived row is spaced by every parameter's name, so the + // decision waits until all of them are read (below). + stored = append(stored, pointFromGen(el)) params = append(params, p) continue } @@ -489,6 +498,9 @@ func splitFlowObjects(coll element.Element) ([]*microflows.MicroflowParameter, [ } objs = append(objs, o) } + for i, p := range params { + p.Position = microflows.AuthoredParameterPosition(stored[i], i, params) + } return params, objs } diff --git a/mdl/backend/modelsdk/microflow_annotation_test.go b/mdl/backend/modelsdk/microflow_annotation_test.go index 297041f587..1941bd4691 100644 --- a/mdl/backend/modelsdk/microflow_annotation_test.go +++ b/mdl/backend/modelsdk/microflow_annotation_test.go @@ -66,3 +66,28 @@ func TestAnnotationFlowToGen(t *testing.T) { t.Errorf("Line (BezierCurve) missing on major>=10: %v", err) } } + +// TestAnnotationFlowToGen_WritesTheChosenSides guards that the sides the builder +// picked reach the document, and that a flow nobody oriented keeps the +// note-above drawing (2 -> 0) every earlier write produced +// (mendixlabs/mxcli#1598, case 2). +func TestAnnotationFlowToGen_WritesTheChosenSides(t *testing.T) { + sides := func(af *microflows.AnnotationFlow) (int32, int32) { + t.Helper() + raw, err := (&codec.Encoder{}).Encode(annotationFlowToGen(af, newestFlowVersion)) + if err != nil { + t.Fatalf("encode: %v", err) + } + r := bson.Raw(raw) + return r.Lookup("OriginConnectionIndex").Int32(), r.Lookup("DestinationConnectionIndex").Int32() + } + below := µflows.AnnotationFlow{OriginID: "ann-1", DestinationID: "act-1", + Sides: µflows.ConnectionSides{Origin: 0, Destination: 2}} + if o, d := sides(below); o != 0 || d != 2 { + t.Errorf("note below its activity: written %d -> %d, want 0 -> 2", o, d) + } + unoriented := µflows.AnnotationFlow{OriginID: "ann-1", DestinationID: "act-1"} + if o, d := sides(unoriented); o != 2 || d != 0 { + t.Errorf("unoriented flow: written %d -> %d, want the note-above default 2 -> 0", o, d) + } +} diff --git a/mdl/backend/modelsdk/microflow_parameter_position_test.go b/mdl/backend/modelsdk/microflow_parameter_position_test.go index d46ab57819..22e56863a1 100644 --- a/mdl/backend/modelsdk/microflow_parameter_position_test.go +++ b/mdl/backend/modelsdk/microflow_parameter_position_test.go @@ -3,6 +3,7 @@ package modelsdkbackend import ( + "fmt" "testing" "github.com/mendixlabs/mxcli/model" @@ -18,19 +19,73 @@ func TestMicroflowParameterToGenKeepsAuthoredPosition(t *testing.T) { Name: "Feedback", Position: &model.Point{X: -77, Y: 0}, } - g := microflowParameterToGen(authored, 0, newestFlowVersion).(*genMf.MicroflowParameter) + g := microflowParameterToGen(authored, 0, nil, newestFlowVersion).(*genMf.MicroflowParameter) if got := g.RelativeMiddlePoint(); got != "-77;0" { t.Errorf("authored position = %q, want -77;0", got) } // Control: unannotated parameters still derive from the index. derived := µflows.MicroflowParameter{Name: "Feedback"} - g = microflowParameterToGen(derived, 0, newestFlowVersion).(*genMf.MicroflowParameter) + g = microflowParameterToGen(derived, 0, nil, newestFlowVersion).(*genMf.MicroflowParameter) if got := g.RelativeMiddlePoint(); got != "200;53" { t.Errorf("derived position at index 0 = %q, want 200;53", got) } - g = microflowParameterToGen(derived, 1, newestFlowVersion).(*genMf.MicroflowParameter) + g = microflowParameterToGen(derived, 1, nil, newestFlowVersion).(*genMf.MicroflowParameter) if got := g.RelativeMiddlePoint(); got != "300;53" { t.Errorf("derived position at index 1 = %q, want 300;53", got) } } + +// Parameters with long names were written 100px apart whatever their names +// (mendixlabs/mxcli#1598, case 5): LongParameterNameOne / Two / Three at +// 200;53, 300;53 and 400;53, labels of 20-22 characters overlapping. Studio Pro +// authors space a row by the names: in 95 Studio Pro-authored rows the median +// pitch is 95-106px for names under 15 characters and 160px for 20-24. +func TestDerivedParameterRowIsSpacedByTheWidestName(t *testing.T) { + positions := func(names ...string) []int { + mf := µflows.Microflow{} + for _, n := range names { + mf.Parameters = append(mf.Parameters, µflows.MicroflowParameter{Name: n}) + } + var xs []int + for _, el := range microflowToGen(mf, newestFlowVersion).ObjectCollection().(*genMf.MicroflowObjectCollection).ObjectsItems() { + if p, ok := el.(*genMf.MicroflowParameter); ok { + var x, y int + fmt.Sscanf(p.RelativeMiddlePoint(), "%d;%d", &x, &y) + xs = append(xs, x) + } + } + return xs + } + + long := positions("LongParameterNameOne", "LongParameterNameTwo", "LongParameterNameThree") + // A 22-character label at Studio Pro's ~7px per character. + const widestLabel = 22 * 7 + for i := 1; i < len(long); i++ { + if pitch := long[i] - long[i-1]; pitch < widestLabel { + t.Errorf("parameters %d and %d are %dpx apart, under the %dpx a 22-character name needs (row %v)", + i-1, i, pitch, widestLabel, long) + } + } + + // Control: short names keep the 100px row every earlier write produced, so + // an ordinary flow's parameters do not move. + if short := positions("Customer", "Order"); len(short) != 2 || short[0] != 200 || short[1] != 300 { + t.Errorf("short names moved: %v, want [200 300]", short) + } +} + +// A flow an earlier mxcli wrote has its long-named parameters on the old 100px +// row. That row is mxcli's arithmetic, not a person's placement, so a reader +// must not mistake it for one and pin it. +func TestLegacyParameterRowReadsAsDerived(t *testing.T) { + params := []*microflows.MicroflowParameter{ + {Name: "LongParameterNameOne"}, {Name: "LongParameterNameTwo"}, {Name: "LongParameterNameThree"}, + } + for i := range params { + legacy := model.Point{X: 200 + i*100, Y: 53} + if p := microflows.AuthoredParameterPosition(legacy, i, params); p != nil { + t.Errorf("parameter %d on the old 100px row read as authored at %v", i, *p) + } + } +} diff --git a/mdl/backend/modelsdk/microflow_write.go b/mdl/backend/modelsdk/microflow_write.go index cda3698a76..fe9686c0d7 100644 --- a/mdl/backend/modelsdk/microflow_write.go +++ b/mdl/backend/modelsdk/microflow_write.go @@ -304,7 +304,7 @@ func microflowToGen(mf *microflows.Microflow, v flowVersion) *genMf.Microflow { // Object collection (parameters merged first, then objects). oc := genMf.NewMicroflowObjectCollection() for i, p := range mf.Parameters { - oc.AddObjects(microflowParameterToGen(p, i, v)) + oc.AddObjects(microflowParameterToGen(p, i, mf.Parameters, v)) } if mf.ObjectCollection != nil { for _, obj := range mf.ObjectCollection.Objects { @@ -494,11 +494,17 @@ func annotationFlowToGen(af *microflows.AnnotationFlow, v flowVersion) element.E g.SetID(element.ID(af.ID)) g.SetOriginID(element.ID(af.OriginID)) g.SetDestinationID(element.ID(af.DestinationID)) - // A note sits ABOVE the element it documents, so its line leaves the note's - // bottom edge and enters the element's top. Both indexes were 0 (top), which - // drew the line out of the top of the note and back down around it. - g.SetOriginConnectionIndex(2) - g.SetDestinationConnectionIndex(0) + // The builder picks the sides that face each other (orientAnnotationFlows); + // a flow nothing oriented is drawn as for a note ABOVE its element — out of + // the note's bottom edge, into the element's top. Both indexes were once 0 + // (top), which drew the line out of the top of the note and back down + // around it. + origin, destination := int32(2), int32(0) + if af.Sides != nil { + origin, destination = int32(af.Sides.Origin), int32(af.Sides.Destination) + } + g.SetOriginConnectionIndex(origin) + g.SetDestinationConnectionIndex(destination) // An annotation flow is a Flow like a sequence flow, with the same floors: // the Line was written from 10.0 on, six minors before it existed. if v.hasLine() { @@ -1515,15 +1521,15 @@ func memberChangeToGen(m *microflows.MemberChange) element.Element { } // microflowParameterToGen builds a gen MicroflowParameter. An authored position -// is written as given; without one the position derives from the index, matching -// the legacy serializer. -func microflowParameterToGen(p *microflows.MicroflowParameter, idx int, v flowVersion) element.Element { +// is written as given; without one the position derives from the index and the +// names of all the flow's parameters (params), which space the row. +func microflowParameterToGen(p *microflows.MicroflowParameter, idx int, params []*microflows.MicroflowParameter, v flowVersion) element.Element { g := genMf.NewMicroflowParameter() g.SetID(element.ID(p.ID)) g.SetDocumentation(p.Documentation) g.SetHasVariableNameBeenChanged(false) g.SetName(p.Name) - pos := microflows.DerivedParameterPosition(idx) + pos := microflows.DerivedParameterPosition(idx, params) if p.Position != nil { pos = *p.Position } diff --git a/mdl/backend/modelsdk/nanoflow_write.go b/mdl/backend/modelsdk/nanoflow_write.go index ce1f4ab441..674878f4b2 100644 --- a/mdl/backend/modelsdk/nanoflow_write.go +++ b/mdl/backend/modelsdk/nanoflow_write.go @@ -102,7 +102,7 @@ func nanoflowToGen(nf *microflows.Nanoflow, v flowVersion) *genMf.Nanoflow { oc := genMf.NewMicroflowObjectCollection() for i, p := range nf.Parameters { - oc.AddObjects(microflowParameterToGen(p, i, v)) + oc.AddObjects(microflowParameterToGen(p, i, nf.Parameters, v)) } if nf.ObjectCollection != nil { for _, obj := range nf.ObjectCollection.Objects { diff --git a/mdl/backend/modelsdk/rule_write.go b/mdl/backend/modelsdk/rule_write.go index 57f837166d..169736742d 100644 --- a/mdl/backend/modelsdk/rule_write.go +++ b/mdl/backend/modelsdk/rule_write.go @@ -120,7 +120,7 @@ func ruleToGen(rule *microflows.Rule, v flowVersion) *genMf.Rule { oc := genMf.NewMicroflowObjectCollection() for i, p := range rule.Parameters { - oc.AddObjects(microflowParameterToGen(p, i, v)) + oc.AddObjects(microflowParameterToGen(p, i, rule.Parameters, v)) } if rule.ObjectCollection != nil { for _, obj := range rule.ObjectCollection.Objects { diff --git a/mdl/executor/cmd_alter_flow.go b/mdl/executor/cmd_alter_flow.go index fe9a1aa944..365e18f446 100644 --- a/mdl/executor/cmd_alter_flow.go +++ b/mdl/executor/cmd_alter_flow.go @@ -579,7 +579,12 @@ func (a *alterFlowContext) buildFragment(ctx *ExecContext, body []ast.MicroflowS return nil, fmt.Errorf("every path through the fragment ends the flow with a return, so nothing would lead on " + "to the rest of it; a return can be inserted only on a path that branches off the rest (a guard clause)") } - return cutFragment(oc, fb.fallThroughEndID, fb.returnEndIDs) + frag, err := cutFragment(oc, fb.fallThroughEndID, fb.returnEndIDs) + if err != nil { + return nil, err + } + frag.Pinned = fb.statedNotes + return frag, nil } // cutFragment removes the builder's start event and the end event the body diff --git a/mdl/executor/cmd_flow_modify_patch.go b/mdl/executor/cmd_flow_modify_patch.go index 859c180cba..dcd6cf6eaf 100644 --- a/mdl/executor/cmd_flow_modify_patch.go +++ b/mdl/executor/cmd_flow_modify_patch.go @@ -271,7 +271,7 @@ func parameterMoves(d *flowDecl, stored []*microflows.MicroflowParameter) []flow if sp.Name != p.Name { continue } - cur := microflows.DerivedParameterPosition(i) + cur := microflows.DerivedParameterPosition(i, stored) if sp.Position != nil { cur = *sp.Position } diff --git a/mdl/executor/cmd_microflows_annotation_sides_test.go b/mdl/executor/cmd_microflows_annotation_sides_test.go new file mode 100644 index 0000000000..a8ce66c1dc --- /dev/null +++ b/mdl/executor/cmd_microflows_annotation_sides_test.go @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// A note's line leaves the side of the note that faces its element and enters +// the side of the element that faces the note (mendixlabs/mxcli#1598, case 2). +// The writer stored 2 -> 0 (note bottom -> element top) for every note, so a +// note placed BELOW its activity drew its line out of its own bottom edge and +// round the outside of both boxes. +// +// The expected sides are Studio Pro's, measured rather than assumed: of 523 +// distinct Studio Pro-authored AnnotationFlows (ako/TestApp, PedApp and nine +// customer apps), the element's side follows "vertically separated -> top or +// bottom, else left or right" in 513 (98%), and the numbering is the activity +// one (0 top, 1 right, 2 bottom, 3 left): a note level with its activity on the +// left stores 1 -> 3, on the right 3 -> 1. The issue's guess that a note is +// numbered "top 0, bottom 1, left 2, right 3" does not survive those two. +func TestAnnotationFlowLeavesTheFacingSide(t *testing.T) { + cases := []struct { + name string + note string + origin, dst int + }{ + {"above", "(400, 80)", AnchorBottom, AnchorTop}, + {"below", "(400, 360)", AnchorTop, AnchorBottom}, + {"left", "(120, 200)", AnchorRight, AnchorLeft}, + {"right", "(680, 200)", AnchorLeft, AnchorRight}, + // Diagonal: vertically clear of the activity, so the vertical sides win, + // as they do in Studio Pro's own drawings of a note above and to one side. + {"above-left", "(150, 60)", AnchorBottom, AnchorTop}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fb := buildFlowFromMDL(t, ` + @position(400, 200) + @annotation(text: 'note', position: `+tc.note+`, size: (200, 60)) + log info node 'A' 'a';`) + if len(fb.annotationFlows) != 1 { + t.Fatalf("got %d annotation flows, want 1", len(fb.annotationFlows)) + } + got := fb.annotationFlows[0].Sides + if got == nil { + t.Fatalf("note %s: no sides chosen; the writer falls back to %d -> %d", + tc.note, AnchorBottom, AnchorTop) + } + if got.Origin != tc.origin || got.Destination != tc.dst { + t.Errorf("note %s of an activity at (400, 200): line %d -> %d, want %d -> %d", + tc.note, got.Origin, got.Destination, tc.origin, tc.dst) + } + }) + } +} + +// A note on an activity INSIDE a loop is wired from the parent collection, and +// the two positions are in different coordinate planes: the body's are +// relative to the loop box. Compared raw, a note above the loop would read as +// below an activity whose body-relative y is small. +func TestAnnotationFlowSidesAcrossALoopBoundary(t *testing.T) { + note := µflows.Annotation{BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: "note"}, + Position: model.Point{X: 500, Y: 400}, + Size: model.Size{Width: 200, Height: 60}, + }} + inner := µflows.ActionActivity{BaseActivity: microflows.BaseActivity{BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: "inner"}, + Position: model.Point{X: 200, Y: 80}, // relative to the loop's top-left + Size: model.Size{Width: ActivityWidth, Height: ActivityHeight}, + }}} + loop := µflows.LoopedActivity{ + BaseMicroflowObject: microflows.BaseMicroflowObject{ + BaseElement: model.BaseElement{ID: "loop"}, + Position: model.Point{X: 500, Y: 200}, + Size: model.Size{Width: 400, Height: 160}, // top-left (300, 120) + }, + ObjectCollection: µflows.MicroflowObjectCollection{Objects: []microflows.MicroflowObject{inner}}, + } + af := µflows.AnnotationFlow{OriginID: "note", DestinationID: "inner"} + orientAnnotationFlows([]microflows.MicroflowObject{note, loop}, []*microflows.AnnotationFlow{af}) + // The inner activity is at absolute (500, 200); the note, at y=400, is below it. + if af.Sides == nil || af.Sides.Origin != AnchorTop || af.Sides.Destination != AnchorBottom { + t.Fatalf("sides %+v, want %d -> %d (note below the loop-body activity)", af.Sides, AnchorTop, AnchorBottom) + } +} + +// A note whose position the script states is recorded, so a spliced fragment +// keeps it where it is instead of moving it with the fragment +// (mendixlabs/mxcli#1598, case 1); a note placed by the layout is not. +func TestStatedNotesAreRecordedForTheSplice(t *testing.T) { + fb := buildFlowFromMDL(t, ` + @annotation(text: 'stated', position: (1500, 600), size: (200, 80)) + log info node 'A' 'a'; + @annotation(text: 'placed', size: (200, 80)) + log info node 'B' 'b';`) + stated := map[string]bool{} + for _, o := range fb.objects { + if n, ok := o.(*microflows.Annotation); ok { + stated[n.Caption] = fb.statedNotes[n.ID] + } + } + if !stated["stated"] || stated["placed"] { + t.Errorf("recorded as stated: %v, want only the note with a position", stated) + } +} diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index 89bcc35c2b..a518e6f014 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -93,6 +93,16 @@ type flowBuilder struct { // splitID→nextActivity flow — which is emitted one iteration later by the // outer loop, not by addIfStatement. nextFlowAnchor *ast.FlowAnchors + + // atBodyTail is set while the LAST statement of a loop body is built, and + // passed on to the last statement of each branch of an IF there. An IF/ELSE + // at the tail joins nothing — both branches end the iteration — so it gets + // no merge (mendixlabs/mxcli#884). Whoever reads it clears it. + atBodyTail bool + + // statedNotes are the notes whose position the script states, which a + // spliced fragment pins in place (see recordStatedNote). + statedNotes map[model.ID]bool // curveByOrigin holds each statement's @curve keyed by the activity it was // written on, applied to that activity's outgoing flows by applyFlowCurves // once the graph is complete. (#884) diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index c27b1e3213..818487e212 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -557,11 +557,15 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID { } origins := enumSplitOriginAnchors(branchYs, centerY) + var caseAnchors []ast.CaseAnchor + if s.Annotations != nil { + caseAnchors = s.Annotations.CaseAnchors + } slot := func(i int) splitCaseSlot { if origins == nil { - return splitCaseSlot{order: i, origin: -1} + return splitCaseSlot{order: i, origin: -1, anchors: caseAnchors} } - return splitCaseSlot{order: i, origin: origins[i]} + return splitCaseSlot{order: i, origin: origins[i], anchors: caseAnchors} } savedEndsWithReturn := fb.endsWithReturn @@ -609,6 +613,11 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID { // through describe → exec without silently dropping. if thisAnchor != nil && len(fb.flows) > 0 { applyUserAnchors(fb.flows[len(fb.flows)-1], nil, thisAnchor) + // The outcome's own `to:` on the CASE is the more specific + // of the two, so it wins over the first statement's. + if len(br.values) == 1 { + slot(i).applyCaseAnchor(fb.flows[len(fb.flows)-1], br.values[0]) + } } } else { var flow *microflows.SequenceFlow @@ -665,6 +674,9 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID { fb.endsWithReturn = true } else { fb.nextConnectionPoint = ensureMerge().ID + // `@anchor(from: X)` on a CASE is the flow leaving it — out of its + // merge — as on an IF (#767). It was accepted and dropped (#1587). + fb.nextFlowAnchor = mergeExitFlowAnchor(s) } return splitID } @@ -918,6 +930,7 @@ func (fb *flowBuilder) addEnumSplitFlows(originID, destinationID model.ID, value for _, value := range values { flow := newHorizontalFlowWithEnumCase(originID, destinationID, value) slot.apply(flow, split, target) + slot.applyCaseAnchor(flow, value) fb.flows = append(fb.flows, flow) } } @@ -928,12 +941,43 @@ func (fb *flowBuilder) addEnumSplitFlows(originID, destinationID model.ID, value type splitCaseSlot struct { order int origin int + // anchors are the CASE's `@anchor(A: (from: …, to: …))` entries. + anchors []ast.CaseAnchor +} + +// applyCaseAnchor sets the sides the CASE's @anchor names for this value's +// flow, over the defaults apply chose (mendixlabs/mxcli#1587). +func (slot splitCaseSlot) applyCaseAnchor(flow *microflows.SequenceFlow, value string) { + if fa := caseAnchorFor(slot.anchors, value); fa != nil { + applyUserAnchors(flow, fa, fa) + } +} + +// caseAnchorFor returns the anchors written for value, matched as WHEN values +// are: case-insensitively, quotes ignored. +func caseAnchorFor(anchors []ast.CaseAnchor, value string) *ast.FlowAnchors { + for _, ca := range anchors { + if sameCaseValue(ca.Value, value) { + return ca.Anchors + } + } + return nil +} + +func sameCaseValue(a, b string) bool { + return strings.EqualFold(strings.Trim(a, "\"`"), strings.Trim(b, "\"`")) } // apply sets the flow's sides. split and target are where the two ends actually are, // which is not where the stack was planned once a branch carries @position: a line is // only sent out of the top corner towards something above the split, and out of the // bottom towards something below, whatever third the case was counted into. +// +// Up to three cases keep the pair table's side whatever the geometry, because that +// side is how DESCRIBE reads the WHEN order back (splitCaseOrder), and the order has +// to survive a branch moved anywhere with @position. Holding them to the geometry +// rule reordered a CASE whose branches were placed out of order. A side that +// differs is written explicitly, `@anchor(A: (from: right))` (mendixlabs/mxcli#1587). func (slot splitCaseSlot) apply(flow *microflows.SequenceFlow, split, target model.Point) { if flow == nil { return diff --git a/mdl/executor/cmd_microflows_builder_annotations.go b/mdl/executor/cmd_microflows_builder_annotations.go index 8402bacc8b..b5b2f3c5b7 100644 --- a/mdl/executor/cmd_microflows_builder_annotations.go +++ b/mdl/executor/cmd_microflows_builder_annotations.go @@ -86,6 +86,9 @@ func (fb *flowBuilder) mergeStatementAnnotations(stmt ast.MicroflowStatement) { if ann.BodyTailAnchor != nil { fb.pendingAnnotations.BodyTailAnchor = ann.BodyTailAnchor } + if len(ann.CaseAnchors) > 0 { + fb.pendingAnnotations.CaseAnchors = ann.CaseAnchors + } } // applyAnnotations applies pending annotations to the activity identified by activityID. @@ -361,6 +364,19 @@ func (fb *flowBuilder) attachAnnotation(note ast.MicroflowAnnotation, activityID fb.linkAnnotation(fb.newAnnotation(note, pos, size).ID, activityID) } +// recordStatedNote remembers a note whose position the script states. A +// spliced fragment keeps such a note where it is instead of moving it with +// the fragment (MicroflowFragment.Pinned). +func (fb *flowBuilder) recordStatedNote(note ast.MicroflowAnnotation, id model.ID) { + if note.Position == nil { + return + } + if fb.statedNotes == nil { + fb.statedNotes = map[model.ID]bool{} + } + fb.statedNotes[id] = true +} + // attachFreeAnnotation creates a free-floating Annotation not connected to any // activity. A label on one is accepted and reused, so a note can be shared // between the canvas and an activity. @@ -396,6 +412,7 @@ func (fb *flowBuilder) newAnnotation(note ast.MicroflowAnnotation, pos model.Poi Caption: note.Text, } fb.objects = append(fb.objects, annotation) + fb.recordStatedNote(note, annotation.ID) if note.Label != "" { if fb.annotationsByLabel == nil { fb.annotationsByLabel = map[string]*microflows.Annotation{} @@ -405,6 +422,71 @@ func (fb *flowBuilder) newAnnotation(note ast.MicroflowAnnotation, pos model.Poi return annotation } +// orientAnnotationFlows picks, for every note line that has no sides yet, the +// side of the note that faces its element and the side of the element that +// faces the note (mendixlabs/mxcli#1598, case 2). It runs once every object has +// its final position, since a note is placed before rows and containers settle. +// +// Studio Pro's own drawings are the rule: of 523 distinct note lines in +// Studio Pro-authored apps, the element's side is top or bottom whenever the +// two boxes are vertically clear of each other, and left or right only when +// they overlap vertically — 98% agreement. Writing 2 -> 0 for every note drew +// a note below its activity with a line that leaves its own bottom edge. +func orientAnnotationFlows(objects []microflows.MicroflowObject, flows []*microflows.AnnotationFlow) { + boxes := map[model.ID]canvasBox{} + collectCanvasBoxes(objects, model.Point{}, boxes) + for _, af := range flows { + if af.Sides != nil { + continue + } + note, ok1 := boxes[af.OriginID] + target, ok2 := boxes[af.DestinationID] + if !ok1 || !ok2 { + continue + } + af.Sides = facingSides(note, target) + } +} + +// canvasBox is an object's box in absolute canvas coordinates. +type canvasBox struct{ left, top, right, bottom int } + +// collectCanvasBoxes records the absolute box of every object, descending into +// loop bodies. A loop's children are positioned relative to the loop box's +// top-left corner, and a note outside a loop may be wired to an activity +// inside it, so comparing raw positions would mix two planes. +func collectCanvasBoxes(objects []microflows.MicroflowObject, origin model.Point, out map[model.ID]canvasBox) { + for _, o := range objects { + if o == nil { + continue + } + p, sz := o.GetPosition(), objectSize(o) + x, y := origin.X+p.X, origin.Y+p.Y + out[o.GetID()] = canvasBox{x - sz.Width/2, y - sz.Height/2, x + sz.Width/2, y + sz.Height/2} + if loop, ok := o.(*microflows.LoopedActivity); ok && loop.ObjectCollection != nil { + collectCanvasBoxes(loop.ObjectCollection.Objects, + model.Point{X: x - sz.Width/2, Y: y - sz.Height/2}, out) + } + } +} + +// facingSides returns the sides at which a line from note to target attaches. +// Vertical separation wins over horizontal; boxes that overlap fall back to the +// note-above drawing the writer has always used. +func facingSides(note, target canvasBox) *microflows.ConnectionSides { + switch { + case note.bottom < target.top: + return µflows.ConnectionSides{Origin: AnchorBottom, Destination: AnchorTop} + case note.top > target.bottom: + return µflows.ConnectionSides{Origin: AnchorTop, Destination: AnchorBottom} + case note.right < target.left: + return µflows.ConnectionSides{Origin: AnchorRight, Destination: AnchorLeft} + case note.left > target.right: + return µflows.ConnectionSides{Origin: AnchorLeft, Destination: AnchorRight} + } + return µflows.ConnectionSides{Origin: AnchorBottom, Destination: AnchorTop} +} + func (fb *flowBuilder) linkAnnotation(annotationID, activityID model.ID) { fb.annotationFlows = append(fb.annotationFlows, µflows.AnnotationFlow{ BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, diff --git a/mdl/executor/cmd_microflows_builder_calls.go b/mdl/executor/cmd_microflows_builder_calls.go index 1953ffc269..2252e93a07 100644 --- a/mdl/executor/cmd_microflows_builder_calls.go +++ b/mdl/executor/cmd_microflows_builder_calls.go @@ -687,7 +687,7 @@ func (fb *flowBuilder) addCallWebServiceAction(s *ast.CallWebServiceStmt) model. } if s.ErrorHandling != nil && len(s.ErrorHandling.Body) > 0 { - errorY := fb.posY + VerticalSpacing + errorY := fb.posY + ErrorHandlerPitch mergeID := fb.addErrorHandlerFlow(activity.ID, activityX, s.ErrorHandling.Body) fb.handleErrorHandlerMerge(mergeID, activity.ID, errorY) } diff --git a/mdl/executor/cmd_microflows_builder_control.go b/mdl/executor/cmd_microflows_builder_control.go index a032231ff0..7cb5719e80 100644 --- a/mdl/executor/cmd_microflows_builder_control.go +++ b/mdl/executor/cmd_microflows_builder_control.go @@ -37,6 +37,20 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID { thenNeedsErrorMerge := thenReturns && bodyHasContinuingCustomErrorHandler(s.ThenBody) elseNeedsErrorMerge := elseReturns && bodyHasContinuingCustomErrorHandler(s.ElseBody) + // The last statement of a loop body: an IF/ELSE whose branches both carry + // on there joins nothing, since both end the iteration, and a loop-body + // activity with no outgoing flow is how Mendix ends one. Its merge had no + // way out, Studio Pro authors delete it by hand, and every re-run drew it + // again (mendixlabs/mxcli#884). Both branches must hold something — a split + // needs a flow per case — and an explicit @merge keeps the merge, which is + // how DESCRIBE carries a stored one through a rebuild. + atTail := fb.atBodyTail + fb.atBodyTail = false + tailWithoutMerge := atTail && hasElseBody && len(s.ThenBody) > 0 && len(s.ElseBody) > 0 && + !thenReturns && !elseReturns && + !bodyHasContinuingCustomErrorHandler(s.ThenBody) && !bodyHasContinuingCustomErrorHandler(s.ElseBody) && + (s.Annotations == nil || s.Annotations.Merge == nil) + // Save/restore endsWithReturn around branch processing to avoid // a branch's RETURN affecting the parent flow state prematurely savedEndsWithReturn := fb.endsWithReturn @@ -120,7 +134,7 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID { // Determine if the merge would have 2+ incoming edges (non-redundant). // Skip merge when only one branch flows into it (the other returns). needMerge := false - if !bothReturn { + if !bothReturn && !tailWithoutMerge { if hasElseBody { needMerge = (!thenReturns && !elseReturns) || thenNeedsErrorMerge || elseNeedsErrorMerge } else { @@ -159,7 +173,9 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID { var pendingThenAnchor *ast.FlowAnchors for i, stmt := range s.ThenBody { thisAnchor := stmtOwnAnchor(stmt) + fb.atBodyTail = tailWithoutMerge && i == len(s.ThenBody)-1 actID := fb.addStatement(stmt) + fb.atBodyTail = false if fb.takeBranchJoin(lastThenID, splitID, "true", pendingThenCase, prevThenAnchor, trueBranchAnchor) { pendingThenCase, pendingThenAnchor = "", nil continue @@ -255,7 +271,9 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID { var pendingElseAnchor *ast.FlowAnchors for i, stmt := range s.ElseBody { thisAnchor := stmtOwnAnchor(stmt) + fb.atBodyTail = tailWithoutMerge && i == len(s.ElseBody)-1 actID := fb.addStatement(stmt) + fb.atBodyTail = false if fb.takeBranchJoin(lastElseID, splitID, "false", pendingElseCase, prevElseAnchor, falseBranchAnchor) { pendingElseCase, pendingElseAnchor = "", nil continue @@ -533,9 +551,10 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID { return splitID } -// mergeExitFlowAnchor is the origin anchor an if's own `@anchor(from: X)` gives -// the flow leaving its closing merge, or nil when it sets none. -func mergeExitFlowAnchor(s *ast.IfStmt) *ast.FlowAnchors { +// mergeExitFlowAnchor is the origin anchor an if's or a case's own +// `@anchor(from: X)` gives the flow leaving its closing merge, or nil when it +// sets none. +func mergeExitFlowAnchor(s ast.MicroflowStatement) *ast.FlowAnchors { own := stmtOwnAnchor(s) if own == nil || own.From == ast.AnchorSideUnset { return nil @@ -606,10 +625,10 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID { // First, measure the loop body to determine size bodyBounds := fb.measurer.measureStatementsSpan(s.Body) - // Calculate loop box size with padding - // Extra width for iterator icon and its label (100 pixels) - iteratorSpace := 100 - loopWidth := max(bodyBounds.Width+2*LoopPadding+iteratorSpace, MinLoopWidth) + // Calculate loop box size with padding, plus the iterator column. The + // measurer uses the same constant, so an IF holding this loop measures it at + // the width it is built at. + loopWidth := max(bodyBounds.Width+2*LoopPadding+LoopIteratorSpace, MinLoopWidth) loopHeight := max(bodyBounds.Height+2*LoopPadding, MinLoopHeight) // Inner positioning: activities start after the iterator icon on the left, @@ -617,10 +636,10 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID { // (IF/CASE) centred on innerStartY stays within [0, loopHeight]. // // A position is a CENTRE, so the first activity has to start half its own - // width past the iterator column; at LoopPadding+iteratorSpace its left edge + // width past the iterator column; at LoopPadding+LoopIteratorSpace its left edge // landed 60px inside that column, on top of the iterator's icon and the list // variable's label. - innerStartX := LoopPadding + iteratorSpace + ActivityWidth/2 + innerStartX := LoopPadding + LoopIteratorSpace + ActivityWidth/2 innerStartY := loopHeight / 2 // posX is where the builder would CENTRE the next element. A loop box placed @@ -683,8 +702,10 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID { // its false flow entirely and mx check reports CE0079 (ledger #52). var lastBodyID model.ID pendingCase := "" - for _, stmt := range s.Body { + for i, stmt := range s.Body { + loopBuilder.atBodyTail = i == len(s.Body)-1 actID := loopBuilder.addStatement(stmt) + loopBuilder.atBodyTail = false if actID != "" { loopBuilder.applyPendingAnnotations(actID) if lastBodyID != "" { @@ -1049,8 +1070,10 @@ func (fb *flowBuilder) addWhileStatement(s *ast.WhileStmt) model.ID { // since ledger #52 and this one did not, so the two disagreed on the same body. var lastBodyID model.ID pendingCase := "" - for _, stmt := range s.Body { + for i, stmt := range s.Body { + loopBuilder.atBodyTail = i == len(s.Body)-1 actID := loopBuilder.addStatement(stmt) + loopBuilder.atBodyTail = false if actID != "" { loopBuilder.applyPendingAnnotations(actID) if lastBodyID != "" { diff --git a/mdl/executor/cmd_microflows_builder_flows.go b/mdl/executor/cmd_microflows_builder_flows.go index 3f159724ee..1d281cbf3e 100644 --- a/mdl/executor/cmd_microflows_builder_flows.go +++ b/mdl/executor/cmd_microflows_builder_flows.go @@ -86,6 +86,40 @@ func (fb *flowBuilder) rejoinMergePosition(source model.ID, computed model.Point return computed } +// mergeOnEdge places a merge inserted into the normal edge from originID to +// destinationID: midway between the right edge of the one and the left edge of +// the other, on the line of the node it enters. +// +// The fallback — the layout cursor less half a spacing — is only right while +// the cursor still points at the successor, and by the time a rejoin is wired +// the successor has been placed and the cursor moved past it. The merge then +// landed ON an end event (same centre, the end inside the 40px merge) and +// RIGHT of an activity, so the edge into it ran backwards (mendixlabs/mxcli#884, +// ako/mxcli#1273). The fallback still covers an edge that does not run left to +// right, such as one that wraps onto the next row. +func (fb *flowBuilder) mergeOnEdge(originID, destinationID model.ID, fallback model.Point) model.Point { + origin, dest := fb.objectByID(originID), fb.objectByID(destinationID) + if origin == nil || dest == nil { + return fallback + } + right := origin.GetPosition().X + objectSize(origin).Width/2 + left := dest.GetPosition().X - objectSize(dest).Width/2 + if left <= right { + return fallback + } + return model.Point{X: (right + left) / 2, Y: dest.GetPosition().Y} +} + +// objectByID returns this builder's object with the given ID, or nil. +func (fb *flowBuilder) objectByID(id model.ID) microflows.MicroflowObject { + for i := len(fb.objects) - 1; i >= 0; i-- { + if o := fb.objects[i]; o != nil && o.GetID() == id { + return o + } + } + return nil +} + func isEmptyCustomErrorHandler(eh *ast.ErrorHandlingClause) bool { if eh == nil || len(eh.Body) != 0 { return false @@ -392,7 +426,7 @@ func (fb *flowBuilder) addEmptyErrorHandlerRejoinFlowFrom(normalOriginID, errorO merge := µflows.ExclusiveMerge{ BaseMicroflowObject: microflows.BaseMicroflowObject{ BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, - Position: fb.rejoinMergePosition(errorOriginID, model.Point{X: fb.posX - HorizontalSpacing/2, Y: fb.baseY}), + Position: fb.rejoinMergePosition(errorOriginID, fb.mergeOnEdge(normalOriginID, destinationID, model.Point{X: fb.posX - HorizontalSpacing/2, Y: fb.baseY})), Size: model.Size{Width: MergeSize, Height: MergeSize}, }, } @@ -445,7 +479,7 @@ func (fb *flowBuilder) addErrorHandlerRejoinFlowForState(state pendingErrorHandl merge := µflows.ExclusiveMerge{ BaseMicroflowObject: microflows.BaseMicroflowObject{ BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, - Position: fb.rejoinMergePosition(state.source, model.Point{X: fb.posX - HorizontalSpacing/2, Y: fb.baseY}), + Position: fb.rejoinMergePosition(state.source, fb.mergeOnEdge(originID, destinationID, model.Point{X: fb.posX - HorizontalSpacing/2, Y: fb.baseY})), Size: model.Size{Width: MergeSize, Height: MergeSize}, }, } @@ -733,7 +767,7 @@ func (fb *flowBuilder) addErrorHandlerFlow(sourceActivityID model.ID, sourceX in } // Position error handler below the main flow - errorY := fb.posY + VerticalSpacing + errorY := fb.posY + ErrorHandlerPitch errorX := sourceX // Build error handler activities @@ -841,6 +875,14 @@ func (fb *flowBuilder) addErrorHandlerFlow(sourceActivityID model.ID, sourceX in fb.flows = append(fb.flows, errBuilder.flows...) fb.annotationFlows = append(fb.annotationFlows, errBuilder.annotationFlows...) fb.errors = append(fb.errors, errBuilder.errors...) + // The handler's objects land in this collection, so its stated notes are + // this builder's to pin too. + for id := range errBuilder.statedNotes { + if fb.statedNotes == nil { + fb.statedNotes = map[model.ID]bool{} + } + fb.statedNotes[id] = true + } // A @curve inside the handler is recorded on errBuilder, and applied by the // builder that owns the flows once its graph is complete — this one, since // the handler's flows (and the edge that rejoins the main flow) land here. @@ -1196,10 +1238,21 @@ func (fb *flowBuilder) mergeOverConnectedEndEvents() { if len(idxs) < 2 { continue } + // On the edge of the path that arrives along the end event's own line, so + // the merge does not land on the end event (see mergeOnEdge). + pos := model.Point{X: fb.posX - HorizontalSpacing/2, Y: fb.baseY} + if end := fb.objectByID(endID); end != nil { + for _, i := range idxs { + if o := fb.objectByID(fb.flows[i].OriginID); o != nil && o.GetPosition().Y == end.GetPosition().Y { + pos = fb.mergeOnEdge(o.GetID(), endID, pos) + break + } + } + } merge := µflows.ExclusiveMerge{ BaseMicroflowObject: microflows.BaseMicroflowObject{ BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, - Position: model.Point{X: fb.posX - HorizontalSpacing/2, Y: fb.baseY}, + Position: pos, Size: model.Size{Width: MergeSize, Height: MergeSize}, }, } diff --git a/mdl/executor/cmd_microflows_builder_graph.go b/mdl/executor/cmd_microflows_builder_graph.go index 90dbaf9bcc..1a4ca5122d 100644 --- a/mdl/executor/cmd_microflows_builder_graph.go +++ b/mdl/executor/cmd_microflows_builder_graph.go @@ -266,6 +266,10 @@ func (fb *flowBuilder) buildFlowGraph(stmts []ast.MicroflowStatement, returns *a // this is the first point where a row's real depth is known. fb.separateRows() + // Note lines after everything has moved: the sides that face each other + // depend on where the note and its element finally sit. + orientAnnotationFlows(fb.objects, fb.annotationFlows) + return µflows.MicroflowObjectCollection{ BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, Objects: fb.objects, @@ -605,7 +609,7 @@ func (fb *flowBuilder) addTerminalEndEventForPendingHandler(returns *ast.Microfl end := µflows.EndEvent{ BaseMicroflowObject: microflows.BaseMicroflowObject{ BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, - Position: model.Point{X: fb.posX + HorizontalSpacing/2, Y: fb.baseY + VerticalSpacing}, + Position: model.Point{X: fb.posX + HorizontalSpacing/2, Y: fb.baseY + ErrorHandlerPitch}, Size: model.Size{Width: EventSize, Height: EventSize}, }, ReturnValue: returnValue, @@ -616,6 +620,13 @@ func (fb *flowBuilder) addTerminalEndEventForPendingHandler(returns *ast.Microfl // addStatement converts an AST statement to a microflow activity and returns its ID. func (fb *flowBuilder) addStatement(stmt ast.MicroflowStatement) model.ID { + // The loop-body tail is this statement's alone: only an IF reads it, and a + // CASE or anything else must not leave it set for an IF nested inside, + // which is not at the tail (atBodyTail). + if _, isIf := stmt.(*ast.IfStmt); !isIf { + fb.atBodyTail = false + } + // Extract annotations from the statement and merge into pendingAnnotations fb.mergeStatementAnnotations(stmt) diff --git a/mdl/executor/cmd_microflows_case_outcome_anchor_test.go b/mdl/executor/cmd_microflows_case_outcome_anchor_test.go new file mode 100644 index 0000000000..d3d1250234 --- /dev/null +++ b/mdl/executor/cmd_microflows_case_outcome_anchor_test.go @@ -0,0 +1,188 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// A CASE stored each outcome's side by WHEN order alone — 1st top, 2nd right, +// 3rd bottom — and MDL had no way to say otherwise (mendixlabs/mxcli#1587, +// ako/mxcli#1492): `@anchor(A: (from: right))` was refused with MDL092 and +// `@anchor(from: right)` on the CASE was accepted and ignored. The WHEN-order +// default stays for up to three cases — it is how DESCRIBE reads the case order +// back (TestEnumSplitRoundtripPreservesEmptyCaseBeforeNamedCases) — so the +// per-outcome form is how a script, and a description, says otherwise. + +const caseOutcomeMDL = ` + case $E + when A then + log info node 'T' 'a'; + when B then + log info node 'T' 'b'; + when (empty) then + log info node 'T' 'empty'; + end case; + log info node 'T' 'after';` + +// caseOutcomeSides returns, per case value, the sides of the flow leaving the split. +func caseOutcomeSides(t *testing.T, fb *flowBuilder) map[string][2]int { + t.Helper() + var splitID model.ID + for _, o := range fb.objects { + if _, ok := o.(*microflows.ExclusiveSplit); ok { + splitID = o.GetID() + } + } + out := map[string][2]int{} + for _, f := range fb.flows { + if f.OriginID != splitID { + continue + } + if v, ok := enumCaseValue(f); ok { + out[v] = [2]int{f.OriginConnectionIndex, f.DestinationConnectionIndex} + } + } + if len(out) == 0 { + t.Fatal("no case flows built") + } + return out +} + +func TestCaseOutcomeAnchorChoosesEachOutcomesSide(t *testing.T) { + fb := buildFlowFromMDL(t, " @anchor(A: (from: right), (empty): (from: bottom, to: top))"+caseOutcomeMDL) + if len(fb.errors) > 0 { + t.Fatalf("build errors: %v", fb.errors) + } + got := caseOutcomeSides(t, fb) + if got["A"][0] != AnchorRight { + t.Errorf("A leaves side %d, want right (%d) as written", got["A"][0], AnchorRight) + } + if got["(empty)"] != [2]int{AnchorBottom, AnchorTop} { + t.Errorf("(empty) runs %d -> %d, want bottom -> top (%d -> %d) as written", + got["(empty)"][0], got["(empty)"][1], AnchorBottom, AnchorTop) + } + // An outcome the annotation does not name keeps its default. + if got["B"][0] != AnchorRight { + t.Errorf("B leaves side %d, want its default right (%d)", got["B"][0], AnchorRight) + } +} + +// A keyword as an enumeration value is still a key: `Open` is a lexer keyword, +// and a keyword key used to fall through to the positional form and be refused. +func TestCaseOutcomeAnchorAcceptsAKeywordValue(t *testing.T) { + fb := buildFlowFromMDL(t, ` + @anchor(Open: (from: bottom)) + case $State + when Open then log info node 'T' 'o'; + when Closed then log info node 'T' 'c'; + end case;`) + if got := caseOutcomeSides(t, fb); got["Open"][0] != AnchorBottom { + t.Errorf("Open leaves side %d, want bottom (%d) as written", got["Open"][0], AnchorBottom) + } +} + +// `@anchor(from: …)` on a CASE is the flow leaving it — out of its closing +// merge — exactly as on an IF (#767). It was accepted and dropped. +func TestCaseFromAnchorIsTheFlowLeavingItsMerge(t *testing.T) { + fb := buildFlowFromMDL(t, " @anchor(from: bottom)"+caseOutcomeMDL) + var merge model.ID + for _, o := range fb.objects { + if _, ok := o.(*microflows.ExclusiveMerge); ok { + merge = o.GetID() + } + } + for _, f := range fb.flows { + if f.OriginID == merge { + if f.OriginConnectionIndex != AnchorBottom { + t.Errorf("the flow leaving the CASE's merge starts at side %d, want bottom (%d)", f.OriginConnectionIndex, AnchorBottom) + } + return + } + } + t.Fatal("no flow leaves the CASE's merge") +} + +// DESCRIBE writes an outcome's side only where it differs from what the +// builder would choose, so a description rebuilds the same sides and an +// ordinary CASE gains no annotation. +func TestDescribeCaseOutcomeAnchorsRoundTrip(t *testing.T) { + const src = `create microflow M.Case ($E: M.Kind) returns Boolean +begin + @anchor(A: (from: right), (empty): (from: bottom, to: top)) + case $E + when A then + log info node 'T' 'a'; + when B then + log info node 'T' 'b'; + when (empty) then + log info node 'T' 'empty'; + end case; + return true; +end;` + out, oc := describeBuilt(t, src) + // The (empty) outcome's `to: top` lands on its first activity, which says + // so itself as `@anchor(to: top)`; the CASE only carries what no statement + // can. Its `from: bottom` is the third outcome's default, so it is not + // restated, and neither is anything about B. + for _, want := range []string{"A: (from: right)", "@anchor(to: top)"} { + if !strings.Contains(out, want) { + t.Errorf("description lacks %q:\n%s", want, out) + } + } + for _, unwanted := range []string{"B: (", "(empty): ("} { + if strings.Contains(out, unwanted) { + t.Errorf("description restates a default with %q:\n%s", unwanted, out) + } + } + again, _ := describeBuilt(t, "create microflow M.Case ($E: M.Kind) returns Boolean\nbegin\n"+out+"\nend;") + if again != out { + t.Errorf("not a fixed point:\nfirst:\n%s\nsecond:\n%s", out, again) + } + _ = oc + + plain, _ := describeBuilt(t, "create microflow M.Case ($E: M.Kind) returns Boolean\nbegin\n"+caseOutcomeMDL+"\n return true;\nend;") + if strings.Contains(plain, "@anchor(") { + t.Errorf("an unannotated CASE is described with an anchor:\n%s", plain) + } +} + +func TestCaseOutcomeAnchorValidation(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + {"unknown value", " @anchor(Z: (from: right))" + caseOutcomeMDL, "Z"}, + {"true: on a CASE", " @anchor(true: (from: right))" + caseOutcomeMDL, "true"}, + {"value key on an IF", ` + @anchor(A: (from: right)) + if $F then log info node 'T' 'x'; end if;`, "A"}, + } { + t.Run(tc.name, func(t *testing.T) { + var found bool + for _, v := range checkMicroflowSource(t, "create microflow M.F ($E: M.Kind, $F: Boolean)\nbegin\n"+tc.src+"\nend;") { + if v.RuleID == invalidAnchorRule && strings.Contains(v.Message, tc.want) { + found = true + } + } + if !found { + t.Errorf("no %s refusal naming %q", invalidAnchorRule, tc.want) + } + }) + } +} + +// The CASE's own `from:` round-trips like an IF's: describe reads it off the +// flow leaving the merge. +func TestDescribeCaseFromAnchorRoundTrips(t *testing.T) { + src := "create microflow M.Case ($E: M.Kind) returns Boolean\nbegin\n @anchor(from: bottom)" + caseOutcomeMDL + "\n return true;\nend;" + out, _ := describeBuilt(t, src) + if !strings.Contains(out, "from: bottom") { + t.Errorf("the CASE's from: is not described:\n%s", out) + } + again, _ := describeBuilt(t, "create microflow M.Case ($E: M.Kind) returns Boolean\nbegin\n"+out+"\nend;") + if again != out { + t.Errorf("not a fixed point:\nfirst:\n%s\nsecond:\n%s", out, again) + } +} diff --git a/mdl/executor/cmd_microflows_derived_layout.go b/mdl/executor/cmd_microflows_derived_layout.go index aeaf1ced97..e2f2fae8ee 100644 --- a/mdl/executor/cmd_microflows_derived_layout.go +++ b/mdl/executor/cmd_microflows_derived_layout.go @@ -263,7 +263,11 @@ func (m *flowAnalysisMemo) splitMergePoints(oc *microflows.MicroflowObjectCollec // mergeLabels is the labelling of a flow's merges through the memo. func (m *flowAnalysisMemo) mergeLabels(oc *microflows.MicroflowObjectCollection) mergeLabels { - compute := func() mergeLabels { return mergeAllLabels(labelRejoinMerges(oc), labelCrossedMerges(oc)) } + compute := func() mergeLabels { + l := mergeAllLabels(labelRejoinMerges(oc), labelCrossedMerges(oc)) + l.inline = newInlineMerges(oc) + return l + } if m == nil { return compute() } diff --git a/mdl/executor/cmd_microflows_describe_irreducible_test.go b/mdl/executor/cmd_microflows_describe_irreducible_test.go index 1b9529cab4..bc24707ba7 100644 --- a/mdl/executor/cmd_microflows_describe_irreducible_test.go +++ b/mdl/executor/cmd_microflows_describe_irreducible_test.go @@ -209,3 +209,27 @@ end;` } } } + +// `while true` with early returns, from saingge's re-test of mendixlabs/mxcli#923: +// describe -> exec wrote nothing, yet describe printed three "NOT equivalent ... +// must not be re-executed" warnings, one per decision, and lint raised +// MDL-FLOW01 three times. The graph analysis walked each decision's false +// branch round the loop's back edge into its own true branch. +func TestDescribeIrreducible_WhileTrueWithEarlyReturnsHasNoWarning(t *testing.T) { + const src = `create microflow M.ACT_Loop ($Code: string) returns string as $R +begin + declare $Status string = 'new'; + while true + begin + if $Code = 'a' then return 'A'; end if; + if $Code = 'b' then return 'B'; end if; + if $Code = 'c' then return 'C'; end if; + set $Status = 'x'; + end while; +end;` + out, oc := describeBuilt(t, src) + if strings.Contains(out, "WARNING") { + t.Errorf("a while-true with early returns is described with a warning:\n%s", out) + } + assertDescriptionRebuildsGraph(t, out, oc) +} diff --git a/mdl/executor/cmd_microflows_describe_single_inbound_merge_test.go b/mdl/executor/cmd_microflows_describe_single_inbound_merge_test.go new file mode 100644 index 0000000000..556a594761 --- /dev/null +++ b/mdl/executor/cmd_microflows_describe_single_inbound_merge_test.go @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// A merge with ONE incoming path is a node mxcli's own `exec` writes from +// ordinary MDL — `join finish; merge finish;` — and DESCRIBE walked straight +// through it, printing a warning that re-executing the description deletes it +// (mendixlabs/mxcli#923, ako/mxcli#1277). On the main line it is represented +// the way it was written, so the description rebuilds it and has nothing to +// warn about. +func TestDescribe_SingleInboundMergeOnTheMainLineIsRepresented(t *testing.T) { + const src = `create microflow M.MF_RT_Chain () returns string as $Out +begin + declare $Out string = 'a'; + if 1 = 1 then + if 2 = 2 then set $Out = 'healthy'; join emit; + else set $Out = 'watch'; join review; end if; + else set $Out = 'low'; join review; end if; + merge review; log info node 'T' 'review'; join emit; + merge emit; log info node 'T' 'emit'; join finish; + merge finish; return $Out; +end;` + out, oc := describeBuilt(t, src) + if strings.Contains(out, "WARNING") { + t.Errorf("the description still warns:\n%s", out) + } + assertDescriptionRebuildsGraph(t, out, oc) + if got, want := countMerges(rebuildDescription(t, out)), countMerges(oc); got != want { + t.Errorf("the description rebuilds %d merges, the flow has %d:\n%s", got, want, out) + } +} + +// The simplest shape: one activity, a one-input merge, the end. Also the +// fixture of TestDroppedMergeWarnings_PassThroughMergeIsFlagged, built from MDL. +func TestDescribe_SingleInboundMergeAfterAnActivityIsRepresented(t *testing.T) { + const src = `create microflow M.MF_Pass () returns Boolean +begin + log info node 'T' 'one'; + join onward; + merge onward; + log info node 'T' 'two'; + return true; +end;` + out, oc := describeBuilt(t, src) + if strings.Contains(out, "WARNING") { + t.Errorf("the description still warns:\n%s", out) + } + if got, want := countMerges(rebuildDescription(t, out)), countMerges(oc); got != want || want != 1 { + t.Errorf("the description rebuilds %d merges, the flow has %d (want 1):\n%s", got, want, out) + } + // Executing the description again describes identically: a fixed point. + again, _ := describeBuilt(t, "create microflow M.MF_Pass () returns Boolean\nbegin\n"+out+"\nend;") + if again != out { + t.Errorf("not a fixed point:\nfirst:\n%s\nsecond:\n%s", out, again) + } +} + +func countMerges(oc *microflows.MicroflowObjectCollection) int { + n := 0 + for _, o := range oc.Objects { + switch v := o.(type) { + case *microflows.ExclusiveMerge: + n++ + case *microflows.LoopedActivity: + if v.ObjectCollection != nil { + n += countMerges(v.ObjectCollection) + } + } + } + return n +} + +// rebuildDescription builds the flow a description body defines. +func rebuildDescription(t *testing.T, body string) *microflows.MicroflowObjectCollection { + t.Helper() + prog, errs := visitor.Build("create microflow M.Described () returns string\nbegin\n" + body + "\nend;") + if len(errs) > 0 { + t.Fatalf("description does not parse: %v\n%s", errs[0], body) + } + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + fb := &flowBuilder{posX: 100, posY: 100, spacing: HorizontalSpacing, measurer: &layoutMeasurer{}, varTypes: map[string]string{}} + return fb.buildFlowGraph(mf.Body, mf.ReturnType) +} diff --git a/mdl/executor/cmd_microflows_layout.go b/mdl/executor/cmd_microflows_layout.go index a35447b31d..dcc2fe835d 100644 --- a/mdl/executor/cmd_microflows_layout.go +++ b/mdl/executor/cmd_microflows_layout.go @@ -378,7 +378,7 @@ func pairFlowLayout(stored, rebuilt *layoutGraph) (*layoutPlan, error) { // gets the position the writer derives from its index. byName := map[string]model.Point{} for i, bp := range rebuilt.parameters { - pos := microflows.DerivedParameterPosition(i) + pos := microflows.DerivedParameterPosition(i, rebuilt.parameters) if bp.Position != nil { pos = *bp.Position } diff --git a/mdl/executor/cmd_microflows_loop_tail_merge_test.go b/mdl/executor/cmd_microflows_loop_tail_merge_test.go new file mode 100644 index 0000000000..5b787fecf8 --- /dev/null +++ b/mdl/executor/cmd_microflows_loop_tail_merge_test.go @@ -0,0 +1,196 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// An IF/ELSE that ends a loop body joins nothing: both branches end the +// iteration. The builder still drew a merge for them, with no way out +// (mendixlabs/mxcli#884, ako/mxcli#1273) — a node Studio Pro's authors delete +// by hand, and which every re-run put back. A loop-body activity with no +// outgoing flow ends the iteration; two of them are two such activities. + +func loopBody(t *testing.T, fb *flowBuilder) *microflows.MicroflowObjectCollection { + t.Helper() + for _, o := range fb.objects { + if l, ok := o.(*microflows.LoopedActivity); ok { + return l.ObjectCollection + } + } + t.Fatal("no loop built") + return nil +} + +func countMergesIn(oc *microflows.MicroflowObjectCollection) int { + n := 0 + for _, o := range oc.Objects { + if _, ok := o.(*microflows.ExclusiveMerge); ok { + n++ + } + } + return n +} + +func TestIfElseEndingALoopBodyHasNoMerge(t *testing.T) { + for _, tc := range []struct{ name, loop string }{ + {"loop", "loop $i in $L begin\n%s\nend loop;"}, + {"while", "while $Flag begin\n%s\nend while;"}, + } { + t.Run(tc.name, func(t *testing.T) { + body := ` log info node 'x' 'first'; + if $Other then + log info node 'x' 'then'; + else + log info node 'x' 'else'; + end if;` + fb := buildFlowFromMDL(t, " retrieve $L from M.Item;\n "+strings.Replace(tc.loop, "%s", body, 1)) + oc := loopBody(t, fb) + if n := countMergesIn(oc); n != 0 { + t.Errorf("the IF ending the %s body has %d merge(s), want none", tc.name, n) + } + // Both branch activities end the iteration: no flow leaves them. + out := map[model.ID]int{} + for _, f := range fb.flows { + out[f.OriginID]++ + } + branches := 0 + for _, o := range oc.Objects { + if _, ok := o.(*microflows.ExclusiveSplit); !ok { + continue + } + for _, f := range fb.flows { + if f.OriginID == o.GetID() { + branches++ + if out[f.DestinationID] != 0 { + t.Errorf("a branch activity has %d outgoing flow(s), want 0", out[f.DestinationID]) + } + } + } + } + if branches != 2 { + t.Errorf("%d branch flows, want 2", branches) + } + }) + } +} + +// Nested: the merge-less tail reaches into each branch, so an IF/ELSE that ends +// a branch of the tail IF has no merge either. +func TestNestedIfElseAtTheLoopTailHasNoMerge(t *testing.T) { + fb := buildFlowFromMDL(t, ` + retrieve $L from M.Item; + loop $i in $L begin + if $A then + if $B then log info node 'x' 'ab'; else log info node 'x' 'a'; end if; + else + log info node 'x' 'none'; + end if; + end loop;`) + if n := countMergesIn(loopBody(t, fb)); n != 0 { + t.Errorf("%d merge(s) at the loop tail, want none", n) + } +} + +// Controls: a merge that joins something stays. An IF that is not last, an IF +// with an empty branch (the split needs a flow per case), and an explicit +// @merge, which is how DESCRIBE keeps a stored tail merge through a rebuild. +func TestIfInALoopBodyKeepsAMergeThatJoinsSomething(t *testing.T) { + for _, tc := range []struct{ name, body string }{ + {"not last", ` + if $Other then log info node 'x' 'then'; else log info node 'x' 'else'; end if; + log info node 'x' 'after';`}, + {"empty else", ` + if $Other then log info node 'x' 'then'; else end if;`}, + {"no else", ` + if $Other then log info node 'x' 'then'; end if;`}, + {"explicit @merge", ` + @merge(600, 80) + if $Other then log info node 'x' 'then'; else log info node 'x' 'else'; end if;`}, + } { + t.Run(tc.name, func(t *testing.T) { + fb := buildFlowFromMDL(t, " retrieve $L from M.Item;\n loop $i in $L begin"+tc.body+"\n end loop;") + if n := countMergesIn(loopBody(t, fb)); n != 1 { + t.Errorf("%d merge(s), want 1", n) + } + }) + } +} + +// A tail merge that IS stored (an earlier mxcli wrote one, or a person kept +// it) is described with @merge, so a rebuild from the description keeps it. +func TestDescribeKeepsAStoredLoopTailMerge(t *testing.T) { + const src = `create microflow M.Tail ($Other: Boolean) returns Boolean +begin + retrieve $L from M.Item; + loop $i in $L begin + @merge(600, 80) + if $Other then log info node 'x' 'then'; else log info node 'x' 'else'; end if; + end loop; + return true; +end;` + out, oc := describeBuilt(t, src) + if !strings.Contains(out, "@merge(") { + t.Errorf("a stored loop-tail merge is described without @merge:\n%s", out) + } + if got, want := countMerges(rebuildDescription(t, out)), countMerges(oc); got != want { + t.Errorf("the description rebuilds %d merges, the flow has %d:\n%s", got, want, out) + } +} + +// The same, through the canonical describe that omits derivable positions: a +// tail merge an earlier mxcli wrote stands where the old layout put it, so its +// position is "derived" — but the current layout draws no merge there at all. +func TestCanonicalDescribeKeepsAStoredLoopTailMerge(t *testing.T) { + stored := buildMicroflowFromMDL(t, `create microflow M.Tail ($Other: Boolean) returns Boolean +begin + retrieve $L from M.Item; + loop $i in $L begin + @merge(600, 80) + if $Other then log info node 'x' 'then'; else log info node 'x' 'else'; end if; + end loop; + return true; +end;`) + mf := µflows.Microflow{ObjectCollection: stored} + e := newTestExecutor() + ctx := e.newExecContext(t.Context()) + // A keep set that keeps nothing: every position reads as derivable. + ctx.describeLayout = newFlowLayoutKeep() + out := strings.Join(formatMicroflowActivities(ctx, mf, nil, nil), "\n") + if !strings.Contains(out, "@merge(") { + t.Errorf("a stored loop-tail merge is described without @merge when positions are derived:\n%s", out) + } +} + +// The tail belongs to the last statement only. A CASE at the tail did not +// clear it, so an IF/ELSE inside one of its arms — followed by another +// statement — lost its merge and the statement after it lost its way in +// (02c-complex-layout-examples.mdl, CX_ACT_BatchProcessFactories). +func TestIfInsideACaseAtTheLoopTailKeepsItsMerge(t *testing.T) { + fb := buildFlowFromMDL(t, ` + retrieve $L from M.Item; + loop $i in $L begin + case $E + when A then + if $Other then log info node 'x' 'then'; else log info node 'x' 'else'; end if; + log info node 'x' 'after'; + when (empty) then + log info node 'x' 'empty'; + end case; + end loop;`) + oc := loopBody(t, fb) + in := map[model.ID]int{} + for _, f := range fb.flows { + in[f.DestinationID]++ + } + for _, o := range oc.Objects { + if _, ok := o.(*microflows.ActionActivity); ok && in[o.GetID()] == 0 { + t.Errorf("an activity at %v has no incoming flow", o.GetPosition()) + } + } +} diff --git a/mdl/executor/cmd_microflows_parameter_position.go b/mdl/executor/cmd_microflows_parameter_position.go index f015dba593..b2ab36f979 100644 --- a/mdl/executor/cmd_microflows_parameter_position.go +++ b/mdl/executor/cmd_microflows_parameter_position.go @@ -34,11 +34,11 @@ func positionFromAST(p *ast.Position) *model.Point { // Readers already normalise, so Position is non-nil only when it is intent; // the derived comparison here is belt-and-braces for a model assembled in // memory rather than read from disk. -func parameterPositionAnnotation(p *microflows.MicroflowParameter, idx int, indent string) string { +func parameterPositionAnnotation(p *microflows.MicroflowParameter, idx int, params []*microflows.MicroflowParameter, indent string) string { if p == nil || p.Position == nil { return "" } - if *p.Position == microflows.DerivedParameterPosition(idx) { + if *p.Position == microflows.DerivedParameterPosition(idx, params) { return "" } return fmt.Sprintf("%s@position(%d, %d)", indent, p.Position.X, p.Position.Y) @@ -56,7 +56,7 @@ func parameterPositionAnnotation(p *microflows.MicroflowParameter, idx int, inde func describeMicroflowParameters(params []*microflows.MicroflowParameter, formatType func(*microflows.MicroflowParameter) string) []string { lines := make([]string, 0, len(params)*2) for i, param := range params { - if ann := parameterPositionAnnotation(param, i, " "); ann != "" { + if ann := parameterPositionAnnotation(param, i, params, " "); ann != "" { lines = append(lines, ann) } paramType := "Object" diff --git a/mdl/executor/cmd_microflows_parameter_position_test.go b/mdl/executor/cmd_microflows_parameter_position_test.go index ba9af032ec..4929ea66c3 100644 --- a/mdl/executor/cmd_microflows_parameter_position_test.go +++ b/mdl/executor/cmd_microflows_parameter_position_test.go @@ -22,7 +22,7 @@ import ( // carries no intent, so it is re-derived rather than pinned. func TestDerivedParameterPositionMatchesTheWriters(t *testing.T) { for idx, want := range []model.Point{{X: 200, Y: 53}, {X: 300, Y: 53}, {X: 400, Y: 53}} { - if got := microflows.DerivedParameterPosition(idx); got != want { + if got := microflows.DerivedParameterPosition(idx, nil); got != want { t.Errorf("DerivedParameterPosition(%d) = %v, want %v", idx, got, want) } } @@ -33,14 +33,14 @@ func TestDerivedParameterPositionMatchesTheWriters(t *testing.T) { // over: inserting a parameter would leave the existing ones on the old grid // while the new one lands on top of them. func TestAuthoredParameterPositionIgnoresTheDerivedSpot(t *testing.T) { - if p := microflows.AuthoredParameterPosition(model.Point{X: 200, Y: 53}, 0); p != nil { + if p := microflows.AuthoredParameterPosition(model.Point{X: 200, Y: 53}, 0, nil); p != nil { t.Errorf("index 0 at the derived spot: got %v, want nil", *p) } - if p := microflows.AuthoredParameterPosition(model.Point{X: 300, Y: 53}, 1); p != nil { + if p := microflows.AuthoredParameterPosition(model.Point{X: 300, Y: 53}, 1, nil); p != nil { t.Errorf("index 1 at the derived spot: got %v, want nil", *p) } // The same point at a DIFFERENT index is not the derived one, so it is intent. - got := microflows.AuthoredParameterPosition(model.Point{X: 200, Y: 53}, 1) + got := microflows.AuthoredParameterPosition(model.Point{X: 200, Y: 53}, 1, nil) if got == nil || *got != (model.Point{X: 200, Y: 53}) { t.Errorf("index 1 at index 0's spot: got %v, want 200;53 kept", got) } @@ -51,7 +51,7 @@ func TestAuthoredParameterPositionIgnoresTheDerivedSpot(t *testing.T) { // pointer, and the test exists because a bool-free `if p.Position != (Point{})` // would pass every other case here. func TestAuthoredParameterPositionKeepsOrigin(t *testing.T) { - got := microflows.AuthoredParameterPosition(model.Point{X: 0, Y: 0}, 0) + got := microflows.AuthoredParameterPosition(model.Point{X: 0, Y: 0}, 0, nil) if got == nil { t.Fatal("0;0 was dropped as if unset; it is a position a person can choose") } @@ -65,14 +65,14 @@ func TestAuthoredParameterPositionKeepsOrigin(t *testing.T) { // every rewritten flow — the round-trip failure startAnnotationLines documents. func TestParameterPositionAnnotation(t *testing.T) { derived := µflows.MicroflowParameter{Position: &model.Point{X: 200, Y: 53}} - if got := parameterPositionAnnotation(derived, 0, " "); got != "" { + if got := parameterPositionAnnotation(derived, 0, nil, " "); got != "" { t.Errorf("derived position emitted %q, want no line", got) } authored := µflows.MicroflowParameter{Position: &model.Point{X: -77, Y: 0}} - if got := parameterPositionAnnotation(authored, 0, " "); got != " @position(-77, 0)" { + if got := parameterPositionAnnotation(authored, 0, nil, " "); got != " @position(-77, 0)" { t.Errorf("got %q, want \" @position(-77, 0)\"", got) } - if got := parameterPositionAnnotation(µflows.MicroflowParameter{}, 0, " "); got != "" { + if got := parameterPositionAnnotation(µflows.MicroflowParameter{}, 0, nil, " "); got != "" { t.Errorf("unset position emitted %q, want no line", got) } } diff --git a/mdl/executor/cmd_microflows_rejoin_merge_position_test.go b/mdl/executor/cmd_microflows_rejoin_merge_position_test.go new file mode 100644 index 0000000000..903688e1ac --- /dev/null +++ b/mdl/executor/cmd_microflows_rejoin_merge_position_test.go @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// The merge an ON ERROR handler falls through to sat on top of the node after +// the guarded activity (mendixlabs/mxcli#884, TDSC0706's repro; ako/mxcli#1273): +// +// ExclusiveMerge @(560, 200) 40x40 +// EndEvent @(560, 200) 20x20 <- same centre, the end inside the merge +// +// It was placed at the layout cursor less half a spacing, and the cursor had +// already moved past the successor; with an activity after the guarded one it +// landed RIGHT of that activity, so the edge into it ran backwards. It now sits +// on the edge it splits, between the two boxes. +func TestRejoinMergeSitsBetweenTheGuardedActivityAndItsSuccessor(t *testing.T) { + for _, tc := range []struct{ name, body string }{ + {"successor is the end event", ` + @position(240, 200) + call microflow M.SUB_Callee() on error without rollback { + @position(240, 380) + log error node 'Repro' 'handled'; + }; + @position(560, 200) + return;`}, + {"successor is an activity, no positions", ` + call microflow M.SUB_Callee() on error without rollback { + log error node 'Repro' 'handled'; + }; + log info node 'Repro' 'done';`}, + {"inside an else branch", ` + if $Flag then + log info node 'Repro' 'then'; + else + call microflow M.SUB_Callee() on error without rollback { + log error node 'Repro' 'handled'; + }; + log info node 'Repro' 'after'; + end if;`}, + } { + t.Run(tc.name, func(t *testing.T) { + fb := buildFlowFromMDL(t, tc.body) + objects := map[model.ID]microflows.MicroflowObject{} + for _, o := range fb.objects { + objects[o.GetID()] = o + } + var checked int + for _, f := range fb.flows { + merge, ok := objects[f.OriginID].(*microflows.ExclusiveMerge) + if !ok || f.IsErrorHandler { + continue + } + // The merge the handler rejoins at: one normal and one handler + // flow in, the successor out. + var fromHandler bool + var guarded microflows.MicroflowObject + for _, in := range fb.flows { + if in.DestinationID != merge.ID { + continue + } + if _, isAct := objects[in.OriginID].(*microflows.ActionActivity); isAct && !in.IsErrorHandler { + if a := objects[in.OriginID]; a.GetPosition().Y == merge.Position.Y || guarded == nil { + guarded = a + } + } + if in.IsErrorHandler || objects[in.OriginID].GetPosition().Y > merge.Position.Y { + fromHandler = true + } + } + if !fromHandler || guarded == nil { + continue + } + next := objects[f.DestinationID] + mergeL, mergeR := merge.Position.X-MergeSize/2, merge.Position.X+MergeSize/2 + guardedR := guarded.GetPosition().X + objectSize(guarded).Width/2 + nextL := next.GetPosition().X - objectSize(next).Width/2 + if mergeL < guardedR || mergeR > nextL { + t.Errorf("rejoin merge spans x %d..%d; the guarded activity ends at %d and the %T after it starts at %d", + mergeL, mergeR, guardedR, next, nextL) + } + if merge.Position.Y != next.GetPosition().Y { + t.Errorf("rejoin merge at y=%d, off the line of the %T it enters (y=%d)", + merge.Position.Y, next, next.GetPosition().Y) + } + checked++ + } + if checked == 0 { + t.Fatal("no rejoin merge found") + } + }) + } +} diff --git a/mdl/executor/cmd_microflows_show_helpers.go b/mdl/executor/cmd_microflows_show_helpers.go index 3d9b34a1f6..b2b7c2ee59 100644 --- a/mdl/executor/cmd_microflows_show_helpers.go +++ b/mdl/executor/cmd_microflows_show_helpers.go @@ -314,7 +314,7 @@ func emitAnchorAnnotationWithActivityMap( if _, isSplit := obj.(*microflows.ExclusiveSplit); isSplit { emitSplitAnchorAnnotation(id, flowsByOrigin, flowsByDest, lines, indentStr, false, - mergeExitAnchor(id, flowsByOrigin, activityMap)) + mergeExitAnchor(id, flowsByOrigin, activityMap), enumCaseAnchorFragments(id, flowsByOrigin, activityMap)...) return } if _, isSplit := obj.(*microflows.InheritanceSplit); isSplit { @@ -445,7 +445,14 @@ func emitMergeAnnotation( return } merge := commonMergeAfter(obj.GetID(), flowsByOrigin, activityMap) - if merge == nil || !layout.keepsPosition(merge.GetID()) { + if merge == nil { + return + } + // A merge with no way out ends a loop body. The builder no longer draws one + // there (mendixlabs/mxcli#884), so a stored one — an earlier mxcli's, or one + // a person kept — exists in a rebuild only if the description says so, + // wherever it stands. + if !layout.keepsPosition(merge.GetID()) && len(findNormalFlows(flowsByOrigin[merge.GetID()])) > 0 { return } p := merge.GetPosition() @@ -588,6 +595,7 @@ func emitSplitAnchorAnnotation( indentStr string, preserveDefaultIncoming bool, mergeExitFrom string, + caseParts ...string, ) { // Incoming flow anchor (where the previous activity's flow lands on the split). var inTo string @@ -607,7 +615,7 @@ func emitSplitAnchorAnnotation( falseTo = anchorSideKeyword(falseFlow.DestinationConnectionIndex) } - if inTo == "" && trueFrom == "" && trueTo == "" && falseFrom == "" && falseTo == "" && mergeExitFrom == "" { + if inTo == "" && trueFrom == "" && trueTo == "" && falseFrom == "" && falseTo == "" && mergeExitFrom == "" && len(caseParts) == 0 { return } @@ -631,6 +639,7 @@ func emitSplitAnchorAnnotation( if p := branchAnchorFragmentWithDefaultSides("false", falseFrom, falseTo, falseDefaultFroms, falseDefaultTos); p != "" { parts = append(parts, p) } + parts = append(parts, caseParts...) if len(parts) == 0 { return } @@ -656,7 +665,7 @@ func mergeExitAnchor( if activityMap == nil { return "" } - if trueFlow, falseFlow := findBranchFlows(flowsByOrigin[splitID]); trueFlow == nil && falseFlow == nil { + if trueFlow, falseFlow := findBranchFlows(flowsByOrigin[splitID]); trueFlow == nil && falseFlow == nil && !hasEnumCaseFlow(flowsByOrigin[splitID]) { return "" } merge := commonMergeAfter(splitID, flowsByOrigin, activityMap) @@ -675,6 +684,91 @@ func mergeExitAnchor( return "" } +// hasEnumCaseFlow reports whether any of a split's flows carries an +// enumeration case — a CASE, whose `from:` is the flow leaving its merge as on +// an IF (mendixlabs/mxcli#1587). +func hasEnumCaseFlow(flows []*microflows.SequenceFlow) bool { + for _, f := range flows { + if _, ok := enumCaseValue(f); ok { + return true + } + } + return false +} + +// enumCaseAnchorFragments returns the `Value: (from: …[, to: …])` entries a +// CASE's @anchor needs: one per outcome whose stored sides differ from what the +// builder would draw for it there (mendixlabs/mxcli#1587). Without them a CASE +// drawn in Studio Pro lost every side on describe -> exec. +// +// The default is computed exactly as addEnumSplit computes it — the branch's +// place in describe order, the branch count, where split and target stand — so +// an ordinary CASE gets no entries. `to:` is only written for a flow into a +// merge: one into an activity is carried by that activity's own `@anchor(to:)`. +func enumCaseAnchorFragments( + splitID model.ID, + flowsByOrigin map[model.ID][]*microflows.SequenceFlow, + activityMap map[model.ID]microflows.MicroflowObject, +) []string { + split := activityMap[splitID] + if split == nil { + return nil + } + type branch struct { + dest model.ID + flows []*microflows.SequenceFlow + } + var branches []branch + byDest := map[model.ID]int{} + for _, f := range orderedEnumSplitFlows(findNormalFlows(flowsByOrigin[splitID]), activityMap) { + if _, ok := enumCaseValue(f); !ok { + continue + } + if i, ok := byDest[f.DestinationID]; ok { + branches[i].flows = append(branches[i].flows, f) + continue + } + byDest[f.DestinationID] = len(branches) + branches = append(branches, branch{dest: f.DestinationID, flows: []*microflows.SequenceFlow{f}}) + } + targetOf := func(id model.ID) model.Point { + if o := activityMap[id]; o != nil { + return o.GetPosition() + } + return model.Point{} + } + ys := make([]int, len(branches)) + for i, b := range branches { + ys[i] = targetOf(b.dest).Y + } + origins := enumSplitOriginAnchors(ys, split.GetPosition().Y) + + var out []string + for i, b := range branches { + slot := splitCaseSlot{order: i, origin: -1} + if origins != nil { + slot.origin = origins[i] + } + _, intoMerge := activityMap[b.dest].(*microflows.ExclusiveMerge) + for _, f := range b.flows { + def := µflows.SequenceFlow{} + slot.apply(def, split.GetPosition(), targetOf(b.dest)) + var inner []string + if f.OriginConnectionIndex != def.OriginConnectionIndex { + inner = append(inner, "from: "+anchorSideKeyword(f.OriginConnectionIndex)) + } + if intoMerge && f.DestinationConnectionIndex != def.DestinationConnectionIndex { + inner = append(inner, "to: "+anchorSideKeyword(f.DestinationConnectionIndex)) + } + if len(inner) > 0 { + value, _ := enumCaseValue(f) + out = append(out, formatEnumSplitCaseValue(value)+": ("+strings.Join(inner, ", ")+")") + } + } + } + return out +} + // branchAnchorFragment builds a `key: (from: X, to: Y)` fragment for a branch // anchor. Returns "" when both sides are empty (default). func branchAnchorFragment(label, from, to string) string { @@ -1168,6 +1262,15 @@ func traverseFlow( return } visited[currentID] = true + // A merge with one way in, on the main line, is spelled the way exec + // was given it — `join L; merge L;` — so the description rebuilds it + // instead of warning that it is lost (see inlineMerges). + if indent == 0 { + if label, ok := labels.inlineLabel(currentID); ok { + *lines = append(*lines, "join "+label+";") + *lines = append(*lines, mergeDeclarationLines(indent, label, obj, annotationsByTarget.layoutKeep(), flowsByOrigin)...) + } + } for _, flow := range flowsByOrigin[currentID] { traverseFlow(ctx, flow.DestinationID, activityMap, flowsByOrigin, flowsByDest, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget, labels) } diff --git a/mdl/executor/cmd_microflows_show_merge.go b/mdl/executor/cmd_microflows_show_merge.go index 07d593eabc..9a2e19b0f2 100644 --- a/mdl/executor/cmd_microflows_show_merge.go +++ b/mdl/executor/cmd_microflows_show_merge.go @@ -47,6 +47,77 @@ import ( type mergeLabels struct { byID map[model.ID]string crossed map[model.ID]bool + // inline names the one-input merges of the main line, which a description + // spells as written: `join