Skip to content

Commit ebc4f8e

Browse files
authored
Merge pull request fsprojects#3234 from nojaf/fix-2944
Fix indentation loss for comments in no-break infix expressions
2 parents 8f3c408 + cac2132 commit ebc4f8e

File tree

5 files changed

+66
-21
lines changed

5 files changed

+66
-21
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
- Dynamic operator on result of qualified function call causes formatting error. [#3135](https://github.com/fsprojects/fantomas/issues/3135)
1515
- Long delegate type with generic args no longer breaks around arrow. [#2468](https://github.com/fsprojects/fantomas/issues/2468)
1616
- Idempotency problem when formatting NUnit Assert.That with lambda argument. [#1740](https://github.com/fsprojects/fantomas/issues/1740)
17+
- Comment between lines of no-break infix expression no longer loses indentation. [#2944](https://github.com/fsprojects/fantomas/issues/2944)
1718

1819
## [8.0.0-alpha-002] - 2025-12-15
1920

src/Fantomas.Core.Tests/OperatorTests.fs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,3 +1422,21 @@ let allDecls =
14221422
@+ iimplsLs
14231423
@+ ctorLs
14241424
"""
1425+
1426+
[<Test>]
1427+
let ``comment between lines breaks indentation, 2944`` () =
1428+
formatSourceString
1429+
"""
1430+
5 =
1431+
// a comment
1432+
5
1433+
"""
1434+
{ config with IndentSize = 2 }
1435+
|> prepend newline
1436+
|> should
1437+
equal
1438+
"""
1439+
5 =
1440+
// a comment
1441+
5
1442+
"""

src/Fantomas.Core.Tests/QuotationTests.fs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,10 @@ test
122122
.Replace("8.12", "8.13") // CRAP score rounding
123123
.Replace("4.12", "4.13") // CRAP score rounding
124124
.Trim([| '\u00FF' |]) = expected
125-
.Replace('\r', '\u00FF')
126-
.Replace('\n', '\u00FF')
127-
.Replace("\u00FF\u00FF", "\u00FF")
128-
.Trim([| '\u00FF' |])
125+
.Replace('\r', '\u00FF')
126+
.Replace('\n', '\u00FF')
127+
.Replace("\u00FF\u00FF", "\u00FF")
128+
.Trim([| '\u00FF' |])
129129
@>
130130
"""
131131

src/Fantomas.Core/CodePrinter.fs

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -812,36 +812,55 @@ let genExpr (e: Expr) =
812812
ctx
813813

814814
| Expr.InfixApp node ->
815-
let genOnelinerInfixExpr (node: ExprInfixAppNode) =
816-
let genExpr e =
817-
match e with
818-
| Expr.Record _
819-
| Expr.AnonStructRecord _ -> atCurrentColumnIndent (genExpr e)
820-
| _ -> genExpr e
815+
let genRhsExpr e =
816+
match e with
817+
| Expr.Record _
818+
| Expr.AnonStructRecord _ -> atCurrentColumnIndent (genExpr e)
819+
| _ -> genExpr e
821820

821+
let genShortInfixExpr =
822822
genExpr node.LeftHandSide
823823
+> sepSpace
824824
+> genSingleTextNode node.Operator
825825
+> sepNlnWhenWriteBeforeNewlineNotEmpty
826826
+> sepSpace
827-
+> genExpr node.RightHandSide
827+
+> genRhsExpr node.RightHandSide
828+
829+
let isNewLineInfixOp = newLineInfixOps.Contains node.Operator.Text
830+
let isNoBreakInfixOp = noBreakInfixOps.Contains node.Operator.Text
828831

829-
if
830-
isLambdaOrIfThenElse node.LeftHandSide
831-
&& newLineInfixOps.Contains node.Operator.Text
832-
then
832+
// Lambdas or if/then/else on the LHS of pipe-like operators (|>, ||>, >>)
833+
// always use the multiline layout to avoid confusing indentation.
834+
if isLambdaOrIfThenElse node.LeftHandSide && isNewLineInfixOp then
833835
genNode node (genMultilineInfixExpr node)
836+
// No-break operators (=, >, <, %) keep the operator on the same line as the LHS.
837+
// When the expression doesn't fit on one line, indent the RHS to preserve
838+
// correct indentation when trivia (comments) precedes it. See #2944.
839+
elif isNoBreakInfixOp then
840+
let genLongNoBreakInfixExpr =
841+
genExpr node.LeftHandSide
842+
+> sepSpace
843+
+> genSingleTextNode node.Operator
844+
+> indent
845+
+> sepNlnWhenWriteBeforeNewlineNotEmpty
846+
+> sepSpace
847+
+> genRhsExpr node.RightHandSide
848+
+> unindent
849+
850+
fun ctx ->
851+
genNode
852+
node
853+
(isShortExpression ctx.Config.MaxInfixOperatorExpression genShortInfixExpr genLongNoBreakInfixExpr)
854+
ctx
855+
// All other infix operators place the operator at the start of the next line.
834856
else
835857
fun ctx ->
836858
genNode
837859
node
838860
(isShortExpression
839861
ctx.Config.MaxInfixOperatorExpression
840-
(genOnelinerInfixExpr node)
841-
(ifElse
842-
(noBreakInfixOps.Contains(node.Operator.Text))
843-
(genOnelinerInfixExpr node)
844-
(genMultilineInfixExpr node)))
862+
genShortInfixExpr
863+
(genMultilineInfixExpr node))
845864
ctx
846865

847866
| Expr.IndexWithoutDot node ->

src/Fantomas.Core/SyntaxOak.fs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,14 @@ type NodeBase(range: range) =
9797
if hasChildren then
9898
for n in x.Children do
9999
match n with
100-
| :? SingleTextNode as stn -> sb.Append(contentIndent).Append(stn.ToString()).AppendLine() |> ignore
100+
| :? SingleTextNode as stn ->
101+
for tn in stn.ContentBefore do
102+
sb.Append(contentIndent).Append(tn.ToString()).AppendLine() |> ignore
103+
104+
sb.Append(contentIndent).Append(stn.ToString()).AppendLine() |> ignore
105+
106+
for tn in stn.ContentAfter do
107+
sb.Append(contentIndent).Append(tn.ToString()).AppendLine() |> ignore
101108
| :? NodeBase as nb ->
102109
nb.AppendToStringWithIndent(sb, depth + 1)
103110
sb.AppendLine() |> ignore

0 commit comments

Comments
 (0)