Skip to content

Added basic error reporting #14

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

Merged
merged 14 commits into from
Dec 3, 2019
Merged

Added basic error reporting #14

merged 14 commits into from
Dec 3, 2019

Conversation

Noble-Mushtak
Copy link
Contributor

@Noble-Mushtak Noble-Mushtak commented Nov 28, 2019

In this pull request, I added some basic error reporting, where, if jsonValue is unable to parse the given String, then it reports an error saying how many characters it had read in from the String when it encountered the problem, what it was expecting at that character, and what it actually found. For right now, the error reporting is pretty basic, as only charP, stringP, and parseIf actually report error messages. Here is an example:

*Main> runParser jsonValue "1e-s"
Right (1,"e-s",JsonNumber 1.0)
*Main> runParser jsonValue "[1e-s]"
Left (KnownError (2,"Expected ']', but found 'e'"))

In the first command, the jsonValue successfully parses the number 1 and then leaves the e-s alone because that exponent part is invalid. However, when we run jsonValue on an array containing 1e-s, it says we expected a ] when they found the e. Arguably, it should actually say that the exponent part of the number is invalid, but I think this will require some more complex error handling.

Also, as shown above, I report the number of characters the parser has read in so far. However, I had trouble figuring out how to get the number of characters that reads had parsed, since jsonNumber was implemented using reads. (One way to do this was to subtract the length of the whole string by the length of the unparsed input that reads returned, but this solution is O(n) where n is the length of the string, which I think is pretty inefficient and clunky.) Therefore, I rewrote jsonNumber based off the JSON Data Interchange Syntax, so that it could use charP, parseIf, and spanP, which makes it easier to update the number of characters read in so far.

Noble-Mushtak and others added 14 commits November 27, 2019 22:17
Without this import, I can not use the (<>) function to concatenate strings because ghc tells me that the (<>) function is not in scope.
However, with this import, the GitHub check says that the Data.Semigroup is redundant.
Therefore, to avoid this issue, I simply replaced the (<>) function with (++).
It doesn't make any sense to me and used in a very limited case which
is not needed anyway.
@rexim
Copy link
Member

rexim commented Dec 3, 2019

@Noble-Mushtak hey! Thank you very much for the contribution! 👍

@rexim rexim merged commit 0c785be into tsoding:master Dec 3, 2019
@rexim rexim mentioned this pull request Dec 3, 2019
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.

2 participants