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 (.:!) and partially revert d0414be92ee6bf5b4c057978955b89d111767dab #288

Merged
merged 3 commits into from Jan 15, 2016

Conversation

phadej
Copy link
Collaborator

@phadej phadej commented Sep 19, 2015

Handle #83 by introducing .:! as @gregwebs proposed (I bikeshedded .:?? to .:!). Fixes undocumented breaking change in 0.10

Related: #287

@phadej phadej force-pushed the dot-colon-qmark branch 2 times, most recently from 1df7cad to c613f92 Compare September 19, 2015 14:27
@phadej phadej changed the title Add (.:??) and partially revert d0414be92ee6bf5b4c057978955b89d111767dab Add (.:!) and partially revert d0414be92ee6bf5b4c057978955b89d111767dab Sep 19, 2015
@phadej phadej force-pushed the dot-colon-qmark branch 2 times, most recently from ed7a12b to 7565ae6 Compare September 19, 2015 15:48
@dmjio
Copy link
Contributor

dmjio commented Sep 19, 2015

+1

@gregwebs
Copy link
Contributor

great, this needs to be merged. only problem is that we have a bunch of operators with slightly different semantics. A big improvement would be to give a non-operator name to all of them. And better yet would be to make combinators to describe how maybe is treated.

undefinedAsNull "maybeField"

@phadej
Copy link
Collaborator Author

phadej commented Sep 20, 2015

Probably shouldn't be part of this PR, but I'd like to have names too.

How about

parseField = (.:)
parseOptionalField = (.:?)
parseNotNullOptionalField = (.:!) -- this one is huge :(
withDefault = (.!=)

@gregwebs
Copy link
Contributor

Can we get this released now as version 0.11 to avoid breaking everyone with 0.10 ?

@k-bx
Copy link
Contributor

k-bx commented Oct 16, 2015

@phadej this needs to be rebased

@phadej
Copy link
Collaborator Author

phadej commented Oct 16, 2015

Rebased (and fixed the build sdist): https://travis-ci.org/phadej/aeson/builds/85721736

Please, setup travis for all of us!

@nikomi
Copy link

nikomi commented Dec 14, 2015

+1!

This was almost 2 months ago - is anything happening with this?

bos added a commit that referenced this pull request Jan 15, 2016
@bos bos merged commit 81f8fb5 into haskell:master Jan 15, 2016
@bos
Copy link
Collaborator

bos commented Jan 15, 2016

Applied, thanks.

@gregwebs
Copy link
Contributor

@phadej does it make sense to apply this now that 0.10 has been released or is this a breaking change back in the other direction?

@phadej
Copy link
Collaborator Author

phadej commented Jan 15, 2016

@gregwebs, if the next release is 0.11...

@phadej phadej deleted the dot-colon-qmark branch January 15, 2016 20:38
@gregwebs
Copy link
Contributor

But what is the point of making a 0.11 release that makes things like 0.9 if 0.10 is out? Isn't this merge 3 months late?

@phadej
Copy link
Collaborator Author

phadej commented Jan 15, 2016

I cannot really comment on that, as I'm author and user of aeson-compat which makes (.:?) in 0.10 behave like it was in 0.9. IMO 0.9 behaviour is more useful, your mileage may vary.

@gregwebs
Copy link
Contributor

This is obviously better and it would have been great to be merged before the 0.10 release. However, many people changed their code to work with 0.10. Those that did not take a compatible approach will just need to change it back to 0.9 behavior now.

@tolysz
Copy link
Contributor

tolysz commented Jan 15, 2016

I am just testing if this PR actually fixes my Maybe ~ Null case; otherwise my patch makes aeson behave correctly (i.e. pre 10 )

@codygman
Copy link
Contributor

Should this close #287?

@wereHamster
Copy link

@gregwebs there are also many people who haven't upgraded their package yet (due to this and other problems with the 0.10 release), for them keeping the 0.9 semantics would be preferable.

@nikomi
Copy link

nikomi commented Jan 17, 2016

+1

I assumed the null-handling API change to be an unintended glitch that would be corrected in the next version, probably marking 0.10.0.0 as deprecated.

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.

None yet

9 participants