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

pretty-simple drops characters that come after non-balanced parenthesis #52

Open sjakobi opened 4 years ago

sjakobi commented 4 years ago

Sorry for the large example!

> x
Union (Cat (Cat Empty (Char 'f')) (Cat (Char ' ') (Cat (Char '"') (Cat Empty (Cat (Text 2 "\\n") (Char '"')))))) (Union (Cat (Cat Empty (Char 'f')) (Cat (Char ' ') (Cat (Char '"') (Cat Empty (Cat (Text 2 "\\n") (Char '"')))))) (FlatAlt (Column [(10,Nesting [(10,Cat (Cat Empty (Char 'f')) (Cat Line (Cat Empty (Column [(10,Nesting [(10,Nest 2 (Cat (Text 2 "  ") (FlatAlt (Column [(10,Nesting [(10,Cat (Text 2 "''") (Cat Line (Cat (Column [(10,Nesting [(10,Cat Empty (Cat Empty (Cat Line Empty)))])]) (Text 2 "''"))))])]) (Cat (Char '"') (Cat Empty (Cat (Text 2 "\\n") (Char '"')))))))])]))))])]) (Cat (Cat Empty (Char 'f')) (Cat (Char ' ') (FlatAlt (Column [(10,Nesting [(10,Cat (Text 2 "''") (Cat Line (Cat (Column [(10,Nesting [(10,Cat Empty (Cat Empty (Cat Line Empty)))])]) (Text 2 "''"))))])]) (Cat (Char '"') (Cat Empty (Cat (Text 2 "\\n") (Char '"')))))))))
> pPrintNoColor  x
Union 
    ( Cat 
        ( Cat Empty ( Char 'f' ) ) 
        ( Cat ( Char ' ' ) 
            ( Cat ( Char '"') (Cat Empty (Cat (Text 2 " \\n") (Char '" ' ) )
        )
    )
> show x
"Union (Cat (Cat Empty (Char 'f')) (Cat (Char ' ') (Cat (Char '\"') (Cat Empty (Cat (Text 2 \"\\\\n\") (Char '\"')))))) (Union (Cat (Cat Empty (Char 'f')) (Cat (Char ' ') (Cat (Char '\"') (Cat Empty (Cat (Text 2 \"\\\\n\") (Char '\"')))))) (FlatAlt (Column [(10,Nesting [(10,Cat (Cat Empty (Char 'f')) (Cat Line (Cat Empty (Column [(10,Nesting [(10,Nest 2 (Cat (Text 2 \"  \") (FlatAlt (Column [(10,Nesting [(10,Cat (Text 2 \"''\") (Cat Line (Cat (Column [(10,Nesting [(10,Cat Empty (Cat Empty (Cat Line Empty)))])]) (Text 2 \"''\"))))])]) (Cat (Char '\"') (Cat Empty (Cat (Text 2 \"\\\\n\") (Char '\"')))))))])]))))])]) (Cat (Cat Empty (Char 'f')) (Cat (Char ' ') (FlatAlt (Column [(10,Nesting [(10,Cat (Text 2 \"''\") (Cat Line (Cat (Column [(10,Nesting [(10,Cat Empty (Cat Empty (Cat Line Empty)))])]) (Text 2 \"''\"))))])]) (Cat (Char '\"') (Cat Empty (Cat (Text 2 \"\\\\n\") (Char '\"')))))))))"

The Union constructor has two fields, but pPrintNoColor shows only the first!

sjakobi commented 4 years ago

Also note the incorrect representation of the second '"' Char as '" '! The first one is actually fine.

cdepillabout commented 4 years ago

@sjakobi Thanks for reporting this. Could you give the whole code used to produce x?

My guess is that this happens because pretty-simple doesn't have any support for reading Haskell-style single characters (like 'a'), but it does have support for reading strings (like "foobar"), so what is happening here is that '"' ... '"' actually gets interpreted as something like ' "'some string that starts and ends with a single quote'" '.

Here are two related issues. I imagine solving either of them would also solve this problem, if you'd like to take a shot at it:

cdepillabout commented 4 years ago

I tried looking into this a little more. Here's my reproduction of this issue:

image

You can see that everything after the final ) gets lost. This happens because pretty-simple has no support for Haskell characters (#45), and it doesn't support non-balanced brackets very well (#27).

I'd happily accept fixes for either of these! I imagine #45 should be particularly easy to fix.

sjakobi commented 4 years ago

Could you give the whole code used to produce x?

  1. Checkout https://github.com/dhall-lang/dhall-haskell/commit/3b222cda91a1ee2c3c7a1cf7b51a9803b9fa9454
  2. Update stack.yaml to use this prettyprinter version.
  3. In stack repl, run
> app = App (Var (V "f" 0)) (TextLit (Chunks [] "\n")) :: Expr () ()
> x = diag $ unAnnotate $ prettyExpr app
sjakobi commented 4 years ago

Thanks for the quick response! I'll try to look for alternative packages first, but may come back if I don't find anything better! :)

sjakobi commented 4 years ago

BTW I don't understand the new title. There's no unbalanced parenthesis here – it's apparently just that the characters are misparsed.

cdepillabout commented 4 years ago

There's no unbalanced parenthesis here – it's apparently just that the characters are misparsed.

Ah, so like I said in https://github.com/cdepillabout/pretty-simple/issues/52#issuecomment-557365228, this is caused because of #45 and #27.

The unbalanced parenthesis are happening because there is no smart parsing for Haskell characters. However, if the unbalanced parenthesis thing was fixed, then at least all the characters would be output, even if there is no pretty-printing of them.

sjakobi commented 4 years ago

Also note the incorrect representation of the second '"' Char as '" '! The first one is actually fine.

This is fixed as of v3.2.0.0, probably due to https://github.com/cdepillabout/pretty-simple/commit/0b95aecb420c631942726d4beb6fc68f8a6a3641. :)