fourmolu / fourmolu

A fourk of ormolu that uses four space indentation and allows arbitrary configuration. Don't like it? PRs welcome!
https://fourmolu.github.io/
Other
356 stars 62 forks source link

Break up "respectful" configuration #281

Open brandonchinn178 opened 1 year ago

brandonchinn178 commented 1 year ago

Today, respectful does three things:

  1. Preserve import groups or merge into one import group
  2. Omit newlines between top-level declarations if user omitted newlines (or force newlines between)
  3. Keep where keyword on same line as closing parens in an export list (https://github.com/fourmolu/fourmolu/pull/245)

IMO it's perfectly reasonable for someone to want to respect some of these and not respect others.

@georgefst: What do you think about replacing respectful: true | false with separate options for each of these scenarios (and have independent options for future "respectful"-related formatting), e.g.

# default: preserve
import-groups: "preserve" | "merge"

# default: false
force-newlines-between-decls: true | false

# default: false
force-where-newline-exports: true | false

Pros:

Cons:

Related: https://github.com/fourmolu/fourmolu/issues/280

georgefst commented 1 year ago

Yeah, I knew we'd have to do this eventually. I think we need to keep a --respectful flag which implies all the others though (backwards-compatibility, for one thing). This might be awkward to implement - I don't think any of our current flags work this way?

brandonchinn178 commented 1 year ago

@georgefst why do we need to keep --respectful? It's on by default (which all of these options would too), and I've never heard of anyone turning it off. IIUC it was only added to keep parity with ormolu styling (which requires it disabled).

IMO we shouldn't worry about the config being backwards compatible. I would be in favor of erroring if the config file contained any unknown options, though (which would've helped with #306). In the off chance someone manually set respectful: false, they'll get an error to update their config, a one time thing when upgrading fourmolu

georgefst commented 1 year ago

IMO we shouldn't worry about the config being backwards compatible.

In general, I really think we should. I mentioned something in another thread about how I'd even like to provide migration scripts.

But it's a very good point that the existence of --respectful is kind of a historical curiosity, which very few people are likely to have manually disabled. So I'm willing to make an exception here.

I would be in favor of erroring if the config file contained any unknown options, though (which would've helped with https://github.com/fourmolu/fourmolu/issues/306).

Agreed.

georgefst commented 1 year ago

But it's a very good point that the existence of --respectful is kind of a historical curiosity, which very few people are likely to have manually disabled. So I'm willing to make an exception here.

I think I've changed my mind on this, unfortunately. I've seen repos that have copy-pasted one of the config files from this repo, so they will have an explicit respectful setting.

brandonchinn178 commented 1 year ago

If we kept respectful for backwards compatibility, would you be amenable to showing a deprecation warning and removing it X versions later, or would you be opposed to any removal?