CURL review request

[jdrewsen]

Den 16-08-2011 15:13, dsimcha skrev:
> On 8/16/2011 7:48 AM, Jonas Drewsen wrote:
>> Hi all,
>>
>> This is a review request for the curl wrapper. Please read the "known
>> issues" in the top of the source file and if possible suggest a solution.
>>
>> We also need somebody for running the review process. Anyone?
>>
>> Code:
>> https://github.com/jcd/phobos/blob/curl-wrapper/etc/curl.d
>> Docs:
>> http://freeze.steamwinter.com/D/web/phobos/etc_curl.html
>>
>> Demolish!
>>
>> /Jonas
>
>  From a quick look, this looks very well thought out. I'll review it
> more thoroughly when I have more time. A few questions/comments from a
> quick look at the docs:
>
> Does the async stuff use D threads, or does Curl have its own async API?

It uses the spawn method from std.concurrency which uses D threads i think.

> In your examples for postData, you have onReceive a ubyte[] and write it
> out to console. Did you mean to cast this to some kind of string?

Not really. It is just an example and writeln will output a byte array 
just fine.

> For onReceive, what's the purpose of the return value?

To specify the number of bytes that have been handled. If this is less 
that the entire buffer the rest of the request will fail. Additionally 
both the onSend and onReceive callbacks can return  CURL_WRITEFUNC_PAUSE 
pr CURL_READFUNC_PAUSE to pause the transfer. And onSend can return 
CURL_READFUNC_ABORT to abort as well. I see that the is missing from the 
docs... will add.

>
> If/when this module makes it into Phobos, are we going to start
> including a libcurl binary with DMD distributions so that std.curl feels
> truly **standard** and requires zero extra configuration?

I think that would be convenient but don't believe it is the correct 
thing to do. In my opinion D modules that are not pure D or wrapping a 
system library (which libcurl is definitely not) should be the only ones 
allowed in the std package. Other wrappers in phobos should be in the 
etc package and the lib binaries should not be included.

Just how I feel about it though. Anyone who disagrees?

/Jonas