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

(Either e) instance of decodeText #1

Open
RobertFischer opened this issue Apr 6, 2018 · 5 comments
Open

(Either e) instance of decodeText #1

RobertFischer opened this issue Apr 6, 2018 · 5 comments

Comments

@RobertFischer
Copy link

Would you be interesting in updating the library to use MonadFail instead of Maybe to denote error conditions in decodeText? We can specialize it (through the SPECIALIZE pragma) to Maybe, but it also opens up a whole world of other use cases and call sites.

The catch with this suggestion is that MonadFail is being rolled out rather slowly...with effectively full adoption not coming into play until GHC 8.8 as per https://wiki.haskell.org/MonadFail_Proposal -- but the support in Maybe has been there since base 4.9.0.0 as per https://hackage.haskell.org/package/base-4.11.0.0/docs/Control-Monad-Fail.html#t:MonadFail so for backwards compatibility, we could define the MonadFail instance as an {-# OVERLAPPABLE #-} instance (so as to not conflict with the Monad instance) and conditionally included based on the CPP #if MIN_VERSION_base(4,9,0).

If you're open to this change, I'm happy to submit the pull request.

@jxv
Copy link
Contributor

jxv commented Apr 6, 2018

Using a Maybe is fairly easy to deal with, but I'm certainly open to one or two use cases for generalizing.

@lexi-lambda
Copy link
Collaborator

Currently, it isn’t actually locked down to Maybe; decodeText may produce an arbitrary functor. However, it is true that currently only a Maybe instance is provided.

I think it would be useful to include an Either instance where possible, but the problem is that the underlying libraries do not often include error messages of any kind that can be placed in the Left side of an Either. For that reason, Maybe is really the only logical choice; neither Either nor fail can be effectively used.

That said, Data.Text.Encoding.decodeUtf8' does actually produce an Either UnicodeException Text, so it would be a good idea to add an instance of the shape e ~ UnicodeException => DecodeText (Either e) (UTF8 ByteString) (where the type equality is used to defer the choice of e to improve type inference). I don’t think MonadFail makes much sense here, either, however, because fail only accepts a String, and we’d really like to provide the more structured UnicodeException where we can.

I used to be more enthusiastic about MonadFail, but I’ve found it’s far less useful in practice than I had hoped. The instances vary so wildly in behavior that I think it ends up often doing the flat-out wrong thing when used with a stack of monad transformers. So while I’m in favor of adding more instances for DecodeText, I vote no on utilizing MonadFail here.

@RobertFischer
Copy link
Author

Okay, then I am going to close this issue and open another one for the Either implementation.

An IO (or MonadThrow?) implementation is the other one that I would like, because that's useful for "Be UTF8 or die" with the functions defined in #2.

@lexi-lambda
Copy link
Collaborator

“Be UTF8 or die” is antithetical to the spirit of this library, which is designed to never throw any exceptions, no matter what you do, so I don’t think it would be good to have an IO instance. If I want to die, I will explicitly handle the failure and die the way I want to (signaling an appropriate error).

@RobertFischer
Copy link
Author

K. Then I'll just remake this issue to be for the Either implementation alone.

@RobertFischer RobertFischer reopened this Apr 6, 2018
@RobertFischer RobertFischer changed the title Use MonadFail (Either e) instance of decodeText Apr 6, 2018
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

No branches or pull requests

3 participants