Skip to content

Unset NVM_BIN when is not necessary#1033

Merged
ljharb merged 1 commit into
nvm-sh:masterfrom
robsonpeixoto:unset-unnecessary-envvar
Mar 11, 2016
Merged

Unset NVM_BIN when is not necessary#1033
ljharb merged 1 commit into
nvm-sh:masterfrom
robsonpeixoto:unset-unnecessary-envvar

Conversation

@robsonpeixoto

@robsonpeixoto robsonpeixoto commented Mar 10, 2016

Copy link
Copy Markdown
Contributor

I would like to know if my current shell are using a node configured by the nvm.
But the are only 3 cheap ways to know

  • check if there are the nvm directory on PATH
  • check the version using the NVM_BIN or NVM_PATH

Using the NVM_BIN is very simple. Using this simple function:

function __node_ps1() {
  if [[ ! -z "$NVM_BIN" ]]; then
    local array=(${NVM_BIN//// })
    local version=${array[-2]}
    echo " js(${version##*v})"
  fi
}

But after a nvm deactivate or nvm use system the nvm keep these environment variables.

ljharb
ljharb reviewed Mar 10, 2016
View reviewed changes
Comment thread nvm.sh Outdated
if [ $NVM_USE_SILENT -ne 1 ]; then
echo "Now using system version of node: $(node -v 2>/dev/null)$(nvm_print_npm_version)"
fi
unset NVM_BIN NVM_PATH

@ljharb ljharb Mar 10, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this line shouldn't be needed, since you're doing it in nvm deactivate already

@robsonpeixoto robsonpeixoto Mar 10, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 0227822

@ljharb

ljharb commented Mar 10, 2016

Copy link
Copy Markdown
Member

@robsonpeixoto it's much simpler than that. type nvm >/dev/null 2>&1 && nvm current will exit 0 and print out the current nvm-managed node version if nvm is available. I use that in my own prompt and it's very fast.

@ljharb ljharb added the needs followup We need some info or action from whoever filed this issue/PR. label Mar 10, 2016
ljharb
ljharb reviewed Mar 10, 2016
View reviewed changes
Comment thread nvm.sh
echo "$NVM_DIR/*/lib/node_modules removed from \$NODE_PATH"
fi
fi
unset NVM_BIN NVM_PATH

@ljharb ljharb Mar 10, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should this have -f so that it will try to unset it even if it happens not to be set?

@robsonpeixoto robsonpeixoto Mar 10, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

-f is useful to remove function definition.
And I do not see any problem if it unset a variable that no dot exists.

~> export OI=123
~> echo $?
0
~> unset OI
~> echo $?
0
~> unset OI
~> echo $?
0

@ljharb ljharb Mar 10, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

k thanks, just confirming :-)

@ljharb

ljharb commented Mar 10, 2016

Copy link
Copy Markdown
Member

Assuming tests pass, I think this looks great! Would you mind rebasing it down to a single commit?

@ljharb ljharb added bugs Oh no, something's broken :-( needs rebase Please rebase your branch onto latest master! This removes merge commits, & keeps the git log clean. and removed needs followup We need some info or action from whoever filed this issue/PR. labels Mar 10, 2016
@robsonpeixoto

robsonpeixoto commented Mar 10, 2016

Copy link
Copy Markdown
Contributor Author

I can, @ljharb. Before or after the tests finish ?

@ljharb

ljharb commented Mar 10, 2016

Copy link
Copy Markdown
Member

Either way, but I suppose we might as well wait til they pass to make sure there's no further tweaks needed. Thanks!

@robsonpeixoto

robsonpeixoto commented Mar 11, 2016

Copy link
Copy Markdown
Contributor Author

Done @ljharb.

@ljharb

ljharb commented Mar 11, 2016

Copy link
Copy Markdown
Member

@robsonpeixoto awesome! if you'd mind one more rebase that'd be awesome, otherwise i'll merge this in later today :-D

@robsonpeixoto

robsonpeixoto commented Mar 11, 2016

Copy link
Copy Markdown
Contributor Author

A rebase with the master done, @ljharb

@ljharb ljharb removed the needs rebase Please rebase your branch onto latest master! This removes merge commits, & keeps the git log clean. label Mar 11, 2016
@ljharb

ljharb commented Mar 11, 2016

Copy link
Copy Markdown
Member

Thanks!

@ljharb ljharb merged commit 8fb4ee0 into nvm-sh:master Mar 11, 2016
@robsonpeixoto robsonpeixoto deleted the unset-unnecessary-envvar branch March 11, 2016 19:24
sandinmyjoints added a commit to sandinmyjoints/nvm that referenced this pull request Jun 24, 2016
@sandinmyjoints
Merge tag 'v0.31.1'
faefb90 
v0.31.1 Changelog

New Stuff:
 - `nvm uninstall`: Check installation dir permissions before uninstalling; display "fix" commands (nvm-sh#847)
 - `nvm alias`: colorize output to match `nvm ls`
 - `nvm alias`: colorize output when creating aliases
 - `nvm ls`/`nvm alias`/`nvm ls-remote`: only colorize when colors are supported

Fixes:
 - don’t use bash `==` in conditionals
 - `nvm run`: pass through `--silent` on bare `nvm run`
 - `nvm exec`: show “io.js” for io.js versions
 - `set -e`: ensure `nvm_version` returning 3, and `nvm_alias` returning 2, doesn’t terminate the process
 - `nvm alias`: explicitly forbid user aliases in subdirs
 - `read` exits 1 when `.nvmrc` lacks a trailing newline; avoid that
 - `set -x`: avoid an unbound variable
 - `deactivate`: unset `$NVM_BIN` and `$NVM_PATH` (nvm-sh#1033)

Performance:
 - `nvm alias`: slightly speed up alias resolution
 - Use `awk` to improve version comparison performance

Robustness:
 - add a missing `command` to a `sed` call

Misc:
 - Various README tweaks
 - Various testing improvements
 - Prefer `nvm --help` over `nvm help`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bugs Oh no, something's broken :-(

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@robsonpeixoto @ljharb