Skip to content

Commit

Permalink
Fix: print line comment after arrow in lamda between paranthesis (#557)
Browse files Browse the repository at this point in the history
* WIP: refactor IfThenElse

* WIP: refactor IfThenElse, more comments after keywords

* WIP: refactor IfThenElse, comments everywhere

* WIP: refactor IfThenElse, fix all tests

* WIP: refactor IfThenElse, added MaxIfThenElseShortWidth setting

* WIP: refactor IfThenElse, added MaxIfThenElseShortWidth setting to global tool

* Updated Documentation.md with --maxIfThenElseShortWidth

* Removed warnings introduced by IfThenElse refactor

* merged with master
  • Loading branch information
nojaf authored Nov 22, 2019
1 parent 6ed1572 commit 818c5a9
Show file tree
Hide file tree
Showing 28 changed files with 1,329 additions and 159 deletions.
21 changes: 21 additions & 0 deletions docs/Documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,27 @@ match meh with

will remain the same, the newline after `->` was detected and preserved.


- `--maxIfThenElseShortWidth <number>`: `number` if being set, controls when if/then/else expressions will be formatted as single line or as multiple lines.

Fantomas tries to follow [the F# style guide](https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-if-expressions) as close as possible when formatting if expressions.

The style guide says:

> If either cond, e1 or e2 are longer, but not multi-line:
```fsharp
if cond
then e1
else e2
```

But what exactly is longer right? By default Fantomas will use 40 width to determine if the expression needs to be formatted to the example above or remain as a oneliner (`if cond then e1 else e2`).


So if either `cond`, `e1` or `e2` is longer than `maxIfThenElseShortWidth` but not multiline, it will format on three lines.
See [unit tests](https://github.com/fsprojects/fantomas/blob/9d4b499c09a1f06f5485835817844657cc51215b/src/Fantomas.Tests/IfThenElseTests.fs#L734) for more examples.

That said, most of the preferences are very simple.
But they demonstrate the flexibility of Fantomas on a set of configurations.
More preferences will be added depending on use cases.
Expand Down
15 changes: 13 additions & 2 deletions src/Fantomas.CoreGlobalTool/Program.fs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ let [<Literal>] indentOnTryWithText = "Enable indentation on try/with block (def
let [<Literal>] reorderOpenDeclarationText = "Enable reordering open declarations (default = false)."
let [<Literal>] spaceAroundDelimiterText = "Disable spaces after starting and before ending of lists, arrays, sequences and records (default = true)."
let [<Literal>] keepNewlineAfterText = "Keep newlines found after = in let bindings, -> in pattern matching and chained function calls (default = false)."
let [<Literal>] maxIfThenElseShortWidthText = "Set the max length of any expression in an if expression before formatting on multiple lines (default = 40)."
let [<Literal>] strictModeText = "Enable strict mode (ignoring directives and comments and printing literals in canonical forms) (default = false)."

let time f =
Expand Down Expand Up @@ -129,6 +130,7 @@ let main _args =
let spaceAroundDelimiter = ref FormatConfig.Default.SpaceAroundDelimiter
let keepNewlineAfter = ref FormatConfig.Default.KeepNewlineAfter
let strictMode = ref FormatConfig.Default.StrictMode
let maxIfThenElseShortWidth = ref FormatConfig.Default.MaxIfThenElseShortWidth

let handleOutput s =
if not !stdOut then
Expand Down Expand Up @@ -163,6 +165,13 @@ let main _args =
eprintfn "Page width should be at least 60."
exit 1

let handleMaxIfThenElseShortWidth i =
if i >= 0 then
maxIfThenElseShortWidth := i
else
eprintfn "Max IfThenElse short width should be greater than zero."
exit 1

let fileToFile (inFile : string) (outFile : string) config =
try
printfn "Processing %s" inFile
Expand Down Expand Up @@ -244,7 +253,8 @@ let main _args =
ArgInfo("--reorderOpenDeclaration", ArgType.Set reorderOpenDeclaration, reorderOpenDeclarationText);

ArgInfo("--noSpaceAroundDelimiter", ArgType.Clear spaceAroundDelimiter, spaceAroundDelimiterText);
ArgInfo("--keepNewlineAfter", ArgType.Set keepNewlineAfter, keepNewlineAfterText);
ArgInfo("--keepNewlineAfter", ArgType.Set keepNewlineAfter, keepNewlineAfterText)
ArgInfo("--maxIfThenElseShortWidth", ArgType.Int handleMaxIfThenElseShortWidth, maxIfThenElseShortWidthText);
ArgInfo("--strictMode", ArgType.Set strictMode, strictModeText) |]

ArgParser.Parse(options, handleInput, sprintf "Fantomas <input_path>%sCheck out https://github.com/fsprojects/fantomas/blob/master/docs/Documentation.md#using-the-command-line-tool for more info." Environment.NewLine )
Expand All @@ -262,7 +272,8 @@ let main _args =
ReorderOpenDeclaration = !reorderOpenDeclaration
SpaceAroundDelimiter = !spaceAroundDelimiter
StrictMode = !strictMode
KeepNewlineAfter = !keepNewlineAfter }
KeepNewlineAfter = !keepNewlineAfter
MaxIfThenElseShortWidth = !maxIfThenElseShortWidth }

