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

Add autocompletion generation for more shells #337

Merged
merged 1 commit into from
Jun 2, 2021
Merged

Add autocompletion generation for more shells #337

merged 1 commit into from
Jun 2, 2021

Conversation

falzm
Copy link
Contributor

@falzm falzm commented May 26, 2021

This change adds shell autocompletion support for Fish, PowerShell &
Zsh.

@shortcut-integration
Copy link

This pull request has been linked to Clubhouse Story #30750: Aditional shell completion for exo cli.

@falzm falzm requested a review from retrack May 26, 2021 14:27
Copy link
Member

@zyegfryed zyegfryed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't wait to test it with my zsh shell!

Copy link
Member

@retrack retrack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot get it to work properly and get the below on zsh: (-> is tab)

acr@mbp13ar-3 zsh % exo li-> _arguments:comparguments:325: invalid option definition: (-C --config)-C[Specify an alternate config file [env EXOSCALE_CONFIG]]:

did you manage to get is work ?

.goreleaser.yml Show resolved Hide resolved
Makefile Show resolved Hide resolved
@falzm
Copy link
Contributor Author

falzm commented May 27, 2021

@retrack indeed Zsh seems to require completion files to begin with an underscore.

@falzm
Copy link
Contributor Author

falzm commented May 27, 2021

@zyegfryed can you confirm it works with your zsh shell?

@retrack
Copy link
Member

retrack commented May 27, 2021

@zyegfryed can you confirm it works with your zsh shell?

@falzm can you remind me how to run a local release with goreleaser? maybe using the output of that will help. I tested doing make build and make completions and then moving the file around. But there might be more to it and I am not a zsh user, good old bash and a mile long .bash_profile ...

@zyegfryed
Copy link
Member

@zyegfryed can you confirm it works with your zsh shell?

@falzm can you remind me how to run a local release with goreleaser? maybe using the output of that will help. I tested doing make build and make completions and then moving the file around. But there might be more to it and I am not a zsh user, good old bash and a mile long .bash_profile ...

I'm doing the same and having the exact some behavior as you. I also remove the .zcompdump files on my home directory (rm .zcompdump*), but still the same issue with wrong argument...
Will continue to try to make it work and will keep you update don my progress.

@falzm
Copy link
Contributor Author

falzm commented May 27, 2021

Same, not a zsh user so I'm not sure what I'm doing wrong here...

@retrack you can run goreleaser build --snapshot --rm-dist at the root of the source tree.

@zyegfryed
Copy link
Member

@falzm @retrack OK, I understand what's the issue: in our commands description we sometime refer to a way to define an option via an environment variable, for example, the config option. To do so, we surround the environment variable with brackets in the description – i.e. [env EXOSCALE_CONFIG], which breaks the _autocomplete function.
A quick fix would be to use parentheses instead. Doable?

@zyegfryed
Copy link
Member

zyegfryed commented May 27, 2021

^^ an alternate solution will be to escape the bracket from the description. But I guess we'll need to modify the underlying library (cobra) to do so, wouldn't we?

Edit: already mentionned in cobra project as Issue spf13/cobra#989 (mentioning a closed PR spf13/cobra#899)

^^ bumping cobra to v1.1.0 should fix the issue since a revamped auto-completion for zsh was rolled out in that release.

@falzm
Copy link
Contributor Author

falzm commented May 27, 2021

^^ bumping cobra to v1.1.0 should fix the issue since a revamped auto-completion for zsh was rolled out in that release.

@zyegfryed cool, good thing that I have an on-going branch where I've upgraded Cobra to the latest version then. Thank you for looking into this!

@falzm falzm added the WIP 🚧 Work in progress label May 27, 2021
This change adds shell autocompletion support for Fish, PowerShell &
Zsh.
@falzm falzm removed the WIP 🚧 Work in progress label Jun 2, 2021
@falzm
Copy link
Contributor Author

falzm commented Jun 2, 2021

@zyegfryed I've updated the branch, can I ask you for a new test?

@zyegfryed
Copy link
Member

@zyegfryed I've updated the branch, can I ask you for a new test?

Sure. Test coming...

@zyegfryed
Copy link
Member

So, completion works as expected for the first level commands/options, but fails when going deeper:

~ ❯ exo v<TAB>                                                                                                                                                                                                                                                                                                                                                    
version  -- Print the version of exo
vm       -- Compute instances management

~ ❯ exo --c<TAB>
~ ❯ exo --config

but, this will fail:

~ ❯ exo vm cre
_arguments:comparguments:325: invalid option definition: (-C --config)-C[Specify an alternate config file [env EXOSCALE_CONFIG]]:

@zyegfryed
Copy link
Member

zyegfryed commented Jun 2, 2021

^^ forget my previous comment, I forgot to delete the completion history or made another mistake while trying it, and after cleaning and re-installing it works as expected! GG! 🎉

@zyegfryed zyegfryed self-requested a review June 2, 2021 09:20
@falzm falzm merged commit 8d5e1f6 into master Jun 2, 2021
@falzm falzm deleted the ch30750 branch June 2, 2021 09:34
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 this pull request may close these issues.

3 participants