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.

Author: epage

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"

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(
             "\

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 => {

src/bin/cargo/commands/mod.rs

@@ -11,6 +11,7 @@ pub fn builtin() -> Vec<App> {
         doc::cli(),
         fetch::cli(),
         fix::cli(),
+        help::cli(),
         generate_lockfile::cli(),
         git_checkout::cli(),
         init::cli(),
@@ -52,6 +53,7 @@ pub fn builtin_exec(cmd: &str) -> Option<fn(&mut Config, &ArgMatches) -> CliResu
         "doc" => doc::exec,
         "fetch" => fetch::exec,
         "fix" => fix::exec,
+        "help" => help::exec,
         "generate-lockfile" => generate_lockfile::exec,
         "git-checkout" => git_checkout::exec,
         "init" => init::exec,