Preference: avoid dangling closing parentheses

[chreekat]

(do
    v <- addVolumeSetPublic aiAcct
    c <- makeAddContainer v (Just ReleasePUBLIC) Nothing
    Just v' <- lookupVolume ((volumeId . volumeRow) v)
    Just s <- lookupSlotByContainerId c
    pure (v', slotContainer s)
)

versus

(do
    v <- addVolumeSetPublic aiAcct
    c <- makeAddContainer v (Just ReleasePUBLIC) Nothing
    Just v' <- lookupVolume ((volumeId . volumeRow) v)
    Just s <- lookupSlotByContainerId c
    pure (v', slotContainer s))

I believe the latter is more in the spirit of Haskell's whitespace-delimited style.

[eborden]

There are two camps in this situation. I see both forms, but I prefer the former. This form is consistent with brittany's handling of ] and } as well.

examples with --columns 20

foo =
  [ some
  , looonnnggg
  , list
  ]

bar = record
  { update = some
  , extra = fields
  }

[chreekat]

I'm in favor of consistency when the situations are analogous, like in export lists or other tuples. Where dangling parentheses become an eyesore is in a $-free style with a couple levels of nesting: they aren't book-ending a list, they're just duplicating the effort of whitespace-sensitive layouting. My introductory example is exactly that (and it's a relatively minor example).

Having said that, I would like to collect more real examples that I consider bad before I make a strong stand on this matter. :) I'd also like to fix the bug that causes layouts like

(some line that's 80 chars long
)

Since that's basically unrelated, and also really ugly.

[lspitzner]

There are two technical hurdles for such a feature: Firstly, there is an invariant for the docSeq/BDSeq part of the brittany DSL which assumes that we don't have multi-line nodes in the middle of such a sequence. But this layout would demand exactly that: Essentially docSeq [lparen, childnode, rparen] for a potentially multi-line childnode(-layout). Breaking this invariant might be harmless, but it also could lead to really strange layouting bugs.

Secondly, in the internal algorithm, the spacing information for each potential layout of a node in the syntax tree only tracks two things: The length in the first line, plus the length of the block below the first line. Best explained via example: For a simple function application node like myFunc myArg myOtherArg we have two potential layouts

myFunc myArg myOtherArg
myFunc
  myArg
  myOtherArg

where the first has size (23, Nothing) and the second has size (6, Just 12).

The issue with this it does not track the length of the last line, so if we now add parenthesis (i.e. we calculate the sizes of the parent node, which happens to be (myFunc myArg myOtherArg)), we get (25, Nothing) and (8, Just ?) where ? is.. 12 or 13 - we cannot tell. And with a bit of nesting we can get arbitrary amounts of closing parentheses.

For this second part, we could either a) rewrite the size-tracking to also save the length of the last line (if such exists) b) default to the lower value (optimistic, but parentheses might overflow column limit) or c) default to the higher value (might reject certain valid layouts).

--

in summary, i think a proper implementation would be a significant bit of work, and a quicker, somewhat sloppy impl that does not always give optimal results might be possible, but no promises.

[lspitzner]

I agree with @eborden that such a setting should also affect other kinds of parentheses, for consistency. With that, it is not a counterargument any more.

@chreekat what is the better alternative for

(some line that's 80 chars long
)

? Or perhaps I don't understand the context of that example?

--

as for my personal preference on this whole thing - nothing strong, really. "inline-closing-parens" breaks with the "same level in the ast => same indentation for all involved syntax" postulation, but the I admit the (line) cost for this can indeed be high. Also you might get a bit more ambiguity regarding "optimal layout" (assuming the algorithm supported finding the optimum), consider the question of

((((((foo bar baz))
))))
-- versus
((((((foo bar baz
))))))

(for a 20 column limit) so you get a bit more of "child node affects layout of parents". but that is a weak argument, as single-line versus multiple-line size already affects parents, too.

[jerbaroo]

A situation where I would like to avoid the dangling parentheses is the following:

import           Control.Monad.IO.Class         ( MonadIO
                                                , liftIO
                                                )
import           Telescope.Class                ( Telescope(..) )

To me these simple imports, which are the only ones in this module, should not be taking up 4 lines. Furthermore, I would like to avoid having 1 line that only contains 1 parenthesis.

[noughtmare]

Here is my response to:

This form is consistent with brittany's handling of ] and } as well.

Perhaps it is already answered by @chreekat

Where dangling parentheses become an eyesore is in a $-free style with a couple levels of nesting: they aren't book-ending a list, they're just duplicating the effort of whitespace-sensitive layouting.

But I would like to add that you can see it from yet another perspective, namely that expressions with a single "element" should have the closing brace/bracket/paren on the same line and multi "element" expression should have the closer on a new line.

And function application is always a single "element", so parentheses would almost always be placed on the same line. But list brackets can also be placed on the same line if there is only one element. I just ran into this example:

x = Def
  noPos
  Nothing
  (th2agPat pat)
  (Expression
    noPos
    [ HsToken (render $ to_HPJ_Doc (pprBody True body $$ where_clause wheres))
              noPos
    ]
  )
  False
  True
  False

I would like to see:

x = Def
  noPos
  Nothing
  (th2agPat pat)
  (Expression
    noPos
    [HsToken (render $ to_HPJ_Doc (pprBody True body $$ where_clause wheres))
             noPos])
  False
  True
  False

Both the list bracket and the function application parenthesis are closed on the line with noPos.

My definition of "element" would involve being on the same indentation level. List elements are all indented on the same level, but function arguments are indented further than the function they are applied to, so they are not separate elements: the whole function application is one element.

By the way, this means that I do like the separate line for the closing parenthesis in export and import lists, when they have multiple elements. So I'm not in favour of @jerbaroo's suggestion, but I am of course not against having that as a separate configuration option.