Update format of toolchain list

Update the format of `rustup toolchain list` to have a similar flavour to the new `rustup show` format.
rust-lang · May 8, 2024 · ac3fc5b · ac3fc5b
1 parent 3deaf97
commit ac3fc5b
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 75 deletions.
diff --git a/src/cli/common.rs b/src/cli/common.rs
@@ -14,7 +14,6 @@ use once_cell::sync::Lazy;
 
 use super::self_update;
 use crate::cli::download_tracker::DownloadTracker;
-use crate::config::ActiveReason;
 use crate::currentprocess::{
     argsource::ArgSource,
     filesource::{StdinSource, StdoutSource},
@@ -24,6 +23,7 @@ use crate::currentprocess::{
 use crate::dist::dist::{TargetTriple, ToolchainDesc};
 use crate::dist::manifest::ComponentStatus;
 use crate::install::UpdateStatus;
+use crate::toolchain::names::{LocalToolchainName, ToolchainName};
 use crate::utils::notifications as util_notifications;
 use crate::utils::notify::NotificationLevel;
 use crate::utils::utils;
@@ -426,78 +426,72 @@ fn list_items(
     Ok(utils::ExitCode(0))
 }
 
-fn print_toolchain_path(
-    cfg: &Cfg,
-    toolchain: &str,
-    if_default: &str,
-    if_override: &str,
-    verbose: bool,
-) -> Result<()> {
-    let toolchain_path = cfg.toolchains_dir.join(toolchain);
-    let toolchain_meta = fs::symlink_metadata(&toolchain_path)?;
-    let toolchain_path = if verbose {
-        if toolchain_meta.is_dir() {
-            format!("\t{}", toolchain_path.display())
-        } else {
-            format!("\t{}", fs::read_link(toolchain_path)?.display())
-        }
-    } else {
-        String::new()
-    };
-    writeln!(
-        process().stdout().lock(),
-        "{}{}{}{}",
-        &toolchain,
-        if_default,
-        if_override,
-        toolchain_path
-    )?;
-    Ok(())
-}
-
 pub(crate) fn list_toolchains(cfg: &Cfg, verbose: bool) -> Result<utils::ExitCode> {
-    // Work with LocalToolchainName to accommodate path based overrides
-    let toolchains = cfg
-        .list_toolchains()?
-        .iter()
-        .map(Into::into)
-        .collect::<Vec<_>>();
+    let toolchains = cfg.list_toolchains()?;
     if toolchains.is_empty() {
         writeln!(process().stdout().lock(), "no installed toolchains")?;
     } else {
-        let def_toolchain_name = cfg.get_default()?.map(|t| (&t).into());
+        let default_toolchain_name = cfg.get_default()?;
         let cwd = utils::current_dir()?;
-        let ovr_toolchain_name =
-            if let Ok(Some((toolchain, reason))) = cfg.find_active_toolchain(&cwd) {
-                match reason {
-                    ActiveReason::Default => None,
-                    _ => Some(toolchain),
-                }
+        let active_toolchain_name: Option<ToolchainName> =
+            if let Ok(Some((LocalToolchainName::Named(toolchain), _reason))) =
+                cfg.find_active_toolchain(&cwd)
+            {
+                Some(toolchain)
             } else {
                 None
             };
+
         for toolchain in toolchains {
-            let if_default = if def_toolchain_name.as_ref() == Some(&toolchain) {
-                " (default)"
-            } else {
-                ""
-            };
-            let if_override = if ovr_toolchain_name.as_ref() == Some(&toolchain) {
-                " (override)"
-            } else {
-                ""
-            };
+            let is_default_toolchain = default_toolchain_name.as_ref() == Some(&toolchain);
+            let is_active_toolchain = active_toolchain_name.as_ref() == Some(&toolchain);
 
-            print_toolchain_path(
+            print_toolchain(
                 cfg,
                 &toolchain.to_string(),
-                if_default,
-                if_override,
+                is_default_toolchain,
+                is_active_toolchain,
                 verbose,
             )
             .context("Failed to list toolchains' directories")?;
         }
     }
+
+    fn print_toolchain(
+        cfg: &Cfg,
+        toolchain: &str,
+        is_default: bool,
+        is_active: bool,
+        verbose: bool,
+    ) -> Result<()> {
+        let toolchain_path = cfg.toolchains_dir.join(toolchain);
+        let toolchain_meta = fs::symlink_metadata(&toolchain_path)?;
+        let toolchain_path = if verbose {
+            if toolchain_meta.is_dir() {
+                format!(" {}", toolchain_path.display())
+            } else {
+                format!(" {}", fs::read_link(toolchain_path)?.display())
+            }
+        } else {
+            String::new()
+        };
+        let status_str = match (is_default, is_active) {
+            (true, true) => " (active, default)",
+            (true, false) => " (default)",
+            (false, true) => " (active)",
+            (false, false) => "",
+        };
+
+        writeln!(
+            process().stdout().lock(),
+            "{}{}{}",
+            &toolchain,
+            status_str,
+            toolchain_path
+        )?;
+        Ok(())
+    }
+
     Ok(utils::ExitCode(0))
 }
 

diff --git a/tests/suite/cli_rustup.rs b/tests/suite/cli_rustup.rs
@@ -834,27 +834,36 @@ fn list_default_toolchain() {
             config.expect_ok(&["rustup", "default", "nightly"]);
             config.expect_ok_ex(
                 &["rustup", "toolchain", "list"],
-                for_host!(
-                    r"nightly-{0} (default)
-"
-                ),
+                for_host!("nightly-{0} (active, default)\n"),
                 r"",
             );
         })
     });
 }
 
 #[test]
-fn list_override_toolchain() {
+fn list_no_default_toolchain() {
+    test(&|config| {
+        config.with_scenario(Scenario::SimpleV2, &|config| {
+            config.expect_ok(&["rustup", "install", "nightly"]);
+            config.expect_ok(&["rustup", "default", "none"]);
+            config.expect_ok_ex(
+                &["rustup", "toolchain", "list"],
+                for_host!("nightly-{0}\n"),
+                r"",
+            );
+        })
+    });
+}
+
+#[test]
+fn list_no_default_override_toolchain() {
     test(&|config| {
         config.with_scenario(Scenario::SimpleV2, &|config| {
             config.expect_ok(&["rustup", "override", "set", "nightly"]);
             config.expect_ok_ex(
                 &["rustup", "toolchain", "list"],
-                for_host!(
-                    r"nightly-{0} (override)
-"
-                ),
+                for_host!("nightly-{0} (active)\n"),
                 r"",
             );
         })
@@ -869,10 +878,7 @@ fn list_default_and_override_toolchain() {
             config.expect_ok(&["rustup", "override", "set", "nightly"]);
             config.expect_ok_ex(
                 &["rustup", "toolchain", "list"],
-                for_host!(
-                    r"nightly-{0} (default) (override)
-"
-                ),
+                for_host!("nightly-{0} (active, default)\n"),
                 r"",
             );
         })

diff --git a/tests/suite/cli_v1.rs b/tests/suite/cli_v1.rs
@@ -91,11 +91,11 @@ fn list_toolchains() {
         config.expect_ok(&["rustup", "update", "nightly"]);
         config.expect_ok(&["rustup", "update", "beta-2015-01-01"]);
         config.expect_stdout_ok(&["rustup", "toolchain", "list"], "nightly");
-        config.expect_stdout_ok(&["rustup", "toolchain", "list", "-v"], "(default)\t");
+        config.expect_stdout_ok(&["rustup", "toolchain", "list", "-v"], "(active, default) ");
         #[cfg(windows)]
         config.expect_stdout_ok(
             &["rustup", "toolchain", "list", "-v"],
-            for_host!("\\toolchains\\nightly-{}"),
+            for_host!(r"\toolchains\nightly-{}"),
         );
         #[cfg(not(windows))]
         config.expect_stdout_ok(
@@ -106,7 +106,7 @@ fn list_toolchains() {
         #[cfg(windows)]
         config.expect_stdout_ok(
             &["rustup", "toolchain", "list", "-v"],
-            "\\toolchains\\beta-2015-01-01",
+            r"\toolchains\beta-2015-01-01",
         );
         #[cfg(not(windows))]
         config.expect_stdout_ok(

diff --git a/tests/suite/cli_v2.rs b/tests/suite/cli_v2.rs
@@ -129,11 +129,11 @@ fn list_toolchains() {
         config.expect_ok(&["rustup", "update", "nightly"]);
         config.expect_ok(&["rustup", "update", "beta-2015-01-01"]);
         config.expect_stdout_ok(&["rustup", "toolchain", "list"], "nightly");
-        config.expect_stdout_ok(&["rustup", "toolchain", "list", "-v"], "(default)\t");
+        config.expect_stdout_ok(&["rustup", "toolchain", "list", "-v"], "(active, default) ");
         #[cfg(windows)]
         config.expect_stdout_ok(
             &["rustup", "toolchain", "list", "-v"],
-            for_host!("\\toolchains\\nightly-{}"),
+            for_host!(r"\toolchains\nightly-{}"),
         );
         #[cfg(not(windows))]
         config.expect_stdout_ok(
@@ -144,7 +144,7 @@ fn list_toolchains() {
         #[cfg(windows)]
         config.expect_stdout_ok(
             &["rustup", "toolchain", "list", "-v"],
-            "\\toolchains\\beta-2015-01-01",
+            r"\toolchains\beta-2015-01-01",
         );
         #[cfg(not(windows))]
         config.expect_stdout_ok(