pretty-simple icon indicating copy to clipboard operation
pretty-simple copied to clipboard

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

Open sjakobi opened this issue 5 years ago • 8 comments

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 avatar Nov 22 '19 02:11 sjakobi

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

sjakobi avatar Nov 22 '19 02:11 sjakobi

@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:

  • implement support for haskell characters like 'a': https://github.com/cdepillabout/pretty-simple/issues/45
  • allow non-balanced brackets: https://github.com/cdepillabout/pretty-simple/issues/27

cdepillabout avatar Nov 22 '19 02:11 cdepillabout

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.

cdepillabout avatar Nov 22 '19 03:11 cdepillabout

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 avatar Nov 22 '19 03:11 sjakobi

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 avatar Nov 22 '19 03:11 sjakobi

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

sjakobi avatar Nov 22 '19 03:11 sjakobi

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.

cdepillabout avatar Nov 22 '19 04:11 cdepillabout

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. :)

sjakobi avatar Dec 21 '19 14:12 sjakobi