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

Pretty print improvements #67

Merged
merged 4 commits into from Aug 29, 2015

Conversation

Projects
None yet
2 participants
@bitonic
Copy link
Collaborator

commented Aug 27, 2015

No description provided.

bitonic added some commits Aug 27, 2015

Use `prettyPrintParseException` in `decodeEither`.
The rationale is that if you're getting back a raw `String`, you care
more about its readability rather than how faithfully it represent the
data structure.  If you want information about the data structure,
you're better off with `decodeEither'`.
@bitonic

This comment has been minimized.

Copy link
Owner Author

commented on Data/Yaml/Internal.hs in 18ce568 Aug 27, 2015

@snoyberg, you might want to check that this makes sense to you.

This comment has been minimized.

Copy link

replied Aug 27, 2015

I'd prefer using an assert there, having an exception throw an exception in production seems dangerous (even if it should never happen).

This comment has been minimized.

Copy link
Owner Author

replied Aug 27, 2015

How do you suggest to use assert here -- since we're not dealing with a boolean property?

This comment has been minimized.

Copy link
Owner Author

replied Aug 29, 2015

I took another look at the code, and I'm not so sure two UnexpectedEvents cannot happen. I reverted to the old code for that section.

--
-- Since 0.8.11
prettyPrintParseException :: ParseException -> String
prettyPrintParseException NonScalarKey = "Non scalar key"

This comment has been minimized.

Copy link
@snoyberg

snoyberg Aug 27, 2015

Owner

Is there an actual change in behavior here, or is it just restructuring the code?

This comment has been minimized.

Copy link
@bitonic

bitonic Aug 27, 2015

Author Collaborator

There is, the part that I care most about is nice reporting when there is a YamlError, but I also report in a friendlier way when there is an UnexpectedEvent.

bitonic added some commits Aug 28, 2015

Be less strict when pretty-printing `UnexpectedEvent`.
Rationale: while I don't think that two `Nothing`s make sense there, I
think the current code might allow that to happen, and I don't want to
take a bet and throw unwarranted exceptions or wrong error messages.  So
I'm reverting to the old behaviour.

snoyberg added a commit that referenced this pull request Aug 29, 2015

@snoyberg snoyberg merged commit 8db0d58 into snoyberg:master Aug 29, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

snoyberg added a commit that referenced this pull request Aug 30, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.