bash_completion fixes

[PeterDaveHello]

1. Since NVM_PROFILE will be a full path, the detection was not working properly without */ prefix.
2. The origin bash_completion source prompt in installation was:

[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"  # This loads nvm
 && [ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion"  # This loads nvm bash_completion

Which will cause this error if you don't manually remove the comment:

-bash: x: line 2: syntax error near unexpected token `&&'
-bash: x: line 2: ` && [ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion"  # This loads nvm bash_completion'

Which is inconvenient, so I removed the && prefix, and let the bash_completion to detect if nvm loaded.

3. bash_completion didn't detect if nvm already loaded or not, so it'll work even if nvm is not loaded, now it'll detect if nvm loaded yet.

[ljharb]

should this error out, instead of just returning?

[PeterDaveHello]

Refer to the common bash_completions, I think it's fine to just silently do nothing, but we can print error message if you want.

[ljharb]

I guess this is fine as-is.

[ljharb]

How did this work in your original testing? Can we add "install script" tests that would have caught this?

[PeterDaveHello]

Since I can't run the tests locally, I might mess it up when manually mocking up the variables, or something like that, I'll be happy to write test, which part would you like to have?

[ljharb]

Ideally I'd like a test that fails without the */, and passes with it.

[PeterDaveHello]

Then should we separate this part of detection out to a single function?

[ljharb]

It's already in nvm_do_install - if that's not tested or is hard to test, then splitting it into a separate function is fine.

[PeterDaveHello]

Not sure if I understand it, looks like test/install_script/nvm_do_install is just for testing nvm_do_install function available?

[ljharb]

Right - so let's refactor out the detection into a separate function, and make tests just for it.

[PeterDaveHello]

@ljharb updated! would you please help me review? Thanks!

[PeterDaveHello]

Hello @ljharb, is the update fine with you 😄 ? Thanks.

[ljharb]

Thanks, looks good. It'd be good to rebase this down to one commit on latest master, but I can take care of it when I get a chance to merge it if you don't have the time :-)

[PeterDaveHello]

@ljharb no problem, branch rebased, hope could be merged this time, thanks.