tweag / ormolu

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

do with unnecessary Curly braces and BlockArguments #473

Closed amesgen closed 4 years ago

amesgen commented 4 years ago
{-# LANGUAGE BlockArguments #-}

x = f
  do 1
  do 2
  do 3

results in

{-# LANGUAGE BlockArguments #-}

x = f
  do { 1 }
  do { 2 }
  do 3
amesgen commented 4 years ago

This fixes the problem above, and all tests pass:

diff --git a/src/Ormolu/Printer/Meat/Declaration/Value.hs b/src/Ormolu/Printer/Meat/Declaration/Value.hs
index 4d81fbc..3079092 100644
--- a/src/Ormolu/Printer/Meat/Declaration/Value.hs
+++ b/src/Ormolu/Printer/Meat/Declaration/Value.hs
@@ -18,6 +18,7 @@ import Ctype (is_space)
 import Data.Bool (bool)
 import Data.Char (isPunctuation, isSymbol)
 import Data.Data hiding (Infix, Prefix)
+import Data.Functor ((<&>))
 import Data.List (intersperse, sortOn)
 import Data.List.NonEmpty ((<|), NonEmpty ((:|)))
 import qualified Data.List.NonEmpty as NE
@@ -565,10 +566,13 @@ p_hsExpr' s = \case
                   if isOneLineSpan spn
                     then inci
                     else id
+        ub <- getLayout <&> \case
+          SingleLine -> id
+          MultiLine -> dontUseBraces
         useBraces $ do
           located func (p_hsExpr' s)
           breakpoint
-          indent $ sep breakpoint (located' p_hsExpr) initp
+          indent $ sep breakpoint (ub . located' p_hsExpr) initp
         indent $ do
           unless (null initp) breakpoint
           located lastp p_hsExpr

But I am not sure if this is the "correct" fix.


EDIT: This works too:

diff --git a/src/Ormolu/Printer/Meat/Declaration/Value.hs b/src/Ormolu/Printer/Meat/Declaration/Value.hs
index 4d81fbc..c5f99f1 100644
--- a/src/Ormolu/Printer/Meat/Declaration/Value.hs
+++ b/src/Ormolu/Printer/Meat/Declaration/Value.hs
@@ -18,6 +18,7 @@ import Ctype (is_space)
 import Data.Bool (bool)
 import Data.Char (isPunctuation, isSymbol)
 import Data.Data hiding (Infix, Prefix)
+import Data.Functor ((<&>))
 import Data.List (intersperse, sortOn)
 import Data.List.NonEmpty ((<|), NonEmpty ((:|)))
 import qualified Data.List.NonEmpty as NE
@@ -565,7 +566,10 @@ p_hsExpr' s = \case
                   if isOneLineSpan spn
                     then inci
                     else id
-        useBraces $ do
+        ub <- getLayout <&> \case
+          SingleLine -> useBraces
+          MultiLine -> id
+        ub $ do
           located func (p_hsExpr' s)
           breakpoint
           indent $ sep breakpoint (located' p_hsExpr) initp
mrkkrp commented 4 years ago

@utdemir Thoughts?

utdemir commented 4 years ago

@mrkkrp Both look good to me, I slightly prefer the second patch since then the function application is also covered by possible useBraces; also it looks simpler.

I couldn't come up with a counter-example. I think this patch works out in the end since it only omits braces on multi-line layout and not on the last argument.

mrkkrp commented 4 years ago

@amesgen Would you like to open a PR?