Skip to content

Commit

Permalink
Fix unnecessary big indentation (fsprojects#418)
Browse files Browse the repository at this point in the history
* atColumn

* delayed atColumn

* TestExternalProjects: add fulma-demo

* atColumn fixes

* tests update

* comments

* merge fix

* merge fix

* comments
  • Loading branch information
jindraivanek authored and nojaf committed Feb 18, 2019
1 parent fb1fba0 commit c0ccb19
Show file tree
Hide file tree
Showing 12 changed files with 69 additions and 30 deletions.
4 changes: 2 additions & 2 deletions src/Fantomas.Tests/ClassTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ System.String.Concat
("a",
"b"
+ (longNamedFunlongNamedFunlongNamedFunlongNamedFunlongNamedFun
(longNamedClasslongNamedClasslongNamedClasslongNamedClasslongNamedClasslongNamedClass)).Property)
(longNamedClasslongNamedClasslongNamedClasslongNamedClasslongNamedClasslongNamedClass)).Property)
"""

[<Test>]
Expand All @@ -360,5 +360,5 @@ type Exception with
type Exception with
member inline __.FirstLine =
(__.Message.Split
([| Environment.NewLine |], StringSplitOptions.RemoveEmptyEntries)).[0]
([| Environment.NewLine |], StringSplitOptions.RemoveEmptyEntries)).[0]
"""
2 changes: 1 addition & 1 deletion src/Fantomas.Tests/ControlStructureTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ let x =
|> should equal """
let x =
if try
true
true
with Failure _ -> false
then ()
else ()
Expand Down
6 changes: 3 additions & 3 deletions src/Fantomas.Tests/DataStructureTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ let ``should keep -> notation``() =
|> should equal """
let environVars target =
[ for e in Environment.GetEnvironmentVariables target ->
let e1 = e :?> Collections.DictionaryEntry
e1.Key, e1.Value ]
let e1 = e :?> Collections.DictionaryEntry
e1.Key, e1.Value ]
"""

[<Test>]
Expand Down Expand Up @@ -140,7 +140,7 @@ let a2 = [| 0..99 |]
let a3 =
[| for n in 1..100 do
if isPrime n then yield n |]
if isPrime n then yield n |]
"""

[<Test>]
Expand Down
4 changes: 2 additions & 2 deletions src/Fantomas.Tests/FormattingSelectionTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ let a3 =
|> should equal """
let a3 =
[| for n in 1..100 do
if isPrime n then yield n |]"""
if isPrime n then yield n |]"""

[<Test>]
let ``should format around the cursor inside an object expression``() =
Expand All @@ -233,7 +233,7 @@ let ``should format around the cursor inside an object expression``() =
|> should equal """
let obj1 =
{ new System.Object() with
member x.ToString() = "F#" }"""
member x.ToString() = "F#" }"""

[<Test>]
let ``should format around the cursor inside a computation expression``() =
Expand Down
8 changes: 4 additions & 4 deletions src/Fantomas.Tests/InterfaceTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ let ``object expressions``() =
|> should equal """
let obj1 =
{ new System.Object() with
member x.ToString() = "F#" }
member x.ToString() = "F#" }
"""

[<Test>]
Expand All @@ -72,8 +72,8 @@ let ``object expressions and interfaces``() =
|> should equal """
let implementer() =
{ new ISecond with
member this.H() = ()
member this.J() = ()
member this.H() = ()
member this.J() = ()
interface IFirst with
member this.F() = ()
member this.G() = () }
Expand All @@ -92,7 +92,7 @@ let f () =
|> should equal """
let f() =
{ new obj() with
member x.ToString() = "INotifyEnumerableInternal"
member x.ToString() = "INotifyEnumerableInternal"
interface INotifyEnumerableInternal<'T>
interface IEnumerable<_> with
member x.GetEnumerator() = null }
Expand Down
10 changes: 5 additions & 5 deletions src/Fantomas.Tests/LetBindingTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ let f () =
|> should equal """let f() =
let x = 1
(while true do
()
()
x)
"""

