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

Record on single line setting #697

Closed
nojaf opened this issue Feb 21, 2020 · 4 comments
Closed

Record on single line setting #697

nojaf opened this issue Feb 21, 2020 · 4 comments

Comments

@nojaf
Copy link
Contributor

nojaf commented Feb 21, 2020

Proposal

Fantomas currently formats a record on multiple lines if there are multiple fields.
Even though the record itself can still be short.

For example

type A = { A: int; B: int }

let a = { C = 1; B = 2 }

let b = { a with C = 2; B = 3 }

will be formatted as

type A =
    { A: int
      B: int }

let a =
    { C = 1
      B = 2 }

let b =
    { a with
          C = 2
          B = 3 }

If two or more fields are involved, every expression is put on multiple lines.
The suggestion from G-Research is to introduce a new setting to control with maximum allowed record width. And only format on multiple lines if the record exceeds this setting.

Proposed setting name : MaxSingleLineRecordWidth
Default value : 40

Impact

This would introduce a breaking change stylistically speaking.
It would require a change in multiline and has some impact on CodePrinter.

Gains

The outcome would be more concise code.

@jindraivanek
Copy link
Contributor

+1

Actually, I would like global setting like this that would do this for all potentially multiline expressions. (but that is potentially hard to implement)

@auduchinok
Copy link
Contributor

Could it maybe check if the line fits line length with single-line record formatting instead?

@nojaf
Copy link
Contributor Author

nojaf commented Feb 22, 2020

Not sure you would always want this.
For example:

let a = { LongIdentifier = "foo"; X = a; BaaaarrIdentifier = "meeeeeeh"; Meh = FooAgain; A = y; Y = e; C = c; D = dd }

This is 118 long and would in theory fit on a PageWidth 120 setting.
However, for me, this should be on multiple lines.

I think a similar setting could also be useful for deciding when an array should be on multiple lines or not. Debatable whether this should be a separate setting or the same setting.

f.ex.

let a = [ "Fooooooooooooo";  "Baaaaaaaaaaaaaaaaaar"; "Meeeeeeeeeeeeeeeh"; "Turn"; "To"; "Stooooooooooooooooooooonees" ]

This again is 119 characters long and I would use multiple lines instead of placing it on one as it does now.

We could introduce some new construct that tries a naive solution to format an expression and then check if that exceeds a certain length.

Something along the lines of

    let isExpressionLongerThen f maxWidth ctx =
        // create empty context
        let dummyCtx =  f { ctx with
                              WriterModel = WriterModel.init
                              WriterEvents = Queue.empty }
        let writerEvents =
            dummyCtx.WriterEvents
            |> Seq.takeWhile (function | WriteLine _ -> false | _ -> true)
            |> Seq.toArray

        let totalColumnSize events =
            Seq.fold (fun acc ev ->
                match ev with
                | Write w -> String.length w
                | _ -> 0
                |> (+) acc) 0 events
            |> fun total -> total > maxWidth

        let isLonger =
            if dummyCtx.WriterEvents.Length > writerEvents.Length
            then true // multiple lines
            else totalColumnSize writerEvents // checks if single line is longer then maxWidth

        isLonger

    let isLong (ctx: Context) =
        isExpressionLongerThen shortExpr ctx.Config.MaxSingleLineRecordWidth ctx

    (ifElseCtx isLong multilineExpr shortExpr) ctx

@nojaf
Copy link
Contributor Author

nojaf commented Apr 24, 2020

Implemented in #721

@nojaf nojaf closed this as completed Apr 24, 2020
@nojaf nojaf added the v4 label Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants