cdepillabout / pretty-simple

pretty-printer for Haskell data types that have a Show instance
https://hackage.haskell.org/package/pretty-simple
BSD 3-Clause "New" or "Revised" License
243 stars 29 forks source link

Make output less vertical #43

Closed buggymcbugfix closed 4 years ago

buggymcbugfix commented 5 years ago

Thank you for this awesome library. I was trying to make the following ugly debug output (for granule in case you were wondering) more palatable.

AST [] [Def ((3,1),(5,30)) (Id "fact'" "fact'") [Equation ((4,1),(4,13)) () [PBox ((4,7),(4,9)) () (PCon
str ((4,8),(4,8)) () (Id "Z" "Z") [])] (Val ((4,13),(4,13)) () (NumInt 1)),Equation ((5,1),(5,30)) () [P
Box ((5,7),(5,11)) () (PConstr ((5,8),(5,10)) () (Id "S" "S") [PVar ((5,10),(5,10)) () (Id "m" "m_1")])]
 (Binop ((5,30),(5,30)) () "*" (App ((5,15),(5,27)) () (Val ((5,15),(5,15)) () (Var () (Id "natToInt" "n
atToInt"))) (App ((5,25),(5,27)) () (Val ((5,25),(5,25)) () (Constr () (Id "S" "S") [])) (Val ((5,27),(5
,27)) () (Var () (Id "m" "m_1"))))) (App ((5,32),(5,40)) () (Val ((5,32),(5,32)) () (Var () (Id "fact'"
"fact'"))) (Val ((5,38),(5,40)) () (Promote () (Val ((5,39),(5,39)) () (Var () (Id "m" "m_1")))))))] (Fo
rall ((3,9),(3,26)) [((Id "n" "n_0"),KPromote (TyCon (Id "Nat" "Nat")))] (FunTy (Box (CInterval {lowerBo
und = CNat 1, upperBound = CVar (Id "n" "n_0")}) (TyApp (TyCon (Id "N" "N")) (TyVar (Id "n" "n_0")))) (T
yCon (Id "Int" "Int"))))]

Yikes! However, pretty-simple is a bit overzealous and makes it too pretty:

AST []
    [ Def
        (
            ( 3
            , 1
            )
        ,
            ( 5
            , 30
            )
        ) ( Id "fact'" "fact'" )
        [ Equation
            (
                ( 4
                , 1
                )
            ,
                ( 4
                , 13
                )
            ) ()
            [ PBox
                (
                    ( 4
                    , 7
                    )
                ,
                    ( 4
                    , 9
                    )
                ) ()
                ( PConstr
                    (
                        ( 4
                        , 8
                        )
                    ,
                        ( 4
                        , 8
                        )
                    ) () ( Id "Z" "Z" ) []
                )
            ]
            ( Val
                (
                    ( 4
                    , 13
                    )
                ,
                    ( 4
                    , 13
                    )
                ) () ( NumInt 1 )
            )
        , Equation
            (
                ( 5
                , 1
                )
            ,
                ( 5
                , 30
                )
            ) ()
            [ PBox
                (
                    ( 5
                    , 7
                    )
                ,
                    ( 5
                    , 11
                    )
                ) ()
                ( PConstr
                    (
                        ( 5
                        , 8
                        )
                    ,
                        ( 5
                        , 10
                        )
                    ) () ( Id "S" "S" )
                    [ PVar
                        (
                            ( 5
                            , 10
                            )
                        ,
                            ( 5
                            , 10
                            )
                        ) () ( Id "m" "m_1" )
[etc.]

Is there an easy way to make things less vertical?

cdepillabout commented 5 years ago

@buggymcbugfix Unfortunately there is no way currently to do this.

If you wanted to send a PR that adds an option called compact that tries to do something like this, I would accept it. Although it would probably be non-trivial to add.

If you wanted to add something like this, I think you would mainly be adding code in the OutputPrinter module.

If I'm not mistaken, I think brittany tries to do something like this. It tries to put as much information on one line as long as it is still readable.

mpdairy commented 5 years ago

Hey, pretty-simple is almost perfect for what I'm doing, but the things I print are too vertical as well. How hard would it be to have an option where you clump together trailing parens on the same line? For example, the regular way:

Baz
    { unBaz =
        [ "hello"
        , "bye"
        ]
    }

the compact way:

Baz
    { unBaz =
        [ "hello"
        , "bye" ] }

That would save me from seeing things like this:

                                                    } 
                                                )
                                            } 
                                        , _version = 1
                                        } 
                                    }
                                )
                            } 
                        } 
                    )
                } 
            , _true = 30
            , _false = 31
            } 
        )
    } 

If you point me in the right direction I'll do a PR.

cdepillabout commented 5 years ago

@mpdairy Thanks for taking an interest in this.

The code for outputting stuff is the following:

https://github.com/cdepillabout/pretty-simple/blob/d06378ebd77dcc24198b756cf1c99ddb8fbb62ac/src/Text/Pretty/Simple/Internal/OutputPrinter.hs#L94

However, this is going from output tokens directly to a Text Builder. It would probably be easier to use a proper pretty-printer:

http://hackage.haskell.org/package/prettyprinter

If you wanted to change the code to instead use prettyprinter, it would probably be much easier to change the output format after that.

Or, you could just go ahead and try to add a change like your are suggesting.

Also, I mentioned this above, but I think that doing something like brittany does where it tries to put as much information on one line as possible (while not going past 80 characters) might be a better change than just wrapping trailing parens/braces on one line.

georgefst commented 4 years ago

@buggymcbugfix How does this look?

AST []
    [ Def
        ( ( 3, 1 ), ( 5, 30 ) )
        ( Id "fact'" "fact'" )
        [ Equation
            ( ( 4, 1 ), ( 4, 13 ) ) ()
            [ PBox
                ( ( 4, 7 ), ( 4, 9 ) ) ()
                ( PConstr ( ( 4, 8 ), ( 4, 8 ) ) () ( Id "Z" "Z" ) [] )
            ]
            ( Val ( ( 4, 13 ), ( 4, 13 ) ) () ( NumInt 1 ) )
        , Equation
            ( ( 5, 1 ), ( 5, 30 ) ) ()
            [ PBox
                ( ( 5, 7 ), ( 5, 11 ) ) ()
                ( PConstr
                    ( ( 5, 8 ), ( 5, 10 ) ) ()
                    ( Id "S" "S" )
                    [ PVar ( ( 5, 10 ), ( 5, 10 ) ) () ( Id "m" "m_1" ) ]
                )
            ]
            ( Binop
                ( ( 5, 30 ), ( 5, 30 ) ) () "*"
                ( App
                    ( ( 5, 15 ), ( 5, 27 ) ) ()
                    ( Val
                        ( ( 5, 15 ), ( 5, 15 ) ) ()
                        ( Var () ( Id "natToInt" "natToInt" ) )
                    )
                    ( App
                        ( ( 5, 25 ), ( 5, 27 ) ) ()
                        ( Val
                            ( ( 5, 25 ), ( 5, 25 ) ) ()
                            ( Constr () ( Id "S" "S" ) [] )
                        )
                        ( Val
                            ( ( 5, 27 ), ( 5, 27 ) ) ()
                            ( Var () ( Id "m" "m_1" ) )
                        )
                    )
                )
                ( App
                    ( ( 5, 32 ), ( 5, 40 ) ) ()
                    ( Val
                        ( ( 5, 32 ), ( 5, 32 ) ) ()
                        ( Var () ( Id "fact'" "fact'" ) )
                    )
                    ( Val
                        ( ( 5, 38 ), ( 5, 40 ) ) ()
                        ( Promote ()
                            ( Val
                                ( ( 5, 39 ), ( 5, 39 ) ) ()
                                ( Var () ( Id "m" "m_1" ) )
                            )
                        )
                    )
                )
            )
        ]
        ( Forall
            ( ( 3, 9 ), ( 3, 26 ) )
            [ ( ( Id "n" "n_0" ), KPromote ( TyCon ( Id "Nat" "Nat" ) ) ) ]
            ( FunTy
                ( Box
                    ( CInterval
                        { lowerBound = CNat 1
                        , upperBound = CVar
                            ( Id "n" "n_0" )
                        }
                    )
                    ( TyApp
                        ( TyCon ( Id "N" "N" ) )
                        ( TyVar ( Id "n" "n_0" ) )
                    )
                )
                ( TyCon ( Id "Int" "Int" ) )
            )
        )
    ]

With #67 now merged, we can do this with a two-line change (essentially we just call group on each subexpression). I imagine @cdepillabout wouldn't want that as the default, but it could easily be made configurable.

georgefst commented 4 years ago

Also, #67 has actually made @mpdairy's example print on just two lines, unless you add at least six more "bye"s.

georgefst commented 4 years ago

@cdepillabout This has made me realise that, since we use defaultOptions, we wrap at a fairly arbitrary 80 spaces.

I'll open a PR making that configurable as well. Do you think 80 is a sensible default? The existing tests actually pass even with Unbounded, but examples like @buggymcbugfix's become a lot uglier (they collapse to a single line when the grouping is applied).

mpdairy commented 4 years ago

Yeah that looks a lot better. Should we put all the closing parens on one line as well?

so instead of:

                        ( Promote ()
                            ( Val
                                ( ( 5, 39 ), ( 5, 39 ) ) ()
                                ( Var () ( Id "m" "m_1" ) )
                            )
                        )
                    )
                )
            )
        ]
        ( Forall

you'd have:

                        ( Promote ()
                            ( Val
                                ( ( 5, 39 ), ( 5, 39 ) ) ()
                                ( Var () ( Id "m" "m_1" ) )
        )    )   )   )   )   ]
        ( Forall

or this:

                        ( Promote ()
                            ( Val
                                ( ( 5, 39 ), ( 5, 39 ) ) ()
                                ( Var () ( Id "m" "m_1" ) )   )   )   )   )   )   ]
        ( Forall

I don't really know which looks best. I've actually sort of gotten used to the vertical default.

georgefst commented 4 years ago

I think the first version there is nicer, since it means matching parens are in the same column. But that might be non-trivial to implement.

I've never actually personally been bothered by the spacing of closing parens so I won't be proactive about it, but I'd be willing to help if someone else started a PR.

buggymcbugfix commented 4 years ago

@georgefst that looks awesome!

@mpdairy purely for readability I prefer having all the closing parens on one line like in the last example since the indentation already gives me the nesting depth. I don't mind the spaces between the closing brackets.

buggymcbugfix commented 4 years ago

Awesome work! I enjoyed playing with this: https://georgefst.github.io/pretty-simple/ ❤️

georgefst commented 4 years ago

Awesome work! I enjoyed playing with this: https://georgefst.github.io/pretty-simple/

Thanks! Your example turned out to be a perfect showcase for the new features.

(Well, almost - it could do with some non-printable characters, but we plan to include other examples eventually anyway: #81)