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 shell completions #1032

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Add shell completions #1032

wants to merge 2 commits into from

Conversation

itowlson
Copy link
Contributor

@itowlson itowlson commented Jan 17, 2023

I had a quick play with this to see what it could give us and there is good news and there is bad news.

The good news is that we can get completions for things like commands and options. E.g. spin de<TAB> -> spin deploy, spin new --va<TAB> -> --value --values-file.

The bad news is that nothing else really works - at least on bash. (Maybe it would have worked better on zsh.) All value completions seem to be "any path." clap_complete supports a bunch of value hints but ignores all of them, or at least all the ones I tried. For example, ValueHint::FilePath, ValueHint::DirPath and ValueHint::Url all resolve to compgen -f which is files and directories. I don't know if this is fixed in more recent versions of clap_complete, but I did find an issue where people were asking for dynamic completions (e.g. spin template new <TAB> and it would give you a list of templates) and it wasn't happening.

In this draft, the user experience is bad news too. You would run spin generate-completions {bash|zsh|...} >spincomps, then chmod +x spincomps, then add source spincomps to your .bashrc or user completions file or something like that. Then you upgrade Spin and do the dance again. If we think command and option completion is worth it, we can discuss a better UI, and whether to support other shells.

And right now the code is also bad news, but I wanted to get opinions on whether we wanted this and if so what we wanted it to look like before I invested in niceing it up.

Signed-off-by: itowlson [email protected]

@kate-goldenring
Copy link
Contributor

@itowlson I wasn't able to get the flow working on my end. I did the following:

cargo build
./target/debug/spin generate-bash-completions > ~/spincomps
echo "sourch spincomps" >> ~/.bashrc
# fails to autocomplete `spin deploy`
./target/debug/spin de

Am i missing a step?

@itowlson
Copy link
Contributor Author

@kate-goldenring It looks like it only works if spin is unqualified (i.e. on the PATH). I see the same thing if I try to autocomplete via a relative path. CURSES.

@itowlson
Copy link
Contributor Author

I dunno if this is a bash limitation or if there is an incantation we can do to make it behave properly.

@itowlson itowlson changed the title Add bash completions Add shell completions Jan 18, 2023
@itowlson
Copy link
Contributor Author

Kate reports that it now blows up on zsh with _arguments:comparguments:325: can only be called from completion function so not sure how to proceed from here... might have to call it a day!

@itowlson
Copy link
Contributor Author

All right, it looks like on zsh you need to pipe it to a file named _spin and put this into .zshrc (assuming you put the file in a ~/completions directory):

autoload -Uz compinit
fpath=(~/completions $fpath)
compinit

Thanks @kate-goldenring for working through this!

@mikkelhegn
Copy link
Member

I wanted to tru this in my Fish shell, but got stuck here:

spin generate-bash-completions > ~/spincomps
Unknown command: plugin 'generate-bash-completions' not found at expected location /Users/mikkel/Library/Application Support/spin/plugins/manifests/generate-bash-completions.json: No such file or directory (os error 2)

I've built the debug version from this PR branch.

@mikkelhegn
Copy link
Member

As a totally unrelated side-note, I just realized the word play of having a Fish shell!!! Have only been using thing thing for at least two years now...

@kate-goldenring
Copy link
Contributor

I wanted to tru this in my Fish shell, but got stuck here:

spin generate-bash-completions > ~/spincomps
Unknown command: plugin 'generate-bash-completions' not found at expected location /Users/mikkel/Library/Application Support/spin/plugins/manifests/generate-bash-completions.json: No such file or directory (os error 2)

I've built the debug version from this PR branch.

@mikkelhegn I believe it has been renamed to remove bash: spin generate-completions > ~/spincomps

@itowlson
Copy link
Contributor Author

@mikkelhegn I updated the description without leaving a comment; my bad. I changed the syntax to spin generate-completions <SHELL> because my first tester used zsh not bash. I think fish is supported but have lost all confidence in how you actually wire up the generated completion file...!

@radu-matei
Copy link
Member

Paging @bacongobbler and @adamreese, who might have more background on building completions for CLIs.

@adamreese
Copy link
Member

Thanks for getting this started. We should exclude hidden commands like spin trigger (and the recursive spin generate-completions)

Adding completers for commands like spin new http<TAB> would be great but isn't needed for the first iteration

@mikkelhegn
Copy link
Member

Works nicely in fish

image

image

spin generate-completions fish > ~/.config/fish/completions/spin.fish

@mikkelhegn
Copy link
Member

@itowlson - Is this something we could consider for 2.0?

@itowlson
Copy link
Contributor Author

@mikkelhegn The question, for me, is: given what we now know about the limitations of doing it the quick way, and the cost of doing it a nicer way, how do we want to proceed:

  1. The quick way is too limited, and the nice way too costly, to be worth it. Leave it for now.
  2. The quick way is limited, but better than nothing - do it that way.
  3. The feature is worth the cost of doing it right - invest in the nice way.

We can then define what we want the user experience to be (e.g. the generate-completions dump command or something fancier).

This is independent of 2.0 (at least at the technical level) - nothing in this would involve breaking changes as far as I know.

@mikkelhegn
Copy link
Member

My preference, stack ranked would be:

3, 2, 1 :-) I'll leave it to the overall prioritization for the team to decide if this is above the cut line.

I think we should have some completion in 2.0 (as the project now starts to really mature). Will either of the above also work for plug-ins out-of-the-box?

@mikkelhegn mikkelhegn added this to the 2.0 milestone Sep 22, 2023
@itowlson
Copy link
Contributor Author

The quick way won't for plugins, though with the Clap shenanigans in #1560 it might list initial plugin names - but it is purely static, derived from the Clap command structures.

The nice way... who knows? I wouldn't be optimistic, and at minimum it would depend on the plugin providing a way to
get completions that we could then fold into our completions (or hand off to the plugin to complete). I don't know enough about completions to be sure if or how this might work.

@mikkelhegn
Copy link
Member

Another effort related to this: https://github.com/karthik2804/spin-autocomplete @karthik2804 @itowlson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Post 1.0
Status: Post Release
Development

Successfully merging this pull request may close these issues.

6 participants