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

[spikegrobstein]

the nvm.sh file assigns and exports an NVM_CD_FLAGS variable if it
was sourced from a zsh shell. the fact that it's exported means that
it'll be assigned in all child processes, including the nvm-exec
script, which uses bash as the interpreter.

Bash's cd command doesn't have a -q flag, so if the NVM_CD_FLAGS
is assigned -q, the script will error out and incorrectly claim that
the node version isn't installed.

this also manifests itself in the nvm exec command.

Example:

$ 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.

To address this, we unset the NVM_CD_FLAGS at the start of the
nvm-exec script, before loading nvm.sh.

[ljharb]

This was added in 00a8b36 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.

[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.