search

lukechilds / zsh-nvm

Zsh plugin for installing, updating and loading nvm
MIT License
2.22k stars 112 forks source link

Added autocompletion for nvm #58

Closed Glennmen closed 11 months ago

Glennmen commented 4 years ago

This PR adds autocompletion to the zsh-nvm plugin based on the official nvm plugin _nvm file: https://github.com/robbyrussell/oh-my-zsh/blob/52f58785645c18aa88ea515d070a42bcfe97508d/plugins/nvm/_nvm

Closes #57

lukechilds commented 4 years ago

How does this work?

As far as I can see the _nvm file isn't being called or loaded anywhere. Doesn't it need to be registered with compdef?

Have you tested this and is it working for you?

Also, this just adds completion for the default npm commands. What about adding support for the extra commands we add such as nvm upgrade, nvm revert, nvm install nightly etc.

Glennmen commented 4 years ago

@lukechilds I have been running my fork for 4 months now and the autocompletion works. image

zsh by default searches the plugin name with an underscore prefix for tab completion, the file doesn't need to be called or loaded. (https://github.com/zsh-users/zsh-completions/blob/master/zsh-completions-howto.org)

Those are NVM commands, not NPM commands (they might look similar). I pulled the list from the original NVM plugin in ohmyzsh, but it hasn't been updated since 2018.

lukechilds commented 4 years ago

zsh by default searches the plugin name with an underscore prefix for tab completion, the file doesn't need to be called or loaded.

Ahh! Ok thanks.

Those are NVM commands, not NPM commands (they might look similar). I pulled the list from the original NVM plugin in ohmyzsh, but it hasn't been updated since 2018.

Yep, zsh-nvm wraps nvm with some additional functionality. So when using nvm via this plugin you will have some extra commands such as nvm upgrade, nvm revert, nvm install nightly`. It would be great if we could add completion for these too.

See the readme for more details on these https://github.com/lukechilds/zsh-nvm.

Glennmen commented 4 years ago

@lukechilds Ok I will update the list with the newer NVM commands that have since been added. Shame that the default NVM plugin hasn't been updated... Another reason why this plugin is better than the default NVM ohmyzsh plugin 😉

Glennmen commented 4 years ago

@lukechilds I updated all commands so that they are the same as the official NVM help text (https://github.com/nvm-sh/nvm/blob/b9536327ae35ab60d082b1d5a5462b1173c9aa71/nvm.sh#L2365-L2429) and added nvm upgrade and nvm revert from this package.

This PR only adds autocompletion for 1st level arguments (nvm install), I will need to do more research how to add support for 2nd level arguments (nvm install nightly or nvm install --lts) but I hope that this PR could already be merged.

Let me know if any other changes are needed.

lukechilds commented 4 years ago

@Glennmen this looks great and I'm happy to merge with just 1st level arguments. However these changes don't seem to be working.

Just checking out your branch and sourcing the plugin file is not enough to load the completions.

Do you have some other zsh options enabled that's a requirement to enable the completions to be loaded? Or are you using another zsh framework like OMZ? Possibly you're inheriting some settings or functionality from there that's required for these changes to work.

I've verified this by disabling my entire .zshrc. Just running standard zsh 5.5.1 (x86_64-apple-darwin17.5.0) with no configuration settings. Sourcing the plugin in your branch loads nvm but doesn't have completion. nvm <tab> just lists the files in the current directory.

Screenshot 2020-03-03 at 14 20 39
Glennmen commented 4 years ago

@lukechilds I am using OMZ, but not sure if that auto loads the completion files. If you aren't using any zsh plugin manager maybe this will help: add directory to fpath fpath=(~/newdir $fpath)

lukechilds commented 4 years ago

@Glennmen you need to test with OMZ (and ideally any other settings) disabled to make sure this will work for normal users. We can't guarantee which plugin manager they will be using or if they'll be using one at all.

So it looks like we need to manually add the containing folder to $fpath in the main plugin.

Although, as @memark suggested nvm does already come with completion: https://github.com/nvm-sh/nvm/blob/258938ef66a2a49a4a400554a6dce890226ae34c/bash_completion

It might make more sense to just source this file. That way we know we're always giving the correct suggestions because they'll be up to date with the current installed version of nvm. If we hardcode the completions in zsh-nvm it means that if nvm adds/removes/changes commands, our completions will be incorrect.

We should source the file by default but allow it be disabled by a user setting NVM_LOAD_COMPLETIONS to false.

Glennmen commented 4 years ago

@lukechilds I added documentation how to add completion manually.

Personally I still prefer this completion over the included bash completion, I explained why in #58 I really hope this will get merged, it will also make it easier to add zsh-nvm commands. Like I already did in this PR and once I can work out support for 2nd level arguments this will only get better.