[Fix] 'nvm exec' no longer prints error about '-q' being an invalid option

[spikegrobstein]

The NVM_CD_FLAGS variable is assigned a value if nvm is being run in zsh, so that cd'ing around won't print any error messages. This flag, however, is not compatible with bash's cd built-in.

Because this variable was exported, it meant that a user running zsh would be unable to run nvm exec ... commands without encountering errors.

This issue manifests itself in the following manner:

$ nvm exec 16.14.0 npm --version
Running node v16.14.0 (npm v8.3.1)
/Users/<ME>/.nvm/nvm.sh: line 28: cd: -q: invalid option
cd: usage: cd [-L|[-P [-e]] [-@]] [dir]
both the tree and the node path are required
N/A: version "v16.14.0 -> N/A" is not yet installed.

You need to run "nvm install v16.14.0" to install it before using it.

in this case, node 16.14.0 is installed.

By not exporting the variable, it will not be visible inside the nvm-exec script, and the NVM_CD_FLAGS will be assigned fresh on each run when the nvm.sh file is sourced, and commands like this will be successful.

[ljharb]

This was added in 00a8b36b78a7c99fe96ec0416fcc2a3b00f20dae to fix #868. This change seems like it regresses that?

[spikegrobstein]

this will not regress it because the variable is still being assigned a value; it's just not being exported.

When it's exported, it means that it's going to be assigned to all child processes that are called from this point on. so when the nvm-exec shell script is called, which is using the bash interpretter, the variable will already be assigned the -q variable even though it's not being run in zsh.

[ljharb]

Is an alternative for nvm-exec to always unset NVM_CD_FLAGS?

[spikegrobstein]

that was the fix that I was originally going to propose but it doesn't address the root cause of the bug.

The codebase only references the NVM_CD_FLAGS in 4 places and in none of them, does the variable need to be exported, so by not exporting it, it potentially addresses other (not yet found) cases where the leaking of this variable could affect other operations.

if explicitly unset'ing it in nvm-exec is more desirable, that's a fine compromise since it still addresses this bug.

[emily-michelle0620]

Pass through proxy

[ljharb]

@spikegrobstein any interest in updating to that approach?

[spikegrobstein]

I totally didn't notice this reply and I totally forgot about this. I'll get this updated later today when I have time and push the requested change.

[spikegrobstein]

I replaced my previous commit with a new fix/explanation and rebased against master. Also updated the PR body to accurate explain the change (matching the commit).

Let me know how this is.