New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement CURLOPT_CURLU #3227

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@xquery
Contributor

xquery commented Nov 3, 2018

Implement CURLOPT_CURLU (as per https://github.com/curl/curl/wiki/URL-API) so one may pass via curl_easy_setopt aka:

CURL *handle;
CURLU *urlp;
CURLUcode uc;

urlp = curl_url();
uc = curl_url_set(urlp, CURLUPART_URL, "http://example.com/path/index.html", 0);

curl_easy_setopt(handle, CURLOPT_CURLU, urlp);
...

this approach expands on the prior attempts ( #3128, #3160,#3188) ... updated docs (CURLOPT_CURLU.3) to highlight override behavior as well as adding demonstration of this in libtest658.c.

ps - yes, git push --force is now my friend

@xquery

This comment has been minimized.

Contributor

xquery commented Nov 4, 2018

I think appveyor borked due to other reasons ...will nudge with doc change

@bagder

bagder approved these changes Nov 6, 2018

@@ -29,7 +29,7 @@ CURLOPT_CURLU \- set URL with CURLU *
CURLcode curl_easy_setopt(CURL *handle, CURLOPT_CURLU, void *pointer);
.SH DESCRIPTION
Pass in a pointer to the \fIURL\fP to work with. The parameter should be a
CURLU *.
CURLU *. Setting CURLOPT_CURLU will explicitly override CURLOPT_URL.

This comment has been minimized.

@bagder

bagder Nov 6, 2018

Member

if you write them as \fICURLOPT_CURLU(3)\fP and \fICURLOPT_URL(3)\fP they will appear suitably highlighted in man pages and hyperlinked on the web pages.

This comment has been minimized.

@xquery

xquery Nov 8, 2018

Contributor

ack

.SH RETURN VALUE
Returns CURLE_OK if the option is supported, and CURLE_UNKNOWN_OPTION if not.
.SH "SEE ALSO"
.BR CURLOPT_URL "(3),

This comment has been minimized.

@bagder

bagder Nov 6, 2018

Member

maybe also some of the URL API functions can be mentioned in the SEE ALSO?

This comment has been minimized.

@xquery

xquery Nov 8, 2018

Contributor

ack

@bagder

This comment has been minimized.

Member

bagder commented Nov 6, 2018

If you rebase and force-push, you can get the merge conflict resolved and all test should be able to run green now.

@xquery xquery force-pushed the xquery:curlopt_curlu8 branch from 7359183 to d8e8e7e Nov 7, 2018

xquery added some commits Nov 7, 2018

@bagder bagder closed this in 5c4fe0d Nov 9, 2018

@moteus

This comment has been minimized.

moteus commented Nov 9, 2018

Can you please specify in doc Is it possible change URL after CULR_URLU was set.
So make multible requests but instead of reset URL on easy handle change it only in url handle

@xquery

This comment has been minimized.

Contributor

xquery commented Nov 10, 2018

Can you please specify in doc Is it possible change URL after CULR_URLU was set.
So make multible requests but instead of reset URL on easy handle change it only in url handle

seems reasonable, though will be the subject of a different PR if we do it.

@bagder

This comment has been minimized.

Member

bagder commented Nov 10, 2018

I think that feature is implied but maybe we should spell it out better. And possibly also spell out that the handle is used read-only by libcurl itself...

@xquery xquery deleted the xquery:curlopt_curlu8 branch Dec 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment