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 Servant.Client.Free #920

Merged
merged 2 commits into from Mar 11, 2018

Conversation

Projects
None yet
3 participants
@phadej
Copy link
Member

commented Mar 11, 2018

this is useful in testing, it nothing else.

@@ -48,3 +51,12 @@ decodedAs response contentType = do
Right val -> return val
where
accept = toList $ contentTypes contentType

instance ClientF ~ f => RunClient (Free f) where

This comment has been minimized.

Copy link
@phadej

phadej Mar 11, 2018

Author Member

dunno if should rather have RunClientF f => RunClient (Free f)

runRequest req = liftF (RunRequest req id)
streamingRequest req = liftF (StreamingRequest req id)
throwServantError = liftF . Throw
catchServantError x h = go x where

This comment has been minimized.

Copy link
@phadej

phadej Mar 11, 2018

Author Member

this is not particularly correct, as RunRequest could throw things catchable by catchServantError too.

@jkarni

This comment has been minimized.

Copy link
Member

commented Mar 11, 2018

Is this missing a file (ClientF)?

@phadej phadej force-pushed the phadej:free-client branch from cff7144 to 80188e0 Mar 11, 2018

@phadej

This comment has been minimized.

Copy link
Member Author

commented Mar 11, 2018

oops

@jkarni
Copy link
Member

left a comment

I've wanted this before!

data ClientF a
= RunRequest Request (Response -> a)
| StreamingRequest Request (StreamingResponse -> a)
| Throw ServantError

This comment has been minimized.

Copy link
@jkarni

jkarni Mar 11, 2018

Member

Maybe adding a Catch constructor would help fix the problem with catchServantError that you mention below?

This comment has been minimized.

Copy link
@phadej

phadej Mar 11, 2018

Author Member

catch isn't an algebraic effect. So it won't really work out well.

@phadej

This comment has been minimized.

Copy link
Member Author

commented Mar 11, 2018

@jkarni @alpmestan I'd rather remove catchServantError, we don't use it itself, and I doubt anyone else

@jkarni

This comment has been minimized.

Copy link
Member

commented Mar 11, 2018

@alpmestan

This comment has been minimized.

Copy link
Contributor

commented Mar 11, 2018

LGTM, and I don't mind the removal of catchServantError as well.

@phadej phadej merged commit fe20b5a into haskell-servant:master Mar 11, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@phadej phadej deleted the phadej:free-client branch Mar 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.