From ad85ef4151563d289922bdcec9fe51a1774c95da Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
Date: Wed, 21 Feb 2024 21:19:45 -0600
Subject: [PATCH 1/5] refactor(cli): Shrink main

I'm going to add more `config_configure`s and this will make it more
obvious that these are done in early returns.
---
 src/bin/cargo/cli.rs | 202 ++++++++++++++++++++++---------------------
 1 file changed, 105 insertions(+), 97 deletions(-)

diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs
index f6c3f5f3497..9d021884664 100644
--- a/src/bin/cargo/cli.rs
+++ b/src/bin/cargo/cli.rs
@@ -53,50 +53,7 @@ pub fn main(gctx: &mut GlobalContext) -> CliResult {
         .map(String::as_str)
         == Some("help")
     {
-        let header = style::HEADER;
-        let literal = style::LITERAL;
-        let placeholder = style::PLACEHOLDER;
-
-        let options = CliUnstable::help();
-        let max_length = options
-            .iter()
-            .filter(|(_, help)| help.is_some())
-            .map(|(option_name, _)| option_name.len())
-            .max()
-            .unwrap_or(0);
-        let z_flags = options
-            .iter()
-            .filter(|(_, help)| help.is_some())
-            .map(|(opt, help)| {
-                let opt = opt.replace("_", "-");
-                let help = help.unwrap();
-                format!("    {literal}-Z {opt:<max_length$}{literal:#}  {help}")
-            })
-            .join("\n");
-        drop_println!(
-            gctx,
-            "\
-{header}Available unstable (nightly-only) flags:{header:#}
-
-{z_flags}
-
-Run with `{literal}cargo -Z{literal:#} {placeholder}[FLAG] [COMMAND]{placeholder:#}`",
-        );
-        if !gctx.nightly_features_allowed {
-            drop_println!(
-                gctx,
-                "\nUnstable flags are only available on the nightly channel \
-                 of Cargo, but this is the `{}` channel.\n\
-                 {}",
-                features::channel(),
-                features::SEE_CHANNELS
-            );
-        }
-        drop_println!(
-            gctx,
-            "\nSee https://doc.rust-lang.org/nightly/cargo/reference/unstable.html \
-             for more information about these flags."
-        );
+        print_zhelp(gctx);
         return Ok(());
     }
 
@@ -114,59 +71,7 @@ Run with `{literal}cargo -Z{literal:#} {placeholder}[FLAG] [COMMAND]{placeholder
     }
 
     if expanded_args.flag("list") {
-        // Maps from commonly known external commands (not builtin to cargo)
-        // to their description, for the help page. Reserved for external
-        // subcommands that are core within the rust ecosystem (esp ones that
-        // might become internal in the future).
-        let known_external_command_descriptions = HashMap::from([
-            (
-                "clippy",
-                "Checks a package to catch common mistakes and improve your Rust code.",
-            ),
-            (
-                "fmt",
-                "Formats all bin and lib files of the current crate using rustfmt.",
-            ),
-        ]);
-        drop_println!(
-            gctx,
-            color_print::cstr!("<green,bold>Installed Commands:</>")
-        );
-        for (name, command) in list_commands(gctx) {
-            let known_external_desc = known_external_command_descriptions.get(name.as_str());
-            let literal = style::LITERAL;
-            match command {
-                CommandInfo::BuiltIn { about } => {
-                    assert!(
-                        known_external_desc.is_none(),
-                        "known_external_commands shouldn't contain builtin `{name}`",
-                    );
-                    let summary = about.unwrap_or_default();
-                    let summary = summary.lines().next().unwrap_or(&summary); // display only the first line
-                    drop_println!(gctx, "    {literal}{name:<20}{literal:#} {summary}");
-                }
-                CommandInfo::External { path } => {
-                    if let Some(desc) = known_external_desc {
-                        drop_println!(gctx, "    {literal}{name:<20}{literal:#} {desc}");
-                    } else if is_verbose {
-                        drop_println!(
-                            gctx,
-                            "    {literal}{name:<20}{literal:#} {}",
-                            path.display()
-                        );
-                    } else {
-                        drop_println!(gctx, "    {literal}{name}{literal:#}");
-                    }
-                }
-                CommandInfo::Alias { target } => {
-                    drop_println!(
-                        gctx,
-                        "    {literal}{name:<20}{literal:#} alias: {}",
-                        target.iter().join(" ")
-                    );
-                }
-            }
-        }
+        print_list(gctx, is_verbose);
         return Ok(());
     }
 
@@ -185,6 +90,109 @@ Run with `{literal}cargo -Z{literal:#} {placeholder}[FLAG] [COMMAND]{placeholder
     exec.exec(gctx, subcommand_args)
 }
 
+fn print_zhelp(gctx: &GlobalContext) {
+    let header = style::HEADER;
+    let literal = style::LITERAL;
+    let placeholder = style::PLACEHOLDER;
+
+    let options = CliUnstable::help();
+    let max_length = options
+        .iter()
+        .filter(|(_, help)| help.is_some())
+        .map(|(option_name, _)| option_name.len())
+        .max()
+        .unwrap_or(0);
+    let z_flags = options
+        .iter()
+        .filter(|(_, help)| help.is_some())
+        .map(|(opt, help)| {
+            let opt = opt.replace("_", "-");
+            let help = help.unwrap();
+            format!("    {literal}-Z {opt:<max_length$}{literal:#}  {help}")
+        })
+        .join("\n");
+    drop_println!(
+        gctx,
+        "\
+{header}Available unstable (nightly-only) flags:{header:#}
+
+{z_flags}
+
+Run with `{literal}cargo -Z{literal:#} {placeholder}[FLAG] [COMMAND]{placeholder:#}`",
+    );
+    if !gctx.nightly_features_allowed {
+        drop_println!(
+            gctx,
+            "\nUnstable flags are only available on the nightly channel \
+                 of Cargo, but this is the `{}` channel.\n\
+                 {}",
+            features::channel(),
+            features::SEE_CHANNELS
+        );
+    }
+    drop_println!(
+        gctx,
+        "\nSee https://doc.rust-lang.org/nightly/cargo/reference/unstable.html \
+             for more information about these flags."
+    );
+}
+
+fn print_list(gctx: &GlobalContext, is_verbose: bool) {
+    // Maps from commonly known external commands (not builtin to cargo)
+    // to their description, for the help page. Reserved for external
+    // subcommands that are core within the rust ecosystem (esp ones that
+    // might become internal in the future).
+    let known_external_command_descriptions = HashMap::from([
+        (
+            "clippy",
+            "Checks a package to catch common mistakes and improve your Rust code.",
+        ),
+        (
+            "fmt",
+            "Formats all bin and lib files of the current crate using rustfmt.",
+        ),
+    ]);
+    drop_println!(
+        gctx,
+        color_print::cstr!("<green,bold>Installed Commands:</>")
+    );
+    for (name, command) in list_commands(gctx) {
+        let known_external_desc = known_external_command_descriptions.get(name.as_str());
+        let literal = style::LITERAL;
+        match command {
+            CommandInfo::BuiltIn { about } => {
+                assert!(
+                    known_external_desc.is_none(),
+                    "known_external_commands shouldn't contain builtin `{name}`",
+                );
+                let summary = about.unwrap_or_default();
+                let summary = summary.lines().next().unwrap_or(&summary); // display only the first line
+                drop_println!(gctx, "    {literal}{name:<20}{literal:#} {summary}");
+            }
+            CommandInfo::External { path } => {
+                if let Some(desc) = known_external_desc {
+                    drop_println!(gctx, "    {literal}{name:<20}{literal:#} {desc}");
+                } else if is_verbose {
+                    drop_println!(
+                        gctx,
+                        "    {literal}{name:<20}{literal:#} {}",
+                        path.display()
+                    );
+                } else {
+                    drop_println!(gctx, "    {literal}{name}{literal:#}");
+                }
+            }
+            CommandInfo::Alias { target } => {
+                drop_println!(
+                    gctx,
+                    "    {literal}{name:<20}{literal:#} alias: {}",
+                    target.iter().join(" ")
+                );
+            }
+        }
+    }
+}
+
 pub fn get_version_string(is_verbose: bool) -> String {
     let version = cargo::version();
     let mut version_string = format!("cargo {}\n", version);

From 808b4f64b1dcd48471eef7a0869964c87534f411 Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
Date: Wed, 21 Feb 2024 21:22:02 -0600
Subject: [PATCH 2/5] refactor(cli): Make it obvious that the cases are
 mutually exclusive

---
 src/bin/cargo/cli.rs | 48 ++++++++++++++++++--------------------------
 1 file changed, 20 insertions(+), 28 deletions(-)

diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs
index 9d021884664..106e92d689b 100644
--- a/src/bin/cargo/cli.rs
+++ b/src/bin/cargo/cli.rs
@@ -48,46 +48,38 @@ pub fn main(gctx: &mut GlobalContext) -> CliResult {
 
     let (expanded_args, global_args) = expand_aliases(gctx, args, vec![])?;
 
+    let is_verbose = expanded_args.verbose() > 0;
+
     if expanded_args
         .get_one::<String>("unstable-features")
         .map(String::as_str)
         == Some("help")
     {
         print_zhelp(gctx);
-        return Ok(());
-    }
-
-    let is_verbose = expanded_args.verbose() > 0;
-    if expanded_args.flag("version") {
+    } else if expanded_args.flag("version") {
         let version = get_version_string(is_verbose);
         drop_print!(gctx, "{}", version);
-        return Ok(());
-    }
-
-    if let Some(code) = expanded_args.get_one::<String>("explain") {
+    } else if let Some(code) = expanded_args.get_one::<String>("explain") {
         let mut procss = gctx.load_global_rustc(None)?.process();
         procss.arg("--explain").arg(code).exec()?;
-        return Ok(());
-    }
-
-    if expanded_args.flag("list") {
+    } else if expanded_args.flag("list") {
         print_list(gctx, is_verbose);
-        return Ok(());
-    }
-
-    let (cmd, subcommand_args) = match expanded_args.subcommand() {
-        Some((cmd, args)) => (cmd, args),
-        _ => {
-            // No subcommand provided.
-            cli(gctx).print_help()?;
-            return Ok(());
-        }
-    };
-    let exec = Exec::infer(cmd)?;
-    config_configure(gctx, &expanded_args, subcommand_args, global_args, &exec)?;
-    super::init_git(gctx);
+    } else {
+        let (cmd, subcommand_args) = match expanded_args.subcommand() {
+            Some((cmd, args)) => (cmd, args),
+            _ => {
+                // No subcommand provided.
+                cli(gctx).print_help()?;
+                return Ok(());
+            }
+        };
+        let exec = Exec::infer(cmd)?;
+        config_configure(gctx, &expanded_args, subcommand_args, global_args, &exec)?;
+        super::init_git(gctx);
 
-    exec.exec(gctx, subcommand_args)
+        exec.exec(gctx, subcommand_args)?;
+    }
+    Ok(())
 }
 
 fn print_zhelp(gctx: &GlobalContext) {

From 1834244c987b4ec41ee5a45ec69d0661468c4d29 Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
Date: Wed, 21 Feb 2024 21:22:52 -0600
Subject: [PATCH 3/5] refactor(cli): Allow calling config_configure without
 Exec

---
 src/bin/cargo/cli.rs | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs
index 106e92d689b..b73567bff5f 100644
--- a/src/bin/cargo/cli.rs
+++ b/src/bin/cargo/cli.rs
@@ -74,7 +74,13 @@ pub fn main(gctx: &mut GlobalContext) -> CliResult {
             }
         };
         let exec = Exec::infer(cmd)?;
-        config_configure(gctx, &expanded_args, subcommand_args, global_args, &exec)?;
+        config_configure(
+            gctx,
+            &expanded_args,
+            Some(subcommand_args),
+            global_args,
+            Some(&exec),
+        )?;
         super::init_git(gctx);
 
         exec.exec(gctx, subcommand_args)?;
@@ -365,16 +371,18 @@ For more information, see issue #12207 <https://github.com/rust-lang/cargo/issue
 fn config_configure(
     gctx: &mut GlobalContext,
     args: &ArgMatches,
-    subcommand_args: &ArgMatches,
+    subcommand_args: Option<&ArgMatches>,
     global_args: GlobalArgs,
-    exec: &Exec,
+    exec: Option<&Exec>,
 ) -> CliResult {
-    let arg_target_dir = &subcommand_args.value_of_path("target-dir", gctx);
+    let arg_target_dir = &subcommand_args.and_then(|a| a.value_of_path("target-dir", gctx));
     let mut verbose = global_args.verbose + args.verbose();
     // quiet is unusual because it is redefined in some subcommands in order
     // to provide custom help text.
-    let mut quiet = args.flag("quiet") || subcommand_args.flag("quiet") || global_args.quiet;
-    if matches!(exec, Exec::Manifest(_)) && !quiet {
+    let mut quiet = args.flag("quiet")
+        || subcommand_args.map(|a| a.flag("quiet")).unwrap_or_default()
+        || global_args.quiet;
+    if matches!(exec, Some(Exec::Manifest(_))) && !quiet {
         // Verbosity is shifted quieter for `Exec::Manifest` as it is can be used as if you ran
         // `cargo install` and we especially shouldn't pollute programmatic output.
         //

From f525e1f383fc0d0a7adbdfb1bac6d6228ae79b33 Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
Date: Thu, 22 Feb 2024 10:55:27 -0600
Subject: [PATCH 4/5] fix(context): Configure Shell before emitting messages

---
 src/cargo/util/config/mod.rs | 66 ++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs
index cf9402c7bc4..30536b1adf9 100644
--- a/src/cargo/util/config/mod.rs
+++ b/src/cargo/util/config/mod.rs
@@ -1018,41 +1018,14 @@ impl GlobalContext {
         unstable_flags: &[String],
         cli_config: &[String],
     ) -> CargoResult<()> {
-        for warning in self
-            .unstable_flags
-            .parse(unstable_flags, self.nightly_features_allowed)?
-        {
-            self.shell().warn(warning)?;
-        }
-        if !unstable_flags.is_empty() {
-            // store a copy of the cli flags separately for `load_unstable_flags_from_config`
-            // (we might also need it again for `reload_rooted_at`)
-            self.unstable_flags_cli = Some(unstable_flags.to_vec());
-        }
-        if !cli_config.is_empty() {
-            self.cli_config = Some(cli_config.iter().map(|s| s.to_string()).collect());
-            self.merge_cli_args()?;
-        }
-        if self.unstable_flags.config_include {
-            // If the config was already loaded (like when fetching the
-            // `[alias]` table), it was loaded with includes disabled because
-            // the `unstable_flags` hadn't been set up, yet. Any values
-            // fetched before this step will not process includes, but that
-            // should be fine (`[alias]` is one of the only things loaded
-            // before configure). This can be removed when stabilized.
-            self.reload_rooted_at(self.cwd.clone())?;
-        }
-        let extra_verbose = verbose >= 2;
-        let verbose = verbose != 0;
-
         // Ignore errors in the configuration files. We don't want basic
         // commands like `cargo version` to error out due to config file
         // problems.
         let term = self.get::<TermConfig>("term").unwrap_or_default();
 
-        let color = color.or_else(|| term.color.as_deref());
-
         // The command line takes precedence over configuration.
+        let extra_verbose = verbose >= 2;
+        let verbose = verbose != 0;
         let verbosity = match (verbose, quiet) {
             (true, true) => bail!("cannot set both --verbose and --quiet"),
             (true, false) => Verbosity::Verbose,
@@ -1066,16 +1039,17 @@ impl GlobalContext {
                 _ => Verbosity::Normal,
             },
         };
-
-        let cli_target_dir = target_dir.as_ref().map(|dir| Filesystem::new(dir.clone()));
-
         self.shell().set_verbosity(verbosity);
+        self.extra_verbose = extra_verbose;
+
+        let color = color.or_else(|| term.color.as_deref());
         self.shell().set_color_choice(color)?;
         if let Some(hyperlinks) = term.hyperlinks {
             self.shell().set_hyperlinks(hyperlinks)?;
         }
+
         self.progress_config = term.progress.unwrap_or_default();
-        self.extra_verbose = extra_verbose;
+
         self.frozen = frozen;
         self.locked = locked;
         self.offline = offline
@@ -1084,8 +1058,34 @@ impl GlobalContext {
                 .ok()
                 .and_then(|n| n.offline)
                 .unwrap_or(false);
+        let cli_target_dir = target_dir.as_ref().map(|dir| Filesystem::new(dir.clone()));
         self.target_dir = cli_target_dir;
 
+        for warning in self
+            .unstable_flags
+            .parse(unstable_flags, self.nightly_features_allowed)?
+        {
+            self.shell().warn(warning)?;
+        }
+        if !unstable_flags.is_empty() {
+            // store a copy of the cli flags separately for `load_unstable_flags_from_config`
+            // (we might also need it again for `reload_rooted_at`)
+            self.unstable_flags_cli = Some(unstable_flags.to_vec());
+        }
+        if !cli_config.is_empty() {
+            self.cli_config = Some(cli_config.iter().map(|s| s.to_string()).collect());
+            self.merge_cli_args()?;
+        }
+        if self.unstable_flags.config_include {
+            // If the config was already loaded (like when fetching the
+            // `[alias]` table), it was loaded with includes disabled because
+            // the `unstable_flags` hadn't been set up, yet. Any values
+            // fetched before this step will not process includes, but that
+            // should be fine (`[alias]` is one of the only things loaded
+            // before configure). This can be removed when stabilized.
+            self.reload_rooted_at(self.cwd.clone())?;
+        }
+
         self.load_unstable_flags_from_config()?;
 
         Ok(())

From 2ec04612d288d2054a50a6b230659a6c4dbe138e Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
Date: Wed, 21 Feb 2024 21:24:01 -0600
Subject: [PATCH 5/5] fix(cli): Respect CARGO_TERM_COLOR in '--list' and
 '-Zhelp'

Found this with the test for `-Zhelp` in #13461.
---
 src/bin/cargo/cli.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs
index b73567bff5f..3203f658c35 100644
--- a/src/bin/cargo/cli.rs
+++ b/src/bin/cargo/cli.rs
@@ -55,14 +55,22 @@ pub fn main(gctx: &mut GlobalContext) -> CliResult {
         .map(String::as_str)
         == Some("help")
     {
+        // Don't let config errors get in the way of parsing arguments
+        let _ = config_configure(gctx, &expanded_args, None, global_args, None);
         print_zhelp(gctx);
     } else if expanded_args.flag("version") {
+        // Don't let config errors get in the way of parsing arguments
+        let _ = config_configure(gctx, &expanded_args, None, global_args, None);
         let version = get_version_string(is_verbose);
         drop_print!(gctx, "{}", version);
     } else if let Some(code) = expanded_args.get_one::<String>("explain") {
+        // Don't let config errors get in the way of parsing arguments
+        let _ = config_configure(gctx, &expanded_args, None, global_args, None);
         let mut procss = gctx.load_global_rustc(None)?.process();
         procss.arg("--explain").arg(code).exec()?;
     } else if expanded_args.flag("list") {
+        // Don't let config errors get in the way of parsing arguments
+        let _ = config_configure(gctx, &expanded_args, None, global_args, None);
         print_list(gctx, is_verbose);
     } else {
         let (cmd, subcommand_args) = match expanded_args.subcommand() {