feat: Add custom completer for completing `cargo build --packge <TAB>` #14553

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.

Sign up for GitHub

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

Jump to bottom

Open

shannmu wants to merge 2 commits into rust-lang:master from shannmu:_cargo_package_completer

+44 −2

Contributor

shannmu commented Sep 17, 2024 •

edited by epage

Loading

What does this PR try to resolve?

Tracking issue github.com/rust-lang/cargo/issues/14520

Add custom completer for cargo build --package <TAB>

Fixes #15004

rustbot assigned weihanglo

Collaborator

rustbot commented Sep 17, 2024

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot added A-cli S-waiting-on-review labels

Contributor Author

shannmu commented Sep 17, 2024

rustbot assigned epage and unassigned weihanglo

epage reviewed

View reviewed changes

src/cargo/util/command_prelude.rs Outdated

    
                  Ok(targets)

              }

              fn get_package_candidates() -> Vec<clap_complete::CompletionCandidate> {

Contributor

epage Sep 17, 2024

Suggested change

      
            fn get_package_candidates() -> Vec<clap_complete::CompletionCandidate> {
          
            fn get_ws_member_candidates() -> Vec<clap_complete::CompletionCandidate> {

epage reviewed

View reviewed changes

src/cargo/util/command_prelude.rs Outdated

    
                  get_packages_from_metadata()

                      .unwrap_or_default()

                      .into_iter()

                      .map(|pkg| clap_complete::CompletionCandidate::new(pkg.name().as_str()))

Contributor

epage Sep 17, 2024

Let's include the package descriptions

epage reviewed

View reviewed changes

src/cargo/util/command_prelude.rs Outdated Show resolved Hide resolved

epage reviewed

View reviewed changes

src/cargo/util/command_prelude.rs Outdated

    
                      .collect::<Vec<_>>()

              }

              fn get_packages_from_metadata() -> CargoResult<Vec<Package>> {

Contributor

epage Sep 17, 2024

Suggested change

      
            fn get_packages_from_metadata() -> CargoResult<Vec<Package>> {
          
            fn get_ws_member_packages() -> CargoResult<Vec<Package>> {

Contributor

bors commented Sep 17, 2024

☔ The latest upstream changes (presumably #14535) made this pull request unmergeable. Please resolve the merge conflicts.

epage mentioned this pull request

Tracking Issue for native-completion #14520

Open

30 tasks

shannmu force-pushed the _cargo_package_completer branch from 1aaae04 to 869064b Compare

September 26, 2024 14:49

epage reviewed

View reviewed changes

src/cargo/util/command_prelude.rs Outdated

Comment on lines 93 to 96

    
                              .help_heading(heading::PACKAGE_SELECTION)

                              .add(clap_complete::ArgValueCandidates::new(

                                  get_ws_member_candidates,

                              )),

Contributor

epage Sep 26, 2024

Looks like cargo uninstall uses this flag for the same reason as the positional argment

Contributor

epage Sep 26, 2024

arg_package_spec_no_all( calls this

cargo tree uses to refer to any package in the dependency graph

arg_package_spec calls that but all uses look good

epage reviewed

View reviewed changes

src/cargo/util/command_prelude.rs Show resolved Hide resolved

shannmu added 2 commits

October 8, 2024 17:38


          feat: Add _name to distinguish the same flags of different subcomma…

845bb8b

…nds during compile time.


          feat: Add custom completer for completing `cargo build --packge <TAB>…

2bac0b6

…` / `cargo tree --package <TAB>`

shannmu force-pushed the _cargo_package_completer branch from 869064b to 2bac0b6 Compare

October 8, 2024 09:41

epage reviewed

View reviewed changes

src/cargo/util/command_prelude.rs

    
                              .help_heading(heading::PACKAGE_SELECTION),

                              .help_heading(heading::PACKAGE_SELECTION)

                              .add(clap_complete::ArgValueCandidates::new(move || {

                                  if ["build", "tree"].contains(&name.as_str()) {

Contributor

epage Oct 8, 2024

This is subtle and going to be very confusing. I wonder if we should better organize the arguments by what kind of package is being requested

Contributor Author

shannmu Oct 8, 2024

While implementing the completion, I discovered that parameters with the same name require different values under different subcommands. To minimize disruption to the original code, I added the _name interface, which allows the subcommand being used to be determined at compile time. Is there a better way to determine what kind of package is being requested?

Contributor

epage Oct 24, 2024

Some ideas I thought of

We could have different arg_package functions for different roles
We could accept a completer as an argument

epage mentioned this pull request

cli suggestions for --package #15004

Closed

Collaborator

rustbot commented Mar 20, 2025

☔ The latest upstream changes (possibly 307cbfd) made this pull request unmergeable. Please resolve the merge conflicts.

epage mentioned this pull request

feat: Add custom completer for completing cargo build --packge <TAB> #15338

Draft

Member

weihanglo commented Apr 11, 2025

@rustbot author

rustbot added S-waiting-on-author and removed S-waiting-on-review labels

Collaborator

rustbot commented Apr 11, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

epage mentioned this pull request

Autocomplete cargo remove with installed dependencies #15656

Closed

jakobhellermann mentioned this pull request

Add native completions for --package on various commands #16210

Merged

github-merge-queue bot pushed a commit that referenced this pull request


          Add native completions for --package on various commands (#16210)

88c0fba

### What does this PR try to resolve?

Add dynamic completions for the various `-p <package>` arguments.

Fixes #15004

```
cargo bench|build|check|doc|fix|test -p <tab>`
cargo package|publish -p <tab>
cargo add|pkgid|remove|report|run|rustc|rustdoc -p <tab>

cargo uninstall -p <tab>
cargo tree -p <tab> -i <tab>
cargo clean -p <tab> # not implemented
```

Supersedes #14553 and
#15338

---

EDIT: the second implementation has been chosen

In the first commit I implemented the different completions for
different usages by splitting the `arg_package` methods into multiple
ones if necessary:
```
     * arg_package_spec
     * arg_package_spec_no_all
     * arg_installed_package_spec_no_all <- new (for cargo uninstall)
     * arg_dependency_package_spec_no_all <- new (for cargo tree)
     * arg_clean_package_spec_simple <- new (for cargo clean)
     * arg_package_spec_simple
     * arg_package
```

In the second commit I tried to instead pass an `ArgValueCandidates`
into the function to prevent that duplication.
```
     * arg_package_spec
     * arg_package_spec_no_all (ArgValueCandidates)
     * arg_package_spec_simple (ArgValueCandidates)
     * arg_package
 ```

I think I have a small preference towards the latter, but no strong opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cli S-waiting-on-author