argoproj / argo-rollouts

Progressive Delivery for Kubernetes
https://argo-rollouts.readthedocs.io/
Apache License 2.0
2.8k stars 876 forks source link

zsh completion not enabled when sourced and not extending kubectl completion #1716

Closed gabipetrovay closed 1 year ago

gabipetrovay commented 2 years ago

Summary

I have the zsh completion sourced in ~/.zshrc using:

source <(kubectl-argo-rollouts completion zsh)
# or
source <(kubectl argo rollouts completion zsh)

With only this, no completion works:

But the completion script does not enable the completion (the following line is commented out):

#compdef _kubectl-argo-rollouts kubectl-argo-rollouts

So I have to also add this compdef to my ~/.zshrc as well:

compdef _kubectl-argo-rollouts kubectl-argo-rollouts

Can this be enabled in the generated completion script?

How can I still enable completions for:

What version of Argo Rollouts are you running?

kubectl-argo-rollouts: v1.1.0+ff3471a
  BuildDate: 2021-10-11T20:20:08Z
  GitCommit: ff3471a2fc3ccb90dbb1f370d7e399ff3064043a
  GitTreeState: clean
  GoVersion: go1.16.3
  Compiler: gc
  Platform: darwin/amd64

Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritize the issues with the most 👍.

alexmt commented 2 years ago

@kostis-codefresh FYI

kostis-codefresh commented 2 years ago

@alexmt . Please assign to me. Thanks for the heads up

alexmt commented 2 years ago

Thank you @kostis-codefresh !

gabipetrovay commented 2 years ago

@kostis-codefresh just did a minor update in the issue description (some copy-paste error). Now the "How can I still enable completions for" question is correct.

kostis-codefresh commented 2 years ago

Hello @gabipetrovay

The completion support is coming straight from the popular cobra library which is used by other gotools. There isn't any special instruction just for zsh https://github.com/argoproj/argo-rollouts/blob/master/pkg/kubectl-argo-rollouts/cmd/completion/completion.go#L59

Anyway, if I follow the documentation for zsh as shown here https://argoproj.github.io/argo-rollouts/generated/kubectl-argo-rollouts/kubectl-argo-rollouts_completion/ then in the output of the command I can get kubect-argo-rollouts autocompletion to work just fine. Have you seen that doc page?

# kubectl-argo-rollouts version
kubectl-argo-rollouts: v1.1.0+ff3471a
  BuildDate: 2021-10-11T20:19:55Z
  GitCommit: ff3471a2fc3ccb90dbb1f370d7e399ff3064043a
  GitTreeState: clean
  GoVersion: go1.16.3
  Compiler: gc
  Platform: linux/amd64

% zsh --version
zsh 5.8 (x86_64-ubuntu-linux-gnu)

% echo "autoload -U compinit; compinit" >> ~/.zshrc

% sudo zsh
# kubectl-argo-rollouts completion zsh> "${fpath[1]}/_kubectl-argo-rollouts"
#

# zsh
# kubectl-argo-rollouts    (I pressed Tab here)
abort          -- Abort a rollout
completion     -- Generate completion script
create         -- Create a Rollout, Experiment, AnalysisTemplate, ClusterAnalysisTemplate, or AnalysisRun resource
dashboard      -- Start UI dashboard
get            -- Get details about rollouts and experiments
help           -- Help about any command
[...] more commands here.

I will see if it is possible to also get autocomplete for the "kubectl argo rollouts" command (i.e. with this spaces) but I am not certain if this is possible with this library, or if I have missed something.

But you should be able to follow the docs and get autocompletion at least kubectl-argo-rollouts. The original command that you used source <(kubectl-argo-rollouts completion zsh) is for bash only according to the docs.

kostis-codefresh commented 2 years ago

Also this might be related (for getting the kubectl argo rollouts completion to work)

https://github.com/kubernetes/kubernetes/issues/74178

https://github.com/kubernetes/kubernetes/pull/105867

marckhouzam commented 2 years ago

Also this might be related (for getting the kubectl argo rollouts completion to work)

kubernetes/kubernetes#105867

@kostis-codefresh Would you be able to try out the proposed solution of the above PR? It should be very easy for you as argo-rollouts uses Cobra. It would be good for plugin developers to give feedback on the PR. If you can try, here is what you need to do:

  1. build kubectl with the PR
    1. clone https://github.com/kubernetes/kubernetes
    2. checkout PR https://github.com/kubernetes/kubernetes/pull/105867
    3. build kubectl go build -o /tmp/kubectl ./cmd/kubectl
  2. create your completion executable using Cobra's __complete command:
    
    cat <<EOF > kubectl_complete-argo-rollouts
    #!/usr/bin/env sh

It is important to quote the \$@ variable since the last parameter

could be empty, which indicates that the prefix to complete is empty.

kubectl argo rollouts __complete "\$@" EOF chmod +x kubectl_complete-argo-rollouts

3. put this new `kubectl_complete-argo-rollouts` on $PATH at the same place as `kubectl-argo-rollouts`
4. and then test

/tmp/kubectl ar /tmp/kubectl argo /tmp/kubectl argo rollouts


I can confirm it works for me.

Note that completion for flags won't work for the plugin until kubectl moves to Cobra 1.3.
kostis-codefresh commented 2 years ago

@marckhouzam Yes I can confirm it works for me as well. Of course I had to enable completion for my shell first. Also it is a bit slow during completion. But it still works!

kostis-codefresh commented 2 years ago

@gabipetrovay to circle back on your original question.

danielhelfand commented 2 years ago

I can confirm what @gabipetrovay is reporting. I was unable to get zsh completion working using the instructions from the completion command. I needed to add compdef _kubectl-argo-rollouts kubectl-argo-rollouts to my ~/.zshrc to get everything working.

zsh --version                                 
zsh 5.8 (x86_64-apple-darwin21.0)
marckhouzam commented 2 years ago

I needed to add compdef _kubectl-argo-rollouts kubectl-argo-rollouts to my ~/.zshrc to get everything working

Sourcing the zsh completion script is not supported out of the box for efficiency reasons; sourcing is not the recommended approach coming from the zsh documentation.

If the project wants to support it anyway, it would have to inject the command mentioned by @danielhelfand when generating the completion script. Helm does this for backward-compatibility reasons here: https://github.com/helm/helm/blob/c05bf275df12b7e827720f728c8afa18c9a12943/cmd/helm/completion.go#L196

danielhelfand commented 2 years ago

I needed to add compdef _kubectl-argo-rollouts kubectl-argo-rollouts to my ~/.zshrc to get everything working

Sourcing the zsh completion script is not supported out of the box for efficiency reasons; sourcing is not the recommended approach coming from the zsh documentation.

If the project wants to support it anyway, it would have to inject the command mentioned by @danielhelfand when generating the completion script. Helm does this for backward-compatibility reasons here: https://github.com/helm/helm/blob/c05bf275df12b7e827720f728c8afa18c9a12943/cmd/helm/completion.go#L196

Definitely agree.

I think my comment is more to bring up that the current completion docs for kubectl-argo-rollouts seem to not work for some users.

zachaller commented 1 year ago

Fixed: see dynamic shell completion here https://blog.argoproj.io/argo-rollouts-1-4-release-8275a0d364be