refactor(cli): rewrite rustup (man|completions) with clap-derive

rami3l · rami3l · commit e250bd5f6348 · 2024-05-12T13:17:30.000Z
diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs
@@ -5,7 +5,7 @@ use std::str::FromStr;
 
 use anyhow::{anyhow, Error, Result};
 use clap::{
-    builder::{EnumValueParser, PossibleValue, PossibleValuesParser},
+    builder::{PossibleValue, PossibleValuesParser},
     Arg, ArgAction, ArgMatches, Args, Command, FromArgMatches as _, Parser, Subcommand, ValueEnum,
 };
 use clap_complete::Shell;
@@ -36,9 +36,8 @@ use crate::{
     toolchain::{
         distributable::DistributableToolchain,
         names::{
-            partial_toolchain_desc_parser, CustomToolchainName, LocalToolchainName,
-            MaybeResolvableToolchainName, ResolvableLocalToolchainName, ResolvableToolchainName,
-            ToolchainName,
+            CustomToolchainName, LocalToolchainName, MaybeResolvableToolchainName,
+            ResolvableLocalToolchainName, ResolvableToolchainName, ToolchainName,
         },
         toolchain::Toolchain,
     },
@@ -206,6 +205,24 @@ enum RustupSubcmd {
         #[command(flatten)]
         page: DocPage,
     },
+
+    /// View the man page for a given command
+    #[cfg(not(windows))]
+    Man {
+        command: String,
+
+        #[arg(long, help = OFFICIAL_TOOLCHAIN_ARG_HELP)]
+        toolchain: Option<PartialToolchainDesc>,
+    },
+
+    /// Generate tab-completion scripts for your shell
+    #[command(after_help = COMPLETIONS_HELP, arg_required_else_help = true)]
+    Completions {
+        shell: Shell,
+
+        #[arg(default_value = "rustup")]
+        command: CompletionCommand,
+    },
 }
 
 #[derive(Debug, Subcommand)]
@@ -504,6 +521,11 @@ impl Rustup {
                 topic,
                 page,
             } => doc(cfg, path, toolchain, topic.as_deref(), &page),
+            #[cfg(not(windows))]
+            RustupSubcmd::Man { command, toolchain } => man(cfg, &command, toolchain),
+            RustupSubcmd::Completions { shell, command } => {
+                output_completion_script(shell, command)
+            }
         }
     }
 }
@@ -582,11 +604,9 @@ pub fn main() -> Result<utils::ExitCode> {
             (
                 "dump-testament" | "show" | "update" | "install" | "uninstall" | "toolchain"
                 | "check" | "default" | "target" | "component" | "override" | "run" | "which"
-                | "doc",
+                | "doc" | "man" | "completions",
                 _,
             ) => Rustup::from_arg_matches(&matches)?.dispatch(cfg)?,
-            #[cfg(not(windows))]
-            ("man", m) => man(cfg, m)?,
             ("self", c) => match c.subcommand() {
                 Some(s) => match s {
                     ("update", _) => self_update::update(cfg)?,
@@ -604,18 +624,6 @@ pub fn main() -> Result<utils::ExitCode> {
                 },
                 None => unreachable!(),
             },
-            ("completions", c) => {
-                if let Some(&shell) = c.get_one::<Shell>("shell") {
-                    output_completion_script(
-                        shell,
-                        c.get_one::<CompletionCommand>("command")
-                            .copied()
-                            .unwrap_or(CompletionCommand::Rustup),
-                    )?
-                } else {
-                    unreachable!()
-                }
-            }
             _ => unreachable!(),
         },
         None => {
@@ -626,7 +634,7 @@ pub fn main() -> Result<utils::ExitCode> {
 }
 
 pub(crate) fn cli() -> Command {
-    let mut app = Command::new("rustup")
+    let app = Command::new("rustup")
         .version(common::version())
         .about("The Rust toolchain installer")
         .before_help(format!("rustup {}", common::version()))
@@ -654,24 +662,7 @@ pub(crate) fn cli() -> Command {
                         Err(Error::raw(ErrorKind::InvalidSubcommand, format!("\"{s}\" is not a valid subcommand, so it was interpreted as a toolchain name, but it is also invalid. {TOOLCHAIN_OVERRIDE_ERROR}")))
                     }
                 }),
-        );
-
-    if cfg!(not(target_os = "windows")) {
-        app = app.subcommand(
-            Command::new("man")
-                .about("View the man page for a given command")
-                .arg(Arg::new("command").required(true))
-                .arg(
-                    Arg::new("toolchain")
-                        .help(OFFICIAL_TOOLCHAIN_ARG_HELP)
-                        .long("toolchain")
-                        .num_args(1)
-                        .value_parser(partial_toolchain_desc_parser),
-                ),
-        );
-    }
-
-    app = app
+        )
         .subcommand(
             Command::new("self")
                 .about("Modify the rustup installation")
@@ -717,18 +708,6 @@ pub(crate) fn cli() -> Command {
                                 .default_value(SelfUpdateMode::default_mode()),
                         ),
                 ),
