Fix profile var

[ghost]

I just rebased david’s patch and applied your comments.

Let me know if there are any other changes that are needed to accept this.

Thank you!

[ljharb]

Linking to #833

[ljharb]

this reads pretty confusing to me - could we rename _PROFILE to DETECTED_PROFILE or something?

[ljharb]

just curious, why declare instead of SHELL=/bin/bash PROFILE= nvm_detect_profile?

[ghost]

I was trying to figure out how to set variables inside a command substitution and for some reason that was the way I found. I'm not totally sure of the implications, using declare seems ok. Retrying it now, I think I could have just added semicolons and didn't have to use declare as the following shows.

$ TEST="123"; echo $(TEST="ABC" echo $TEST); echo $TEST
123
123
$ TEST="123"; echo $(TEST="ABC"; echo $TEST); echo $TEST
ABC
123

[ljharb]

It'd be ideal to simplify where possible. generally, you can set env vars for the duration of a command like TEST="ABC" echo $TEST without having to use separate statements. does that work here?

[ghost]

I tried to keep the same logic and rewrote the tests. The [ condition ] || die parts seemed to never trigger urchin when they failed. Let me know if my style is ok, I'm not really a bash programmer.

[ljharb]

The "if" approach is fine with me, altho the || approach should be identical assuming the conditions are the same.

[ghost]

I removed the unnecessary usage of declare. Everything else is ok?

[ljharb]

do these not work without the semicolons? I'd expect just SHELL="/bin/bash" PROFILE="test_profile" nvm_detect_profile to work.

[ghost]

Some of the tests will not return an error even if the semicolon is left out due to side-effects. Though, to obtain the intended logic the semicolons are needed.

# $PROFILE is not a valid file
# In this case no profile is set and nvm_detect_profile returns an empty value,
# still resulting in the test passing
rm "test_profile"
NVM_DETECT_PROFILE="$(PROFILE="test_profile" nvm_detect_profile)"
if [ "$NVM_DETECT_PROFILE" = "test_profile" ]; then
  die "nvm_detect_profile picked \$PROFILE when it was an invalid file"
fi

Play around with the following code in your shell. The command substitution runs in a subshell containing its parent's variables, though, its parent doesn't have access to the child processes variables. As utilized in the tests, command substitutions are replaced by their stdout.

$ TEST="123"; echo $(TEST="ABC" echo $TEST); echo $TEST
123
123
$ TEST="123"; echo $(TEST="ABC"; echo $TEST); echo $TEST
ABC
123

Also, you can try to make the tests fail to confirm they are not passing due to a side-effect. That is what I did.

I'm still trying to learn more about shell and bash programming, there may still be some minor issues, though, I think less than before. I found this article covers a number of potential gotchas.

[ljharb]

Thanks!