Refactor servant-client-core Response+StreamingResponse #899
Conversation
servant-client/src/Servant/Client.hs
Outdated
| {-# LANGUAGE GeneralizedNewtypeDeriving #-} | ||
| {-# LANGUAGE MultiParamTypeClasses #-} | ||
| {-# LANGUAGE OverloadedStrings #-} | ||
| {-# LANGUAGE ScopedTypeVariables #-} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 } |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
It won't work. see performStreamingRequest
phadej
force-pushed
the
response-body-refactor
branch
from
30f8a9c to
f4fc2b3
Compare
No description provided.