Skip to content

[Fix] Add default empty value for NVM_NO_ALIAS variable #2047

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 15, 2019

Conversation

sehrope
Copy link
Contributor

@sehrope sehrope commented May 14, 2019

Adds a default value for NVM_NO_ALIAS so that nvm ls does not error out when run in a bash nounset/-u (no unset vars) environment.

Reproduce error via:

$ set -u ; nvm ls 12
bash: NVM_NO_ALIAS: unbound variable

Adds a default value for NVM_NO_ALIAS so that nvm ls does not error out when run
in a bash nounset/-u (no unset vars) environment.
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Can we add a test that would catch this?

@ljharb
Copy link
Member

ljharb commented May 14, 2019

(Note that when I run set -u; nvm ls 0.13, for example, I don't get this error)

@sehrope sehrope force-pushed the fix-nvm-no-alias-unset branch 2 times, most recently from a10fb10 to bb90d63 Compare May 14, 2019 21:54
@sehrope
Copy link
Contributor Author

sehrope commented May 14, 2019

Added a test for enabling nounset and checking for any stderr in a couple situations of nvm ls .... It fails when applied on it's own: https://travis-ci.org/sehrope/nvm/jobs/532527831#L390

With the fix applied, the test passes. 🎉

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Still confused why this can't repro on my Mac; maybe it's got an outdated or broken bash.

@ljharb ljharb force-pushed the fix-nvm-no-alias-unset branch from bb90d63 to 0b5bb5c Compare May 15, 2019 05:06
@ljharb ljharb merged commit 0b5bb5c into nvm-sh:master May 15, 2019
@NKjoep NKjoep mentioned this pull request Nov 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants