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.

Sign up for GitHub

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 loading of nvm for brew-installed nvm #2162

Merged
merged 3 commits into from
Nov 6, 2024
Merged

Fix loading of nvm for brew-installed nvm #2162

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8eb90f7
Fix loading of nvm for brew-installed nvm
brianphillips Aug 30, 2022
6661159
attempt to be more efficient when checking for brew-managed NVM install
brianphillips Jun 9, 2023
c5fdefe
reuse variable instead of invoking brew again
brianphillips Aug 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions plugins/available/nvm.plugin.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,18 @@ cite about-plugin
about-plugin 'node version manager configuration'

export NVM_DIR="${NVM_DIR:-$HOME/.nvm}"

# first check if NVM is managed by brew
NVM_BREW_PREFIX=""
if _bash_it_homebrew_check
then
NVM_BREW_PREFIX=$(brew --prefix nvm 2>/dev/null)
Copy link
Contributor

Choose a reason for hiding this comment

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

In my (now old) message, I mentioned:

$(brew --prefix --installed $forumula)

With the --installed option, it only returns a value if the formula is installed.

Without it, when the formula is not installed, it would return where the formula would be installed (vs returning nothing)

Maybe you can test this logic locally to confirm, but I think we want the --installed flag here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

brew --prefix --installed $formula runs about 30x slower (over a second on my machine) than brew --prefix $formula (30ms) because it has to exec a ruby script instead of just staying in the bash portion of brew. And since we're checking the existence of the script just below, I don't think we should care about the specifics of brew --prefix --installed nvm vs just brew --prefix nvm

fi

# This loads nvm
if _bash_it_homebrew_check && [[ -s "${BASH_IT_HOMEBREW_PREFIX}/nvm.sh" ]]
if [[ -n "$NVM_BREW_PREFIX" && -s "${NVM_BREW_PREFIX}/nvm.sh" ]]
then
source "${BASH_IT_HOMEBREW_PREFIX}/nvm.sh"
source "${NVM_BREW_PREFIX}/nvm.sh"
else
[[ -s "$NVM_DIR/nvm.sh" ]] && source "$NVM_DIR/nvm.sh"
fi
Expand Down