-        )
-        .subcommand(
-            Command::new("completions")
-                .about("Generate tab-completion scripts for your shell")
-                .after_help(COMPLETIONS_HELP)
-                .arg_required_else_help(true)
-                .arg(Arg::new("shell").value_parser(EnumValueParser::<Shell>::new()))
-                .arg(
-                    Arg::new("command")
-                        .value_parser(EnumValueParser::<CompletionCommand>::new())
-                        .default_missing_value("rustup"),
-                ),
         );
 
     RustupSubcmd::augment_subcommands(app)
@@ -1291,16 +1270,6 @@ fn component_remove(
     Ok(utils::ExitCode(0))
 }
 
-// Make *sure* only to use this for a subcommand whose "toolchain" argument
-// has .value_parser(partial_toolchain_desc_parser), or it will panic.
-// FIXME: Delete this.
-fn explicit_desc_or_dir_toolchain_old<'a>(cfg: &'a Cfg, m: &ArgMatches) -> Result<Toolchain<'a>> {
-    let toolchain = m
-        .get_one::<PartialToolchainDesc>("toolchain")
-        .map(Into::into);
-    explicit_or_dir_toolchain2(cfg, toolchain)
-}
-
 fn explicit_desc_or_dir_toolchain(
     cfg: &Cfg,
     toolchain: Option<PartialToolchainDesc>,
@@ -1546,12 +1515,14 @@ fn doc(
 }
 
 #[cfg(not(windows))]
-fn man(cfg: &Cfg, m: &ArgMatches) -> Result<utils::ExitCode> {
+fn man(
+    cfg: &Cfg,
+    command: &str,
+    toolchain: Option<PartialToolchainDesc>,
+) -> Result<utils::ExitCode> {
     use crate::currentprocess::varsource::VarSource;
 
-    let command = m.get_one::<String>("command").unwrap();
-
-    let toolchain = explicit_desc_or_dir_toolchain_old(cfg, m)?;
+    let toolchain = explicit_desc_or_dir_toolchain(cfg, toolchain)?;
     let mut path = toolchain.path().to_path_buf();
     path.push("share");
     path.push("man");
diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs
@@ -125,15 +125,6 @@ fn validate(candidate: &str) -> Result<&str, InvalidName> {
     }
 }
 
-/// Thunk to avoid errors like
-///  = note: `fn(&'2 str) -> Result<CustomToolchainName, <CustomToolchainName as TryFrom<&'2 str>>::Error> {<CustomToolchainName as TryFrom<&'2 str>>::try_from}` must implement `FnOnce<(&'1 str,)>`, for any lifetime `'1`...
-/// = note: ...but it actually implements `FnOnce<(&'2 str,)>`, for some specific lifetime `'2`
-pub(crate) fn partial_toolchain_desc_parser(
-    value: &str,
-) -> Result<PartialToolchainDesc, anyhow::Error> {
-    value.parse::<PartialToolchainDesc>()
-}
-
 /// A toolchain name from user input.
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
 pub(crate) enum ResolvableToolchainName {
diff --git a/tests/suite/cli-ui/rustup/rustup_completions_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_completions_cmd_help_flag_stdout.toml
@@ -1,14 +1,14 @@
 bin.name = "rustup"
-args = ["completions","--help"]
+args = ["completions", "--help"]
 stdout = """
 ...
 Generate tab-completion scripts for your shell
 
-Usage: rustup[EXE] completions [shell] [command]
+Usage: rustup[EXE] completions <SHELL> [COMMAND]
 
 Arguments:
-  [shell]    [possible values: bash, elvish, fish, powershell, zsh]
-  [command]  [possible values: rustup, cargo]
+  <SHELL>    [possible values: bash, elvish, fish, powershell, zsh]
+  [COMMAND]  [default: rustup] [possible values: rustup, cargo]
 
 Options:
   -h, --help  Print help
diff --git a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml
@@ -9,10 +9,8 @@ The Rust toolchain installer
 Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND]
 
 Commands:
-...
   self         Modify the rustup installation
   set          Alter rustup settings
-  completions  Generate tab-completion scripts for your shell
   show         Show the active and installed toolchains or profiles
   update       Update Rust toolchains and rustup
   check        Check for updates to Rust toolchains and rustup
@@ -24,6 +22,8 @@ Commands:
   run          Run a command with an environment configured for a given toolchain
   which        Display which binary will be run for a given command
   doc          Open the documentation for the current toolchain
+...
+  completions  Generate tab-completion scripts for your shell
   help         Print this message or the help of the given subcommand(s)
 
 Arguments:
diff --git a/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml
@@ -9,10 +9,8 @@ The Rust toolchain installer
 Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND]
 
 Commands:
-...
   self         Modify the rustup installation
   set          Alter rustup settings
-  completions  Generate tab-completion scripts for your shell
   show         Show the active and installed toolchains or profiles
   update       Update Rust toolchains and rustup
   check        Check for updates to Rust toolchains and rustup
@@ -24,6 +22,8 @@ Commands:
   run          Run a command with an environment configured for a given toolchain
   which        Display which binary will be run for a given command
   doc          Open the documentation for the current toolchain
+...
+  completions  Generate tab-completion scripts for your shell
   help         Print this message or the help of the given subcommand(s)
 
 Arguments:
diff --git a/tests/suite/cli-ui/rustup/rustup_man_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_man_cmd_help_flag_stdout.toml
@@ -1,16 +1,16 @@
 bin.name = "rustup"
-args = ["man","--help"]
+args = ["man", "--help"]
 stdout = """
 ...
 View the man page for a given command
 
-Usage: rustup[EXE] man [OPTIONS] <command>
+Usage: rustup[EXE] man [OPTIONS] <COMMAND>
 
 Arguments:
-  <command>  
+  <COMMAND>  
 
 Options:
-      --toolchain <toolchain>  Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more
+      --toolchain <TOOLCHAIN>  Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more
                                information see `rustup help toolchain`
   -h, --help                   Print help
 """
diff --git a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml
@@ -9,10 +9,8 @@ The Rust toolchain installer
 Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND]
 
 Commands:
-...
   self         Modify the rustup installation
   set          Alter rustup settings
-  completions  Generate tab-completion scripts for your shell
   show         Show the active and installed toolchains or profiles
   update       Update Rust toolchains and rustup
   check        Check for updates to Rust toolchains and rustup
@@ -24,6 +22,8 @@ Commands:
   run          Run a command with an environment configured for a given toolchain
   which        Display which binary will be run for a given command
   doc          Open the documentation for the current toolchain
+...
+  completions  Generate tab-completion scripts for your shell
   help         Print this message or the help of the given subcommand(s)
 
 Arguments:
diff --git a/tests/suite/cli_misc.rs b/tests/suite/cli_misc.rs
@@ -863,11 +863,11 @@ fn completion_bad_shell() {
     setup(&|config| {
         config.expect_err(
             &["rustup", "completions", "fake"],
-            r#"error: invalid value 'fake' for '[shell]'"#,
+            r#"error: invalid value 'fake' for '<SHELL>'"#,
         );
         config.expect_err(
             &["rustup", "completions", "fake", "cargo"],
-            r#"error: invalid value 'fake' for '[shell]'"#,
+            r#"error: invalid value 'fake' for '<SHELL>'"#,
         );
     });
 }
@@ -877,7 +877,7 @@ fn completion_bad_tool() {
     setup(&|config| {
         config.expect_err(
             &["rustup", "completions", "bash", "fake"],
-            r#"error: invalid value 'fake' for '[command]'"#,
+            r#"error: invalid value 'fake' for '[COMMAND]'"#,
         );
     });
 }