Expand Down Expand Up @@ -110,8 +110,8 @@ let x =
let x =
[| 1..2 |]
|> Array.mapi (fun _ _ ->
let num =
""
.PadLeft(9)
num)
let num =
""
.PadLeft(9)
num)
"""
4 changes: 2 additions & 2 deletions src/Fantomas.Tests/PatternMatchingTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,8 @@ with
try
fst
(find
(fun (s, (s', ty) : int * int) ->
s' = s0 && can (type_match ty ty0) []) (!the_interface))
(fun (s, (s', ty) : int * int) ->
s' = s0 && can (type_match ty ty0) []) (!the_interface))
with Failure _ -> s0
"""

Expand Down
4 changes: 2 additions & 2 deletions src/Fantomas.Tests/PipingTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,6 @@ let prefetchImages =
let prefetchImages =
[ playerOImage; playerXImage ]
|> List.map (fun img ->
link [ Rel "prefetch"
Href img ])
link [ Rel "prefetch"
Href img ])
"""
2 changes: 1 addition & 1 deletion src/Fantomas.Tests/TypeDeclarationTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ type BlobHelper(Account : CloudStorageAccount) =
configSettingPublisher.Invoke(connectionString) |> ignore)
BlobHelper
(CloudStorageAccount.FromConfigurationSetting
(configurationSettingName))
(configurationSettingName))
"""

[<Test>]
Expand Down
2 changes: 1 addition & 1 deletion src/Fantomas/CodeFormatterImpl.fs
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ let formatRange returnFormattedContentOnly (range : range) (lines : _ []) config
// Mono version of indent text writer behaves differently from .NET one,
// So we add an empty string first to regularize it
|> if returnFormattedContentOnly then Context.str String.Empty else Context.str pre
|> Context.atIndentLevel startCol (Context.col Context.sepNln formatteds Context.str)
|> Context.atIndentLevel true startCol (Context.col Context.sepNln formatteds Context.str)
|> if returnFormattedContentOnly then Context.str String.Empty else Context.str post
|> Context.dump

Expand Down
2 changes: 1 addition & 1 deletion src/Fantomas/CodePrinter.fs
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ and genExpr astContext synExpr =
(sepOpenL +> atCurrentColumn (colAutoNlnSkip0 sepWithPreserveEndOfLine xs (genExpr astContext)) +> sepCloseL)

| Record(inheritOpt, xs, eo) ->
let recordExpr = opt (!- " with ") eo (genExpr astContext) +> atCurrentColumn (col sepSemiNln xs (genRecordFieldName astContext))
let recordExpr = opt (!- " with ") eo (genExpr astContext) +> atCurrentColumnIndent (col sepSemiNln xs (genRecordFieldName astContext))
sepOpenS
+> atCurrentColumn (opt (if xs.IsEmpty then sepNone else ifElseCtx (futureNlnCheck recordExpr sepNone) sepNln sepSemi) inheritOpt
(fun (typ, expr) -> !- "inherit " +> genType astContext false typ +> genExpr astContext expr))
Expand Down
51 changes: 45 additions & 6 deletions src/Fantomas/Context.fs
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,26 @@ type ColumnIndentedTextWriter(tw : TextWriter) =
let indentWriter = new IndentedTextWriter(tw, " ")
let mutable col = indentWriter.Indent

// on newline, bigger from Indent and atColumn is selected
// that way we avoid bigger than indentSpace indentation when indent is used after atCurrentColumn
let mutable atColumn = 0

let applyAtColumn f =
let newIndent = f atColumn
indentWriter.Indent <- newIndent

member __.ApplyAtColumn f = applyAtColumn f

member __.Write(s : string) =
match s.LastIndexOf('\n') with
| -1 -> col <- col + s.Length
| i -> col <- s.Length - i - 1
| i ->
applyAtColumn (fun x -> max indentWriter.Indent x)
col <- s.Length - i - 1
indentWriter.Write(s)

