Add --output/-o flag

[WGH-]

--remote-name-all is good default choice, but overriding the default inferred file name is still useful.

One could argue that one should use plain curl at this point, but wcurl still has lots of good defaults that are missing in plain curl binary and tedious to specify manually.

--curl-options=-owhatever is an option, but that's also very verbose.

Compare

sudo curl -fLOp /usr/local/sbin/wcurl https://raw.githubusercontent.com/curl/wcurl/main/wcurl

sudo wcurl --curl-options=-o/usr/local/sbin/wcurl https://raw.githubusercontent.com/curl/wcurl/main/wcurl

sudo wcurl -o /usr/local/sbin/wcurl https://raw.githubusercontent.com/curl/wcurl/main/wcurl

And the first one with plain curl still misses a lot of good wcurl defaults.

[samueloph]

Thank you for the PR.

So far we have relied on the fact the wcurl does not uses any parameter names from wget and curl (with the exception of -h, -V and --dry-run), the reason for this is to allow for more flexibility if/when wcurl becomes more complete, a possible future rewrite would allow us to not be stuck on old behaviors of parameters we choose when wcurl was still in POSIX shell.

This also allows a more complete wcurl to co-exist, not risking conflicting with limited parameters from POSIX shell wcurl, thus not having the same parameter name behaving differently between flavors of wcurl.

If a future wcurl decides to mimic all of the wget parameters, for example, then this becomes impossible if we add --output/-o (wget uses this for a logfile name).

I'm not saying this is a blocker, but merging this means we are making a decision that wcurl will not try to have the same parameters as wget in the future, it's a one-way door decision.

I would like to allow myself some time to think about this, and also to gather feedback from others.

And lastly, and less importantly, we need to decide how this should work when the user provides multiple URLs to be downloaded (wcurl will do it in parallel and use the same name for all). I believe that due to the usage --no-clobber, the files don't get overwritten, but this needs to be double-checked, and that's also likely not the behavior the user wants, unless we can make sure the filenames are ordered like the input URLs were (URL1 -> filename.1, URL2 -> filename2).

[samueloph]

After discussing this with @sergiodj and a few others (contributors/users of wcurl), we are currently considering supporting the following parameters:
-O - For wget compatibility;
-o - As it's a commonly used flag for output;
--output - Long form of the parameter, with and without the usage of the equal sign.

This would mean we are making a decision to have parameters which deviates from wget, as wget's -o is for logfiles, and -O concatenates all URLs into the same file, whereas we will save each URL into its own file, appending a number to the end of the file to avoid overwrites.

The effect of this is that we make it easier for wget users to use wcurl, as long as they only need the -O flag, but that's the most used one anyway. It also let curl users to use the same flag as curl, and everyone else will be happy with not having to remember whether the parameter is lowercase or uppercase, both will work the same way for wcurl.

This said, we still want to spend more time considering this decision, possibly 1 or 2 more weeks.

I just wanted to provide an update with what's on our minds currently.

[WGH-]

I'm glad you (seem to have) liked the idea in general.

[samueloph]

We need to document what happens if this value is passed multiple times, I believe that today only the latest value is used.

Implementing logic to allow it to be set multiple times is possible, but we need to decide if it makes sense and check if it will not increase the complexity too much.

[samueloph]

TODO: Consider making the new parameter behave just like curl's with regards to ordering, from the manpage:

You may use this option as many times as the number of URLs you have. For example, if you specify two URLs on > the same command line, you can use it like this:

curl -o aa example.com -o bb example.net

and the order of the -o options and the URLs does not matter, just that the first -o is for the first URL and so on, so > the above command line can also be written as

curl example.com example.net -o aa -o bb

...

--output is associated with a single URL. Use it once per URL when you use several URLs in a command line.

@sergiodj is going to check how we could do this.

[WGH-]

I'm a bit confused whether you still wish me to make some changes, or you've completely took over my PR :)

(I'm fine either way)

[samueloph]

I'm a bit confused whether you still wish me to make some changes, or you've completely took over my PR :)

I initially started writing comments in the PR, then I realized it would be quicker to address them myself, I had permission to push to the fork (it's a checkbox when you open a PR).

(I'm fine either way)

Thanks, when you open a PR you have the option to allow maintainers to push to your fork, you had that enabled and so I assumed it was fine, but let me know if you prefer me not to do that, I would happily comply.

When we merge the PR, we will use a merge commit, so your original commits will still be there and you will be credited.

There are a few things that are pending before we can merge:

1. Rebase on main - I just pushed some changes.
2. Address readonly variable comment - Add --output/-o flag #28 (comment).
3. Allow for -o to be set multiple times - Add --output/-o flag #28 (comment).
4. Add more tests covering the feature.

@sergiodj already has an idea on how to do 3, but if you would like to do it yourself, just let us know.
For numbers 1, 2 and 4, you can also do it, just let us know. Otherwise I will do them tomorrow.

Cheers,

[samueloph]

I've merged main into the fork, now we are just missing:
3- Allow for -o/-O/--output to be set multiple times - #28 (comment).

[sergiodj]

Thanks!

Here's my review. A couple of nitpicks about the PR, and one possibly unneded variable. Otherwise, it looks good.

[sergiodj]

I agree with this change, but it doesn't belong in this PR :-).

[sergiodj]

Likewise: these are formatting changed that should go into their own PR IMHO.

[sergiodj]

Given that my remaining comments can be considered "cosmetic", I think we should go ahead and merge this PR now. Improvements can always be made later.

Thanks!

[samueloph]

Thank you for the suggestion and contribution @WGH- , we should have a new wcurl release with this feature within the next few days.