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

NVM installation resets completion logic of other commands #2489

Closed
akefirad opened this issue Apr 16, 2021 · 13 comments
Closed

NVM installation resets completion logic of other commands #2489

akefirad opened this issue Apr 16, 2021 · 13 comments

Comments

@akefirad
Copy link
Contributor

akefirad commented Apr 16, 2021

Operating system and version:

ProductName:	macOS
ProductVersion:	11.2.3
BuildVersion:	20D91

nvm debug output:

nvm --version: v0.38.0
$TERM_PROGRAM: iTerm.app
$SHELL: /bin/zsh
$SHLVL: 1
whoami: 'foo'
${HOME}: /Users/foo
${NVM_DIR}: '${HOME}/.nvm'
${PATH}: ${HOME}/.sdkman/candidates/maven/current/bin:${HOME}/.sdkman/candidates/java/current/bin:${HOME}/.sdkman/candidates/gradle/current/bin:${NVM_DIR}/versions/node/v15.14.0/bin:${HOME}/bin:/usr/local/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/opt/fzf/bin
$PREFIX: ''
${NPM_CONFIG_PREFIX}: ''
$NVM_NODEJS_ORG_MIRROR: ''
$NVM_IOJS_ORG_MIRROR: ''
shell version: 'zsh 5.8 (x86_64-apple-darwin20.0)'
uname -a: 'Darwin 20.3.0 Darwin Kernel Version 20.3.0: Thu Jan 21 00:07:06 PST 2021; root:xnu-7195.81.3~1/RELEASE_X86_64 x86_64'
checksum binary: 'shasum'
OS version: macOS 11.2.3 20D91
curl: /usr/bin/curl, curl 7.64.1 (x86_64-apple-darwin20.0) libcurl/7.64.1 (SecureTransport) LibreSSL/2.8.3 zlib/1.2.11 nghttp2/1.41.0
wget: not found
sed: /usr/bin/sed
cut: /usr/bin/cut
basename: /usr/bin/basename
rm: /bin/rm
mkdir: /bin/mkdir
xargs: /usr/bin/xargs
git: /usr/bin/git, git version 2.24.3 (Apple Git-128)
ls: grep:: No such file or directory
grep: grep: aliased to grep --color=auto --exclude-dir={.bzr,CVS,.git,.hg,.svn,.idea,.tox} (grep --color=auto --exclude-dir={.bzr,CVS,.git,.hg,.svn,.idea,.tox}), grep (BSD grep) 2.5.1-FreeBSD
awk: /usr/bin/awk, awk version 20200816
nvm current: v15.14.0
which node: ${NVM_DIR}/versions/node/v15.14.0/bin/node
which iojs: iojs not found
which npm: ${NVM_DIR}/versions/node/v15.14.0/bin/npm
npm config get prefix: ${NVM_DIR}/versions/node/v15.14.0
npm notice
npm notice New minor version of npm available! 7.7.6 -> 7.10.0
npm notice Changelog: <https://github.com/npm/cli/releases/tag/v7.10.0>
npm notice Run `npm install -g [email protected]` to update!
npm notice
npm root -g: ${NVM_DIR}/versions/node/v15.14.0/lib/node_modules

nvm ls output:

