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

New zsh completion script seems broken #881

Closed
corneliusweig opened this issue Jun 10, 2019 · 19 comments
Closed

New zsh completion script seems broken #881

corneliusweig opened this issue Jun 10, 2019 · 19 comments

Comments

@corneliusweig
Copy link

corneliusweig commented Jun 10, 2019

I just tried the zsh completion as merged recently via #646. Unfortunately, I couldn't get it to work for example with skaffold (https://github.com/corneliusweig/skaffold/tree/upgrade-cobra).

The generated completion script looks ok (see below), but when I type skaffold <Tab><Tab>, I only get files from my current directory as suggestion instead of subcommands and options. I have sourced the generated completion script in a running zsh via source skaffold-completion.zsh.

/cc @rsteube @babysnakes (because you were active on the referenced PR)

#compdef _skaffold skaffold


function _skaffold {
  local -a commands

  _arguments -C \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:' \
    "1: :->cmnds" \
    "*::arg:->args"

  case $state in
  cmnds)
    commands=(
      "build:Builds the artifacts"
      "completion:Output shell completion for the given shell (bash or zsh)"
      "config:A set of commands for interacting with the Skaffold config."
      "debug:Runs a pipeline file in debug mode"
      "delete:Delete the deployed resources"
      "deploy:Deploys the artifacts"
      "dev:Runs a pipeline file in development mode"
      "diagnose:Run a diagnostic on Skaffold"
      "fix:Converts old Skaffold config to newest schema version"
      "help:Help about any command"
      "init:Automatically generate Skaffold configuration for deploying an application"
      "run:Runs a pipeline file"
      "version:Print the version information"
    )
    _describe "command" commands
    ;;
  esac

  case "$words[1]" in
  build)
    _skaffold_build
    ;;
  completion)
    _skaffold_completion
    ;;
  config)
    _skaffold_config
    ;;
  debug)
    _skaffold_debug
    ;;
  delete)
    _skaffold_delete
    ;;
  deploy)
    _skaffold_deploy
    ;;
  dev)
    _skaffold_dev
    ;;
  diagnose)
    _skaffold_diagnose
    ;;
  fix)
    _skaffold_fix
    ;;
  help)
    _skaffold_help
    ;;
  init)
    _skaffold_init
    ;;
  run)
    _skaffold_run
    ;;
  version)
    _skaffold_version
    ;;
  esac
}

function _skaffold_build {
  _arguments \
    '(*-b *--build-image)'{\*-b,\*--build-image}'[Choose which artifacts to build. Artifacts with image names that contain the expression will be built only. Default is to build sources for all artifacts]:' \
    '--cache-artifacts[Set to true to enable caching of artifacts]' \
    '--cache-file[Specify the location of the cache file (default $HOME/.skaffold/cache)]:' \
    '(-d --default-repo)'{-d,--default-repo}'[Default repository value (overrides global config)]:' \
    '--enable-rpc[Enable gRPC for exposing Skaffold events (true by default for `skaffold dev`)]' \
    '(-f --filename)'{-f,--filename}'[Filename or URL to the pipeline file]:' \
    '*--insecure-registry[Target registries for built images which are not secure]:' \
    '(-n --namespace)'{-n,--namespace}'[Run deployments in the specified namespace]:' \
    '(-o --output)'{-o,--output}'[Used in conjuction with --quiet flag. Format output with go-template. For full struct documentation, see https://godoc.org/github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/flags#BuildOutput]:' \
    '(*-p *--profile)'{\*-p,\*--profile}'[Activate profiles by name]:' \
    '(-q --quiet)'{-q,--quiet}'[Suppress the build output and print image built on success. See --output to format output.]' \
    '--rpc-http-port[tcp port to expose event REST API over HTTP]:' \
    '--rpc-port[tcp port to expose event API]:' \
    '--skip-tests[Whether to skip the tests after building]' \
    '--toot[Emit a terminal beep after the deploy is complete]' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

function _skaffold_completion {
  _arguments \
    '(-h --help)'{-h,--help}'[help for completion]' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:' \
    '1: :("bash" "zsh")'
}


function _skaffold_config {
  local -a commands

  _arguments -C \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:' \
    "1: :->cmnds" \
    "*::arg:->args"

  case $state in
  cmnds)
    commands=(
      "list:List all values set in the global Skaffold config"
      "set:Set a value in the global Skaffold config"
      "unset:Unset a value in the global Skaffold config"
    )
    _describe "command" commands
    ;;
  esac

  case "$words[1]" in
  list)
    _skaffold_config_list
    ;;
  set)
    _skaffold_config_set
    ;;
  unset)
    _skaffold_config_unset
    ;;
  esac
}

