fix(add): Focus on error, rather than large feature lists (#15200)

weihanglo · web-flow · commit 2d61f602a094 · 2025-02-18T21:52:32.000Z
### What does this PR try to resolve? Inspired by #11100 and previous work to collapse feature lists down. ### How should we test and review this PR? ### Additional information
diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs
@@ -43,6 +43,8 @@ use crate::CargoResult;
 use crate::GlobalContext;
 use crate_spec::CrateSpec;
 
+const MAX_FEATURE_PRINTS: usize = 30;
+
 /// Information on what dependencies should be added
 #[derive(Clone, Debug)]
 pub struct AddOptions<'a> {
@@ -166,36 +168,44 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<(
                 write!(message, "no features available for crate {}", dep.name)?;
             } else {
                 if !deactivated.is_empty() {
-                    writeln!(
-                        message,
-                        "disabled features:\n    {}",
-                        deactivated
-                            .iter()
-                            .map(|s| s.to_string())
-                            .coalesce(|x, y| if x.len() + y.len() < 78 {
-                                Ok(format!("{x}, {y}"))
-                            } else {
-                                Err((x, y))
-                            })
-                            .into_iter()
-                            .format("\n    ")
-                    )?
+                    if deactivated.len() <= MAX_FEATURE_PRINTS {
+                        writeln!(
+                            message,
+                            "disabled features:\n    {}",
+                            deactivated
+                                .iter()
+                                .map(|s| s.to_string())
+                                .coalesce(|x, y| if x.len() + y.len() < 78 {
+                                    Ok(format!("{x}, {y}"))
+                                } else {
+                                    Err((x, y))
+                                })
+                                .into_iter()
+                                .format("\n    ")
+                        )?;
+                    } else {
+                        writeln!(message, "{} disabled features available", deactivated.len())?;
+                    }
                 }
                 if !activated.is_empty() {
-                    writeln!(
-                        message,
-                        "enabled features:\n    {}",
-                        activated
-                            .iter()
-                            .map(|s| s.to_string())
-                            .coalesce(|x, y| if x.len() + y.len() < 78 {
-                                Ok(format!("{x}, {y}"))
-                            } else {
-                                Err((x, y))
-                            })
-                            .into_iter()
-                            .format("\n    ")
-                    )?
+                    if deactivated.len() + activated.len() <= MAX_FEATURE_PRINTS {
+                        writeln!(
+                            message,
+                            "enabled features:\n    {}",
+                            activated
+                                .iter()
+                                .map(|s| s.to_string())
+                                .coalesce(|x, y| if x.len() + y.len() < 78 {
+                                    Ok(format!("{x}, {y}"))
+                                } else {
+                                    Err((x, y))
+                                })
+                                .into_iter()
+                                .format("\n    ")
+                        )?;
+                    } else {
+                        writeln!(message, "{} enabled features available", activated.len())?;
+                    }
                 }
             }
             anyhow::bail!(message.trim().to_owned());
@@ -1114,7 +1124,6 @@ fn print_dep_table_msg(shell: &mut Shell, dep: &DependencyUI) -> CargoResult<()>
 
         writeln!(stderr, "{prefix}Features{suffix}:")?;
 
-        const MAX_FEATURE_PRINTS: usize = 30;
         let total_activated = activated.len();
         let total_deactivated = deactivated.len();
 
diff --git a/tests/testsuite/cargo_add/features_error_activated_over_limit/in/Cargo.toml b/tests/testsuite/cargo_add/features_error_activated_over_limit/in/Cargo.toml
@@ -0,0 +1,51 @@
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+edition = "2015"
+
+[dependencies.your-face]
+version = "99999.0.0"
+features = [
+  "eyes000",
+  "eyes001",
+  "eyes002",
+  "eyes003",
+  "eyes004",
+  "eyes005",
+  "eyes006",
+  "eyes007",
+  "eyes008",
+  "eyes009",
+  "eyes010",
+  "eyes011",
+  "eyes012",
+  "eyes013",
+  "eyes014",
+  "eyes015",
+  "eyes016",
+  "eyes017",
+  "eyes018",
+  "eyes019",
+  "eyes020",
+  "eyes021",
+  "eyes022",
+  "eyes023",
+  "eyes024",
+  "eyes025",
+  "eyes026",
+  "eyes027",
+  "eyes028",
+  "eyes029",
+  "eyes030",
+  "eyes031",
+  "eyes032",
+  "eyes033",
+  "eyes034",
+  "eyes035",
+  "eyes036",
+  "eyes037",
+  "eyes038",
+  "eyes039",
+]
diff --git a/tests/testsuite/cargo_add/features_error_activated_over_limit/in/src/lib.rs b/tests/testsuite/cargo_add/features_error_activated_over_limit/in/src/lib.rs
diff --git a/tests/testsuite/cargo_add/features_error_activated_over_limit/mod.rs b/tests/testsuite/cargo_add/features_error_activated_over_limit/mod.rs
@@ -0,0 +1,35 @@
+use cargo_test_support::compare::assert_ui;
+use cargo_test_support::current_dir;
+use cargo_test_support::file;
+use cargo_test_support::prelude::*;
+use cargo_test_support::str;
+use cargo_test_support::Project;
+
+#[cargo_test]
+fn case() {
+    const MANY_FEATURES_COUNT: usize = 50;
+
+    cargo_test_support::registry::init();
+    let mut test_package =
+        cargo_test_support::registry::Package::new("your-face", "99999.0.0+my-package");
+    for i in 0..MANY_FEATURES_COUNT {
+        test_package.feature(format!("eyes{i:03}").as_str(), &[]);
+    }
+    test_package.publish();
+
+    let project = Project::from_template(current_dir!().join("in"));
+    let project_root = project.root();
+    let cwd = &project_root;
+
+    let features = "eees100,eees101";
+    snapbox::cmd::Command::cargo_ui()
+        .arg("add")
+        .arg_line(format!("your-face --features {features}").as_str())
+        .current_dir(cwd)
+        .assert()
+        .failure()
+        .stdout_eq(str![""])
+        .stderr_eq(file!["stderr.term.svg"]);
+
+    assert_ui().subset_matches(current_dir!().join("out"), &project_root);
+}
diff --git a/tests/testsuite/cargo_add/features_error_activated_over_limit/out/Cargo.toml b/tests/testsuite/cargo_add/features_error_activated_over_limit/out/Cargo.toml
@@ -0,0 +1,51 @@
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+edition = "2015"
+
+[dependencies.your-face]
+version = "99999.0.0"
+features = [
+  "eyes000",
+  "eyes001",
+  "eyes002",
+  "eyes003",
+  "eyes004",
+  "eyes005",
+  "eyes006",
+  "eyes007",
+  "eyes008",
+  "eyes009",
+  "eyes010",
+  "eyes011",
+  "eyes012",
+  "eyes013",
+  "eyes014",
+  "eyes015",
+  "eyes016",
+  "eyes017",
+  "eyes018",
+  "eyes019",
+  "eyes020",
+  "eyes021",
+  "eyes022",
+  "eyes023",
+  "eyes024",
+  "eyes025",
+  "eyes026",
+  "eyes027",
+  "eyes028",
+  "eyes029",
+  "eyes030",
+  "eyes031",
+  "eyes032",
+  "eyes033",
+  "eyes034",
+  "eyes035",
+  "eyes036",
+  "eyes037",
+  "eyes038",
+  "eyes039",
+]
diff --git a/tests/testsuite/cargo_add/features_error_activated_over_limit/stderr.term.svg b/tests/testsuite/cargo_add/features_error_activated_over_limit/stderr.term.svg
@@ -0,0 +1,40 @@
+<svg width="740px" height="164px" xmlns="http://www.w3.org/2000/svg">
+  <style>
+    .fg { fill: #AAAAAA }
+    .bg { background: #000000 }
+    .fg-green { fill: #00AA00 }
+    .fg-red { fill: #AA0000 }
+    .container {
+      padding: 0 10px;
+      line-height: 18px;
+    }
+    .bold { font-weight: bold; }
+    tspan {
+      font: 14px SFMono-Regular, Consolas, Liberation Mono, Menlo, monospace;
+      white-space: pre;
+      line-height: 18px;
+    }
+  </style>
+
+  <rect width="100%" height="100%" y="0" rx="4.5" class="bg" />
+
+  <text xml:space="preserve" class="container fg">
+    <tspan x="10px" y="28px"><tspan class="fg-green bold">    Updating</tspan><tspan> `dummy-registry` index</tspan>
+</tspan>
+    <tspan x="10px" y="46px"><tspan class="fg-green bold">      Adding</tspan><tspan> your-face v99999.0.0 to dependencies</tspan>
+</tspan>
+    <tspan x="10px" y="64px"><tspan class="fg-red bold">error</tspan><tspan class="bold">:</tspan><tspan> unrecognized features for crate your-face: eees100, eees101</tspan>
+</tspan>
+    <tspan x="10px" y="82px"><tspan>disabled features:</tspan>
+</tspan>
+    <tspan x="10px" y="100px"><tspan>    eyes040, eyes041, eyes042, eyes043, eyes044, eyes045, eyes046, eyes047, eyes048</tspan>
+</tspan>
+    <tspan x="10px" y="118px"><tspan>    eyes049</tspan>
+</tspan>
+    <tspan x="10px" y="136px"><tspan>40 enabled features available</tspan>
+</tspan>
+    <tspan x="10px" y="154px">
+</tspan>
+  </text>
+
+</svg>
diff --git a/tests/testsuite/cargo_add/features_error_deactivated_over_limit/in/Cargo.toml b/tests/testsuite/cargo_add/features_error_deactivated_over_limit/in/Cargo.toml
@@ -0,0 +1,12 @@
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+edition = "2015"
+
+[dependencies.your-face]
+version = "99999.0.0"
+features = [
+  "eyes000",
+]
diff --git a/tests/testsuite/cargo_add/features_error_deactivated_over_limit/in/src/lib.rs b/tests/testsuite/cargo_add/features_error_deactivated_over_limit/in/src/lib.rs
diff --git a/tests/testsuite/cargo_add/features_error_deactivated_over_limit/mod.rs b/tests/testsuite/cargo_add/features_error_deactivated_over_limit/mod.rs
@@ -0,0 +1,35 @@
+use cargo_test_support::compare::assert_ui;
+use cargo_test_support::current_dir;
+use cargo_test_support::file;
+use cargo_test_support::prelude::*;
+use cargo_test_support::str;
+use cargo_test_support::Project;
+
+#[cargo_test]
+fn case() {
+    const MANY_FEATURES_COUNT: usize = 200;
+
+    cargo_test_support::registry::init();
+    let mut test_package =
+        cargo_test_support::registry::Package::new("your-face", "99999.0.0+my-package");
+    for i in 0..MANY_FEATURES_COUNT {
+        test_package.feature(format!("eyes{i:03}").as_str(), &[]);
+    }
+    test_package.publish();
+
+    let project = Project::from_template(current_dir!().join("in"));
+    let project_root = project.root();
+    let cwd = &project_root;
+
+    let features = "eees100,eees101";
+    snapbox::cmd::Command::cargo_ui()
+        .arg("add")
+        .arg_line(format!("your-face --features {features}").as_str())
+        .current_dir(cwd)
+        .assert()
+        .failure()
+        .stdout_eq(str![""])
+        .stderr_eq(file!["stderr.term.svg"]);
+
+    assert_ui().subset_matches(current_dir!().join("out"), &project_root);
+}
diff --git a/tests/testsuite/cargo_add/features_error_deactivated_over_limit/out/Cargo.toml b/tests/testsuite/cargo_add/features_error_deactivated_over_limit/out/Cargo.toml
@@ -0,0 +1,12 @@
+[workspace]
+
+[package]
+name = "cargo-list-test-fixture"
+version = "0.0.0"
+edition = "2015"
+
+[dependencies.your-face]
+version = "99999.0.0"
+features = [
+  "eyes000",
+]
diff --git a/tests/testsuite/cargo_add/features_error_deactivated_over_limit/stderr.term.svg b/tests/testsuite/cargo_add/features_error_deactivated_over_limit/stderr.term.svg
@@ -0,0 +1,36 @@
+<svg width="740px" height="128px" xmlns="http://www.w3.org/2000/svg">
+  <style>
+    .fg { fill: #AAAAAA }
+    .bg { background: #000000 }
+    .fg-green { fill: #00AA00 }
+    .fg-red { fill: #AA0000 }
+    .container {
+      padding: 0 10px;
+      line-height: 18px;
+    }
+    .bold { font-weight: bold; }
+    tspan {
+      font: 14px SFMono-Regular, Consolas, Liberation Mono, Menlo, monospace;
+      white-space: pre;
+      line-height: 18px;
+    }
+  </style>
+
+  <rect width="100%" height="100%" y="0" rx="4.5" class="bg" />
+
+  <text xml:space="preserve" class="container fg">
+    <tspan x="10px" y="28px"><tspan class="fg-green bold">    Updating</tspan><tspan> `dummy-registry` index</tspan>
+</tspan>
+    <tspan x="10px" y="46px"><tspan class="fg-green bold">      Adding</tspan><tspan> your-face v99999.0.0 to dependencies</tspan>
+</tspan>
+    <tspan x="10px" y="64px"><tspan class="fg-red bold">error</tspan><tspan class="bold">:</tspan><tspan> unrecognized features for crate your-face: eees100, eees101</tspan>
+</tspan>
+    <tspan x="10px" y="82px"><tspan>199 disabled features available</tspan>
+</tspan>
+    <tspan x="10px" y="100px"><tspan>1 enabled features available</tspan>
+</tspan>
+    <tspan x="10px" y="118px">
+</tspan>
+  </text>
+
+</svg>
diff --git a/tests/testsuite/cargo_add/mod.rs b/tests/testsuite/cargo_add/mod.rs
@@ -25,6 +25,8 @@ mod features;
 mod features_activated_over_limit;
 mod features_deactivated_over_limit;
 mod features_empty;
+mod features_error_activated_over_limit;
+mod features_error_deactivated_over_limit;
 mod features_multiple_occurrences;
 mod features_preserve;
 mod features_spaced_values;
