prettyprinter icon indicating copy to clipboard operation
prettyprinter copied to clipboard

Move grouping in `encloseSep`

Open Lev135 opened this issue 1 year ago • 0 comments

Currently encloseSep is defined like

encloseSep l r s ds = case ds of
    []  -> l <> r
    [d] -> l <> d <> r
    _   -> cat (zipWith (<>) (l : repeat s) ds) <> r

Taking in mind that cat = group . vcat the last line is equivalent to

    _ -> group (vcat (zipWith (<>) (l : repeat s) ds)) <> r

This works fine in case we want r to be in one line with last element:

[ 1
, 2
, 3 ]

But if we want it to be at the next line:

[ 1
, 2
, 3
]

it (with r = line <> "]") won't group correctly:

[ 1, 2, 3
]

since r is out of group scope in the current definition of encloseSep.

I propose to change last line in the definition of the encloseSep in such way, that r will be grouped in all cases:

encloseSep l r s ds = group $ case ds of
    []  -> l <> r
    [d] -> l <> d <> r
    _   -> vcat (zipWith (<>) (l : repeat s) ds) <> r

This won't change behavior when r doesn't contain line (like list and tupled), but will produce better behavior in this case.

Lev135 avatar Jun 01 '23 16:06 Lev135