DannyBen / alf

Bash Alias Generator and Manager
MIT License
90 stars 5 forks source link

Autocompletion in zsh conflicts with NVM's autocompletion #39

Closed akefirad closed 3 years ago

akefirad commented 3 years ago

Hi mate,

If you remember we discussed the support for ZSH (#31) and you managed to get it working including the auto-completion (if I'm not mistaken). I think it did work specifically for sub-commands but I'm not sure. Now I noticed that the auto-completion doesn't work for sub-commands in (it works just fine for the root commands). I have no idea what has broken the feature. Can you please help me to troubleshoot this? My setup is (for the record, I'm setting it up on a clean machine):

Alf version: 92c38febbb80dfad27554612cf2c9e2edbebcf8b Bash version: GNU bash, version 5.1.4(1)-release (x86_64-apple-darwin20.2.0) ZSH version: zsh 5.8 (x86_64-apple-darwin20.0) OS version: macOS 11.2.3 20D91

Thanks.

DannyBen commented 3 years ago

Hmm.

Well - first of all, the autocompletion of the root alf commands is something that is actually not handled by alf at all - since alf generates the ~/.bash_aliases, each function in this file is recognized by the shell and is autocompleted like any other function.

Alf autocompletion is only responsible for the subcommands. You can see its definition in the generated ~/.bash_aliases, at the end of the file.

Now - I have tested it now using the docker-compose file that is in this repository, and I am able to make it work as expected.

These are the versions I am using:

$ bash --version |head -1
GNU bash, version 5.1.0(1)-release (x86_64-alpine-linux-musl)

$ zsh --version
zsh 5.8 (x86_64-alpine-linux-musl)

$ alf -v
0.4.6

Here is a screencast of the autocomplete working inside the zsh container.

Things to check:

  1. Are you getting any errors when you do source ~/.bash_aliases?
  2. Do you see the expected autocomplete commands at the end of ~/.bash_aliases?
  3. Did you add something new to your alf.conf since the last time that it worked?
akefirad commented 3 years ago

Thanks for the quick reply. So I went back and checked all the changes one by one (didn't know when exactly it broke) and found this in my .zshrc:

export NVM_DIR="$HOME/.nvm"
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"  # This loads nvm
[ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion"  # This loads nvm bash_completion

This is after source ~/bash_aliases line and when I move it before that everything seems to work just fine. The problematic part is the bash completion script. I'm not a bash guru, you might find this interesting (~/.nvm/bash_completion, especially the last lines talking about bash vs zsh):

#!/usr/bin/env bash

# bash completion for Node Version Manager (NVM)

if ! command -v nvm &> /dev/null; then
  return
fi

__nvm_generate_completion() {
  declare current_word
  current_word="${COMP_WORDS[COMP_CWORD]}"
  # shellcheck disable=SC2207
  COMPREPLY=($(compgen -W "$1" -- "${current_word}"))
  return 0
}

__nvm_commands() {
  declare current_word
  declare command

  current_word="${COMP_WORDS[COMP_CWORD]}"

  COMMANDS='
    help install uninstall use run exec
    alias unalias reinstall-packages
    current list ls list-remote ls-remote
    install-latest-npm
    cache deactivate unload
    version version-remote which'

  if [ ${#COMP_WORDS[@]} == 4 ]; then

    command="${COMP_WORDS[COMP_CWORD - 2]}"
    case "${command}" in
      alias) __nvm_installed_nodes ;;
    esac

  else

    case "${current_word}" in
      -*) __nvm_options ;;
      *) __nvm_generate_completion "${COMMANDS}" ;;
    esac

  fi
}

__nvm_options() {
  OPTIONS=''
  __nvm_generate_completion "${OPTIONS}"
}

__nvm_installed_nodes() {
  __nvm_generate_completion "$(nvm_ls) $(__nvm_aliases)"
}

__nvm_aliases() {
  declare aliases
  aliases=""
  if [ -d "${NVM_DIR}/alias" ]; then
    aliases="$(cd "${NVM_DIR}/alias" && command find "${PWD}" -type f | command sed "s:${PWD}/::")"
  fi
  echo "${aliases} node stable unstable iojs"
}

__nvm_alias() {
  __nvm_generate_completion "$(__nvm_aliases)"
}

__nvm() {
  declare previous_word
  previous_word="${COMP_WORDS[COMP_CWORD - 1]}"

  case "${previous_word}" in
    use | run | exec | ls | list | uninstall) __nvm_installed_nodes ;;
    alias | unalias) __nvm_alias ;;
    *) __nvm_commands ;;
  esac

  return 0
}

