tweag / ormolu

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

MultiWayIf formatted with four spaces #1002

Closed oberblastmeister closed 1 year ago

oberblastmeister commented 1 year ago

Describe the bug

testing =
  if
    | otherwise -> ()
    | otherwise -> ()
    | otherwise -> ()

gets formatted into this

testing =
  if
      | otherwise -> ()
      | otherwise -> ()
      | otherwise -> ()

Expected behavior It should not be formatted with four spaces.

Environment

mrkkrp commented 1 year ago

There is a reason for this, see #488.

amesgen commented 1 year ago

Yeah, I wonder whether we might want to revisit that, given that we have the same problem with do:

testing = 
  do
    succ
   1

In https://github.com/tweag/ormolu/issues/707#issuecomment-888359615, we decided that this should be a fixed point, as indenting the bodies of do blocks by 4 spaces to account just for this edge case seems to be more severe than violating our "indentation should be divisible by 2" principle.

From a consistency standpoint, shouldn't we change MultiWayIf to also use only 2 spaces and accept the 1 space indentation if an argument is applied to it? WDYT?

mitchellwrosen commented 1 year ago

From a consistency standpoint, shouldn't we change MultiWayIf to also use only 2 spaces and accept the 1 space indentation if an argument is applied to it? WDYT?

I agree with this. Aside: I have never even seen code that applies the RHS of a multi-way if to an argument; I didn't know it was possible. :sweat_smile:

mrkkrp commented 1 year ago

Note that if we start using 1 space, then whatever is nested inside that 1-space-indented block is going to be at odd (literally) indentations: 3, 5, etc., and it this kind of code block can be potentially large, which is the worst.

I do understand the consistency argument though.

amesgen commented 1 year ago

Another alternative that would fix this issue and also improve consistency: What about identing do blocks/cases/MultiWayIf with with 4 spaces iff it is the applicand (i.e. f in f a)?

I.e. this would be a fixed point, note how applied do blocks and MultiWayIf are now formatted consistently compared to the current output, and foo1 no longer has odd indentation:

Proposed fixed point Current output
```haskell foo0 = do 2 foo1 = do succ 1 foo2 = if | otherwise -> 2 foo3 = if | otherwise -> succ 1 ``` ```haskell foo0 = do 2 foo1 = do succ 1 foo2 = if | otherwise -> 2 foo3 = if | otherwise -> succ 1 ```
mrkkrp commented 1 year ago

@amesgen I quite like this idea.