derekkraan / curl_req

req to curl and back
https://hex.pm/packages/curl_req
MIT License
53 stars 3 forks source link

exact/approiximate Mode for `to_curl/2` #7

Closed kevinschweikert closed 3 months ago

kevinschweikert commented 3 months ago

I have been thinking already about doing this kind of processing. Woytek mentioned converting -H accept_encoding... to --compressed instead of the header version here but I am hesitant. Why? Because this isn't the request that Req is sending. So I think there are two modes here perhaps. One is "exact" mode, like the request exactly as Req is going to send it (or as close as we can get). And the second is the "approximate" mode (for lack of better terminology), which converts things like basic auth to -u and a compression header to `--compressed.

There is then also the question of implementation. The code, as written here, won't work for all cases. Someone could, for instance, add their own auth header, or define a step that does something with the auth option.

I am wondering if we should pull these changes out into a new PR for now. Then we can continue to discuss and refine the hard part but still merge the easy parts for now.

_Originally posted by @derekkraan in https://github.com/derekkraan/curl_req/pull/5#discussion_r1628882584_

kevinschweikert commented 3 months ago

Wording ideas for staying true to Req:

Wording ideas for curl style

derekkraan commented 3 months ago

There is also the question of which mode should be the standard one. If you look at it from that perspective, perhaps we do the "approximate" mode as the standard mode, and then you only need to come up with one name, let's say "debug", which gives you as close to exactly what req is going to do.

What do you think?

kevinschweikert commented 3 months ago

Yeah i'm with you. The default should be the curl native flags and we strip the user agent and modify the headers into curls own functionality even when that is not the exact request. That would make it easier for curl user to understand what's happening and is more readable.

My idea would be to add a option to to_curl/2 which would look something like

Req.new(url: "http://example.com", ) |> CurlReq.to_curl(style: :curl) #default if no style specified
"curl --compressed -X GET http://example.com"

Req.new(url: "http://example.com") |> CurlReq.to_curl(style: :req)
"curl -H "accept-encoding: gzip" -H "user-agent: req/0.5.0" -X GET http://example.com"

If you're happy with this, i can work on a PR

derekkraan commented 3 months ago

I am happy with this. PR would be much appreciated.