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

Splitting ExperimentalStroustrupStyle to separate settings #2276

Closed
Thorium opened this issue May 24, 2022 · 3 comments
Closed

Splitting ExperimentalStroustrupStyle to separate settings #2276

Thorium opened this issue May 24, 2022 · 3 comments

Comments

@Thorium
Copy link
Member

Thorium commented May 24, 2022

I love the ExperimentalStroustrupStyle setting in records, but I hate it in computation expressions.
Computation expressions are so crucial part of F# that it should be very clear what goes on there.

What we want to optimize? Maintenance.
Let's observe the following code:

let myMethod() =
    async {
        let! x =
           async {
               do! waitSomething()
               return 4
           }
        return {
            Id = 1
            Data1 = "asdf"
            Data2 = "asdf"
            Data3 = x
        }
    }

There is no way to create this properly on any settings in Fantomas.

@nojaf
Copy link
Contributor

nojaf commented May 25, 2022

Hello, thank you for opening this discussion on the Fantomas repository.

Splitting ExperimentalStroustrupStyle is an interesting case.

let (|StroustrupStyleExpr|_|) (isStroustrupStyleEnabled: bool) (e: SynExpr) =
if not isStroustrupStyleEnabled then
None
else
match e with
// { foo with Bar = bar }
| SynExpr.Record(copyInfo = Some _) -> None
| SynExpr.Record _
| SynExpr.AnonRecd _
// task { ... }
| SynExpr.App (ExprAtomicFlag.NonAtomic, false, SynExpr.Ident _, SynExpr.ComputationExpr _, _)
| ArrayOrList _ -> Some e
| _ -> None

Stroupstrous current works on records, array/list and computation expressions.
You can imagine that if we split one thing, people will eventually ask for everything to be split.
This will bring in an additional churn to maintain Fantomas.
It might be worth having, I would like to see what the community thinks about this.
Are enough people interested in this to carry it?
And of course, what big of an impact it will have on the codebase. Because there would be more changes of course than just the snippet above.

@josh-degraw
Copy link
Contributor

josh-degraw commented Aug 19, 2022

Is your issue here essentially that you want async { to be on its own line, rather than on the same line as the binding? Because if so, in my opinion that's not (or shouldn't be) directly related to the Stroustrup style, which is mainly just about where the braces are placed relative to identifiers. IMO your example still conforms to Stroustrup style with the brackets, so I'd say it would make sense to allow that, but as a different flag, like compututation_expressions_on_newline or something like that.

@nojaf
Copy link
Contributor

nojaf commented Apr 13, 2023

Available in v6.

@nojaf nojaf closed this as completed Apr 13, 2023
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