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

zsh completion: allow setting custom registries for 'cargo add --registry' #14084

Closed

Conversation

pseyfert
Copy link

What does this PR try to resolve?

So far when I type in zsh cargo add --regi<TAB>, I get cargo 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 add

zstyle ':completion:*:complete:cargo:*:registries' registries "sevensense" "my employer"

In my .zshrc and get sevensense suggested. The format for the style is that of an associative array. I.e. for multiple registries:

zstyle ':completion:*:complete:cargo:*:registries' registries "wubbel" "I like wubbel" "wopper" "you can eat that at: Burger King" "yuhu" "the best"

And the tab completion would suggest

 > cargo add --registry=<TAB>
registry:
wubbel  -- I like wubbel
wopper  -- you can eat that at: Burger King
yuhu    -- the best

I also looked into reading the configured registries from ~/.cargo/config.toml but afaiu cargo 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.

@rustbot
Copy link
Collaborator

rustbot commented Jun 16, 2024

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 (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added A-completions Area: shell completions S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 16, 2024
@pseyfert pseyfert changed the title allow setting custom registries for 'cargo add --registry' zsh completion: allow setting custom registries for 'cargo add --registry' Jun 16, 2024
Copy link
Member

@weihanglo weihanglo Jun 17, 2024

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:


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.

Copy link
Member

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 🥲 )

Copy link
Author

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.

Copy link
Member

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-completions Area: shell completions S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants