Curl wrapper

[Robert Jacques]

On Tue, 17 May 2011 07:24:16 -0400, Jonas Drewsen <jdrewsen at nospam.com>  
wrote:
> On 17/05/11 08.03, Robert Jacques wrote:
>> On Mon, 16 May 2011 17:07:57 -0400, jdrewsen <jdrewsen at nospam.com>  
>> wrote:
>>
>>> Hi,
>>>
>>> I've been working on a wrapper for the etc.c.curl module. It is now
>>> pretty stable and I would very much like some feedback on the API.
>>>
>>> http://freeze.steamwinter.com/D/web/phobos/etc_curl.html
>>>
>>> BTW I use template mixins which doesn't seem to get included in the
>>> generated docs. Is there any way I can make this work?
>>>
>>> /Jonas
[snip]

> I don't know what ditto does. I'll try to look at the ddoc documentation.

"If a documentation comment for a declaration consists only of the  
identifier ditto then the documentation comment for the previous  
declaration at the same declaration scope is applied to this declaration  
as well."

[snip]

>> http.onReceiveHeader has issues with it's 'See' link. It's example is
>> too wide. You don't need really need the parameter documentation,
>> particularly if you improve the example, i.e.:
>> string[string] headerInformation;
>> with(auto http = new Http("http://www.google.com")) {
>> onReceiveHeader = (string key, string value) {
>> if ( value.length <= 10 )
>> headerInformation[key.idup] = value.idup;
>> };
>> onReceive = (ubyte[] data) { };
>> perform;
>> }
>>
>
> I see your point. The question is how we would like phobos docs to be:
>
> 1, consistent with all parameters documented for each method and each  
> method explained and exemplified
> 2, minimal with examples and explanations where absolutely needed.
>
> or somewhere in between. The current style if the curl wrapper is closer  
> to 1 than 2.
>
> Anything official about this?

Official? I don't know. But looking over std.algorithm, Param: is only  
used 3 times, for each of the find variants, in order to clarify what  
haystack and needle are and in one case make the limitations/requirements  
on haystack/needle explicit. Looking over the rest of phobos, I see a mix  
of styles, depending on the author, and some things (like datetime) seem  
to do both.

[snip]

> CurlTimeCond is not calld Http.TimeCondition because it comes from a  
> etc.c.curl. I could alias it in Http ofcourse.

I think where it's defined is less of an issue than where the  
documentation is.

[snip]

>> HttpResult.text should have huge warning flags with regard to use and/or
>> have its design re-thought. Is there some reason you're not at least
>> caching the result and throwing errors on invalid uses of content, etc.
>> Calling text multiple times should be okay. And calling content after
>> text should be either logically valid, or throw.
>
> It is mostly based on storage concerns. Initially the data is stored as  
> ubyte[] and can be returned by content(). When calling text() to get a  
> string it will be decoded and returned. I now have a coule of options:
>
> 1, to store the string in the result for future calls to text(). I can  
> do that and keep the original data for future calls to content(). This  
> will double the amount of memory needed.
> 2, generated the string on each call to text(). This way no copy of the  
> data is kept in HttpResult.
> 3, generated and store the string and then throw away the original data.  
> Again no copy is kept in HttpResult.
>
> I've chosen the 2. version since I guessed that once you have called  
> text() it is more likely that you'll call text again and not content().

Hmm... I take it that content and text are going to be large enough, and  
result long lived enough to warrant not having the array cached inside  
result. In that case you might want to rename text to toString or decode.  
'text' feels like it should be cheaply reusable, while 'toString'/'decode'  
doesn't. (this is a verb/noun thing) Also, 'toString' tends to compose  
better with the rest of D while 'decode' is similar to the std.utf  
functions. Also, if memory re-use is a concern, a 'toStringInPlace' or  
'decodeInPlace' with the old behavior might be appropriate, although I  
might recommend setting content to the utf-8 string and updating the  
encodingSchemeName as appropriate.