Odd alignment of list patterns

[expipiplus1]

foo = \case
  []     -> _
  [ x ]  -> _
  x : xs -> _

bar = \case
  []          -> _
  [      x ]  -> _
  x : x' : xs -> _

It would be interesting to hear what Brittany's reasoning is here :)

[lspitzner]

hah, thanks for the report. Lets see..

BDLines
  [ BDCols
      ColBindingLine Nothing
      [ BDCols
          ColPatterns
          [BDCols ColPatterns [BDSeq [BDLit (pack "[]"),BDSeparator,BDSeparator]]]
      , BDSeq [BDLit (pack "->"),BDSeparator,BDLit (pack "_")]
      ]
  , BDCols
      ColBindingLine Nothing
      [ BDCols
          ColPatterns
          [BDCols ColPatterns [BDLit (pack "["),BDCols ColPatterns [BDLit (pack "x")],BDSeq [BDLit (pack "]"),BDSeparator,BDSeparator]]]
      , BDSeq [BDLit (pack "->"),BDSeparator,BDLit (pack "_")]
      ]
  , BDCols
      ColBindingLine Nothing
      [ BDCols
          ColPatterns
          [ BDCols
              ColPatterns
              [ BDCols ColPatterns [BDSeq [BDLit (pack "x"),BDSeparator]]
              , BDSeq [BDLit (pack ":"),BDSeparator]
              , BDCols ColPatterns [BDSeq [BDLit (pack "xs"),BDSeparator,BDSeparator]]
              ]
          ]
      , BDSeq [BDLit (pack "->"),BDSeparator,BDLit (pack "_"),BDEmpty]
      ]
  ]

so essentially the column tree looks like

foo = \case

  []     -> _
  11111112222
  aa

  [ x ]  -> _
  11111112222
  aabbcc

  x : xs -> _
  11111112222
  aabbccc

-- and for the bar case:

  x : x' : xs -> _
  1111111111112222
  aaaaaaabbccc
  AABBCCC

so obviously a lot of these trees share the "1abc2" part, all with the ColPatterns column key, and it aligns accordingly.

:-)

of course this is not ideal. guess we should not use the same column type for all pattern stuff. There probably should be ColPatternsBracket, ColPatternsParen in addition to https://github.com/lspitzner/brittany/blob/ac0a5bd7c7e8c9ead2d3a5a1474fdace59194baa/src/Language/Haskell/Brittany/Internal/Layouters/Pattern.hs#L198 that is used for all columns inside patterns currently. I am not completely certain if this would break alignment where we want it, but I think it won't cause problems. Easy enough to try and see.

Do you want to make a shot at fixing this?

[lspitzner]

I realized this is a tiny bit more involved than I had anticipated, because layoutPat really needs to also return the intended ColSig. And while at that, I noticed that colsWrapPat <=< layoutPat is really rather common and we should probably rename "layoutPat" -> "layoutPatAsSeq" and new layoutPat = colsWrapPat <=< layoutPatAsSeq.

I'll have another go at this when I find some time.