Skip to content

Commit 7364284

Browse files
hjothamendixclaude
andcommitted
fix: make retrieve inter-clause whitespace robust to indent changes
Replaces the hard-coded "\n " / "\r\n " suffix matches with a backwards walk that strips a trailing newline plus whatever indent (spaces or tabs) follows it. The formatter re-emits its own indent before each clause, so the original source's trailing newline+indent is structural and would duplicate after a roundtrip — but the old 4-space match silently dropped the entire whitespace gap if the formatter or the source used a different width. Adds a doc comment explaining the contract. Also documents the `not(` heuristic in shouldPreserveExpressionSource so future readers know it preserves the compact form against the parser's `not (<expr>)` re-emission. Addresses ako's review on PR #323. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 9aa2248 commit 7364284

1 file changed

Lines changed: 34 additions & 3 deletions

File tree

mdl/visitor/visitor_microflow_statements.go

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,14 +1078,38 @@ func whitespaceBetween(input antlr.CharStream, start, end int) string {
10781078
return gap
10791079
}
10801080

1081+
// retrieveInterClauseWhitespaceSuffix returns the whitespace gap between a
1082+
// retrieve expression and the next clause keyword (LIMIT/OFFSET), with the
1083+
// trailing newline + indent that the formatter will re-emit stripped off.
1084+
//
1085+
// The formatter writes each subsequent clause on its own line indented by
1086+
// `formatRetrieveContinuationIndent` spaces, so the original source's trailing
1087+
// "\n<indent>" is structural and would duplicate after a roundtrip if kept.
1088+
// Anything before that final newline (blank lines, comments, additional
1089+
// indentation) is preserved as authored. When the gap does not end in a
1090+
// recognisable line-break-then-indent sequence we return "" — the formatter
1091+
// will lay out the clause normally.
10811092
func retrieveInterClauseWhitespaceSuffix(gap string) string {
10821093
if gap == "" {
10831094
return ""
10841095
}
1085-
for _, suffix := range []string{"\r\n ", "\n "} {
1086-
if strings.HasSuffix(gap, suffix) {
1087-
return strings.TrimSuffix(gap, suffix)
1096+
// Trim the trailing newline + structural indentation the formatter will
1097+
// re-emit. We strip whatever indent (spaces or tabs) follows the final
1098+
// newline so this stays robust if the formatter changes its indent width.
1099+
for i := len(gap) - 1; i >= 0; i-- {
1100+
c := gap[i]
1101+
if c == ' ' || c == '\t' {
1102+
continue
1103+
}
1104+
if c == '\n' {
1105+
// Include a preceding \r in the strip so CRLF line endings work.
1106+
cut := i
1107+
if cut > 0 && gap[cut-1] == '\r' {
1108+
cut--
1109+
}
1110+
return gap[:cut]
10881111
}
1112+
break
10891113
}
10901114
return ""
10911115
}
@@ -1263,6 +1287,13 @@ func shouldPreserveExpressionSource(source string) bool {
12631287
}
12641288
}
12651289
}
1290+
// Mendix's `not(<expr>)` function call has no surrounding spaces in
1291+
// idiomatic source, but the parser would re-emit it as `not (<expr>)`
1292+
// (function-call AST node loses the no-space affordance). Preserving the
1293+
// original source keeps the compact form across describe → exec →
1294+
// describe. The substring check is intentionally loose; false positives
1295+
// (e.g. an attribute name containing "not(") only over-preserve and have
1296+
// no semantic effect since the parsed expression is what runs.
12661297
if strings.Contains(strings.ToLower(source), "not(") {
12671298
return true
12681299
}

0 commit comments

Comments
 (0)