->     v15.14.0
default -> node (-> v15.14.0)
iojs -> N/A (default)
unstable -> N/A (default)
node -> stable (-> v15.14.0) (default)
stable -> 15.14 (-> v15.14.0) (default)
lts/* -> lts/fermium (-> N/A)
lts/argon -> v4.9.1 (-> N/A)
lts/boron -> v6.17.1 (-> N/A)
lts/carbon -> v8.17.0 (-> N/A)
lts/dubnium -> v10.24.1 (-> N/A)
lts/erbium -> v12.22.1 (-> N/A)
lts/fermium -> v14.16.1 (-> N/A)

How did you install nvm?

Homebrew

What steps did you perform?

Tried to use the completion by pressing tab.

What happened?

No completion defined before NVM completion work!

What did you expect to happen?

To see the list of sub-commands

Is there anything in any of your profile files that modifies the PATH?

Just adding home/bin and local/bin to the path.

More Details

The whole issue started here DannyBen/alf#39. In my .zshrc, I'm sourcing ~/.bash_aliases before NVM completion script which contains:

if [[ -n ${ZSH_VERSION-} ]]; then
  autoload -U +X compinit && if [[ ${ZSH_DISABLE_COMPFIX-} = true ]]; then
    compinit -u
  else
    compinit
  fi
  autoload -U +X bashcompinit && bashcompinit
fi

The above also exists in NVM complection script as well. Now the issue is, according to @DannyBen's findings, the issue is that compinit should be called exactly once and before any call to complete, otherwise it resets the previous completion logic. (@DannyBen correct me if I'm wrong.)
Now, first, is our understanding correct? Second, what's the best solution to fix this issue?
Thanks.

@ljharb
Copy link
Member

ljharb commented Apr 16, 2021

nvm is explicitly unsupported when installed from homebrew; please brew uninstall it, and install it using the approved installation method (curl/wget command in the readme), and then we can confirm if there's a bug in nvm or in homebrew's formula (which already has a few).

@ljharb
Copy link
Member

ljharb commented Apr 16, 2021

I'm not sure about how compinit works, but if there's a way to improve that completion logic I'm happy to do so.

@DannyBen
Copy link

Yes. Only minor typo, compinit instead of compini.

It would seem that:

  1. In order for bash completions (of any script) to work in zsh, then compinit and bashcompinit must be called first.
  2. If compinit is called more than once, it seems to reset all previously loaded completions.

@akefirad
Copy link
Contributor Author

Confirmed. Installed as you asked. Same issue.

@ljharb
Copy link
Member

ljharb commented Apr 16, 2021

Is there any way to detect if compinit has been called before?

@DannyBen
Copy link

I'm not sure about how compinit works, but if there's a way to improve that completion logic I'm happy to do so.

Well - one thing seems to be certain. compinit should not be called more than once.
I may be wrong, but until someone who knows this stuff better tells me otherwise, this is the conclusion I draw from observing the behavior.

Here is the simplest proof (run in zsh, after compinit was already autoloaded):

$ complete -W "hello world" one
$ one <tab>   # to see autocomplete working
$ compinit 
$ one <tab>   # to see autocomplete NOT working

So - in our case, when someone installs nvm and another script that behaves the same as nvm in regards to calling compinit, then one of the autocomplete sets will be erased.

In my case, I fixed it by checking if compinit was already loaded:

if [[ -n ${ZSH_VERSION-} ]]; then 
  autoload -U +X bashcompinit && bashcompinit 
  if ! command -v compinit > /dev/null; then 
    autoload -U +X compinit && if [[ ${ZSH_DISABLE_COMPFIX-} = true ]]; then 
      compinit -u 
    else 
      compinit 
    fi 
  fi 
fi 

@ljharb
Copy link
Member

ljharb commented Apr 16, 2021

Happy to accept a PR that incorporates that check, thanks!

@akefirad
Copy link
Contributor Author

@DannyBen Let me know if you want me to do the PR.

@DannyBen
Copy link

Alright - another curious point. We noticed nvm changed this piece of code a little. It would seem that you moved bashcompinit to be after the compinit call - was there an issue associated with this change?

@DannyBen Let me know if you want me to do the PR.

Yes please. I am not an mvn user.

@ljharb
Copy link
Member

ljharb commented Apr 16, 2021

@DannyBen yes, see #2393 which fixed #2325.

@DannyBen
Copy link

@DannyBen yes, see #2393 which fixed #2325.

Excellent, thanks. So I will implement a similar change in alf.

@akefirad
Copy link
Contributor Author

There you go #2490.

@marlonrichert
Copy link

For future reference, this is the proper way to install completions in Zsh.

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

No branches or pull requests

4 participants