Skip to content

Commit

Permalink
Revisit if/then/else according to MS style guide. (#1282)
Browse files Browse the repository at this point in the history
* Revisit if/then/else according to MS style guide.

* Removed unused code

* Update documentation.

* Remove foldi function.
  • Loading branch information
nojaf authored Dec 17, 2020
1 parent fc5969a commit 4e28f64
Show file tree
Hide file tree
Showing 16 changed files with 360 additions and 327 deletions.
29 changes: 17 additions & 12 deletions docs/Documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -431,28 +431,26 @@ let b = [|4;5;6|]
Fantomas by default follows the if/then/else conventions listed in the [Microsoft F# style guide](https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-if-expressions).
There is stated:

> If either cond, e1 or e2 are longer, but not multi-line:
> if cond
> then e1
> else e2
> Indentation of conditionals depends on the size and complexity of the expressions that make them up. Write them on one line when:
> cond, e1, and e2 are short
> e1 and e2 are not if/then/else expressions themselves.
This setting controls what exactly short means in terms of character count.
This setting facilitates this by determining the maximum character width where the if/then/else expression stays in one line.
Default = 40.

`defaultConfig`

```fsharp
if System.Char.IsUpper(c) then sprintf "_%s" (c.ToString().ToLower()) else c.ToString()
if myCheck then truth else bogus
```

Neither of the expression is longer than the default 40 here.

`{ defaultConfig with MaxIfThenElseShortWidth = 30 }`
`{ defaultConfig with MaxIfThenElseShortWidth = 10 }`

```fsharp
if System.Char.IsUpper(c)
then sprintf "_%s" (c.ToString().ToLower())
else c.ToString()
if myCheck then
truth
else
bogus
```

### fsharp_max_infix_operator_expression
Expand Down Expand Up @@ -821,6 +819,13 @@ type Range =

### fsharp_keep_if_then_in_same_line

**Deprecated setting!**

This setting will be removed in the next major version of Fantomas.
As of 4.4, it has no effect anymore due to a change in the MS F# style guide.

#### Original description:

Bypasses the situation where `if`,`then` and `else` are placed underneath each other.
This will ensure `if` and `then` are kept in the same line.
Default = false.
Expand Down
10 changes: 6 additions & 4 deletions src/Fantomas.Tests/ActivePatternTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ let (|ParseRegex|_|) regex str =
else None"""
({ config with
MaxValueBindingWidth = 30
MaxFunctionBindingWidth = 30 })
MaxFunctionBindingWidth = 30
MaxIfThenElseShortWidth = 70 })
|> prepend newline
|> should equal """
let (|Even|Odd|) input =
Expand All @@ -59,7 +60,8 @@ let (|Integer|_|) (str: string) =
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
"""
7 changes: 4 additions & 3 deletions src/Fantomas.Tests/AppTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,8 @@ type Queue<'T>(data: list<'T []>, length: int) =
type Queue<'T>(data: list<'T []>, length: int) =
member this.Head =
if length > 0
then (List.head data).[0]
else raise (System.Exception("Queue is empty"))
if length > 0 then
(List.head data).[0]
else
raise (System.Exception("Queue is empty"))
"""
4 changes: 3 additions & 1 deletion src/Fantomas.Tests/CompilerDirectivesTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1850,7 +1850,9 @@ let getDefaultProxyFor =
match calcEnvProxies.Force().TryFind uri.Scheme with
| Some p -> if p.GetProxy uri <> uri then p else getDefault()
| None -> getDefault())
""" config
"""
{ config with
MaxIfThenElseShortWidth = 50 }
|> prepend newline
|> should equal """
let getDefaultProxyFor =
Expand Down
16 changes: 10 additions & 6 deletions src/Fantomas.Tests/ComputationExpressionTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -792,7 +792,9 @@ let ``let + let + let bang + if/then/else in ce`` () =
else
return ()
}
""" config
"""
{ config with
MaxIfThenElseShortWidth = 75 }
|> prepend newline
|> should equal """
let rec private appendToAzureTableStorage (cosmoEvents: EventWrite<JsonValue> seq) =
Expand Down Expand Up @@ -1643,7 +1645,9 @@ let create: Highlighter =
}
|> List.ofSeq
|> FormattedText.fromList
""" config
"""
{ config with
MaxIfThenElseShortWidth = 80 }
|> prepend newline
|> should equal """
let create: Highlighter =
Expand All @@ -1664,10 +1668,10 @@ let create: Highlighter =
yield TextSpan.highlight ms.[i].Value
let regStart = ms.[i].Index + ms.[i].Length
if i < ms.Count - 1
then yield TextSpan.normal (s.Substring(regStart, ms.[i + 1].Index - regStart))
elif regStart < s.Length
then yield TextSpan.normal (s.Substring(regStart))
if i < ms.Count - 1 then
yield TextSpan.normal (s.Substring(regStart, ms.[i + 1].Index - regStart))
elif regStart < s.Length then
yield TextSpan.normal (s.Substring(regStart))
}
|> List.ofSeq
Expand Down
13 changes: 10 additions & 3 deletions src/Fantomas.Tests/ControlStructureTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ let test x y =
else "Don't know"
if age < 10
then printfn "You are only %d years old and already learning F#? Wow!" age""" config
then printfn "You are only %d years old and already learning F#? Wow!" age"""
{ config with
MaxIfThenElseShortWidth = 60 }
|> prepend newline
|> should equal """
let rec tryFindMatch pred list =
Expand Down Expand Up @@ -141,7 +143,10 @@ 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 @@ -542,7 +547,9 @@ let internal coli f' (c: seq<'T>) f (ctx: Context) =
i <- i + 1
st
""" config
"""
{ config with
MaxIfThenElseShortWidth = 50 }
|> prepend newline
|> should equal """
let internal coli f' (c: seq<'T>) f (ctx: Context) =
Expand Down
7 changes: 4 additions & 3 deletions src/Fantomas.Tests/DynamicOperatorTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ 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
else NoColor
if f.errors?(fieldNameY) && f.touched?(fieldNameZ) then
IsDanger
else
NoColor
|> Input.Color
"""

Expand Down
Loading

0 comments on commit 4e28f64

Please sign in to comment.