Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/cargo/ops/cargo_add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<(

unknown_features.sort();

print_dep_table_msg(&mut options.gctx.shell(), &dep, &unknown_features)?;

if !unknown_features.is_empty() {
let (mut activated, mut deactivated) = dep.features();
// Since the unknown features have been added to the DependencyUI we need to remove
Expand Down Expand Up @@ -233,8 +235,6 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<(
anyhow::bail!(message.trim().to_owned());
}

print_dep_table_msg(&mut options.gctx.shell(), &dep)?;

manifest.insert_into_table(
&dep_table,
&dep,
Expand Down Expand Up @@ -1130,7 +1130,11 @@ fn print_action_msg(shell: &mut Shell, dep: &DependencyUI, section: &[String]) -
shell.status("Adding", message)
}

fn print_dep_table_msg(shell: &mut Shell, dep: &DependencyUI) -> CargoResult<()> {
fn print_dep_table_msg(
shell: &mut Shell,
dep: &DependencyUI,
unknown_features: &Vec<&str>,
) -> CargoResult<()> {
if matches!(shell.verbosity(), crate::core::shell::Verbosity::Quiet) {
return Ok(());
}
Expand All @@ -1140,6 +1144,15 @@ fn print_dep_table_msg(shell: &mut Shell, dep: &DependencyUI) -> CargoResult<()>
let error = style::ERROR;

let (activated, deactivated) = dep.features();
let activated: Vec<&str> = activated
.into_iter()
.filter(|&feature_name| {
!unknown_features
.iter()
.any(|&unknown_feature| unknown_feature == feature_name)
})
.collect();

if !activated.is_empty() || !deactivated.is_empty() {
let prefix = format!("{:>13}", " ");
let suffix = format_features_version_suffix(&dep);
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
16 changes: 11 additions & 5 deletions tests/testsuite/cargo_add/feature_suggestion_none/stderr.term.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 15 additions & 5 deletions tests/testsuite/cargo_add/features_unknown/stderr.term.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
40 changes: 30 additions & 10 deletions tests/testsuite/cargo_add/unknown_inherited_feature/stderr.term.svg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#15438 added the "similar feature" messages and the lists of disabled and enabled features that are showcased in these error messages. We even customized the displaying of those specifically for errors (collapsing enabled features while the regular display will collapse disabled features).

So this feels a bit redundant to show the feature list. Is there a reason you feel we should do both?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI our contrib docs recommend opening issues first so we can discuss these things before implementing so we know what direction to implement and avoid you going in a direction we might not end up going with

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops I somehow didn't notice that. I thought the "as of v x.x.x" part gives better insights in cases like the issue I mentioned, however it's undeniable that this feels too redundant.

And I'm sorry about discussion part - I hurried too much to do something; I should have talked on the issue beforehand.

Closing this PR, thanks for taking your time to review!

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.