search

aesiniath / http-streams

Haskell HTTP client library for use with io-streams
https://hackage.haskell.org/package/http-streams
BSD 3-Clause "New" or "Revised" License
50 stars 48 forks source link

Expose Request, Response and Connection constructors #38

Closed flaper87 closed 11 years ago

flaper87 commented 11 years ago

Exposing types' constructor allows implementations to get values from them without re-implementing those functions

istathar commented 11 years ago

I'm not really that excited about this; exposing constructors means you can't change implementation. Which has definitely been necessary a few times in RequestBuilder and Request. Further, a number of the fields there are internal, and that's not shielded from the public interface if you make the constructors public.

AfC

flaper87 commented 11 years ago

I agree with you. However, I think Request and Response are an important piece of the library and represent the input and output types from / to the userspace. This types are built using internal functions - buildRequest, for example - but forbid users to easily access their data.

We could expose the constructor or just create some functions that will help getting info out of them. The whole idea is to don't make users create those functions.

istathar commented 11 years ago

(not saying "no" yet)

We could expose the constructor or just create some functions that will help getting info out of them. The whole idea is to don't make users create those functions.

Ok, sure. What functions (er, fields) have you found you need access to that we don't have exposed as yet? For example, things like the ExpectMode type is very internal and only set if the user has called setExpectMode. So presumably the user knows they called it and ... doesn't need to query this fact, which is internal anyway and has no user visible impact.

AfC

flaper87 commented 11 years ago

Erm, yeah sorry, fields. So far, the ones that I have a need access to are: qHost and qPath. Looking at other http libraries, I think it would be worth it to also grant access to qBody.

If you think there's a better way to do the above, feel free to ignore this request :smile:

istathar commented 11 years ago

@FlaPer87 so you saw the comment in the documentation which said

Note that the Host: header is not set until sendRequest is called, so you will not see it in the Show instance (unless you call setHostname to override the value inherited from the Connection).

yeah? So I'm not sure what use exposing the qHost field is, as again it's only going to be other than target server hostname if you've set it explicitly... which you presumably already know you've done. And qPath? again, you just called in the request builder.

Maybe you can explain the use case that led you to needing these?

AfC