tweag/ormolu

Compact single-line expression with dangling close paren

sellout opened this issue · 2 comments

Is your feature request related to a problem? Please describe.

When cleaning up code, I often find myself with something like

foo
  ( some expression here
  )
  bar

Describe the solution you'd like

I would like Ormolu to reformat that as

foo
  (some expression here)
  bar

because it is guaranteed to not make the line any longer, since the appended ) is balanced by the removed space after (.

Thanks, I see why one might want this. The current behavior is motivated by the following aspect of the design goals:

This means that the choices between single-line/multi-line layouts in certain situations are made by the user, not by an algorithm.

In this particular case, when Ormolu encounters the parentheses, it sees that they are distributed across more than one line, so it uses the multi-line layout accordingly when printing.

We could however choose to make an exception for e.g. cases like yours, i.e. don't take the "multi-line-ness" of the parentheses into account, and instead use the layout of the enclosed expression.


The implementation would look like this

diff --git a/src/Ormolu/Printer/Meat/Declaration/Value.hs b/src/Ormolu/Printer/Meat/Declaration/Value.hs
index ae7e8e4..1639ec0 100644
--- a/src/Ormolu/Printer/Meat/Declaration/Value.hs
+++ b/src/Ormolu/Printer/Meat/Declaration/Value.hs
@@ -705,8 +705,11 @@ p_hsExpr' isApp s = \case
     -- negated literals, as `- 1` and `-1` have differing AST.
     when (negativeLiterals && isLiteral) space
     located e p_hsExpr
-  HsPar _ e ->
-    parens s (located e (dontUseBraces . p_hsExpr))
+  HsPar _ e -> do
+    csSpans <-
+      fmap (flip RealSrcSpan Strict.Nothing . getLoc) <$> getEnclosingComments
+    switchLayout (locA e : csSpans) $
+      parens s (located e (dontUseBraces . p_hsExpr))
   SectionL _ x op -> do
     located x p_hsExpr
     breakpoint

(Something similar should be done for e.g. HsParTy.)

Taking comments into account is necessary to still use multi-line parentheses in cases like

test = (
  -- a
  x)

WDYT @mrkkrp?

I think this is an improvement. Could you open a PR @amesgen ?