Den 16-08-2011 20:49, dsimcha skrev:
== Quote from jdrewsen (jdrew...@nospam.com)'s article
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
Ok, this makes more sense if we define the etc package to be wrappers and
binding
for non-D libraries. I thought this thing was going into std, in which case it
should feel, well, standard. Is that officially how etc is defined?
I don't think there is an official definition yet. But I would like to
suggest the setup described to be official one.
