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

Revisit Elmish code #2180

Closed
nojaf opened this issue Apr 2, 2022 · 3 comments · Fixed by #2244
Closed

Revisit Elmish code #2180

nojaf opened this issue Apr 2, 2022 · 3 comments · Fixed by #2244

Comments

@nojaf
Copy link
Contributor

nojaf commented Apr 2, 2022

Introduction

We currently have multiple settings to control a subset of function applications with array/lists as arguments.
The rationale behind this is described in Formatting Elmish style guide and has a certain overlap with #1408. #1275 is always related in a way and it would be nice if we can consolidate everything in a general fashion.

Proposal

As part of #1408, I propose to move and expand the current Elmish settings. To generalize and apply an alternative style for function applications that end with a list/array.

Scope

Every (nested) SynExp.App that ends with one or two SynExpr.ArrayOrListComputed (or SynExpr.ArrayOrList).

Examples:

f1 a1 [ b1 ; b2 ; b3 ]

f2 [ a1; a2; a3 ] [ b1; b2; b3 ]

f3 a b c [ d1; d2 ]

Arguments before the final list arguments can be of any SynExpr.

Rules

First of, try and fit the expression on one line:

fn a [ b1; b1 ]

If the expression is longer than a threshold (be it max_line_length or fsharp_max_elmish_width)

It should put the elements of the list on the next line indented:

fn a [
    b1
    b2
]

Assuming the function name and additional arguments (fn a) fit on the same line, otherwise the default formatting rules should apply.

fn
   a
   [
        b1
        b2
   ]

When there are two lists at the end, there would be slight variation where the same rules are applied.

// try on single line first
fn [ a1; a2 ] [ b1; b2 ]

// only put last argument on next line, assuming `fn [ a1; a2 ]` fits on one line
fn [ a1; a2 ] [
    b1
    b2
]

// put both list on new lines
fn [
    a1
    a2
] [
    b1
    b2
]

Notice that the ] [ is on a separate line to visually indicate the difference of the elements of both list.
In Elmish code, the first list typically has properties, where the second list will contains children.

Real world samples

These formatted according to the rules above.

fragment [] [
    View.navigation dispatch

    Row.row [
        Row.Custom [ ClassName "no-gutters" Id "main" ] 
    ] [
        View.editor model dispatch
        React.router [ 
            router.onUrlChanged onUrlChanged
            router.children [ routes ] 
        ]
    ]
]
Html.div [
    Html.button [
        prop.style [ style.marginRight 5 ]
        prop.onClick (fun _ -> setCount(count + 1))
        prop.text "Increment"
    ]

    Html.button [
        prop.style [ style.marginLeft 5 ]
        prop.onClick (fun _ -> setCount(count - 1))
        prop.text "Decrement"
    ]

    Html.h1 count
]
testList "A test group" [ 
    test "one test" { Expect.equal (2 + 2) 4 "2+2" }

    test "another test that fails" { Expect.equal (3 + 3) 5 "3+3" }

    testAsync "this is an async test" {
        let! x = async { return 4 }
        Expect.equal x (2 + 2) "2+2"
    }

    testTask "this is a task test" {
        let! n = Task.FromResult 2
        Expect.equal n 2 "n=2"
    } 
]

Open questions

  • Should this all be under the same setting of Stroustrup bracket style? I personally believe so, but I'm open to ideas.
  • How much flexibility should there be for the threshold?
  • Perhaps this should be considered a default in the MS style guide? The proposed rules share some elements with how lambdas and computation expressions are formatted.
@panmona
Copy link
Contributor

panmona commented Jul 19, 2022

The removal of these settings unfortunately seems to make it not possible to have nicely formatted Feliz code like it was possible in V4 with single_argument_web_mode.

With single_argument_web_mode=true it was nicely formatted with default settings: Fantomas Tools

That doesn't seem to be possible now in the newest preview, with the defaults and Stroustroup enabled: Fantomas Tools

Is this actually supported and I am missing something?
If not we should probably have a new issue for this.

@nojaf
Copy link
Contributor Author

nojaf commented Jul 20, 2022

Hello, you need to enable fsharp_multiline_block_brackets_on_same_column as well when using fsharp_experimental_stroustrup_style. Fantomas Tools

@panmona
Copy link
Contributor

panmona commented Jul 20, 2022

Ah, thanks! Good to see that it still works, was worried for a second there :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants