ormolu icon indicating copy to clipboard operation
ormolu copied to clipboard

Repeated "dollar" application forces code blocks down and to the right

Open jkachmar opened this issue 3 years ago • 4 comments

Describe the bug 2efc5d37f9946d67f218f12cc326599ad5dd4b98 seems to have introduced behavior that causes repeated $ application to "stairstep" the code downwards and to the right.

To Reproduce From the examples:

foo =
  bar
    $ baz
    $ quux

...formats to:

foo =
  bar $
    baz $
      quux

And a slightly more degenerate, "real-world" case:

fmapLT mapErr $ ExceptT $ try $ liftIO $
  <...>

...formats to:

fmapLT mapErr $
  ExceptT $
    try $
      liftIO $
        <...>

Expected behavior

  1. Repeated $ application should not reflow text unless forced (e.g. by a newline)
  2. Repeated $ application should not necessarily reflow text in such a way that causes its indentation to "stairstep" inwards

Environment

  • OS name + version: macOS 10.14
  • Version of the code: 0.1.2.0

Additional context N/A

jkachmar avatar Aug 24 '20 18:08 jkachmar

Just letting your know that this behavior is purposeful, it is a feature.

mrkkrp avatar Sep 06 '20 17:09 mrkkrp

Just letting your know that this behavior is purposeful, it is a feature.

I'd argue this isn't a great feature.

Which is nicer::

takeExprA =
  infixOpLeftRecursion p "+" _Add $ -- Additions of
    infixOpLeftRecursion p "*" _Mul $ -- multiplications of
      tryMatchAtom p (prismFallback _Lit) _Show $ -- literals or
        parens takeExprA -- expressions in parens

takeExprB =
  infixOpLeftRecursion p "+" _Add $ -- Additions of
  infixOpLeftRecursion p "*" _Mul $ -- multiplications of
  tryMatchAtom p (prismFallback _Lit) _Show $ -- literals or
  parens takeExprB -- expressions in parens

Imho the second one is nicer. In the example, if we added a third operator to the parsed AST, in the first form we'd have to further indent lines below it.

I understand that the indentation presents the structure, but so does the order, so the indentation is somewhat redundant here!

yairchu avatar Dec 27 '20 19:12 yairchu

For what it’s worth I’ve taken to using . wherever there was previously repeated $ application to avoid this indentation issue, but that’s a mediocre solution at best (in my opinion).

I imagine in the example above, it would work fine, but for other situations it’s not a clean substitution (e.g. anything using the ST monad trick, anything levity-polymorphic).

jkachmar avatar Dec 27 '20 20:12 jkachmar

Actually my example above uses RankNTypes and so it is not a clean substitution as well. If I change it to a . I get:

Spec.hs:21:5: error:
    • Couldn't match type ‘f2’ with ‘f0’
      ‘f2’ is a rigid type variable bound by
        a type expected by the context:
          VerbosePrism' String [[Char]] (Expr, [[Char]])
        at test/Spec.hs:21:5-35

yairchu avatar Dec 28 '20 07:12 yairchu