Skip to content

Commit bb5edc4

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 c643c3f commit bb5edc4

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
@@ -1042,14 +1042,38 @@ func whitespaceBetween(input antlr.CharStream, start, end int) string {
10421042
return gap
10431043
}
10441044

1045+
// retrieveInterClauseWhitespaceSuffix returns the whitespace gap between a
1046+
// retrieve expression and the next clause keyword (LIMIT/OFFSET), with the
1047+
// trailing newline + indent that the formatter will re-emit stripped off.
1048+
//
1049+
// The formatter writes each subsequent clause on its own line indented by
1050+
// `formatRetrieveContinuationIndent` spaces, so the original source's trailing
1051+
// "\n<indent>" is structural and would duplicate after a roundtrip if kept.
1052+
// Anything before that final newline (blank lines, comments, additional
1053+
// indentation) is preserved as authored. When the gap does not end in a
1054+
// recognisable line-break-then-indent sequence we return "" — the formatter
1055+
// will lay out the clause normally.
10451056
func retrieveInterClauseWhitespaceSuffix(gap string) string {
10461057
if gap == "" {
10471058
return ""
10481059
}
1049-
for _, suffix := range []string{"\r\n ", "\n "} {
1050-
if strings.HasSuffix(gap, suffix) {
1051-
return strings.TrimSuffix(gap, suffix)
1060+
// Trim the trailing newline + structural indentation the formatter will
1061+
// re-emit. We strip whatever indent (spaces or tabs) follows the final
1062+
// newline so this stays robust if the formatter changes its indent width.
1063+
for i := len(gap) - 1; i >= 0; i-- {
1064+
c := gap[i]
1065+
if c == ' ' || c == '\t' {
1066+
continue
1067+
}
1068+
if c == '\n' {
1069+
// Include a preceding \r in the strip so CRLF line endings work.
1070+
cut := i
1071+
if cut > 0 && gap[cut-1] == '\r' {
1072+
cut--
1073+
}
1074+
return gap[:cut]
10521075
}
1076+
break
10531077
}
10541078
return ""
10551079
}
@@ -1227,6 +1251,13 @@ func shouldPreserveExpressionSource(source string) bool {
12271251
}
12281252
}
12291253
}
1254+
// Mendix's `not(<expr>)` function call has no surrounding spaces in
1255+
// idiomatic source, but the parser would re-emit it as `not (<expr>)`
1256+
// (function-call AST node loses the no-space affordance). Preserving the
1257+
// original source keeps the compact form across describe → exec →
1258+
// describe. The substring check is intentionally loose; false positives
1259+
// (e.g. an attribute name containing "not(") only over-preserve and have
1260+
// no semantic effect since the parsed expression is what runs.
12301261
if strings.Contains(strings.ToLower(source), "not(") {
12311262
return true
12321263
}

0 commit comments

Comments
 (0)