Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Show-instances to use "#field := val" #37

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Profpatsch
Copy link

@Profpatsch Profpatsch commented Nov 10, 2022

The previous show instance would add another layer of quoting for each
nesting:

ghci> show (rcons (#foo := "bar") rnil)
"[(\"foo\",\"\\\"bar\\\"\")]"

Instead, what we want is to display a nicely readable variant of the
record, using the infix field syntax for both label/value pairs and
full records:

ghci> show (rcons (#hi := (rcons (#lea := "hi") rnil)) (rcons (#foo := "bar") rnil ))
"[#foo := \"bar\",#hi := [(#lea := \"hi\")]]"

ghci> show (#hi := "lea")
"#hi := \"lea\""

That’s better!

Note that we can’t have a roundtripping Read instance anyway, so we
might as well have Show be readable.

Fixes #36

This includes the changes from #35

l-epple and others added 5 commits August 12, 2022 16:55
Arguably, emptyVariant is more like Void than (), given that it is
actually not possible to obtain its value via fromVariant and if it
were, a program would explode due to undefined.

In JSON there is not really a way to have a void value, a JSON value is
always *something*, so it makes no sense to have a Variant '[] parser
that succeeds in any condition. More importantly, the FromJSON instance
for Variant (t ': ts) actually *requires* the instance for Variant '[]
to fail! If parseJSON is called for Variant '[] from that instance, it
means that the value didn't match any of the types the Variant accepts.

This causes the bug that, if parseJSON :: Parser () succeeds for a JSON
value, we can obtain an arbitrary Variant that would fail to evaluate,
as it'd be constructed via extendVariant emptyVariant. For aeson < 1.6
this is a pretty obscure bug, since FromJSON () only accepts the empty
array:

    >>> decode "[]" :: Maybe (Variant '[Int])
    Just *** Exception: Prelude.undefined

Since aeson 2.0, FromJSON () [accepts] all JSON values, meaning that any
failed parse to Variant results in a program crash!

Tagged Variants also exhibit this problem, of course. The bug is fixed
by making the FromJSON (Variant '[]) instance fail always via parseFail.

[accepts]: haskell/aeson@677daf0
This adds a bit more ifdef zoo to be compatible with the breaking
change in aeson. Tested manually against aeson 2.0.3.0 and 1.5.6.0.

Might want to update the CI as well, but I don’t have the time to
touch CircleCI yaml.

Co-authored-by: Lukas Epple <[email protected]>
Support for * is disabled in GHC 9.2 by default and causes a lot of
annoying warnings in 9.0.

Tested against 8.10.7, 9.0.2 and 9.2.4. The base constraint seems to be
correct, as Data.Kind was added in base 4.9.

Co-authored-by: Lukas Epple <[email protected]>
This seems to be required now to compile the Variant code.
The previous show instance would add another layer of quoting for each
nesting:

```
ghci> show (rcons (#foo := "bar") rnil)
"[(\"foo\",\"\\\"bar\\\"\")]"
```

Instead, what we want is to display a nicely readable variant of the
record, using the infix field syntax for both label/value pairs and
full records:

```
ghci> show (rcons (#hi := (rcons (#lea := "hi") rnil)) (rcons (#foo := "bar") rnil ))
"[#foo := \"bar\",#hi := [(#lea := \"hi\")]]"

ghci> show (#hi := "lea")
"#hi := \"lea\""
```

That’s better!

Note that we can’t have a roundtripping `Read` instance anyway, so we
might as well have `Show` be readable.

Fixes agrafix#36
@Profpatsch
Copy link
Author

Note that

ghci> show (FldProxy @"ab cd ef" := "lea")
"#ab cd ef := \"lea\""

So in that case we might want to have a special case? Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show Rec produces nested escaping
2 participants