Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

single line comment trivia treated same as new line trivia (partial fix for issue #513) #526

Merged
merged 5 commits into from
Oct 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion src/Fantomas.Tests/LetBindingTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -277,4 +277,36 @@ let ``should keep space before :`` () =
formatSourceString false "let refl<'a> : Teq<'a, 'a> = Teq(id, id)" config
|> fun formatted -> formatSourceString false formatted config
|> should equal "let refl<'a> : Teq<'a, 'a> = Teq(id, id)
"
"

[<Test>]
let ``newline trivia before simple sequence doesn't force remaining to get offset by last expression column index`` () =
// https://github.com/fsprojects/fantomas/issues/513
formatSourceString false """let a() =
let q = 1

q
b
""" config
|> should equal """let a() =
let q = 1

q
b
"""

[<Test>]
let ``comment trivia before simple sequence doesn't force remaining to get offset by last expression column index`` () =
// https://github.com/fsprojects/fantomas/issues/513
formatSourceString false """let a() =
let q = 1
// comment
q
b
""" config
|> should equal """let a() =
let q = 1
// comment
q
b
"""
20 changes: 11 additions & 9 deletions src/Fantomas/CodePrinter.fs
Original file line number Diff line number Diff line change
Expand Up @@ -726,7 +726,7 @@ and genTuple astContext es =
if i = 0 then f e else noIndentBreakNlnFun f e
))

and genExpr astContext synExpr =
and genExpr astContext synExpr =
let appNlnFun e =
match e with
| CompExpr _
Expand Down Expand Up @@ -1065,14 +1065,16 @@ and genExpr astContext synExpr =
atCurrentColumn (kw "TRY" !-"try " +> indent +> sepNln +> genExpr astContext e1 +> unindent +> kw "FINALLY" !+~"finally"
+> indent +> sepNln +> genExpr astContext e2 +> unindent)

| SequentialSimple es -> atCurrentColumn (colAutoNlnSkip0 sepSemiNln es (genExpr astContext))
// It seems too annoying to use sepSemiNln
| Sequentials es ->
// This is one of those weird situations where the newlines need to printed before atCurrentColumn
// If the newline would be printed in a AtCurrentColumn block that code would be started too far of.
// See https://github.com/fsprojects/fantomas/issues/478
firstNewline es +> atCurrentColumn (col sepSemiNln es (genExpr astContext))

| SequentialSimple es | Sequentials es ->
// This is one situations where the newlines/trivium need to printed before atCurrentColumn due to compiler restriction (last tested 4.7)
// If the trivia would be printed in a AtCurrentColumn block that code would be started too far off,
// and thus, engender compile errors.
// See :
// * https://github.com/fsprojects/fantomas/issues/478
// * https://github.com/fsprojects/fantomas/issues/513

firstNewlineOrComment es +> atCurrentColumn (col sepSemiNln es (genExpr astContext))

| IfThenElse(e1, e2, None) ->
atCurrentColumn (!- "if " +> ifElse (checkBreakForExpr e1) (genExpr astContext e1 ++ "then") (genExpr astContext e1 +- "then")
-- " " +> preserveBreakNln astContext e2)
Expand Down
6 changes: 3 additions & 3 deletions src/Fantomas/TriviaContext.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ open Fantomas
open Fantomas.Context
open Fantomas.TriviaTypes

let firstNewline (es: SynExpr list) (ctx: Context) =
let firstNewlineOrComment (es: SynExpr list) (ctx: Context) =
es
|> List.tryHead
|> Option.bind (fun e -> TriviaHelpers.findByRange ctx.Trivia e.Range)
|> fun cb ->
match cb with
| Some ({ ContentBefore = TriviaContent.Newline::rest } as tn) ->
| Some ({ ContentBefore = (TriviaContent.Newline|TriviaContent.Comment _) as head ::rest } as tn) ->
let updatedTriviaNodes =
ctx.Trivia
|> List.map (fun t ->
Expand All @@ -21,6 +21,6 @@ let firstNewline (es: SynExpr list) (ctx: Context) =
)

let ctx' = { ctx with Trivia = updatedTriviaNodes }
printTriviaContent Newline ctx'
printTriviaContent head ctx'
| _ -> sepNone ctx