function _skaffold_config_list {
  _arguments \
    '(-a --all)'{-a,--all}'[Show values for all kubecontexts]' \
    '(-c --config)'{-c,--config}'[Path to Skaffold config]:' \
    '(-k --kube-context)'{-k,--kube-context}'[Kubectl context to set values against]:' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

function _skaffold_config_set {
  _arguments \
    '(-c --config)'{-c,--config}'[Path to Skaffold config]:' \
    '(-g --global)'{-g,--global}'[Set value for global config]' \
    '(-k --kube-context)'{-k,--kube-context}'[Kubectl context to set values against]:' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

function _skaffold_config_unset {
  _arguments \
    '(-c --config)'{-c,--config}'[Path to Skaffold config]:' \
    '(-g --global)'{-g,--global}'[Set value for global config]' \
    '(-k --kube-context)'{-k,--kube-context}'[Kubectl context to set values against]:' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

function _skaffold_debug {
  _arguments \
    '--cache-artifacts[Set to true to enable caching of artifacts]' \
    '--cache-file[Specify the location of the cache file (default $HOME/.skaffold/cache)]:' \
    '--cleanup[Delete deployments after dev or debug mode is interrupted]' \
    '(-d --default-repo)'{-d,--default-repo}'[Default repository value (overrides global config)]:' \
    '--enable-rpc[Enable gRPC for exposing Skaffold events (true by default for `skaffold dev`)]' \
    '(-f --filename)'{-f,--filename}'[Filename or URL to the pipeline file]:' \
    '--force[Recreate kubernetes resources if necessary for deployment (warning: might cause downtime!)]' \
    '*--insecure-registry[Target registries for built images which are not secure]:' \
    '(*-l *--label)'{\*-l,\*--label}'[Add custom labels to deployed objects. Set multiple times for multiple labels]:' \
    '(-n --namespace)'{-n,--namespace}'[Run deployments in the specified namespace]:' \
    '--no-prune[Skip removing images and containers built by Skaffold]' \
    '--no-prune-children[Skip removing layers reused by Skaffold]' \
    '--port-forward[Port-forward exposed container ports within pods]' \
    '(*-p *--profile)'{\*-p,\*--profile}'[Activate profiles by name]:' \
    '--rpc-http-port[tcp port to expose event REST API over HTTP]:' \
    '--rpc-port[tcp port to expose event API]:' \
    '--skip-tests[Whether to skip the tests after building]' \
    '--tail[Stream logs from deployed objects]' \
    '--toot[Emit a terminal beep after the deploy is complete]' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

function _skaffold_delete {
  _arguments \
    '(-d --default-repo)'{-d,--default-repo}'[Default repository value (overrides global config)]:' \
    '(-f --filename)'{-f,--filename}'[Filename or URL to the pipeline file]:' \
    '(-n --namespace)'{-n,--namespace}'[Run deployments in the specified namespace]:' \
    '(*-p *--profile)'{\*-p,\*--profile}'[Activate profiles by name]:' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

function _skaffold_deploy {
  _arguments \
    '(-a --build-artifacts)'{-a,--build-artifacts}'[Filepath containing build output.
E.g. build.out created by running skaffold build --quiet {{json .}} > build.out]:' \
    '(-d --default-repo)'{-d,--default-repo}'[Default repository value (overrides global config)]:' \
    '--enable-rpc[Enable gRPC for exposing Skaffold events (true by default for `skaffold dev`)]' \
    '(-f --filename)'{-f,--filename}'[Filename or URL to the pipeline file]:' \
    '--force[Recreate kubernetes resources if necessary for deployment (default false, warning: might cause downtime!)]' \
    '(-i --images)'{-i,--images}'[A list of pre-built images to deploy]:' \
    '(*-l *--label)'{\*-l,\*--label}'[Add custom labels to deployed objects. Set multiple times for multiple labels]:' \
    '(-n --namespace)'{-n,--namespace}'[Run deployments in the specified namespace]:' \
    '(*-p *--profile)'{\*-p,\*--profile}'[Activate profiles by name]:' \
    '--rpc-http-port[tcp port to expose event REST API over HTTP]:' \
    '--rpc-port[tcp port to expose event API]:' \
    '--tail[Stream logs from deployed objects (default false)]' \
    '--toot[Emit a terminal beep after the deploy is complete]' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

function _skaffold_dev {
  _arguments \
    '--cache-artifacts[Set to true to enable caching of artifacts]' \
    '--cache-file[Specify the location of the cache file (default $HOME/.skaffold/cache)]:' \
    '--cleanup[Delete deployments after dev or debug mode is interrupted]' \
    '(-d --default-repo)'{-d,--default-repo}'[Default repository value (overrides global config)]:' \
    '--enable-rpc[Enable gRPC for exposing Skaffold events (true by default for `skaffold dev`)]' \
    '(-f --filename)'{-f,--filename}'[Filename or URL to the pipeline file]:' \
    '--force[Recreate kubernetes resources if necessary for deployment (warning: might cause downtime!)]' \
    '*--insecure-registry[Target registries for built images which are not secure]:' \
    '(*-l *--label)'{\*-l,\*--label}'[Add custom labels to deployed objects. Set multiple times for multiple labels]:' \
    '(-n --namespace)'{-n,--namespace}'[Run deployments in the specified namespace]:' \
    '--no-prune[Skip removing images and containers built by Skaffold]' \
    '--no-prune-children[Skip removing layers reused by Skaffold]' \
    '--port-forward[Port-forward exposed container ports within pods]' \
    '(*-p *--profile)'{\*-p,\*--profile}'[Activate profiles by name]:' \
    '--rpc-http-port[tcp port to expose event REST API over HTTP]:' \
    '--rpc-port[tcp port to expose event API]:' \
    '--skip-tests[Whether to skip the tests after building]' \
    '--tail[Stream logs from deployed objects]' \
    '--toot[Emit a terminal beep after the deploy is complete]' \
    '--trigger[How are changes detected? (polling, manual or notify)]:' \
    '(*-w *--watch-image)'{\*-w,\*--watch-image}'[Choose which artifacts to watch. Artifacts with image names that contain the expression will be watched only. Default is to watch sources for all artifacts]:' \
    '(-i --watch-poll-interval)'{-i,--watch-poll-interval}'[Interval (in ms) between two checks for file changes]:' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

function _skaffold_diagnose {
  _arguments \
    '(-f --filename)'{-f,--filename}'[Filename or URL to the pipeline file]:' \
    '(*-p *--profile)'{\*-p,\*--profile}'[Activate profiles by name]:' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

function _skaffold_fix {
  _arguments \
    '(-f --filename)'{-f,--filename}'[Filename or URL to the pipeline file]:' \
    '--overwrite[Overwrite original config with fixed config]' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

function _skaffold_help {
  _arguments \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

function _skaffold_init {
  _arguments \
    '--analyze[Print all discoverable Dockerfiles and images in JSON format to stdout]' \
    '(*-a *--artifact)'{\*-a,\*--artifact}'['\''='\''-delimited dockerfile/image pair to generate build artifact
(example: --artifact=/web/Dockerfile.web=gcr.io/web-project/image)]:' \
    '--compose-file[Initialize from a docker-compose file]:' \
    '(-f --filename)'{-f,--filename}'[Filename or URL to the pipeline file]:' \
    '--force[Force the generation of the Skaffold config]' \
    '--skip-build[Skip generating build artifacts in Skaffold config]' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

function _skaffold_run {
  _arguments \
    '--cache-artifacts[Set to true to enable caching of artifacts]' \
    '--cache-file[Specify the location of the cache file (default $HOME/.skaffold/cache)]:' \
    '--cleanup[Delete deployments after dev or debug mode is interrupted]' \
    '(-d --default-repo)'{-d,--default-repo}'[Default repository value (overrides global config)]:' \
    '--enable-rpc[Enable gRPC for exposing Skaffold events (true by default for `skaffold dev`)]' \
    '(-f --filename)'{-f,--filename}'[Filename or URL to the pipeline file]:' \
    '--force[Recreate kubernetes resources if necessary for deployment (warning: might cause downtime!)]' \
    '*--insecure-registry[Target registries for built images which are not secure]:' \
    '(*-l *--label)'{\*-l,\*--label}'[Add custom labels to deployed objects. Set multiple times for multiple labels]:' \
    '(-n --namespace)'{-n,--namespace}'[Run deployments in the specified namespace]:' \
    '--no-prune[Skip removing images and containers built by Skaffold]' \
    '--no-prune-children[Skip removing layers reused by Skaffold]' \
    '(*-p *--profile)'{\*-p,\*--profile}'[Activate profiles by name]:' \
    '--rpc-http-port[tcp port to expose event REST API over HTTP]:' \
    '--rpc-port[tcp port to expose event API]:' \
    '--skip-tests[Whether to skip the tests after building]' \
    '(-t --tag)'{-t,--tag}'[The optional custom tag to use for images which overrides the current Tagger configuration]:' \
    '--tail[Stream logs from deployed objects (default false)]' \
    '--toot[Emit a terminal beep after the deploy is complete]' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

function _skaffold_version {
  _arguments \
    '(-o --output)'{-o,--output}'[Format output with go-template. For full struct documentation, see https://godoc.org/github.com/GoogleContainerTools/skaffold/pkg/skaffold/version#Info]:' \
    '--color[Specify the default output color in ANSI escape codes]:' \
    '(-v --verbosity)'{-v,--verbosity}'[Log level (debug, info, warn, error, fatal, panic)]:'
}

@rsteube
Copy link
Contributor

rsteube commented Jun 11, 2019

Maybe similar: kubernetes-sigs/kind#522 (comment)

Can look at it tomorrow.

@babysnakes
Copy link
Contributor

Hey @corneliusweig

Sadly I can't reproduce it. I copied the generated script you posted to somewhere in my $fpath as _skaffold and it's working. (actually I made a typo when I copied it first and it seemed to trigger some whatever cache that didn't resolve even after I fixed the filename and
opened new terminal - I actually had to restart to resolve it 🙁). Once I did restart it seems to work:

Screen Shot 2019-06-11 at 10 37 42

Screen Shot 2019-06-11 at 10 37 53

Can you please copy the file to somewhere in your $fpath as _skaffold and see if it works?

Regarding sourcing the completion script, I'm not sure it works. I don't think I ever tested for it, nor was it a target of mine when I wrote it. In my opinion completion scripts should not be manually sourced. I have hundreds of them and it doesn't make sense to load them separately 🙂.

@corneliusweig
Copy link
Author

@babysnakes That does not seem to work either. I am using zsh 5.6.2 with oh-my-zsh.

I think that being able to source the generated script is a major use-case for cobra completion. For example, that is how kubectl, minikube, or skaffold recommend to set up completion. Usually these commands would be put in your zshrc, so that one always gets the latest completion script. The indirection via a file seems unnecessary to me and it has the danger of becoming outdated.

@babysnakes
Copy link
Contributor

Hey @corneliusweig, Let's tackle the sourcing issue later (I'm not even sure what it takes, I have to look).

First let's get it working. I installed docker with oh-my-zsh - default installation, didn't do anything beside the install command - and at first it didn't work. however, after I ran compinit it did work (checkout this ticket). Does it help in your case? if not can you type scaffold and then CTRL-X ? and send me the generated file?

@corneliusweig
Copy link
Author

Hey @babysnakes, thanks for your quick response. Indeed it works also on my system, when I do

  1. skaffold completion zsh > $fpath/_skaffold
  2. compinit

So that is resolved then.


Now what about the direct sourcing, though. I was looking at the old code from the skaffold completion (which uses a zsh wrapper around the bash completion, see below). I wonder why that works without the indirection via a file. In other words, just doing a source <(skaffold completion zsh) with the old version works without having to call compinit or saving the output to a file on $fpath. Can you figure out why? I don't know zsh completions at all :/

#compdef skaffold

__skaffold_bash_source() {
	alias shopt=':'
	alias _expand=_bash_expand
	alias _complete=_bash_comp
	emulate -L sh
	setopt kshglob noshglob braceexpand

	source "$@"
}

__skaffold_type() {
	# -t is not supported by zsh
	if [ "$1" == "-t" ]; then
		shift

		# fake Bash 4 to disable "complete -o nospace". Instead
		# "compopt +-o nospace" is used in the code to toggle trailing
		# spaces. We don't support that, but leave trailing spaces on
		# all the time
		if [ "$1" = "__skaffold_compopt" ]; then
			echo builtin
			return 0
		fi
	fi
	type "$@"
}

__skaffold_compgen() {
	local completions w
	completions=( $(compgen "$@") ) || return $?

	# filter by given word as prefix
	while [[ "$1" = -* && "$1" != -- ]]; do
		shift
		shift
	done
	if [[ "$1" == -- ]]; then
		shift
	fi
	for w in "${completions[@]}"; do
		if [[ "${w}" = "$1"* ]]; then
			echo "${w}"
		fi
	done
}

__skaffold_compopt() {
	true # don't do anything. Not supported by bashcompinit in zsh
}

__skaffold_ltrim_colon_completions()
{
	if [[ "$1" == *:* && "$COMP_WORDBREAKS" == *:* ]]; then
		# Remove colon-word prefix from COMPREPLY items
		local colon_word=${1%${1##*:}}
		local i=${#COMPREPLY[*]}
		while [[ $((--i)) -ge 0 ]]; do
			COMPREPLY[$i]=${COMPREPLY[$i]#"$colon_word"}
		done
	fi
}

__skaffold_get_comp_words_by_ref() {
	cur="${COMP_WORDS[COMP_CWORD]}"
	prev="${COMP_WORDS[${COMP_CWORD}-1]}"
	words=("${COMP_WORDS[@]}")
	cword=("${COMP_CWORD[@]}")
}

__skaffold_filedir() {
	local RET OLD_IFS w qw

	__skaffold_debug "_filedir $@ cur=$cur"
	if [[ "$1" = \~* ]]; then
		# somehow does not work. Maybe, zsh does not call this at all
		eval echo "$1"
		return 0
	fi

	OLD_IFS="$IFS"
	IFS=$'\n'
	if [ "$1" = "-d" ]; then
		shift
		RET=( $(compgen -d) )
	else
		RET=( $(compgen -f) )
	fi
	IFS="$OLD_IFS"

	IFS="," __skaffold_debug "RET=${RET[@]} len=${#RET[@]}"

	for w in ${RET[@]}; do
		if [[ ! "${w}" = "${cur}"* ]]; then
			continue
		fi
		if eval "[[ \"\${w}\" = *.$1 || -d \"\${w}\" ]]"; then
			qw="$(__skaffold_quote "${w}")"
			if [ -d "${w}" ]; then
				COMPREPLY+=("${qw}/")
			else
				COMPREPLY+=("${qw}")
			fi
		fi
	done
}

__skaffold_quote() {
    if [[ $1 == \'* || $1 == \"* ]]; then
        # Leave out first character
        printf %q "${1:1}"
    else
	printf %q "$1"
    fi
}

autoload -U +X bashcompinit && bashcompinit

# use word boundary patterns for BSD or GNU sed
LWORD='[[:<:]]'
RWORD='[[:>:]]'
if sed --help 2>&1 | grep -q GNU; then
	LWORD='\<'
	RWORD='\>'
fi

__skaffold_convert_bash_to_zsh() {
	sed \
	-e 's/declare -F/whence -w/' \
	-e 's/_get_comp_words_by_ref "\$@"/_get_comp_words_by_ref "\$*"/' \
	-e 's/local \([a-zA-Z0-9_]*\)=/local \1; \1=/' \
	-e 's/flags+=("\(--.*\)=")/flags+=("\1"); two_word_flags+=("\1")/' \
	-e 's/must_have_one_flag+=("\(--.*\)=")/must_have_one_flag+=("\1")/' \
	-e "s/${LWORD}_filedir${RWORD}/__skaffold_filedir/g" \
	-e "s/${LWORD}_get_comp_words_by_ref${RWORD}/__skaffold_get_comp_words_by_ref/g" \
	-e "s/${LWORD}__ltrim_colon_completions${RWORD}/__skaffold_ltrim_colon_completions/g" \
	-e "s/${LWORD}compgen${RWORD}/__skaffold_compgen/g" \
	-e "s/${LWORD}compopt${RWORD}/__skaffold_compopt/g" \
	-e "s/${LWORD}declare${RWORD}/builtin declare/g" \
	-e "s/\\\$(type${RWORD}/\$(__skaffold_type/g" \
	<<'BASH_COMPLETION_EOF'
#######################################
# output of `skaffold completion bash` removed 
#######################################
BASH_COMPLETION_EOF
}

__skaffold_bash_source <(__skaffold_convert_bash_to_zsh)
_complete skaffold 2>/dev/null

@babysnakes
Copy link
Contributor

Hey @corneliusweig

I'm not sure what it takes. playing around with it a little I got the infamous error of _arguments:comparguments:325: can only be called from completion function. Quick google search revealed a few people who managed to work around that in their case but it seems to come up a lot. let me check for a few days and see if I can get something to work.

@babysnakes
Copy link
Contributor

Hey @corneliusweig

I'm sorry but looking at it a little further it doesn't seem it can work. Zsh has more then one completion systems. I used the one that is used by the great zsh-users/zsh-completion project. It offers all the niceties of having descriptions on completion, etc. However, it seems that this system can only be used as files inside $fpath, not sourced directly (hence the error message in the previous post). I also checked the few commands I have that uses the system you describe above (eval ...) and what's common to all of them - at least the ones that installed on my laptop - is that they don't offer the full experience of zsh completion.

I'm not sure I can offer any solution for that 🙁

@rsteube
Copy link
Contributor

rsteube commented Jun 12, 2019

@corneliusweig i think the magic happens here: autoload -U +X bashcompinit && bashcompinit and the completion is probably then registered by _complete skaffold 2>/dev/null.
Seems zsh supports bash completions which is why that script looks so unfamiliar to me: https://stackoverflow.com/a/3251836

Regarding adding the completion without fpath this seems to be actually possible with manual invocation of compdef (didn't know about it so far), you still have to initialize the completion system first though.
So something similar to this might work:

autoload -Uz compinit && compinit -C
source <(skaffold completion zsh)
compdef _skaffold skaffold

This is certainly easier to use but skips all the caching mechanisms of zsh regarding the completion system. AFAIK those from fpath get cached in a ~.zcompdump file which speeds up terminal load time (each time you open a new terminal there is a noticable delay before you can start typing). Invoking a command to generate the completion adds even more to that.

see http://zsh.sourceforge.net/Doc/Release/Completion-System.html#Use-of-compinit

(seems i had some assumptions wrong regarding the .zcompdump file, look at the documentation for a correct explanation)

@corneliusweig
Copy link
Author

@rsteube Thanks for investigating this! I think it is safe to assume that compinit has already been correctly initialized when a user tries to source <(cobracmd completion zsh) or so. That means, that the compdef stuff could be moved into the completion template like so:

diff --git a/zsh_completions.go b/zsh_completions.go
index 1275548..538c3f7 100644
--- a/zsh_completions.go
+++ b/zsh_completions.go
@@ -80,6 +80,7 @@ function {{genZshFuncName .}} {
 #compdef _{{.Name}} {{.Name}}
 
 {{template "selectCmdTemplate" .}}
+compdef _{{.Name}} {{.Name}}
 {{end}}
 `
 )

In my manual test that worked like a charm.


The question is: does the extra compdef call have any undesired side-effects when somebody puts the script into a file?

@rsteube
Copy link
Contributor

rsteube commented Jun 12, 2019

Yes, i was thinking the same and had tried it out successfully as well.
Still investigating about side effects though until just now where i've simply grep'd the oh-my-zsh repo for compdef and as it seems quite some scripts do exactly that.
So i think it's safe to assume that it's ok to add that.

Here's some documentation regarding the header line: http://zsh.sourceforge.net/Doc/Release/Completion-System.html#Autoloaded-files

@yashbhutwala
Copy link

Hey @rsteube @corneliusweig I'm running into this issue, and I applied your suggested fix: #881 (comment)

So, now the tab does something, but I run into this error upon tab with global flags:

_arguments:comparguments:319: invalid option definition:

@rsteube
Copy link
Contributor

rsteube commented Jun 12, 2019

That looks more like a problem in the completion script itself. Did a short search and this came up: clap-rs/clap#771 (comment)

Are there some special characters in that line (like the square brackets)? I had a problem with the brackets myself in one completion and needed to add quotation for that.

see: #885

@yashbhutwala
Copy link

yashbhutwala commented Jun 12, 2019

Cool, thanks @rsteube

@corneliusweig
Copy link
Author

corneliusweig commented Jun 12, 2019

i've simply grep'd the oh-my-zsh repo for compdef and as it seems quite some scripts do exactly that.

@rsteube Sounds fine to add that here as well. Do you want to open a PR? Otherwise I'll do that :)

@rsteube
Copy link
Contributor

rsteube commented Jun 12, 2019

can do - just a sec

@babysnakes
Copy link
Contributor

@rsteube very cool, learned something new today 🙂.

@github-actions
Copy link

github-actions bot commented Apr 5, 2020

This issue is being marked as stale due to a long period of inactivity

@corneliusweig
Copy link
Author

I'll close this, because it looks like the cobra authors are happy with the standard zsh way of putting the script into fpath.
Everyone else can take inspiration from https://github.com/GoogleContainerTools/skaffold/pull/3137/files#diff-0b1a7023adc427fe5ea26e6964d2936bR47

@rsteube
Copy link
Contributor

rsteube commented Apr 6, 2020

@corneliusweig i don't remember it exactly but i did notice a small side effect with the explicit compdef call (or the missing _skaffold call).
I think it was that when using fpath the completion initially needed a second <tab> to actually work (so on the very first try it probably just loaded the script).

so instead of:
compdef _skaffold skaffold
maybe sth. like this (compquote is only available during completion, else assume file is direcly sourced):
if compquote '' 2>/dev/null; then _skaffold; else compdef _skaffold skaffold; fi

btw. i did some more work on this here: https://github.com/rsteube/carapace

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

Successfully merging a pull request may close this issue.

4 participants