tweag / ormolu

A formatter for Haskell source code
https://ormolu-live.tweag.io
Other
944 stars 83 forks source link

Options for formatting #1061

Closed epoberezkin closed 6 months ago

epoberezkin commented 11 months ago

Changing arbitrary formatting decisions like this one prompted it.

It's ok to be an opinionated formatter, not allowing any options.

It's no longer ok when such opinions change, as it introduces a lot of annoying noice without much reason when version changes.

Arguably, this was the point when either an option should have been added, or the consistency should be preferred and no changes were made.

Even prettier, that is mentioned in that comment, has evolved to allow some options, however opinionated it was.

mrkkrp commented 11 months ago

Are you aware of Fourmolu?

epoberezkin commented 11 months ago

yes, but it has no option to revert the change in #806.

Also, I don't want to enable the team to have debate about particular options, so fixed ormolu style is better, as long as it remains fixed, or at least offers an option to avoid changes, if any changes happen... The need to add the option would also create additional friction to making any style changes, which is good, I think.

mrkkrp commented 11 months ago

We are fairly conservative regarding style changes at this point. It does not mean, however, that we should be unable to improve or correct our mistakes. #264 actually increased stylistic consistency, and #806 was an improvement for readability and correction of an oversight.

It seems to me that you would not like Ormolu to be configurable, so it is not clear what you want us to do.

epoberezkin commented 11 months ago

It seems to me that you would not like Ormolu to be configurable, so it is not clear what you want us to do.

What I suggest as a policy is this:

This is all in the spirit of maximising consistency and saving time, and minimising disruptions.

We are fairly conservative regarding style changes at this point.

What I am trying to say that the threshold for changing style should be much higher than "it seems better" or "it is more readable", given the role ormolu plays.

Taking "guards" as example, it was a proposal from one person, and implementation by another, without any time or discussion in between. So it doesn't look conservative, sorry - it's about opinion of two people creating work for thousands people... And again, I don't have a view about which style is better, I am just saying that changes in style are disruptive and should be avoided, unless there is a strong enough reason to make this change (e.g., for single constraint parents the compelling reason was changes in AST, and there was some discussion at least). "More readable" is very subjective, and doesn't seem a compelling reason to change the already adopted style...

epoberezkin commented 11 months ago

What you are also not accounting for, oversimplifying the style transition costs, is that with VSCode it is not always obvious how to directly control which ormolu version is used, and also different team members have to communicate and coordinate these style and ormolu version changes, as otherwise this is not just having this diff once - it will be changing back and forth every time somebody makes a PR. So on a whim, it's a lot of communication required between a lot of people, instead of solving the problems of their users, which changing style has nothing to do with...

ocharles commented 11 months ago

This is a pretty extreme position to take, and it assumes that every decision ormolu has made so far has been correct and is immovable. That is an incredibly high bar! I have noticed very few real changes from ormolu, and the few that I have encountered have generally made code more readable. Would you prefer:

I'm not trying to be binary, but I really can't see any real other options in what you're proposing. Ormolu intentionally doesn't have options, and I think that's a fine decision and not one that needs to change. If you want options, then this is something we need to figure out over at fourmolu.

Perhaps what you're asking for is more process around when these changes are made, and who gets to influence whether or not they should be taken?

epoberezkin commented 8 months ago

A poor formatting decision that we're stuck with forever, at the expensive of readability Occasional formatting changes as we improve the status quo?

Rather than having this dilemma, I would prefer a third option, as I wrote initially: to allow to opt out of these occasional formatting changes by introducing an option for at least a year after which it can be deprecated and then removed after one more year. That is, as a goal we don't want to have options, and want to converge to a single style. But any style changes should require an option that would be available for 2 years (the period is not so important as the principle, but given GHC upgrade cycle in most projects 2 years seem a reasonable middle ground). That is a compromise position that I am offering.

That would increase the value of improvement required for it to be accepted, and also allow teams to transition styles at their pace, not at yours.

Perhaps what you're asking for is more process around when these changes are made, and who gets to influence whether or not they should be taken?

That too, and introducing temporary option would support this process.

mrkkrp commented 6 months ago

Whether an option is temporary or not is irrelevant when it comes to maintenance. Having a formatting option, even if it is a single switch means that every example we are testing should be given twice if we are to maintain the quality that we are used to in this project. Two switches already mean 4x test cases that have to be analysed and adjusted manually. Now, Oromlu currently has about 450 unit tests based on particular input/output pairs, that's a lot of work we do not have bandwidth to handle.

I'd like to remind you that this is an open source project that is provided as is. We do strive to develop it in a way that makes sense for the community, but we are under no obligation to cater to a particular project that finds it hard to ensure compatible tooling versions across the entire development team.

To summarize, it does not make sense for us to stop making little tweaks that improve readability. It also does not make sense for us to complicate the project in order to keep promises we never gave in the first place.