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

[style-guide] Lambda closing paren defaults #730

Open
auduchinok opened this issue Feb 21, 2023 · 5 comments
Open

[style-guide] Lambda closing paren defaults #730

auduchinok opened this issue Feb 21, 2023 · 5 comments

Comments

@auduchinok
Copy link
Contributor

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
Copy link
Contributor

kerams commented Feb 21, 2023

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
Copy link
Contributor Author

auduchinok commented Feb 21, 2023

@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
Copy link

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
Copy link
Contributor

nojaf commented Feb 24, 2023

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
Copy link

cmeeren commented Apr 21, 2023

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👍

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

No branches or pull requests

5 participants