👋 @cblp
Thank you for raising your pull request.
Please make sure you have followed our contributing guidelines in DEVELOPER.md. We will review it as soon as possible!

Reviewer: Please verify the following things have been done, if applicable.

• A file has been added to changelog.d/
• Docs have been updated in web/site/pages/config/
• Tests have been added
• Diff files in compat-tests/ either:
  1. Have NOT been changed, or
  2. Modifies fourmolu.yaml with an appropriate option to keep the same formatting, or
  3. Modifies Haskell files with new formatting
     • ONLY as a last resort - see Breaking changes policy

Added unsolved questions to the PR description. Please read.

1. I think one or four lines makes sense. In other words, if it was originally written on one line, keep it one line (same as ifs today). If it's multiline (whether originally written on multiple lines or other formatting forcing it to be), it should be the full four lines
2. Following the spirit of the previous point, I think it would be most intuitive to do

if cond1
  || cond2 then
  something
else
  something else

The multiline condition makes the overall expression four lines. I think it's fine that the indentation in the condition matches the block below, we already do this in a few places like instances with multiline constraints.

I think I solved problems 1 and 2. Sorry, I find merged condition and then-block with then keyword somewhere between unreadable.

Unfortunately, I don't like the code I wrote, I don't know Fourmolu code good enough. The style is great, but implementation sucks a little. I hope somebody will refactor it after me.

Also, I'm not comfortable with the option name. one-level-ifs came from original patch, but I think indent-then-else :: Bool is better.

indent-then-else doesn't quite communicate it to me either. How about compact-ifs?

Which version is more compact? The proposed feature makes code narrower but sometimes taller. E.g. it may reformat 3-line block into 4-line.

I was thinking yours is more compact because it puts the if/then/else keywords on two lines instesd of three (in a multiline context)

Could also do shifted-ifs, since this format is shifting then up one line and shifting the bodies left one indentation

one-level-ifs or same-level-ifs is still fine for me, since it's putting then on the same line as if

Could also do shifted-ifs, since this format is shifting then up one line and shifting the bodies left one indentation

Shift from what? Do you assume one of these style "old" or "default"?

From my taste, "then" on the same line with "if" is the neutral position, and "then" placed on a separate line and indented is shifted, because simply "indented" == "shifted".

Yes, the default Fourmolu format is the default because that's how 90% of Haskell code is formatted in the wild. So the option should be describing how we're transforming from that default style

Oh, from that perspective, ok. I'm still not sure about the naming. I'd like leave the decision for the mainteiner.

I noticed there are no test cases covering multiple chained if ... then ... else ...s, e.g.

if foo then
  doFoo
else if bar then
  doBar
else
  doBaz

This style ^ is what I would love to use with fourmolu, and I'm wondering if this one-level ifs PR will allow that?

@cblp I am the maintainer 🙂 Let's go with shifted-ifs.

Also, can you apply this diff? The key thing here is that we should avoid changing the code too much, to minimize conflicts when merging Ormolu changes back.

diff --git a/src/Ormolu/Printer/Meat/Declaration/Value.hs b/src/Ormolu/Printer/Meat/Declaration/Value.hs
index cc29dad9..0480a462 100644
--- a/src/Ormolu/Printer/Meat/Declaration/Value.hs
+++ b/src/Ormolu/Printer/Meat/Declaration/Value.hs
@@ -1101,6 +1101,8 @@ p_if ::
   LocatedA body ->
   R ()
 p_if placer render anns if' then' else' = do
+  oneLevelIfs <- getPrinterOpt poOneLevelIfs
+
   txt "if"
   space
   located if' p_hsExpr
@@ -1109,6 +1111,9 @@ p_if placer render anns if' then' else' = do
         where
           AnnsIf {aiThen, aiElse} = anns
 
+      locatedToken tokenSpan token =
+        located (L tokenSpan ()) $ \_ -> txt token
+
       betweenSpans spanA spanB s = spanA < s && s < spanB
 
       placeHangingLocated tokenSpan bodyLoc@(L _ body) = do
@@ -1121,33 +1126,23 @@ p_if placer render anns if' then' else' = do
         switchLayout [tokenSpan, bodySpan] $
           placeHanging placement (located bodyLoc render)
 
-  oneLevelIfs <- getPrinterOpt poOneLevelIfs
+      placeBranch tokenSpan body =
+        if oneLevelIfs && isOneLineSpan (getLocA body)
+          then placeHanging Normal (located body render)
+          else placeHangingLocated tokenSpan body
 
-  let placeBranch tokenSpan token body = do
-        located (L tokenSpan ()) $ \_ -> txt token
+  let hangIf m =
         if oneLevelIfs
-          then do
-            if isOneLineSpan $ getLocA body
-              then do
-                breakpoint
-                inci (located body render)
-              else placeHangingLocated tokenSpan body
-          else do
-            space
-            placeHangingLocated tokenSpan body
-
-      branches = do
-        placeBranch thenSpan "then" then'
-        breakpoint
-        placeBranch elseSpan "else" else'
-
-  if oneLevelIfs
-    then do
-      if isOneLineSpan $ getLocA if' then space else breakpoint
-      branches
-    else do
-      breakpoint
-      inci branches
+          then switchLayout [getLocA if'] breakpoint >> m
+          else breakpoint >> inci m
+  hangIf $ do
+    locatedToken thenSpan "then"
+    space
+    placeBranch thenSpan then'
+    breakpoint
+    locatedToken elseSpan "else"
+    space
+    placeBranch elseSpan else'
 
 p_let ::
   -- | True if in do-block

Can you also add @evanrelf's suggestion to the test suite?

@evanrelf I think if-else-if chain is a good feature, but should be implemented separately

@cblp Can you ateast add a test so the behavior is documented?

@brandonchinn178 I've added the patch.
@evanrelf I've added an if-else-if chain example to tests and examples.

@evanrelf I'm not sure aligning nested (chained) ifs is useful since we have MultiWayIf syntax.

Thanks for adding those example cases! Hopefully we can collapse those nested ifs in the future 😄

I'm not sure aligning nested (chained) ifs is useful since we have MultiWayIf syntax.

You're right that they both achieve the same semantic result, but syntactically they're meaningfully different. Nested ifs look like every other programming language, so they read more naturally for me. Also MultiWayIf adds two additional levels of indentation.

f =
  if foo then do
    before
    doFoo
    after
  else if bar then do
    before
    doBar
    after
  else do
    before
    doBaz
    after

f =
  if
    | foo -> do
        before
        doFoo
        after
    | bar -> do
        before
        doBar
        after
    | otherwise -> do
        before
        doBaz
        after