imports long list align + list align behaviour regression when set to newline

[paolino]

In version 0.11.0.3, a configuration containing

     list_align: new_line
     long_list_align: new_line_multiline

was producing a nice consistent layout with all imported stuff in a new line and longer than 80 import lines broken in multiple lines

i.e. this test would have passed

case26b :: Assertion
case26b =
    assertSnippet (step (Just 80) options)
    [ "import Data.Map (Map)"
    , "import Data.List (concat, foldl, foldr, head, init, last, length, map, null, reverse, tail, (++))"
    ]
    [ "import Data.List"
    , "    ( concat"
    , "    , foldl"
    , "    , foldr"
    , "    , head"
    , "    , init"
    , "    , last"
    , "    , length"
    , "    , map"
    , "    , null"
    , "    , reverse"
    , "    , tail"
    , "    , (++)"
    , "    )"
    , "import Data.Map"
    , "    (Map)"
    ]
  where
    options = defaultOptions { listAlign = NewLine, importAlign = None, longListAlign = Multiline}

But now, on 0.14.5.0, I cannot reproduce it. In fact, I get

expected: 
import Data.List
    ( concat
    , foldl
    , foldr
    , head
    , init
    , last
    , length
    , map
    , null
    , reverse
    , tail
    , (++)
    )
import Data.Map
    (Map)

 but got: 
import Data.List
    ( concat
    , foldl
    , foldr
    , head
    , init
    , last
    , length
    , map
    , null
    , reverse
    , tail
    , (++)
    )
import Data.Map (Map)

where the (Map) is not on a new line.

also, without the longListAlign , the listAlign option is respected

i.e. this test passes

case26a :: Assertion
case26a =
    assertSnippet (step (Just 80) options)
    [ "import Data.Map (Map)"
    , "import Data.List (concat, foldl, foldr, head, init, last, length, map, null, reverse, tail, (++))"
    ]
    [ "import Data.List"
    , "    (concat, foldl, foldr, head, init, last, length, map, null, reverse, tail,"
    , "    (++))"
    , "import Data.Map"
    , "    (Map)"
    ]
  where
    options = defaultOptions { listAlign = NewLine, importAlign = None}

Maybe I need a different combination of the options? Any help appreciated

[sorki]

This is a fun one!

In version 0.11.0.3, a configuration containing

     list_align: new_line
     long_list_align: new_line_multiline

was producing a nice consistent layout with all imported stuff in a new line and longer than 80 import lines broken in multiple lines

Note that new_line_multiline corresponds to InlineToMultiline of LongListAlign which actually means try inline first, if too long go multiline.

i.e. this test would have passed

case26b :: Assertion
case26b =
    assertSnippet (step (Just 80) options)
    [ "import Data.Map (Map)"
    , "import Data.List (concat, foldl, foldr, head, init, last, length, map, null, reverse, tail, (++))"
    ]
    [ "import Data.List"
    , "    ( concat"
    , "    , foldl"
    , "    , foldr"
    , "    , head"
    , "    , init"
    , "    , last"
    , "    , length"
    , "    , map"
    , "    , null"
    , "    , reverse"
    , "    , tail"
    , "    , (++)"
    , "    )"
    , "import Data.Map"
    , "    (Map)"
    ]
  where
    options = defaultOptions { listAlign = NewLine, importAlign = None, longListAlign = Multiline}

Multiline vs InlineToMultiline.

The PR #467 tries to address this by introducing better names for constructors and their respective parser options.

But now, on 0.14.5.0, I cannot reproduce it. In fact, I get

expected: 
import Data.List
    ( concat
    , foldl
    , foldr
    , head
    , init
    , last
    , length
    , map
    , null
    , reverse
    , tail
    , (++)
    )
import Data.Map
    (Map)

 but got: 
import Data.List
    ( concat
    , foldl
    , foldr
    , head
    , init
    , last
    , length
    , map
    , null
    , reverse
    , tail
    , (++)
    )
import Data.Map (Map)

where the (Map) is not on a new line.

Which is because InlineToMultiline first tries inline mode, and import Data.Map (Map) fits.

also, without the longListAlign , the listAlign option is respected

i.e. this test passes

case26a :: Assertion
case26a =
    assertSnippet (step (Just 80) options)
    [ "import Data.Map (Map)"
    , "import Data.List (concat, foldl, foldr, head, init, last, length, map, null, reverse, tail, (++))"
    ]
    [ "import Data.List"
    , "    (concat, foldl, foldr, head, init, last, length, map, null, reverse, tail,"
    , "    (++))"
    , "import Data.Map"
    , "    (Map)"
    ]
  where
    options = defaultOptions { listAlign = NewLine, importAlign = None}

This is because the default LongListAlign (Inline) has a special case https://github.com/haskell/stylish-haskell/blob/5503e2726545b79ab9af960999efb538ef35a4e8/lib/Language/Haskell/Stylish/Step/Imports.hs#L482-L484

But it is also the only LongListAlign variant that has it.

Maybe I need a different combination of the options? Any help appreciated

I'm not exactly sure, what is the right solution here

• 467 tries to fix docs and constructor naming to match implementation

• 468 introduces MultilineOnly mode, which almost does what you want, except it would result in

  import Data.Map
  ( Map
  )

  so it almost seems that there should be a LongListAlign Newline mode so this is explicit (but it would clash with new_line parser value at the moment - #467) or ListAlign needs to be respected in other than just the default Inline of LongListAlign.

Another possibility might be to deprecate ListAlign completely in favor of LongListAlign (which seems to be the main type governing the output) and rename LongListAlign to ListAlign but this seems quite difficult to achieve in backwards compatible manner.