Skip to content

Option to encode single-constructor types as tagged sums#522

Merged
bergmark merged 3 commits into
haskell:masterfrom
Lysxia:feature-473
Apr 7, 2017
Merged

Option to encode single-constructor types as tagged sums#522
bergmark merged 3 commits into
haskell:masterfrom
Lysxia:feature-473

Conversation

@Lysxia
Copy link
Copy Markdown
Collaborator

@Lysxia Lysxia commented Mar 18, 2017

As suggested in #206 and possibly #473

@bergmark
Copy link
Copy Markdown
Collaborator

Github showed the already merged PR's in the history so i force pushed a rebase.

@bergmark
Copy link
Copy Markdown
Collaborator

Thanks again, This looks good as well!

Unavoidably this is a breaking change due to the Options constructor being exposed. I'll file an issue for un-exposing in the next major along with this to avoid this in the future.

I'll cut a release with the other PR's tomorrow and then merge this.

@bergmark
Copy link
Copy Markdown
Collaborator

Looks like i also confused myself and travis by doing "git push origin Lysxia/feature-474" so don't mind the test failure!

@Lysxia Lysxia mentioned this pull request Mar 19, 2017
@bergmark
Copy link
Copy Markdown
Collaborator

I noticed that the new field needs to be added to the Show Options instance. I added a regression test so rebasing this PR should break. This should be rebased on top of #526, but let's save some effort and wait for that to get merged!

@bergmark
Copy link
Copy Markdown
Collaborator

#526 was merged so this can be rebased.

@Lysxia Lysxia force-pushed the feature-473 branch 2 times, most recently from 5cb6d6d to 6674f53 Compare March 19, 2017 23:17
@bergmark
Copy link
Copy Markdown
Collaborator

Same here, I didn't get notified of the recent changes.

Thanks a lot for this!

@Lysxia
Copy link
Copy Markdown
Collaborator Author

Lysxia commented Mar 21, 2017

Okay! You're welcome!

Actually this patch may lead to some serious regressions because I've introduced an actual loop in the generic implementation. We can probably avoid it by tracking the new option at the type level, as a parameter of the GToJSON type class, or by splitting that class in some way.

@Lysxia
Copy link
Copy Markdown
Collaborator Author

Lysxia commented Apr 3, 2017

This is now just two simple overlapping instances instead of the previous recursive mess.

Ready to merge.

@bergmark
Copy link
Copy Markdown
Collaborator

bergmark commented Apr 7, 2017

Great stuff!

@bergmark bergmark merged commit 22064e4 into haskell:master Apr 7, 2017
@Lysxia Lysxia deleted the feature-473 branch June 10, 2018 12:06
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.

2 participants