-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
zsh completion: allow setting custom registries for 'cargo add --registry' #14084
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for being interested in improving the shell completion experience!
It is always painful for the team (at least for me) to maintain each shell completion. So, the current proposal is to move the completion to clap. The project is currently a GSoC (Google Summer of Code) project, and is under active development I believe. Some useful links:
- GSoC project link: https://summerofcode.withgoogle.com/programs/2024/projects/jjnidpgn
- Zulip topic for the discussion https://rust-lang.zulipchat.com/#narrow/stream/421156-gsoc/topic/Project.3A.20Move.20cargo.20shell.20completions.20to.20Rust
- Issue in Cargo repo tracking it: Proposal: Better shell completions #6645
- With a Rust native shell completion we may be able to reduce the risk of running code in malicious projects: Invoking cargo completions in malicious project may lead to RCE rustup#3740
Back to your implementation. I don't think it is wrong, yet it is a bit too specific (you need to define zstyle value to enable the auto-completion for a certain flag). Also, like you've mentioned, there is no a good way to document this feature, so users might not find its existence. If there is going to have more shell completions implemented like this one, it may become a burden to maintain in the long run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(In an ideal world, a completion should collect registry names from Cargo configurations, from both environment variables and the hierarchy of .cargo/config.toml
. That's too far a future I know 🥲 )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the prompt feedback and providing the pointers,
I agree that maintaining the completions is not trivial (thanks for doing it so far!) and agree moving it to clap has huge maintainability benefits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tracked this pull request in #6645, so that we can look into a better integration with Cargo configurations when implementing. Going to close this and thanks for providing the use case and idea!
What does this PR try to resolve?
So far when I type in zsh
cargo add --regi<TAB>
, I getcargo add --registry=
as suggestion. So far so good. But even though I have a private not-default registry configured in~/.cargo/config.toml
, there is no further suggestion for what should come after the equal sign. My crates come basically from two registries in equal parts, so setting a default wouldn't help, as I still have to specify the registry on the command line half of the times. In the PR here, I read a zstyle where a user can specify custom registries. In my case, I can addIn my .zshrc and get
sevensense
suggested. The format for the style is that of an associative array. I.e. for multiple registries:And the tab completion would suggest
I also looked into reading the configured registries from
~/.cargo/config.toml
but afaiucargo
can't print them to the command line. I looked at https://github.com/z-shell/zsh-string-lib/tree/main but adding this as dependency seemed a bit too much complexity added.todo: Having this code block where it is, is probably not optimal. Also, the intended usage should be documented somewhere, where users can find it, I'm open for suggestions.
How should we test and review this PR?
cargo add --registry=<TAB>
in zsh, with the _cargo completion in use, should not do anything with the style unset. With the style set as explained above, you should see that zsh suggests the registries, and completes other options as before.I only tested
cargo add
, but also other commands take the --registry argument. I guess generally completion for more complex command lines with registry should be tested.