Skip to content

Refactor servant-client-core Response+StreamingResponse#899

Merged
phadej merged 1 commit intohaskell-servant:masterfrom
phadej:response-body-refactor
Feb 6, 2018
Merged

Refactor servant-client-core Response+StreamingResponse#899
phadej merged 1 commit intohaskell-servant:masterfrom
phadej:response-body-refactor

Conversation

@phadej
Copy link
Copy Markdown
Contributor

@phadej phadej commented Jan 30, 2018

No description provided.

@phadej phadej added this to the 0.13 milestone Jan 30, 2018
Comment thread servant-client/src/Servant/Client.hs Outdated
{-# LANGUAGE GeneralizedNewtypeDeriving #-}
{-# LANGUAGE MultiParamTypeClasses #-}
{-# LANGUAGE OverloadedStrings #-}
{-# LANGUAGE ScopedTypeVariables #-}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for moving this around? I generally prefer to not have too much code in non-Internal modules since then I can export everything for testing etc. without cluttering the haddocks and API.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really (I planned to make non-IO client, but it's difficult given we have streaming now. I'll revert this change.

For the record, I partially disagree, I want e.g. requestToClientRequestto be a part of stable public API (I'd rather export it from Servant.Client)


data StreamingResponse = StreamingResponse { runStreamingResponse :: forall a. ((Status, Seq.Seq Header, HttpVersion, IO BS.ByteString) -> IO a) -> IO a }
type Response = GenResponse LBS.ByteString
newtype StreamingResponse = StreamingResponse { runStreamingResponse :: forall a. (GenResponse (IO BS.ByteString) -> IO a) -> IO a }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this, for symmetry, be something like

newtype StreamingBody = StreamingBody (forall a. ((IO ByteString) -> a) -> a)
type StreamingResponse = GenResponse StreamingBody

(not sure if those details are right, but what I'm referring to is having the continuation within GenResponse rather than containing it)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't work. see performStreamingRequest

@phadej phadej force-pushed the response-body-refactor branch from 30f8a9c to f4fc2b3 Compare January 31, 2018 07:26
@phadej phadej merged commit f5ffdc7 into haskell-servant:master Feb 6, 2018
@phadej phadej deleted the response-body-refactor branch February 6, 2018 09:33
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

Successfully merging this pull request may close these issues.

2 participants