member __.WriteLine(s : string) =
applyAtColumn (fun x -> max indentWriter.Indent x)
col <- indentWriter.Indent
indentWriter.WriteLine(s)

Expand All @@ -32,6 +45,10 @@ type ColumnIndentedTextWriter(tw : TextWriter) =
with get() = indentWriter.Indent
and set i = indentWriter.Indent <- i

member __.AtColumn
with get() = atColumn
and set i = atColumn <- i

member __.InnerWriter = indentWriter.InnerWriter

interface IDisposable with
Expand Down Expand Up @@ -74,6 +91,7 @@ type internal Context =
member x.With(writer : ColumnIndentedTextWriter) =
writer.Indent <- x.Writer.Indent
writer.Column <- x.Writer.Column
writer.AtColumn <- x.Writer.AtColumn
// Use infinite column width to encounter worst-case scenario
let config = { x.Config with PageWidth = Int32.MaxValue }
{ x with Writer = writer; Config = config }
Expand All @@ -86,11 +104,13 @@ let internal dump (ctx: Context) =
/// Indent one more level based on configuration
let internal indent (ctx : Context) =
ctx.Writer.Indent <- ctx.Writer.Indent + ctx.Config.IndentSpaceNum
// if atColumn is bigger then after indent, then we use atColumn as base for indent
ctx.Writer.ApplyAtColumn (fun x -> if x >= ctx.Writer.Indent then x + ctx.Config.IndentSpaceNum else ctx.Writer.Indent)
ctx

/// Unindent one more level based on configuration
let internal unindent (ctx : Context) =
ctx.Writer.Indent <- max 0 (ctx.Writer.Indent - ctx.Config.IndentSpaceNum)
ctx.Writer.Indent <- max ctx.Writer.AtColumn (ctx.Writer.Indent - ctx.Config.IndentSpaceNum)
ctx

/// Increase indent by i spaces
Expand All @@ -104,18 +124,37 @@ let internal decrIndent i (ctx : Context) =
ctx

/// Apply function f at an absolute indent level (use with care)
let internal atIndentLevel level (f : Context -> Context) ctx =
let internal atIndentLevel alsoSetIndent level (f : Context -> Context) ctx =
if level < 0 then
invalidArg "level" "The indent level cannot be negative."
let oldLevel = ctx.Writer.Indent
ctx.Writer.Indent <- level
let oldColumn = ctx.Writer.AtColumn
if alsoSetIndent then ctx.Writer.Indent <- level
ctx.Writer.AtColumn <- level
let result = f ctx
ctx.Writer.AtColumn <- oldColumn
ctx.Writer.Indent <- oldLevel
result

/// Write everything at current column indentation
/// Set minimal indentation (`atColumn`) at current column position - next newline will be indented on `max indent atColumn`
/// Example:
/// { X = // indent=0, atColumn=2
/// "some long string" // indent=4, atColumn=2
/// Y = 1 // indent=0, atColumn=2
/// }
/// `atCurrentColumn` was called on `X`, then `indent` was called, but "some long string" have indent only 4, because it is bigger than `atColumn` (2).
let internal atCurrentColumn (f : _ -> Context) (ctx : Context) =
atIndentLevel ctx.Writer.Column f ctx
atIndentLevel false ctx.Writer.Column f ctx

/// Write everything at current column indentation, set `indent` and `atColumn` on current column position
/// /// Example (same as above):
/// { X = // indent=2, atColumn=2
/// "some long string" // indent=6, atColumn=2
/// Y = 1 // indent=2, atColumn=2
/// }
/// `atCurrentColumn` was called on `X`, then `indent` was called, "some long string" have indent 6, because it is indented from `atCurrentColumn` pos (2).
let internal atCurrentColumnIndent (f : _ -> Context) (ctx : Context) =
atIndentLevel true ctx.Writer.Column f ctx

/// Function composition operator
let internal (+>) (ctx : Context -> Context) (f : _ -> Context) x =
Expand Down

0 comments on commit c0ccb19

Please sign in to comment.