Better combo curl/wget handling

[ljharb]

Fixes #451

[ljharb]

@koenpunt: mind taking a look at this?

[koenpunt]

Looks fine, but maybe we should rename nvm_curl to something like nvm_download?

[ljharb]

Good idea.

Unfortunately, this doesn't work :-/ it seems the curl options we use can't be cleanly passed to wget.

[ljharb]

The other option is to just remove wget support everywhere.

[koenpunt]

The unsupported arguments are --progress-bar, which is the default for wget so we can just strip that, and -L (follow location) is also the default in wget so we can strip that as well, and -C (continue transfer) has an equal in wget: -c:

nvm_curl() {
    ARGS="$* "
    ARGS=${ARGS/--progress-bar /}
    ARGS=${ARGS/-L /}
    ARGS=${ARGS/-s /-q }
    ARGS=${ARGS/-o /-O }
    ARGS=${ARGS/-C /-c }
    wget "$ARGS"
}

And then there is -I, but we can replace that for a version comparison, as it seems like that since version 0.5.1 they all end up in a subdirectory on http://nodejs.org/dist anyway:

[ $(nvm_normalize_version "$VERSION") -ge $(nvm_normalize_version "0.5.1") ]

[ljharb]

Thanks! I think for now I'll just strip the -I check (making the wget less efficient), and we can change the logic in another PR.

[ljharb]

I tested both curl and wget manually here, and it all works.

[koenpunt]

This doesn't look like the correct replacement.. -qO- means silent and output to stdout.

[ljharb]

which part? (i'll take out the echo, oops)

[ljharb]

in curl, -s silences the output but still prints the data to stdout. in wget, -q silences the output but does not print to stdout - to keep the same semantics, -qO- will silence output and print data to stdout in wget.

[koenpunt]

Add -o - to the commands that need it would be a solution. From the manual:

-o, --output
Write output to instead of stdout.
....
Specifying the output as '-' (a single dash) will force the output to be done to stdout.

[koenpunt]

Same here

[mgol]

So it wasn't renamed to nvm_download after all?

[mgol]

Anyway, huge props for it!

[ljharb]

Oops :-) I'll do that in a followup. Still not in a release tag, so nobody should be using it unless they're testing nvm itself.

[koenpunt]

I thought of that, but nvm_curl is not so bad, because we're emulating curl after all.

imho nvm_download should be dedicated to downloading files.

And I think we should extract the checksum verification into its own method.

[ljharb]

@koenpunt i'll be happy to accept PRs for those :-)