soenkehahn / generics-eot

A library for generic programming that aims to be easy to understand
https://generics-eot.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
26 stars 6 forks source link

Makes use of Data.Void from `void` / `base`. #7

Closed dalaing closed 6 years ago

dalaing commented 6 years ago

The void package takes care of the transition for us.

In the general case, this is for greater compatiblity with base / the Haskell ecosystem at large.

My specific desire for the change comes from the fact that generics-eot and some of the classes in contravariant go together particularly well, and the latter works with the Void from Data.Void.

Fixes #6.

dalaing commented 6 years ago

I've update the hpack file and used that to generate the cabal file, and I've boiled the example down to what should be minimal. Sorry for the big example before, I was kind of on autopilot when I added that :/

soenkehahn commented 6 years ago

@dalaing: I still think this PR would be better if we tried to use a simpler example that wouldn't rely on contravariant. Do you disagree? I can try to come up with such an example myself, but I wanted to know whether you think this is better for some reason.

dalaing commented 6 years ago

I'm neutral on the kind of example - this is just one that I'm familiar with.

I guess I haven't been straying from what I'm familiar with because I'm not really sold on the benefits of the test in the first place. I'm still kind of thinking of the existing examples that use the Void from generic-eot as bugs - since they're incompatible with Void from the rest of the Haskell ecosystem - and this change as a bug fix that is exercises by the existing tests.

That's just me though :) And it's not my project. I'm happy to come up with some other example, but it might be quite a while before I can bump that up my list of priorities.

soenkehahn commented 6 years ago

I'm still kind of thinking of the existing examples that use the Void from generic-eot as bugs [...]

Are you talking for example about this here? https://github.com/soenkehahn/generics-eot/blob/master/test/Generics/EotSpec.hs#L30

... but it might be quite a while before I can bump that up my list of priorities.

That's fair. :) Maybe I'll get around to writing up my own tests in the coming days, if you don't beat me to it. Otherwise I'll reconsider merging your PR. (Please, feel free to bump me on this.)

dalaing commented 6 years ago

I'll still be trying to think of simpler examples with my spare cycles :) If I come up with something I'll update the PR pronto.

soenkehahn commented 6 years ago

@dalaing: I looked into this again and found multiple places in the examples where it says something like this: https://github.com/soenkehahn/generics-eot/blob/master/examples/ToString.hs#L49. I think if we just switch those to use Data.Void.absurd that'd be tests and documentation enough that users are supposed to be able to use the Void that ships with base. So I think then we don't need another test-case.

soenkehahn commented 6 years ago

@dalaing: I've created this PR now: #12. I've also re-exported absurd for convenience. I would appreciate a thorough review.

If you can confirm that #12 solves your use-case, I would merge it.

soenkehahn commented 6 years ago

Closing in favor of #12.