Feature request: more flexible record whitespace

[Smaug123]

Issue created from fantomas-ui

G-Research's internal coding guidelines specify that if a record spans more than one line, it should be split one entry per line, and the braces must be symmetrical. The original source below conforms to G-Research's coding standards; Fantomas's output does not. Could we please have a config option for this? I know that the official F# style guidelines say that this layout is "unusual", but it is the completely standard layout for us.

Code

type Foo =
    {
        Field : int
        Field2 : int
        Field244 : int
        Field3 : int
    }

Result

type Foo =
    { Field : int
      Field2 : int
      Field244 : int
      Field3 : int }

Options

Fantomas Next - 3.0.0-7/20/2019

Name	Value
IndentSpaceNum	4
PageWidth	80
PreserveEndOfLine	false
SemicolonAtEndOfLine	false
SpaceBeforeArgument	true
SpaceBeforeColon	true
SpaceAfterComma	true
SpaceAfterSemicolon	true
IndentOnTryWith	false
ReorderOpenDeclaration	false
SpaceAroundDelimiter	true
StrictMode	true

[blumu]

Also, having the curly curly brace on the first field makes it a little more tedious when editing the record. e.g. when adding, removing or moving a field via copy-pasting.
In addition to the symmetric style proposed in the issue description, it would be nice if Fantomas could support the following style:

type Foo = { 
    Field : int
    Field2 : int
    Field244 : int
    Field3 : int
}

[Ciantic]

Original:

            Command.sub ["commit"; "ci"] {
                opts =  [
                    Opts.oneOf (Optional,  [
                        Opt.value ["all"; "a"]
                        Opt.value ["interactive"]
                        Opt.value ["patch"; "p"]
                    ])
                    // To commit you *must* give either message or file where
                    // the message is.
                    Opts.oneOf (Required,  [
                        Opt.value ["file"; "F"]
                        Opt.value ["message"; "m"]
                    ])
                ]
                args = [
                    // You can give optional files to commit as positional
                    // arguments
                    Arg.infinite Optional "file"
                ]
                commands = []
            }

Fantomas:

              Command.sub [ "commit"; "ci" ] { opts =
                                                     [ Opts.oneOf
                                                           (Optional,
                                                            [ Opt.value
                                                                  [ "all"; "a" ]

                                                              Opt.value
                                                                  [ "interactive" ]

                                                              Opt.value
                                                                  [ "patch"; "p" ] ])

                                                       // To commit you *must* give either message or file where
                                                       // the message is.
                                                       Opts.oneOf
                                                           (Required,
                                                            [ Opt.value
                                                                  [ "file"; "F" ]

                                                              Opt.value
                                                                  [ "message";
                                                                    "m" ] ]) ]
                                                 args =
                                                     [ // You can give optional files to commit as positional
                                                       // arguments
                                                       Arg.infinite Optional
                                                           "file" ]
                                                 commands = [] }

I think too that Records should have newline configuration of sorts, that looks way too indented.

[jindraivanek]

@Ciantic This is solved in preview.

[jindraivanek]

To summary we have 3 possible variant of opening bracket:

O1. Opening bracket on same line (current default)

