refactor(cli): Make help behave like other subcommands

Before, we had hacks to intercept raw arguments and to intercept clap
errors and assume what their intention was to be able to implement our
help system.

This flips it around and makes help like any other subcommand,
simplifying cargo initialization.

diff --git a/Cargo.toml b/Cargo.toml
index b887b6e8b9863e6206cb0b7b58867c6dc74cc92e..21fb83dd47d20b73f701111d20ffcc51e23d2c94 100644 (file)
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -61,7 +61,7 @@ toml_edit =  { version = "0.14.3", features = ["serde", "easy", "perf"] }
 unicode-xid = "0.2.0"
 url = "2.2.2"
 walkdir = "2.2"
-clap = "3.2.1"
+clap = "3.2.18"
 unicode-width = "0.1.5"
 openssl = { version = '0.10.11', optional = true }
 im-rc = "15.0.0"

diff --git a/src/bin/cargo/cli.rs b/src/bin/cargo/cli.rs
index 9e5eb70ceb8862523dab805a1643483615d458f3..d5a3a72e065fef26e8b04c8acc0caefd74ab83c4 100644 (file)
--- a/src/bin/cargo/cli.rs
+++ b/src/bin/cargo/cli.rs
@@ -1,10 +1,7 @@
 use anyhow::anyhow;
 use cargo::core::{features, CliUnstable};
 use cargo::{self, drop_print, drop_println, CliResult, Config};
-use clap::{
-    error::{ContextKind, ContextValue},
-    AppSettings, Arg, ArgMatches,
-};
+use clap::{AppSettings, Arg, ArgMatches};
 use itertools::Itertools;
 use std::collections::HashMap;
 use std::fmt::Write;
@@ -29,31 +26,7 @@ pub fn main(config: &mut Config) -> CliResult {
     // In general, try to avoid loading config values unless necessary (like
     // the [alias] table).
 
-    if commands::help::handle_embedded_help(config) {
-        return Ok(());
-    }
-
-    let args = match cli().try_get_matches() {
-        Ok(args) => args,
-        Err(e) => {
-            if e.kind() == clap::ErrorKind::UnrecognizedSubcommand {
-                // An unrecognized subcommand might be an external subcommand.
-                let cmd = e
-                    .context()
-                    .find_map(|c| match c {
-                        (ContextKind::InvalidSubcommand, &ContextValue::String(ref cmd)) => {
-                            Some(cmd)
-                        }
-                        _ => None,
-                    })
-                    .expect("UnrecognizedSubcommand implies the presence of InvalidSubcommand");
-                return super::execute_external_subcommand(config, cmd, &[cmd, "--help"])
-                    .map_err(|_| e.into());
-            } else {
-                return Err(e.into());
-            }
-        }
-    };
+    let args = cli().try_get_matches()?;
 
     // Global args need to be extracted before expanding aliases because the
     // clap code for extracting a subcommand discards global options
@@ -412,7 +385,7 @@ impl GlobalArgs {
     }
 }
 
-fn cli() -> App {
+pub fn cli() -> App {
     let is_rustup = std::env::var_os("RUSTUP_HOME").is_some();
     let usage = if is_rustup {
         "cargo [+toolchain] [OPTIONS] [SUBCOMMAND]"
@@ -425,6 +398,8 @@ fn cli() -> App {
         // Doesn't mix well with our list of common cargo commands.  See clap-rs/clap#3108 for
         // opening clap up to allow us to style our help template
         .disable_colored_help(true)
+        // Provide a custom help subcommand for calling into man pages
+        .disable_help_subcommand(true)
         .override_usage(usage)
         .help_template(
             "\

diff --git a/src/bin/cargo/commands/help.rs b/src/bin/cargo/commands/help.rs
index 16b7e71e6bee83bf363141e4cfbc88e1d53f4ac1..fa0d98fd81ac79fdf8df193eb1c0397ec60a511e 100644 (file)
--- a/src/bin/cargo/commands/help.rs
+++ b/src/bin/cargo/commands/help.rs
@@ -1,4 +1,5 @@
 use crate::aliased_command;
+use crate::command_prelude::*;
 use cargo::util::errors::CargoResult;
 use cargo::{drop_println, Config};
 use cargo_util::paths::resolve_executable;
@@ -10,43 +11,26 @@ use std::path::Path;
 
 const COMPRESSED_MAN: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/man.tgz"));
 
-/// Checks if the `help` command is being issued.
-///
-/// This runs before clap processing, because it needs to intercept the `help`
-/// command if a man page is available.
-///
-/// Returns `true` if help information was successfully displayed to the user.
-/// In this case, Cargo should exit.
-pub fn handle_embedded_help(config: &Config) -> bool {
-    match try_help(config) {
-        Ok(true) => true,
-        Ok(false) => false,
-        Err(e) => {
-            log::warn!("help failed: {:?}", e);
-            false
-        }
-    }
+pub fn cli() -> App {
+    subcommand("help")
+        .about("Displays help for a cargo subcommand")
+        .arg(Arg::new("SUBCOMMAND"))
 }
 
-fn try_help(config: &Config) -> CargoResult<bool> {
-    let mut args = std::env::args_os()
-        .skip(1)
-        .skip_while(|arg| arg.to_str().map_or(false, |s| s.starts_with('-')));
-    if !args
-        .next()
-        .map_or(false, |arg| arg.to_str() == Some("help"))
-    {
-        return Ok(false);
+pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
+    let subcommand = args.get_one::<String>("SUBCOMMAND");
+    if let Some(subcommand) = subcommand {
+        if !try_help(config, subcommand)? {
+            crate::execute_external_subcommand(config, subcommand, &[subcommand, "--help"])?;
+        }
+    } else {
+        let mut cmd = crate::cli::cli();
+        let _ = cmd.print_help();
     }
-    let subcommand = match args.next() {
-        Some(arg) => arg,
-        None => return Ok(false),
-    };
-    let subcommand = match subcommand.to_str() {
-        Some(s) => s,
-        None => return Ok(false),
-    };
+    Ok(())
+}
 
+fn try_help(config: &Config, subcommand: &str) -> CargoResult<bool> {
     let subcommand = match check_alias(config, subcommand) {
         // If this alias is more than a simple subcommand pass-through, show the alias.
         Some(argv) if argv.len() > 1 => {

diff --git a/src/bin/cargo/commands/mod.rs b/src/bin/cargo/commands/mod.rs
index 92ee7d75e450a059d007d3112503ff18482fea02..0499f9a437f528cc7444b1938f7e59c5c627f066 100644 (file)
--- a/src/bin/cargo/commands/mod.rs
+++ b/src/bin/cargo/commands/mod.rs
@@ -13,6 +13,7 @@ pub fn builtin() -> Vec<App> {
         fix::cli(),
         generate_lockfile::cli(),
         git_checkout::cli(),
+        help::cli(),
         init::cli(),
         install::cli(),
         locate_project::cli(),
@@ -54,6 +55,7 @@ pub fn builtin_exec(cmd: &str) -> Option<fn(&mut Config, &ArgMatches) -> CliResu
         "fix" => fix::exec,
         "generate-lockfile" => generate_lockfile::exec,
         "git-checkout" => git_checkout::exec,
+        "help" => help::exec,
         "init" => init::exec,
         "install" => install::exec,
         "locate-project" => locate_project::exec,