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

Add labels to json object and list parsing #695

Merged
merged 4 commits into from
Apr 25, 2019

Conversation

FintanH
Copy link
Contributor

@FintanH FintanH commented Apr 17, 2019

Fixes #558

This PR adds better error messages to the parsing of objects and lists.

I'm aware of #559 but this change seems more to the point of giving a better message :)

@bergmark
Copy link
Collaborator

Thanks a lot!

It would be really nice to have some tests for this, do you think you have time to add them? Probably in tests/ErrorMessages.hs. I'm sure it works but I can't really tell from the code :-) I was thinking about doing a release later this week so I can add them then otherwise.

Also hlint is nitpicking a bit

Data/Aeson/Parser/Internal.hs:128:11: Suggestion: Move brackets to avoid $
Found:
  (A.satisfy $ \ w -> w == 44 || w == 125) A.<?>
    "expecting ',' or '}'"
Perhaps:
  A.satisfy (\ w -> w == 44 || w == 125) A.<?> "expecting ',' or '}'"
Data/Aeson/Parser/Internal.hs:153:13: Suggestion: Move brackets to avoid $
Found:
  (A.satisfy $ \ w -> w == 44 || w == 93) A.<?>
    "expecting ',' or ']'"
Perhaps:
  A.satisfy (\ w -> w == 44 || w == 93) A.<?> "expecting ',' or ']'"

@bergmark bergmark added this to the next milestone Apr 23, 2019
@FintanH
Copy link
Contributor Author

FintanH commented Apr 23, 2019

Sure I'll try get it done this week 👌

@FintanH
Copy link
Contributor Author

FintanH commented Apr 24, 2019

Should be good now 😄 🤞

@bergmark
Copy link
Collaborator

Nice!

The old ghc builds started failing now... looks like we should bump the base compat lower bound to 0.10.1 to get <> in scope.

@FintanH
Copy link
Contributor Author

FintanH commented Apr 24, 2019

The old ghc builds started failing now... looks like we should bump the base compat lower bound to 0.10.1 to get <> in scope.

Since these values are definitely Strings I'll switch it to ++ and it can be backward compat :)

@phadej
Copy link
Collaborator

phadej commented Apr 24, 2019

@bergmark bumping base-compat won't get you <>.
See http://hackage.haskell.org/package/base-compat-batteries

I'm neutral on depending on base-compat-batteries, I guess aeson already depends on all of those dependencies.

@bergmark
Copy link
Collaborator

Ah, thanks, I misread the base-compat changelog.

I pushed a hopefully final fix, weird that it only affected GHC 7.10.

@bergmark
Copy link
Collaborator

re -batteries, i prefer it and would be happy to switch to it.

@FintanH FintanH force-pushed the fintan/better-errors branch from 8f5170b to d1364e4 Compare April 24, 2019 15:14
@FintanH
Copy link
Contributor Author

FintanH commented Apr 25, 2019

@bergmark: This is weird, if you click "Details" to look at the progress on Travis it all looks good but GitHub doesn't seem to be picking this up 🤔

@bergmark
Copy link
Collaborator

I'm guessing there was a hiccup in the travis-github integration. No worries.

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

Successfully merging this pull request may close these issues.

Uninformative invalid JSON error
3 participants