type Foo =
    { Field : int
      ....

O2. Opening bracket on separate line

type Foo =
    {
        Field : int
        ....

O3. Opening bracket on same line

type Foo = {
        Field : int
        ....

And 2 possible variant of closing bracket:

C1. Closing bracket on same line (current default)

      ....
      Field3 : int }

C2. Closing bracket on separate line

        ....
        Field3 : int 
    }

Same for list and arrays.

I support adding these options (shared for records, lists, arrays) to Fantomas.

Maybe:

• (bool) newline before opening bracket
• (bool) newline after opening bracket
• (bool) newline before closing bracket

[Ciantic]

@jindraivanek yeah! Looks much better now, updated to 3.0 preview. It still doesn't add newlines after brackets/square brackets.

Different example, formatted by Fantomas:

    let expect =
        Result<Schema, SetError>.Ok { opts =
                                          [ Opts.anyOf
                                              ([ (Optional, Opt.flagTrue [ "first"; "f" ])
                                                 (Optional, Opt.value [ "second"; "s" ]) ])
                                            Opts.oneOf
                                                (Optional,
                                                 [ Opt.flag [ "third"; "f" ]
                                                   Opt.valueWith "new value" [ "fourth"; "s" ] ]) ]
                                      args = []
                                      commands = [] }

It doesn't look horrible, in fact it's very readable but the amount of indentation looks a lot.

[jindraivanek]

@Ciantic Things like this are definitely a bug, multiline records should be on newline indented. I will create new issue for this.

[blumu]

@jindraivanek Is there a separate issue created to track the feature request mentioned above to have an option to select if a new line return should be inserted before/after opening/closing brackets? (Right now this is a blocker for adoption in our team.)

[jindraivanek]

@blumu There is no such issue, but thanks to remind me about this. We talked about it with @nojaf and we think best way forward is to introduce one new option to enable "bracket on separate line" style for records, list and arrays (cases O2+C2 from above). Will that cover your needs?

[blumu]

@jindraivanek Yes, that would cover our needs. Thanks!

[nojaf]

Hey @Smaug123 would this proposal work out for G-Research?

[Smaug123]

@nojaf Thanks - that would work for us.

[nojaf]

Hey @Smaug123, I've been working on a new setting to facilitate an alternative style of formatting for records. This hasn't been really trivial and I'm not entirely sure yet I've covered all scenarios.

It would be nice if some more code samples were posted here so I can add some more tests.

[ForNeVeR]

I love this new "alternate braces" syntax and have some samples I believe Fantomas should format without any changes. E.g. these particular versions of Types.fs and EmulsionXmpp.fs from the project I maintain.

If you want some more specific samples (e.g. more types/records/CEs or whatever), then please clarify your request: I have plenty of them here and there, and would be happy to share or prepare samples specifically for Fantomas tests.

[nojaf]

Hey @ForNeVeR , thanks for your comment. Let me clarify:
I'm looking for code snippets that contain records in various shapes and forms (nested records, object expressions, records in lists) that are not listed in the unit tests yet.

Please point to the concrete lines in files, f.ex. https://github.com/codingteam/emulsion/blob/3502000a9302324a3abde2bd413befce9727c738/Emulsion/Xmpp/Types.fs#L7-L12

To set the correct expectations, the new setting would format this:

type RoomInfo = {
    RoomJid: JID
    Nickname: string
}

to

type RoomInfo = 
    {
        RoomJid: JID
        Nickname: string
    }

I don't want to introduce too many different styles of formatting at the same time.
And I would like to limit the setting (for now) to records, types & object expressions.
Computation expressions are not in scope.

Hope this helps.

[auduchinok]

@nojaf I believe we should have an option to keep this style:

type RoomInfo = {
    RoomJid: JID
    Nickname: string
}

I've seen it in various projects before and it's clearly what people try to stick there. Could we have a union case instead of a boolean for this style setting?

[nojaf]

We could and I'm not against it.
How does that look for instances of a record, nested records, records in lists, object expressions,...?

It would be nice if someone could sum up everything and prove the consistency between all of it.
The code I've changed so far is consistent and did not require any changes to other parts. (No changes to lists for example)
If a similar story could be told for variant 3 then we can look at it.

[ForNeVeR]

Okay, here're some samples (we could discuss them if you think something here is breaking the guidelines; I've made sure they all compile with no warnings in F# 4.7):

open System

// Records
type Record = {
    foo: int
    bar: int64
    xs: obj seq
}

let recordExpr = {
    foo = 10
    bar = 100L
    xs = seq {
        yield! (seq {
            1
            2
            3
            ()
        } : obj seq)
    }
}

// Object expressions
let disposable = {
    new IDisposable with
        member _.Dispose() = ()
}

// List
let nestedList: obj list = [
    1
    2
    3
    [ // this case looks weird but seen rarely
        1
        2
        3
    ]
]

// Nested CE
let sequence: obj seq = seq {
    1
    2
    3
    async {
        Async.Start <| async {
            return ()
        }
        return 10
    }
}

let functionExpression = Func<_> (fun () ->
    let x = 10
    x + 5
)

Please let me know if you need more.

[Smaug123]

By contrast, G-Research's internal style guidelines suggest the following for that same block of code:

open System

// Records
type Record =
    {
        foo: int
        bar: int64
        xs: obj seq
    }

let recordExpr =
    {
        foo = 10
        bar = 100L
        xs = seq {
            yield! (seq {
                1
                2
                3
                ()
            } : obj seq)
        }
    }

// Object expressions
let disposable =
    { new IDisposable with
        member _.Dispose() = ()
    }

// List
let nestedList: obj list =
    [
        1
        2
        3
        [ // this case looks weird but seen rarely
            1
            2
            3
        ]
    ]

// Nested CE
let sequence: obj seq = seq {
    1
    2
    3
    async {
        Async.Start <| async {
            return ()
        }
        return 10
    }
}

let functionExpression = Func<_> (fun () ->
    let x = 10
    x + 5
)

[Smaug123]

@nojaf If there's a version which contains your new work with records etc, I'd be very happy to run it over some of our internal codebase and see what happens. (As you know, it's quite hard to come up with examples unprompted, but very easy to see when something "looks wrong" in a large existing codebase!)

[nojaf]

Hello @Smaug123 & @ForNeVeR, thanks for the examples.
Well, to be honest, this would have a lot of impacts all over the place 🙃.

I asked for samples related to records and got examples that change lists and computation expressions as well. So clearly we will have to find some middle ground on this.
The most important thing to technically implement this is that there is a method to the madness.

@ForNeVeR

// List
let nestedList: obj list = [
    1
    2
    3
    [ // this case looks weird but seen rarely
        1
        2
        3
    ]
]

The inner list is formatted differently than the outer one. It is not consistent and the technical problem here is that when formatting the inner list we don't know that it is being printed inside the outer list.

@Smaug123
Everything after an = sign that is multiline will start on a new line, so

// Nested CE
let sequence: obj seq = seq {
    1
    2
    3
    async {
        Async.Start <| async {
            return ()
        }
        return 10
    }
}

let functionExpression = Func<_> (fun () ->
    let x = 10
    x + 5
)

will be formatted to

// Nested CE
let sequence: obj seq =
    seq {
        1
        2
        3
        async {
            Async.Start <| async { return () }
            return 10
        }
    }

let functionExpression =
    Func<_>(fun () ->
        let x = 10
        x + 5)

I would not change this behaviour as it would end up being inconsistent with other parts of Fantomas.
F.ex:

let meh () = 
    printfn "meh"
    7 + 7

Please both give an example of records in a list and nested records, haven't seen those yet.

[nojaf]

And how would you format Elmish code like this:

    let navButtons =
      [
        div [Class "navbar-item navbar-social"]
              [ navButton "twitter" "https://twitter.com/FableCompiler" "fab fa-twitter" ]
        div [Class "navbar-item navbar-social"] [
                navButton "gitter" "https://gitter.im/fable-compiler/Fable" "fas fa-comment-dots" ]
      ]

    let menuItem label page currentPage dispatch=
        a [
          classList ["navbar-item", true; "is-active", page = currentPage]
          Href (toHash page)
          OnClick (fun _ -> dispatch ToggleBurger)
        ] [str label]

how do these lists work in your style?

[Smaug123]

Sure, I realise that what I consider to be "stylistically correct" might not even be consistent and I may have to change :) I think I'm fairly strongly attached to having a space after constructors like Func<'T>, but otherwise am happy with the computation expression formatting you gave.

I've never used Elmish, but if I'm reading the code correctly, I think that the two most obvious-to-me renderings are:

let navButtons =
    [
        div
            [ Class "navbar-item navbar-social" ]
            [ navButton "twitter" "https://twitter.com/FableCompiler" "fab fa-twitter" ]
        div
            [ Class "navbar-item navbar-social" ]
            [ navButton "gitter" "https://gitter.im/fable-compiler/Fable" "fas fa-comment-dots" ]
    ]

let menuItem label page currentPage dispatch =
    a
        [
            classList [ ("navbar-item", true) ; ("is-active", page = currentPage) ]
            Href (toHash page)
            OnClick (fun _ -> dispatch ToggleBurger)
        ]
        [ str label ]

or

let navButtons =
    [
        div
            [ Class "navbar-item navbar-social" ]
            [
                navButton
                    "twitter"
                    "https://twitter.com/FableCompiler"
                    "fab fa-twitter"
            ]
        div
            [ Class "navbar-item navbar-social" ]
            [
                navButton
                    "gitter"
                    "https://gitter.im/fable-compiler/Fable"
                    "fas fa-comment-dots"
            ]
    ]

let menuItem label page currentPage dispatch =
    a
        [
            classList
                [
                    "navbar-item", true
                    "is-active", page = currentPage
                ]
            Href (toHash page)
            OnClick (fun _ -> dispatch ToggleBurger)
        ]
        [ str label ]

Both of these are harmonious; perhaps the latter is better because of the multi-line classList in menuItem.

I would note that code of this general form is very unlike code I've seen at G-Research, though, so maybe my idea of "correct" formatting is not good; I could probably count on one finger the number of times I've seen lists of such strange shapes constructed and passed into functions at the call site.

[ForNeVeR]

@nojaf, I understand that this is potentially very big change, and that's why we're discussing it here; I'm okay with looking for some middle ground (i.e. what's possible and what isn't in the current Fantomas) for now. If you think that we better create a couple of more focused separate requests (e.g. for records, lists and CEs), I'll do it (and I'm not in position to require changes all over the place at once). Comments on your points:

The inner list is formatted differently than the outer one.

While I disagree that it's different, I have no strong opinion on that example, because the situation is extremely rare in my sources. What would you propose on that?

Everything after an = sign that is multiline will start on a new line

This is one of the main points of this proposed code style: to avoid this additional indent. I believe we should do our best to preserve this particular "oddity".

Please both give an example of records in a list and nested records, haven't seen those yet.

type MyRecord = {
    xxx: int
    yyy: string
    zzz: MyRecord option 
}

let nested = {
    xxx = 0
    yyy = ""
    zzz = Some {
        xxx = 0
        yyy = ""
        zzz = None
    }
}

let list = [
    {
        xxx = 0
        yyy = ""
        zzz = Some {
            xxx = 0
            yyy = ""
            zzz = None
        }
    }
    {
        xxx = 0
        yyy = ""
        zzz = None
    }
]

And how would you format Elmish code like this

I found this code sample very unusual for my code base (probably just because I've never used Elmish before), and I have no strong opinion on how multiline curried function arguments should be laid out. But one of valid variants in my point of view would be this:

let navButtons = [
    div
        [ Class "navbar-item navbar-social" ]
        [ navButton "twitter" "https://twitter.com/FableCompiler" "fab fa-twitter" ]
    div
        [ Class "navbar-item navbar-social" ]
        [ navButton "gitter" "https://gitter.im/fable-compiler/Fable" "fas fa-comment-dots" ]
]

let menuItem label page currentPage dispatch =
    a
        [ // "a [" would be cool but will create mess for the second argument, so I'll avoid this
            classList [
                ("navbar-item", true) // here, using the newline is optional, but I'll use it for sake of the example
                ("is-active", page = currentPage)
            ]
            Href (toHash page)
            OnClick (fun _ -> dispatch ToggleBurger)
        ]
        [ str label ]

[Smaug123]

Sorry, I just realised I missed out a chunk of your request. ForNeVeR's example I would format as follows:

type MyRecord =
    {
        xxx : int
        yyy : string
        zzz : MyRecord option
    }

let nested =
    {
        xxx = 0
        yyy = ""
        zzz = // although I would always pipe this instead
            Some
                {
                    xxx = 0
                    yyy = ""
                    zzz = None
                }
    }

let list =
    [
        {
            xxx = 0
            yyy = ""
            zzz =
                Some
                    {
                        xxx = 0
                        yyy = ""
                        zzz = None
                    }
        }
        {
            xxx = 0
            yyy = ""
            zzz = None
        }
    ]

[lydell]

Me and my colleagues would be super happy if there was an alternate placement of ), ] and } because we find it hard to edit code when all the ending parens/braces are bunched up at the end of a line.

