CompositionalIT / farmer

Repeatable Azure deployments with ARM templates - made easy!
https://compositionalit.github.io/farmer
MIT License
517 stars 156 forks source link

Fantomas 6 #1027

Closed nojaf closed 10 months ago

nojaf commented 1 year ago

The changes in this PR are as follows:

I have read the contributing guidelines and have completed the following:

Hi there,

We are trying to get some in-the-field feedback on our next major release. If memory serves correctly someone was interested in the Stroustrup style for Farmer as it could be a good fit when there is a lot of configuration. I could be wrong and this was more about when using Farmer versus the actual Farmer codebase. V6 has made a lot of improvements in that area, so I'm curious to hear your thoughts.

nojaf commented 1 year ago

Still found a minor issue: https://github.com/fsprojects/fantomas/issues/2806

isaacabraham commented 1 year ago

Thanks so much - this is definitely highly desired - the stroustrup style makes life a lot easier, particularly in the Arm namespace because we make heavy use of nested list comprehensions, and without stroustrup it pushes everything across more and more.

nojaf commented 1 year ago

Thanks for confirming, I'll get back to you later this week once we have fixed the found issue.

isaacabraham commented 1 year ago

Would also be great to have fsharp_newline_before_multiline_computation_expression = true set as well.

(As an aside, isn't the setting named backwards? Surely it should be false for e.g

let x = foo {
    bar 123
}
nojaf commented 1 year ago

Yeah, naming is hard 😅.

fsharp_newline_before_multiline_computation_expression = true by default. See https://fsprojects.github.io/fantomas/docs/end-users/Configuration.html#fsharp_newline_before_multiline_computation_expression

What we are going for:

There will be a newline before the computation expression by default.

    let something = // Here is the newline before the computation expression
        task {
            let! thing = otherThing ()
            return 5
        }

If you don't want the CE to go to the next line (aka you don't want the newline):

let something = (* no newline before the CE *) task {
    let! thing = otherThing ()
    return 5
}

So, you want the setting to be false?

martinbryant commented 11 months ago

Hi @nojaf - can we look at getting the conflicts dealt with? Thanks

nojaf commented 11 months ago

@martinbryant done!

martinbryant commented 11 months ago

Amazing @nojaf thanks :)

ninjarobot commented 10 months ago

Since we don't have any active PRs right now, I recommend we get this merged as soon as possible.

@nojaf can you please resolve the conflicts one more time?

ninjarobot commented 10 months ago

Or should this use fantomas 6.2.0 instead of 6.1.2 @nojaf ?

nojaf commented 10 months ago

You can check out the version differences at https://github.com/fsprojects/fantomas/blob/main/CHANGELOG.md.

I'll go ahead and close this PR. Whether this gets merged or not doesn't really matter to me anymore. If someone else ever wants to do the upgrade, they can use this PR as a reference. 😊