Skip to content

Conversation

@parsonsmatt
Copy link
Contributor

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this prefers the Right parse then? I think Left would be more common (aeson etc.)

Also, you can probably shorten this using <|>.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup! My thinking is: Attempt to parse the thing we actually want, and if that fails, then attempt to parse the error case. If you have Either String SomeType then String would always succeed even if SomeType could have been parsed.

@paf31
Copy link
Contributor

paf31 commented Nov 1, 2015

Thanks!

@parsonsmatt
Copy link
Contributor Author

The <|> implementation is much nicer, thanks for the tip 👍

@garyb
Copy link
Member

garyb commented Nov 1, 2015

I thought we weren't doing this? Previously: #26 #30

@paf31
Copy link
Contributor

paf31 commented Nov 1, 2015

I think I would prefer to provide this as a function instead of an instance.

@parsonsmatt
Copy link
Contributor Author

I've implemented two of the functions, one Right biased and the other Left biased, and removed the instance.


main = do
print $
readJSON """{ "x":1, "y": 2, "z": 3}""" :: F Response
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this typecheck?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It didn't, and neither psc-ide nor pulp -w build compile the examples directory so I forgot to change it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok, thought I was missing something 😄

@garyb garyb closed this in #45 Oct 10, 2016
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.

3 participants