Skip to content
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

fix failing if check in autoload script #1821

Merged
merged 1 commit into from
May 26, 2018

Conversation

sidsakhadeo
Copy link

Fixes #1812

@ljharb ljharb merged commit b81c120 into nvm-sh:master May 26, 2018
@sidsakhadeo
Copy link
Author

Hi @bjrbhre, as per the script

autoload -U add-zsh-hook
load-nvmrc() {
  local node_version="$(nvm version)" # gets default node version
  local nvmrc_path="$(nvm_find_nvmrc)" # gets path of .nvmrc in current dir

  if [ -n "$nvmrc_path" ]; then
    # following variable is assigned the node version listed in the .nvmrc, if installed
    # else it is assigned "N/A"
    local nvmrc_node_version=$(nvm version "$(cat "${nvmrc_path}")") 
    if [ "$nvmrc_node_version" = "N/A" ]; then
      nvm install # when "N/A" it installs this new node version
    elif [ "$nvmrc_node_version" = "$node_version" ]; then
      nvm use # if already installed it will use this node version
    fi
  elif [ "$node_version" != "$(nvm version default)" ]; then
    echo "Reverting to nvm default version"
    nvm use default
  fi
}
add-zsh-hook chpwd load-nvmrc
load-nvmrc

i think without removing the ! in the second check, it would never execute an nvm use, hence the change

@sidsakhadeo sidsakhadeo deleted the fix-autload-script branch May 30, 2018 14:58
@ljharb ljharb mentioned this pull request May 30, 2018
@bjrbhre
Copy link

bjrbhre commented Jun 19, 2018

Hi @sidsakhadeo. Thanks for your comment and sorry for the delay in answering.

If [ "$nvmrc_node_version" = "$node_version" ] then you just don't need to execute nvm use because it means that you already use the version listed in nvmrc. You need to execute nvm use if they don't match. I do believe the ! is useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants