Remove the IsOption type class #12
Conversation
The function previously performed by the `IsOption` type class, namely,
the ability to control how a value of a specific type is rendered into
an `Options` value, is now taken care of by the specific `Option`
itself. `Option` now also admits a `Contravariant` instance due to this.
The behaviour previously captured by the `IsOption (Maybe a)` instance
is now captured slightly differently. If you had previously
written this:
someOption := (value :: Maybe a)
you can get the old behaviour back (namely, the above expression
evaluating to an empty `Options` if `value` was `Nothing`) by changing
the code to this:
maybe mempty (someOption :=) (value :: Maybe a)
There is also a newly provided helper function which captures the above
pattern: massoc, or its alias (:=?):
someOption :=? (value :: Maybe a)
Most code should continue to work without modification, but note that
the above constitutes a breaking change (as, of course, does the removal
of `IsOption` and a few other related functions which are no longer
needed).
Bonuses:
* Everything is now documented
* There is no longer any use of the FFI
|
Thanks for the PR. I will dig into the changes. @paf31 Since we chatted about the typeclass-based implementation a while back, do you have any thoughts on these changes? Thanks! |
There was a problem hiding this comment.
Why does the key need to be passed here? Surely the Option knows the key already.
There was a problem hiding this comment.
True, I'll see if I can change this to just value -> Options opt.
There was a problem hiding this comment.
Consider using Op from contravariant. Then you get some nice instances.
There was a problem hiding this comment.
There was a problem hiding this comment.
Actually, this is kind of a perfect way to compose options, providing sets of options which can be set at once.
There was a problem hiding this comment.
If we don't need it, I think we should get rid, since we buy the opportunity to represent multiple keys with one Option, which can be very useful when providing an idiomatic interface to things of type Either via tags, for example.
There was a problem hiding this comment.
key was added in 384219d. I really can't recall a specific reason for adding this. I am open to removing it.
There was a problem hiding this comment.
Actually, @garyb does adding key ring any bells for you by chance? I can't recall why I made that commit.
There was a problem hiding this comment.
Sorry! I didn't seem to get the notification on this. I don't really recall anything specifically to do with key, but it's been a while since I made use of -options so it's a little hard to remember.
I've been trying to think where I might have used it, other than tweaking things in gulp-purescript, I feel like there is somewhere I'm forgetting about where I wrote some stuff from scratch and needed some assistance to get it to do what I wanted, but I have no idea what it was now.
There was a problem hiding this comment.
Another option is to keep the one-key-to-one-Option relationship, keeping key, and to add the Op version as an alternative for more complex scenarios.
|
Looks good to me. If there is a major bump happening, maybe it make sense to update for 0.8 now? |
|
Thanks @paf31. I was thinking to bump to 0.6 |
|
Are we okay to merge here, or should we remove |
|
Ping @garyb |
|
@ethul What do you think about moving this library into core or contrib? I think it solves a really common problem, and we'll probably find ourselves wanting a standard solution for FFI libraries. If we want to go the |
|
I am happy to move it to core or contrib, whichever you think is most On Friday, 29 January 2016, Phil Freeman [email protected] wrote:
|
|
Closing via #13 |
4 participants
The function previously performed by the
IsOptiontype class, namely,the ability to control how a value of a specific type is rendered into
an
Optionsvalue, is now taken care of by the specificOptionitself.
Optionnow also admits aContravariantinstance due to this.The behaviour previously captured by the
IsOption (Maybe a)instanceis now captured slightly differently. If you had previously
written this:
you can get the old behaviour back (namely, the above expression
evaluating to an empty
OptionsifvaluewasNothing) by changingthe code to this:
There is also a newly provided helper function which captures the above
pattern: massoc, or its alias (:=?):
Most code should continue to work without modification, but note that
the above constitutes a breaking change (as, of course, does the removal
of
IsOptionand a few other related functions which are no longerneeded).
Bonuses: