"Compact" option sometimes produces ugly/inconsistent output

[mlugg]

I'm using a custom pPrintOpt config which looks like this:

pPrint :: (Show a) => a -> IO ()
pPrint = pPrintOpt CheckColorTty (defaultOutputOptionsDarkBg {outputOptionsCompact=True,outputOptionsPageWidth=60})

I'm getting some weird behaviour when printing long lists of tuples with this compact option. If I print a list of 10 integer values, the printing works as expected:

Prelude> replicate 10 12345
[ 12345
, 12345
, 12345
, 12345
, 12345
, 12345
, 12345
, 12345
, 12345
, 12345
]

If I define a simple ADT, it also works fine:

Prelude> data Foo = Foo Int Int deriving (Show)
Prelude> replicate 10 (Foo 1 2)
[ Foo 1 2
, Foo 1 2
, Foo 1 2
, Foo 1 2
, Foo 1 2
, Foo 1 2
, Foo 1 2
, Foo 1 2
, Foo 1 2
, Foo 1 2
]

However, if I try and print a list of tuples, the output seems to insert extra newlines after the commas:

Prelude> replicate 10 (1, 2)
[
    ( 1, 2 )
,
    ( 1, 2 )
,
    ( 1, 2 )
,
    ( 1, 2 )
,
    ( 1, 2 )
,
    ( 1, 2 )
,
    ( 1, 2 )
,
    ( 1, 2 )
,
    ( 1, 2 )
,
    ( 1, 2 )
]

I would not expect these newlines to be here. Instead, the expected output would be similar to the previous examples, where the comma is on the same line as the value.

[georgefst]

Yep, agreed that's unfortunate. Adding outputOptionsCompact was essentially a quick one line change, so there are bound to be some awkward cases we haven't thought of.

I'll try to look in to this, but it's unlikely to be particularly high-priority for me in the near future. If you're willing to investigate yourself, the key function is prettyExpr. It's admittedly a bit dense, but you'll see that compactness currently just means applying group to each subexpression. Ideally, I'd like to keep it that way, so we'd want to find a way to use combinators like line, line' and flatAlt effectively. Also note that the behaviour of group can be bit odd.

[georgefst]

Well, what do you know, I've actually just hit this myself, and it is a little annoying. So that's a slight bump in priority.

[georgefst]

For posterity, mine is:

x :: IO ()
x = pPrintOpt CheckColorTty opts
    (
        ( 1, 2 )
    , Result
        { a = True, b = False }
    )
  where
    opts =
        defaultOutputOptionsDarkBg
            { outputOptionsCompact = True
            , outputOptionsPageWidth = 40
            }

Ideally, I'd like the constructor (Result) on the same line as well.

[simonmichael]

Thanks for pretty-simple ! hledger has switched to it. Possibly related to this issue: if it could produce slightly more compact output the way pretty-show did, that would be even better. Eg:

pretty-show:

   CsvRules
     { rdirectives = [ ( "skip" , "1" ) ]
     , rcsvfieldindexes = [ ( "date" , 1 ) , ( "amount" , 2 ) ]
     , rassignments = [ ( "amount" , "%2" ) , ( "date" , "%1" ) ]
     , rconditionalblocks = []
     }

pretty-simple, compact mode:

   CsvRules
     { rdirectives=
       [ ( "skip", "1" ) ]
     , rcsvfieldindexes=
       [ ( "date", 1 ), ( "amount", 2 ) ]
     , rassignments=
       [ ( "amount", "%2" ), ( "date", "%1" ) ]
     , rconditionalblocks= []
     }

[georgefst]

@simonmichael Yeah, I agree that would be preferable. As I mentioned further up the thread, the compact mode is really a one line hack, so there hasn't been any thought put in to these edge cases. I'll have a think about how easy this particular one would be to fix.

[georgefst]

CsvRules
{ rdirectives=
[ ( "skip", "1" ) ]
, rcsvfieldindexes=
[ ( "date", 1 ), ( "amount", 2 ) ]
, rassignments=
[ ( "amount", "%2" ), ( "date", "%1" ) ]
, rconditionalblocks= []
}

I assume you're doing something other than calling pPrintOpt on your data directly? Otherwise the lack of space before = is another, separate, bug.

[simonmichael]

@georgefst no I'm just doing:

prettyopts = 
    defaultOutputOptionsDarkBg
    -- defaultOutputOptionsLightBg
    -- defaultOutputOptionsNoColor
    { outputOptionsIndentAmount=2
    , outputOptionsCompact=True
    }

-- | Pretty print. Generic alias for pretty-simple's pPrint.
pprint :: Show a => a -> IO ()
pprint = pPrintOpt CheckColorTty prettyopts

[georgefst]

Okay, with that and:

data CsvRules = CsvRules
    { rdirectives :: [(String, String)]
    , rcsvfieldindexes :: [(String, Int)]
    , rassignments :: [(String, String)]
    , rconditionalblocks :: [()]
    }
    deriving (Show)

main = pprint CsvRules
    { rdirectives = [("skip", "1")]
    , rcsvfieldindexes = [("date", 1), ("amount", 2)]
    , rassignments = [("amount", "%2"), ("date", "%1")]
    , rconditionalblocks = []
    }

I'm getting:

CsvRules
  { rdirectives =
    [ ( "skip", "1" ) ]
  , rcsvfieldindexes =
    [ ( "date", 1 ), ( "amount", 2 ) ]
  , rassignments =
    [ ( "amount", "%2" ), ( "date", "%1" ) ]
  , rconditionalblocks = []
  }

Do you not? Does CsvRules have a custom Show instance?

[simonmichael]

Aha, yes it does: https://github.com/simonmichael/hledger/blob/master/hledger-lib/Hledger/Read/CsvReader.hs#L267 . That's our bug, thanks for looking into it.

[simonmichael]

PS, just more brainstorming: aligning equals signs (capped at some not-too-large width) might be nice for readability:

CsvRules
  { rdirectives        = [ ( "skip" , "1" ) ]
  , rcsvfieldindexes   = [ ( "date" , 1 ) , ( "amount" , 2 ) ]
  , rassignments       = [ ( "amount" , "%2" ) , ( "date" , "%1" ) ]
  , ralignmentisrelaxedforthingswiderthanthelimit = []
  , rconditionalblocks = []
  }