quchen / prettyprinter

A modern, extensible and well-documented prettyprinter.
BSD 2-Clause "Simplified" License
293 stars 34 forks source link

Move grouping in `encloseSep` #245

Open Lev135 opened 1 year ago

Lev135 commented 1 year ago

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.