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

generate-shell-completion: generate uvx shell completions for the fish shell #7323

Closed
wants to merge 2 commits into from

Conversation

ilyagr
Copy link
Contributor

@ilyagr ilyagr commented Sep 12, 2024

Cc: #7258

Summary

uv generate-shell-completion fish will automatically include completion for uvx.

Hopefully, others will contribute something similar to other shells.

Test Plan

I ran

 cargo run -- generate-shell-completion fish |source

in fish and checked uvx --f<TAB> expands to the correct options. You can also do uvx -<TAB>.

@ilyagr ilyagr force-pushed the fish-uvx branch 2 times, most recently from f7f99e4 to 359cc08 Compare September 12, 2024 05:15
@ilyagr ilyagr marked this pull request as ready for review September 12, 2024 05:16
The next commit will be easier to do in the `uv-cli` crate since it
explicitly depends on `clap-complete`.
@ilyagr
Copy link
Contributor Author

ilyagr commented Sep 12, 2024

Another approach to this would be to make a clap::Command for uvx and call clap_complete::generate on it. That would work for all shells. I haven't yet looked into whether this would be easy to do; something like this clap::Command might already exist to make uvx work in the first place.

@bluss
Copy link
Contributor

bluss commented Sep 12, 2024

I think that for bash, you want the uv and uvx completion to end up in different files, because completion is lazy loaded by filename.

There might be some way to work around, that, however -- the recommended way seems to be to put the whole completion in a file called uv and symlink uvx -> uv in the completions dir. That should work, but it's still a solution that requires two files. 😐

I know the PR is about another shell, just bringing this up as a concern that could need to be addressed in general in the design.

@bluss
Copy link
Contributor

bluss commented Sep 12, 2024

The concern about filename that I had for bash, probably applies in a similar way to fish:

A completion file must have a filename consisting of the name of the command to complete and the suffix .fish.

https://fishshell.com/docs/current/completions.html

@ilyagr
Copy link
Contributor Author

ilyagr commented Sep 12, 2024

A completion file must have a filename consisting of the name of the command to complete and the suffix .fish.

https://fishshell.com/docs/current/completions.html

I think this applies if you want to ship the completions together with the fish shell. I don't think it matters if you follow the instructions and do

echo 'eval "$(uv generate-shell-completion bash)"' >> ~/.bashrc
echo 'uv generate-shell-completion fish | source' >> ~/.config/fish/config.fish

I think my statement applies to bash as well, eval "$(uv generate-shell-completion bash)" looks like it will initialize the completion eagerly and that there is no file to call uv.bash. However, I'm less familiar with bash, so I might be missing something.


One situation where your concerns might be much more relevant is for people who package uv for various distributions (Arch, Debian, Chocolatey,...). They might include the shell completions in files as opposed to running uv generation-shell-completion every time, and they would probably want to follow guidelines closer to what you describe. So, perhaps more should be done to support them at some point, e.g. uv generate-shell-completion --uv-only and uv generate-shell-completion --uvx-only. Or we could wait for such people to show up and request something, I'm just guessing what they might need.

@bluss
Copy link
Contributor

bluss commented Sep 12, 2024

I didn't know the docs had those instructions, I can see how it makes it work.

I prefer to use bash completions this way and would recommend it to others:

BASH_COMPLETION_DIR="$HOME/.local/share/bash-completion/completions"
mkdir -p "$BASH_COMPLETION_DIR"
uv generate-shell-completion bash > "$BASH_COMPLETION_DIR"/uv

It sets up lazy loading correctly. If every tool wanted to be called every time you open a shell (to generate completions), it would take a long time, it's not sustainable.

@ilyagr
Copy link
Contributor Author

ilyagr commented Sep 12, 2024

If every tool wanted to be called every time you open a shell (to generate completions), it would take a long time, it's not sustainable.

I would have thought so, but my understanding is that fish seems to load all the completions eagerly, and seems to work fine and open quickly. It's magic. I'm guessing that bash is just as fast. Update: Or maybe not, judging by bluss's comment below, which is a quote from https://fishshell.com/docs/current/completions.html.

If somebody has a better understanding, let me know!

@bluss
Copy link
Contributor

bluss commented Sep 13, 2024

This makes it sound like lazy loading, not eager, "and any completions defined are automatically loaded when needed."

Now that we know about bash's symlinks, assuming that works for fish too, maybe it's just fine to combine both completion scripts in the same output? It will work with uv's instructions. And the more detailed setups with a file per command can use a symlink from uvx.fish -> uv.fish and so on.

@bluss
Copy link
Contributor

bluss commented Sep 14, 2024

There is a proof of concept solution here #7388 but it has some small technical complications

@ilyagr
Copy link
Contributor Author

ilyagr commented Sep 16, 2024

I like the approach of #7388, since it will work for all shells. I haven't thought about the questions you asked in that PR much. I'm happy to close this PR in favor of #7388 if that PR has a promising future.

@ilyagr
Copy link
Contributor Author

ilyagr commented Sep 16, 2024

I'm closing this in favor of #7388.

@ilyagr ilyagr closed this Sep 16, 2024
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.

2 participants