-
Notifications
You must be signed in to change notification settings - Fork 2.7k
feat(completer): Added completion for --features
flag
#15309
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
Conversation
@epage suggestion will be appreciated on how can i improve this like should i include some description ? or something like that, as i have not discussed with yu earlier and pick directly from the parent issue |
src/cargo/util/command_prelude.rs
Outdated
Ok(features | ||
.into_iter() | ||
.map(|name| clap_complete::CompletionCandidate::new(name)) | ||
.collect()) |
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.
Should we sort or de-duplicate?
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 have sorted it in b615137
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.
What about de-duplicate?
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.
So we'll show a feature multiple times if the name is used in different packages? In some workspaces, that could really bloat the output.
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.
However, we can always iterate on this in a future PR.
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.
So we'll show a feature multiple times if the name is used in different packages? In some workspaces, that could really bloat the output.
ahh, we should have a check ig if the package already exists in the completer, that sounds good ?
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 would assume that we put all features into a BTree<String, Vec<String>>
that maps feature names to packages. We then generate the candidates from this so you get one feature name no matter how many packages use it.
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.
should i implement in this pr ? or in the feature iteration ?
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.
My assumption would be this PR.
Is there a reason to push it out?
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.
However, we can always iterate on this in a future PR.
i thought you wanted to iterate on future pr
9674bef
to
b615137
Compare
This comment has been minimized.
This comment has been minimized.
FYI I'm fine with you cleaning up your commits through the review process. This is small and easy to track the differences between each posting. |
a0315e2
to
1f18cdf
Compare
Please clean up the commits for how they should be merged. |
1fbc09a
to
64b6a59
Compare
- Added completion for --features flag - Collect features from all workspaces - TODO: To remove a feature multiple times if the name is used in different packages
64b6a59
to
c5d4950
Compare
I think I missed that the push happened |
### What does this PR try to resolve? Our options are: - Not offer these completions - Hide them - De-prioritize them Since someone could be using `--package`, I felt de-prioritizing would give the best experience. This is a follow up to #15309 ### How to test and review this PR? Along the way, I made things more consistent across completions
Update cargo submodule 19 commits in 24bb93c388fb8c211a37986539f24a819dc669d3..966f94733bbc94ca51ff9f1e4c49ad250ebbdc50 2025-09-10 23:16:07 +0000 to 2025-09-16 17:24:45 +0000 - fix(frontmatter): Improve error quality (rust-lang/cargo#15972) - fix: wrong variable name in documentation (rust-lang/cargo#15968) - Add "Optimizing Build Performance" section to the Cargo book (rust-lang/cargo#15924) - Remove extra apostrophe in environment-variables.md (rust-lang/cargo#15963) - Clarify warning for using `features` or `default-features` in `patch` (rust-lang/cargo#15953) - fix(frontmatter): Try alternative len code fences (rust-lang/cargo#15952) - feat(cli): Allow completions for third-party subcommand names (rust-lang/cargo#15961) - docs(index): Clarify what we mean by omitting features (rust-lang/cargo#15957) - fix(future): Report all content as a single Report (rust-lang/cargo#15943) - fix(complete): Show local crates/features over other members (rust-lang/cargo#15956) - docs(resolver): Describe the role of the lockfile (rust-lang/cargo#15958) - chore: Skip check-version-bump ci job in forks (rust-lang/cargo#15959) - Eliminate the last three "did you mean" warning phrasings (rust-lang/cargo#15356) - fix(info): Suggest a more universal `cargo tree` command (rust-lang/cargo#15954) - feat(cli): Use ellipses when truncating progress (rust-lang/cargo#15955) - feat(completer): Added completion for `--features` flag (rust-lang/cargo#15309) - fix(publish): Switch the 'ctrl-c on wait' line to a help message (rust-lang/cargo#15942) - docs: move docs building process to contributor guide (rust-lang/cargo#15854) - fix(manifest): Show error source to users (rust-lang/cargo#15939) r? ghost
What does this PR try to resolve?
This attempts to complete the autocompleter for
cargo build --features <TAB>
,cargo run --features <TAB>
It loads all the features that are there in the profile section of Cargo.toml
Related to #14520
How should we test and review this PR?
by running
cargo build --features <TAB>
,cargo run --features <TAB>