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

MultiWayIf formatted with four spaces #1002

Closed
oberblastmeister opened this issue Mar 22, 2023 · 6 comments · Fixed by #1011
Closed

MultiWayIf formatted with four spaces #1002

oberblastmeister opened this issue Mar 22, 2023 · 6 comments · Fixed by #1011
Labels
style Nitpicking and things related to purely visual aspect for formatting.

Comments

@oberblastmeister
Copy link

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

  • OS name + version: Arch Linux
  • ormolu 0.5.3.0 1b39d94d0c06aa53a577f7e0476695f9e6f5020b using ghc-lib-parser 9.4.3.20221104
@mrkkrp
Copy link
Member

mrkkrp commented Mar 22, 2023

There is a reason for this, see #488.

@amesgen
Copy link
Member

amesgen commented Mar 22, 2023

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

testing = 
  do
    succ
   1

In #707 (comment), 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?

@amesgen amesgen added the style Nitpicking and things related to purely visual aspect for formatting. label Mar 22, 2023
@mitchellwrosen
Copy link
Contributor

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. 😅

@mrkkrp
Copy link
Member

mrkkrp commented Mar 23, 2023

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
Copy link
Member

amesgen commented Apr 10, 2023

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
foo0 =
  do
    2

foo1 =
  do
      succ
    1

foo2 =
  if
    | otherwise -> 2

foo3 =
  if
      | otherwise -> succ
    1
foo0 =
  do
    2

foo1 =
  do
    succ
   1

foo2 =
  if
      | otherwise -> 2

foo3 =
  if
      | otherwise -> succ
    1

@mrkkrp
Copy link
Member

mrkkrp commented Apr 11, 2023

@amesgen I quite like this idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
style Nitpicking and things related to purely visual aspect for formatting.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants