fsharp / fslang-design

RFCs and docs related to the F# language design process, see https://github.com/fsharp/fslang-suggestions to submit ideas
512 stars 142 forks source link

[style-guide] Lambda closing paren defaults #730

Open auduchinok opened 1 year ago

auduchinok commented 1 year ago

There's inconsistency of how closing bracket is placed with default Fantomas settings:

// closing bracket is on the new line
seq {
    //
    return ()
}

// closing bracket is on the expression line
seq (fun _ ->
    //
    ())

// closing bracket is on the new line
someTupledFunction (
    "A very long string making all of this multi-line",
    "A very long string making all of this multi-line"
)

I suggest we could try to move closing bracket in lambdas to a new line, so it's more inline with other similar constructions. It would also make it easier to edit the lambda if the closing bracket was on a new line:

seq (fun _ ->
    //
    ()
)

Moving the closing bracket like this is already supported in Fantomas, but it's not turned on by default.

kerams commented 1 year ago

I assume this would only apply to the special case of lambda being the last argument?

I personally don't mind

seq
    (fun _ ->
        //
        ())
    55
auduchinok commented 1 year ago

@kerams I don't mind it either, but I found that lately I've been preferring the following:

seq
    (fun _ ->
        //
        ()
    )
    55

Or even this:

[1; 2; 3]
|> List.map (fun i ->
    // some multiline expression here
)
|> List.iter (fun o ->
    // another one
)

It feels a bit more natural to edit the lambda body when the paren is on another line, for some reason. I think it's similar to computation expressions here.

josh-degraw commented 1 year ago

I'd definitely be a fan of this decision.

E M B R A C E    S T R O U S T R U P

nojaf commented 1 year ago

Moving the closing bracket like this is already supported in Fantomas, but it's not turned on by default.

Please be aware that this is controlled by a setting tailored to the G-Research style guide and has influence over more just an application with a single lambda (as in the first post)

Sample

From a pragmatic standpoint, it makes more sense to keep these bundled and not start mixing them.

That all being said, I believe the opening post brings up a valid point. Comparing the lambda to the computation expression, for example, does seem inconsistent.

cmeeren commented 1 year ago

I went here to post a new issue, but I think this issue covers it.

The end paren of lambdas should be on a separate line, corresponding to setting fsharp_multi_line_lambda_closing_newline to true in Fantomas. In other words, I think true should be the default value.

This would make it consistent with other code constructs, such as method calls, where the end parenthesis is on a separate line.

For example, methods are currently formatted like:

MyMethod(
   arg1,
   arg2
)

But lambdas are currently formatted like

(fun x ->
   expr1
   expr2)

Indeed, the inconsistency is more egregious when a method takes a lambda argument; then it uses the lambda formatting instead of the method formatting:

MyMethod(fun x ->
   expr1
   expr2)

E M B R A C E    S T R O U S T R U P

@josh-degraw I wholeheartedly agree👍