# complete is a bash builtin, but recent versions of ZSH come with a function
# called bashcompinit that will create a complete in ZSH. If the user is in
# ZSH, load and run bashcompinit before calling the complete function.
if [[ -n ${ZSH_VERSION-} ]]; then
  autoload -U +X bashcompinit && bashcompinit
  autoload -U +X compinit && if [[ ${ZSH_DISABLE_COMPFIX-} = true ]]; then
    compinit -u
  else
    compinit
  fi
fi

complete -o default -F __nvm nvm

Do you think the comment above is relevant here?

DannyBen commented 3 years ago

Where was this entire nvm completion script? In which file? Was it in your ~/.zshrc as is? If so, it is a bad idea...

DannyBen commented 3 years ago

Well, a few notes:

  1. currently, it looks like the alf autocomplete and nvm autocomplete do not play nice. Depending on the order you source them, only one of them works.
  2. If the entire script you pasted above is directly inside your ~/.zshrc, I would move it to its own file and source it. The ~/.*rc should remain short and simple.

I have devised a simple test, that shows that the problem is with the enablement of the zsh autocomplete (both in alf and in nvm).

complete -W "hello world" zero

# Option 1:
[[ -n $ZSH_VERSION ]] && autoload -U +X compinit && compinit && autoload -U +X bashcompinit && bashcompinit

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

complete -W "hello world" one
complete -W "hello world" two

No matter which of the options is enabled, anything defined before the option 1/2 commands, will be "reset" and deleted. Not sure yet how to fix it, and even if I do find a way, it still probably means that nvm is also doing it wrong....

With the above script, there are three virtual commands: zero, one, two - which have completions. The file just needs to be sourced.

akefirad commented 3 years ago

Where was this entire nvm completion script? In which file? Was it in your ~/.zshrc as is? If so, it is a bad idea...

It's installed by nvm in ~/.nvm/bash_completion.

DannyBen commented 3 years ago

Note I edited my last comment. I am still working on it, but it seems that compinit - called by both alf generated code, and by nvm, simply resets all previous completions....

akefirad commented 3 years ago

You're right. That's unfortunate.

DannyBen commented 3 years ago

The problem can be distilled to this:

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

So - what we need to find, is a way to NOT call compinit, if it was already called. This way, at least I can fix my side of the equation. NVM will need to do the same (of course, assuming my understanding of the situation is correct...).

akefirad commented 3 years ago

Right. Even if one script is fixed, we can resolve the whole issue by proper ordering. (I'll raise an issue on NVM to fix this as well, if there's a solution.)

DannyBen commented 3 years ago

There seems to be an easy solution. Not sure it is the right way, but it seems to be working. I intend to implement it in alf later today.

See this script (source it).

complete -W "hello world" zero

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

complete -W "hello world" one

Both zero and one will have completions.

akefirad commented 3 years ago

Confirmed. It works; it doesn't mess with NVM completion. 👍

DannyBen commented 3 years ago

Alright - version 0.4.7 of alf is now in master. It implements the fix so that sourcing ~/.bash_aliases last, will not recall compinit if it was already called by a previous script.

If you are going to open a ticket with nvm, it would be nice if you can share a reference to the ticket here, so we can follow up. Perhaps even there is a better way of solving it than what I did.

We can keep this issue open for a while, assuming there is still something to learn or improve here.

And of course, thank you for reporting the issue.

akefirad commented 3 years ago

Sure thing. Thanks for the support.

akefirad commented 3 years ago

I just found in the latest version of NVM completion script, the snippet is slightly different:

# complete is a bash builtin, but recent versions of ZSH come with a function
# called bashcompinit that will create a complete in ZSH. If the user is in
# ZSH, load and run bashcompinit before calling the complete function.
if [[ -n ${ZSH_VERSION-} ]]; then
  # Calling compinit first and then bashcompinit as mentioned by zsh man page.
  autoload -U +X compinit && if [[ ${ZSH_DISABLE_COMPFIX-} = true ]]; then
    compinit -u
  else
    compinit
  fi
  autoload -U +X bashcompinit && bashcompinit
fi
DannyBen commented 3 years ago

Alright. Not sure it has an impact, but will keep an eye on it.

akefirad commented 3 years ago

No effect on this specific problem as far as I can tell.

marlonrichert commented 2 years ago

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