On Tue, 20 Jun 2017, Andrew wrote:

He suggests a new API where the form is an opaque handle that you get from curl_form_init, and then you call curl_form_addpart on the form handle to get a handle to a new part. Parts are populated with curl_form_set_name and curl_form_set_data.

I really like the idea of using an opaque handle instead of a pointer to a list of structures, and this is basically how I already present it in my binding.

Right, and that's also what we're using widely in libcurl already so that's more in line with how we usually do things. Opaque handles are way easier to support, especially long term.

It also seems like encoding several CURLFORM_FILE in the same form part might be a problem if curl_form_set_data overwrites the previous data.

How do you think we should provide a "file" in the first place? I can probably see us having a curl_form_set_file() function for that, to be used instead of _data() for a part. Then we could allow multiple such function calls to add more files in the same part. Files are data as well, sure, but they do have some more metadata that might make sense to provide in a separate function call.

I think it might make sense to come up with 3-4 "typical" formposts and how they would be done with this new API so that we feel that it works smoothly with those.

Seemingly missing in the proposal are equivalents for CURLFORM_STREAM and CURLFORM_BUFFERPTR. If curl_form_set_data were to always copy the data then I don't see how these modes could be made to work. Perhaps if instead of separate functions for the name and data there were a single curl_form_setopt function?

Hm, I was trying to make this API without using varargs arguments. I think I
rather try separate functions.

CURLFORM_BUFFERPTR could possibly be done with the curl_form_set_file() function somehow. Perhaps as simple as:

  /* read a file from disk, use the file name in the part */
  curl_form_set_file(part, "filename", 0, NULL);

  /* read a file from disk, set a custom file name in the part */
  curl_form_set_file(part, "filename", 0, "curl.tar.gz");

  /* pass a buffer of size 'len', call the file "monkey" */
  curl_form_set_file(part, ptr, len, "monkey");

For CURLFORM_STREAM, we could maybe make curl_form_set_data() either accept an additional "flags" argument to set this or, I think less good, we could abuse the length argument to have magic values.

This would also mean that you don't need separate functions for
CURLFORM_CONTENTTYPE, CURLFORM_CONTENTHEADER, etc.

Right, we need those too.

  curl_form_set_type(part, "text/html");

  curl_form_set_headers(part, slist); /* standard created list */

--

 / daniel.haxx.se
-------------------------------------------------------------------
Unsubscribe: https://cool.haxx.se/list/listinfo/curl-library
Etiquette:   https://curl.haxx.se/mail/etiquette.html

Reply via email to