feat(completer): Added completion for --features flag
#15309
Conversation
rustbot
added
A-cli
|
@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 |
| for dep in package.dependencies() { | ||
| if dep.is_optional() { | ||
| features.insert(dep.name_in_toml().to_string()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Are these in the package's summary?
There was a problem hiding this comment.
we capture optional dependencies as features even if they aren't explicitly referenced in any feature declaration, whilw cargo automatically creates implicit features for optional dependencies, they might not appear in the features() map if they aren't referenced by other features, this ensures we provide completion for all valid feature names, including those implicit ones
There was a problem hiding this comment.
I commented out this block and still get the same completion results
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.
Should we sort or de-duplicate?
There was a problem hiding this comment.
i have sorted it in b615137
Gmin2
force-pushed
the
completer-features
branch
from
9674bef to
b615137
Compare
| let manifest_path = match find_root_manifest_for_wd(gctx.cwd()) { | ||
| Ok(path) => path, | ||
| Err(_) => return Ok(Vec::new()), | ||
| }; |
There was a problem hiding this comment.
Why not just use ? if the caller does unwrap_or_default()?
| let ws = match Workspace::new(&manifest_path, &gctx) { | ||
| Ok(ws) => ws, | ||
| Err(_) => return Ok(Vec::new()), | ||
| }; |
| // always include the default feature | ||
| features.insert("default".to_string()); |
There was a problem hiding this comment.
Is there a reason we do this? We don't implicitly create a default feature, see #8164.
| // Process all packages in the workspace, collect features from all packages since --package could be used to select any of them | ||
| for package in ws.members() { | ||
| for feature_name in package.summary().features().keys() { | ||
| features.insert(feature_name.as_str().to_string()); |
There was a problem hiding this comment.
In the candidate description, should we mention the package it comes from?
| // try to find this dependency in the workspace | ||
| if let Some(dep_pkg) = ws.members().find(|p| p.name().as_str() == dep_name) { | ||
| for feat in dep_pkg.summary().features().keys() { | ||
| features.insert(format!("{}/{}", dep_name, feat)); |
There was a problem hiding this comment.
This needs to be name_in_toml and
not package_name
| // try to find this dependency in the workspace | ||
| if let Some(dep_pkg) = ws.members().find(|p| p.name().as_str() == dep_name) { | ||
| for feat in dep_pkg.summary().features().keys() { | ||
| features.insert(format!("{}/{}", dep_name, feat)); |
There was a problem hiding this comment.
Should we hide these? Seems like they could bloat the output
|
|
||
| // Add qualified features for dependencies | ||
| for dep in package.dependencies() { | ||
| let dep_name = dep.package_name().as_str().to_string(); |
There was a problem hiding this comment.
Why do we need this to be owned if we're just comparing it and printing it?
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. |
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>