// Handle inputs via pipeline
let isKeyAvailable = ref false
Expand Down
13 changes: 5 additions & 8 deletions src/Fantomas.Tests/ActivePatternTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,15 @@ let (|ParseRegex|_|) regex str =
|> prepend newline
|> should equal """
let (|Even|Odd|) input =
if input % 2 = 0 then Even
else Odd
if input % 2 = 0 then Even else Odd
let (|Integer|_|) (str: string) =
let mutable intvalue = 0
if System.Int32.TryParse(str, &intvalue) then Some(intvalue)
else None
if System.Int32.TryParse(str, &intvalue) then Some(intvalue) else None
let (|ParseRegex|_|) regex str =
let m = Regex(regex).Match(str)
if m.Success then
Some(List.tail [ for x in m.Groups -> x.Value ])
else
None
if m.Success
then Some(List.tail [ for x in m.Groups -> x.Value ])
else None
"""
43 changes: 27 additions & 16 deletions src/Fantomas.Tests/CommentTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -326,12 +326,15 @@ else
""" config
|> prepend newline
|> should equal """
if a then ()
if a then
()
else
// Comment 1
if b then ()
// Comment 2
else ()
// Comment 1
if b then
()
// Comment 2
else
()
"""

[<Test>]
Expand Down Expand Up @@ -643,12 +646,13 @@ type substring =
/// <returns>An integer that indicates the lexical relationship between the two comparands.</returns>
static member CompareOrdinal(strA: substring, strB: substring) =
// If both substrings are empty they are considered equal, regardless of their offset or underlying string.
if strA.Length = 0 && strB.Length = 0 then 0
elif
if strA.Length = 0 && strB.Length = 0 then
0

// OPTIMIZATION : If the substrings have the same (identical) underlying string
// and offset, the comparison value will depend only on the length of the substrings.
strA.String == strB.String && strA.Offset = strB.Offset then compare strA.Length strB.Length
// OPTIMIZATION : If the substrings have the same (identical) underlying string
// and offset, the comparison value will depend only on the length of the substrings.
elif strA.String == strB.String && strA.Offset = strB.Offset then
compare strA.Length strB.Length

else
(* Structural comparison on substrings -- this uses the same comparison
Expand All @@ -675,8 +679,10 @@ if true then //comment
else 0""" config
|> prepend newline
|> should equal """
if true then 1 //comment
else 0
if true then //comment
1
else
0
"""

[<Test>]
Expand All @@ -687,8 +693,11 @@ if //comment
else 0""" config
|> prepend newline
|> should equal """
if true then 1 //comment
else 0
if //comment
true then
1
else
0
"""

[<Test>]
Expand All @@ -699,6 +708,8 @@ else //comment
0""" config
|> prepend newline
|> should equal """
if true then 1
else 0 //comment
if true then
1
else //comment
0
"""
21 changes: 9 additions & 12 deletions src/Fantomas.Tests/ControlStructureTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ then printfn "You are only %d years old and already learning F#? Wow!" age""" co
let rec tryFindMatch pred list =
match list with
| head :: tail ->
if pred (head) then Some(head)
else tryFindMatch pred tail
if pred (head) then Some(head) else tryFindMatch pred tail
| [] -> None
let test x y =
Expand Down Expand Up @@ -138,8 +137,7 @@ let ``try/with and finally``() =
let function1 x y =
try
try
if x = y then raise (InnerError("inner"))
else raise (OuterError("outer"))
if x = y then raise (InnerError("inner")) else raise (OuterError("outer"))
with
| Failure _ -> ()
| InnerError(str) -> printfn "Error1 %s" str
Expand Down Expand Up @@ -222,9 +220,10 @@ let x =
let x =
if try
true
with Failure _ -> false
then ()
else ()
with Failure _ -> false then
()
else
()
"""

[<Test>]
Expand Down Expand Up @@ -297,8 +296,7 @@ let ``multiline if in tuple``() =
""" config
|> prepend newline
|> should equal """
((if true then 1
else 2), 3)
((if true then 1 else 2), 3)
"""

// https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-if-expressions
Expand Down Expand Up @@ -435,9 +433,8 @@ let a ex =
if null = ex then
fooo()
None
elif
// this was None
ex.GetType() = typeof<obj> then
// this was None
elif ex.GetType() = typeof<obj> then
Some ex
else
None
Expand Down
3 changes: 2 additions & 1 deletion src/Fantomas.Tests/DynamicOperatorTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ let ``keep () when dynamic operator inside boolean expr, #476`` () =
|> prepend newline
|> should equal """
let fieldColor (fieldNameX: string) =
if f.errors?(fieldNameY) && f.touched?(fieldNameZ) then IsDanger
if f.errors?(fieldNameY) && f.touched?(fieldNameZ)
then IsDanger
else NoColor
|> Input.Color
"""
1 change: 1 addition & 0 deletions src/Fantomas.Tests/Fantomas.Tests.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
<Compile Include="KeepNewlineAfterTests.fs" />
<Compile Include="ElmishTests.fs" />
<Compile Include="LambdaTests.fs" />
<Compile Include="IfThenElseTests.fs" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\Fantomas\Fantomas.fsproj" />
Expand Down
Loading

0 comments on commit 818c5a9

Please sign in to comment.