let expected =
    (Event.Author "john",
     [ Event.Revenue
         (Event.RevenueId "123",
          Event.Match
              { transactionId = 1337
                amounts =
                    { total = 2500000L
                      vat = 625000L } }) ])

let xml =
    element "supplier-invoice-writeable"
        [ element "foreign-id" [ text foreignId ]
          element "invoice-date" [ text "2020-02-02" ]
          element "accounts"
              [ element "account"
                    [ element "account-nr" [ text "1234" ]
                      element "amount" [ text (String.fromInt64 amount) ] ]
                element "account"
                    [ element "account-nr" [ text "5678" ]
                      element "amount" [ text (String.fromInt64 -amount) ] ] ] ]

When trying to add a new field or list item in the above examples, we spend a lot of time trying to figure out after which )/]/} one should press Enter. And after pressing Enter, VS Code auto-indents to the same level as the last line so one has to dedent a bunch of times and then carefully align with the previous elements.

[nojaf]

The originally reported issue has been implemented and can be used by setting MultilineBlockBracketsOnSameColumn to true.
Example

There were other ideas and suggestions here as well, please create new issues for them.
So we can have focussed discussions on other issues and keep things organized.

[lydell]

@nojaf Awesome, thank you! ❤️

I split out two new issues:

• Something like MultilineBlockBracketsOnSameColumn but for tuples #814
• Something like MultilineBlockBracketsOnSameColumn but for parentheses #815