From 4ad0d4a9e05a56a8d121afc363dbe062b8400894 Mon Sep 17 00:00:00 2001 From: rami3l Date: Sat, 20 Jan 2024 14:28:17 +0800 Subject: [PATCH 01/13] refactor(cli): rewrite `rustup show` with `clap_derive` --- src/cli/rustup_mode.rs | 202 +++++++++++++++-------------------------- 1 file changed, 75 insertions(+), 127 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 6e9a820c66..b8e9d899c3 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -6,7 +6,8 @@ use std::str::FromStr; use anyhow::{anyhow, Error, Result}; use clap::{ builder::{EnumValueParser, PossibleValue, PossibleValuesParser}, - Arg, ArgAction, ArgGroup, ArgMatches, Command, ValueEnum, + Arg, ArgAction, ArgGroup, ArgMatches, Command, FromArgMatches as _, Parser, Subcommand, + ValueEnum, }; use clap_complete::Shell; use itertools::Itertools; @@ -86,6 +87,63 @@ where callee(cfg, matches) } +#[derive(Debug, Parser)] +#[command( + name = "rustup", + bin_name = "rustup[EXE]", + version = common::version(), +)] +struct Rustup { + #[command(subcommand)] + subcmd: RustupSubcmd, +} + +#[derive(Debug, Subcommand)] +enum RustupSubcmd { + /// Show the active and installed toolchains or profiles + #[command(after_help = SHOW_HELP)] + Show { + /// Enable verbose output with rustc information for all installed toolchains + #[arg(short, long)] + verbose: bool, + + #[command(subcommand)] + subcmd: Option, + }, +} + +#[derive(Debug, Subcommand)] +enum ShowSubcmd { + /// Show the active toolchain + #[command(after_help = SHOW_ACTIVE_TOOLCHAIN_HELP)] + ActiveToolchain { + /// Enable verbose output with rustc information + #[arg(short, long)] + verbose: bool, + }, + + /// Display the computed value of RUSTUP_HOME + Home, + + /// Show the default profile used for the `rustup install` command + Profile, +} + +impl Rustup { + fn dispatch(self, cfg: &mut Cfg) -> Result { + match self.subcmd { + RustupSubcmd::Show { verbose, subcmd } => match subcmd { + None => handle_epipe(show(cfg, verbose)), + Some(ShowSubcmd::ActiveToolchain { verbose }) => { + handle_epipe(show_active_toolchain(cfg, verbose)) + } + Some(ShowSubcmd::Home) => handle_epipe(show_rustup_home(cfg)), + Some(ShowSubcmd::Profile) => handle_epipe(show_profile(cfg)), + }, + } + } +} + #[cfg_attr(feature = "otel", tracing::instrument(fields(args = format!("{:?}", process().args_os().collect::>()))))] pub fn main() -> Result { self_update::cleanup_self_updater()?; @@ -158,15 +216,7 @@ pub fn main() -> Result { Ok(match matches.subcommand() { Some(s) => match s { ("dump-testament", _) => common::dump_testament()?, - ("show", c) => match c.subcommand() { - Some(s) => match s { - ("active-toolchain", m) => handle_epipe(show_active_toolchain(cfg, m))?, - ("home", _) => handle_epipe(show_rustup_home(cfg))?, - ("profile", _) => handle_epipe(show_profile(cfg))?, - _ => handle_epipe(show(cfg, c))?, - }, - None => handle_epipe(show(cfg, c))?, - }, + ("show", _) => Rustup::from_arg_matches(&matches)?.dispatch(cfg)?, ("install", m) => deprecated("toolchain install", cfg, m, update)?, ("update", m) => update(cfg, m)?, ("check", _) => check_updates(cfg)?, @@ -287,106 +337,6 @@ pub(crate) fn cli() -> Command { .about("Dump information about the build") .hide(true), // Not for users, only CI ) - .subcommand( - Command::new("show") - .about("Show the active and installed toolchains or profiles") - .after_help(SHOW_HELP) - .arg( - verbose_arg("Enable verbose output with rustc information for all installed toolchains"), - ) - .subcommand( - Command::new("active-toolchain") - .about("Show the active toolchain") - .after_help(SHOW_ACTIVE_TOOLCHAIN_HELP) - .arg( - verbose_arg("Enable verbose output with rustc information"), - ), - ) - .subcommand( - Command::new("home") - .about("Display the computed value of RUSTUP_HOME"), - ) - .subcommand(Command::new("profile").about("Show the default profile used for the `rustup install` command")) - ) - .subcommand( - Command::new("install") - .about("Update Rust toolchains") - .after_help(INSTALL_HELP) - .hide(true) // synonym for 'toolchain install' - .arg( - Arg::new("toolchain") - .help(OFFICIAL_TOOLCHAIN_ARG_HELP) - .required(true) - .value_parser(partial_toolchain_desc_parser) - .num_args(1..) - ) - .arg( - Arg::new("profile") - .long("profile") - .value_parser(PossibleValuesParser::new(Profile::names())) - .num_args(1), - ) - .arg( - Arg::new("no-self-update") - .help("Don't perform self-update when running the `rustup install` command") - .long("no-self-update") - .action(ArgAction::SetTrue) - ) - .arg( - Arg::new("force") - .help("Force an update, even if some components are missing") - .long("force") - .action(ArgAction::SetTrue) - ).arg( - Arg::new("force-non-host") - .help("Install toolchains that require an emulator. See https://github.com/rust-lang/rustup/wiki/Non-host-toolchains") - .long("force-non-host") - .action(ArgAction::SetTrue) - ), - ) - .subcommand( - Command::new("uninstall") - .about("Uninstall Rust toolchains") - .hide(true) // synonym for 'toolchain uninstall' - .arg( - Arg::new("toolchain") - .help(RESOLVABLE_TOOLCHAIN_ARG_HELP) - .required(true) - .value_parser(resolvable_toolchainame_parser) - .num_args(1..) - ), - ) - .subcommand( - Command::new("update") - .about("Update Rust toolchains and rustup") - .aliases(["upgrade", "up"]) - .after_help(UPDATE_HELP) - .arg( - Arg::new("toolchain") - .help(OFFICIAL_TOOLCHAIN_ARG_HELP) - .required(false) - .value_parser(partial_toolchain_desc_parser) - .num_args(1..) - ) - .arg( - Arg::new("no-self-update") - .help("Don't perform self update when running the `rustup update` command") - .long("no-self-update") - .action(ArgAction::SetTrue) - ) - .arg( - Arg::new("force") - .help("Force an update, even if some components are missing") - .long("force") - .action(ArgAction::SetTrue) - ) - .arg( - Arg::new("force-non-host") - .help("Install toolchains that require an emulator. See https://github.com/rust-lang/rustup/wiki/Non-host-toolchains") - .long("force-non-host") - .action(ArgAction::SetTrue) - ), - ) .subcommand(Command::new("check").about("Check for updates to Rust toolchains and rustup")) .subcommand( Command::new("default") @@ -804,20 +754,21 @@ 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::::new())) + .arg( + Arg::new("command") + .value_parser(EnumValueParser::::new()) + .default_missing_value("rustup"), + ), ); - app.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::::new())) - .arg( - Arg::new("command") - .value_parser(EnumValueParser::::new()) - .default_missing_value("rustup"), - ), - ) + RustupSubcmd::augment_subcommands(app) } fn verbose_arg(help: &'static str) -> Arg { @@ -1061,11 +1012,9 @@ fn which(cfg: &Cfg, m: &ArgMatches) -> Result { } #[cfg_attr(feature = "otel", tracing::instrument(skip_all))] -fn show(cfg: &Cfg, m: &ArgMatches) -> Result { +fn show(cfg: &Cfg, verbose: bool) -> Result { common::warn_if_host_is_emulated(); - let verbose = m.get_flag("verbose"); - // Print host triple { let mut t = process().stdout().terminal(); @@ -1201,8 +1150,7 @@ fn show(cfg: &Cfg, m: &ArgMatches) -> Result { } #[cfg_attr(feature = "otel", tracing::instrument(skip_all))] -fn show_active_toolchain(cfg: &Cfg, m: &ArgMatches) -> Result { - let verbose = m.get_flag("verbose"); +fn show_active_toolchain(cfg: &Cfg, verbose: bool) -> Result { let cwd = utils::current_dir()?; match cfg.find_active_toolchain(&cwd)? { Some((toolchain_name, reason)) => { From ac3ce471a7f2a3758e78ae4709f6d532f33078b3 Mon Sep 17 00:00:00 2001 From: rami3l Date: Fri, 10 May 2024 19:19:05 +0800 Subject: [PATCH 02/13] refactor(cli): remove `deprecated()` --- src/cli/rustup_mode.rs | 25 +++---------------------- src/notifications.rs | 3 --- tests/suite/cli_misc.rs | 26 -------------------------- 3 files changed, 3 insertions(+), 51 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index b8e9d899c3..53df09f312 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -46,7 +46,7 @@ use crate::{ toolchain::Toolchain, }, utils::utils, - Cfg, Notification, + Cfg, }; const TOOLCHAIN_OVERRIDE_ERROR: &str = @@ -68,25 +68,6 @@ fn handle_epipe(res: Result) -> Result { } } -fn deprecated(instead: &str, cfg: &mut Cfg, matches: B, callee: F) -> R -where - F: FnOnce(&mut Cfg, B) -> R, -{ - (cfg.notify_handler)(Notification::PlainVerboseMessage( - "Use of (currently) unmaintained command line interface.", - )); - (cfg.notify_handler)(Notification::PlainVerboseMessage( - "The exact API of this command may change without warning", - )); - (cfg.notify_handler)(Notification::PlainVerboseMessage( - "Eventually this command will be a true alias. Until then:", - )); - (cfg.notify_handler)(Notification::PlainVerboseMessage(&format!( - " Please use `rustup {instead}` instead" - ))); - callee(cfg, matches) -} - #[derive(Debug, Parser)] #[command( name = "rustup", @@ -217,10 +198,10 @@ pub fn main() -> Result { Some(s) => match s { ("dump-testament", _) => common::dump_testament()?, ("show", _) => Rustup::from_arg_matches(&matches)?.dispatch(cfg)?, - ("install", m) => deprecated("toolchain install", cfg, m, update)?, + ("install", m) => update(cfg, m)?, ("update", m) => update(cfg, m)?, ("check", _) => check_updates(cfg)?, - ("uninstall", m) => deprecated("toolchain uninstall", cfg, m, toolchain_remove)?, + ("uninstall", m) => toolchain_remove(cfg, m)?, ("default", m) => default_(cfg, m)?, ("toolchain", c) => match c.subcommand() { Some(s) => match s { diff --git a/src/notifications.rs b/src/notifications.rs index 7de6d56b1f..a43561064b 100644 --- a/src/notifications.rs +++ b/src/notifications.rs @@ -31,7 +31,6 @@ pub(crate) enum Notification<'a> { ReadMetadataVersion(&'a str), NonFatalError(&'a anyhow::Error), UpgradeRemovesToolchains, - PlainVerboseMessage(&'a str), /// Both `rust-toolchain` and `rust-toolchain.toml` exist within a directory DuplicateToolchainFile { rust_toolchain: &'a Path, @@ -68,7 +67,6 @@ impl<'a> Notification<'a> { | UpdatingToolchain(_) | ReadMetadataVersion(_) | InstalledToolchain(_) - | PlainVerboseMessage(_) | UpdateHashMatches => NotificationLevel::Verbose, SetDefaultToolchain(_) | SetOverrideToolchain(_, _) @@ -124,7 +122,6 @@ impl<'a> Display for Notification<'a> { f, "this upgrade will remove all existing toolchains. you will need to reinstall them" ), - PlainVerboseMessage(r) => write!(f, "{r}"), DuplicateToolchainFile { rust_toolchain, rust_toolchain_toml, diff --git a/tests/suite/cli_misc.rs b/tests/suite/cli_misc.rs index 63e1720e97..efc9efe224 100644 --- a/tests/suite/cli_misc.rs +++ b/tests/suite/cli_misc.rs @@ -1021,29 +1021,3 @@ fn toolchain_link_then_list_verbose() { config.expect_stdout_ok(&["rustup", "toolchain", "list", "-v"], "/custom-1"); }); } - -#[test] -fn deprecated_interfaces() { - setup(&|config| { - // In verbose mode we want the deprecated interfaces to complain - config.expect_ok_contains( - &["rustup", "--verbose", "install", "nightly"], - "", - "Please use `rustup toolchain install` instead", - ); - config.expect_ok_contains( - &["rustup", "--verbose", "uninstall", "nightly"], - "", - "Please use `rustup toolchain uninstall` instead", - ); - // But if not verbose then they should *NOT* complain - config.expect_not_stderr_ok( - &["rustup", "install", "nightly"], - "Please use `rustup toolchain install` instead", - ); - config.expect_not_stderr_ok( - &["rustup", "uninstall", "nightly"], - "Please use `rustup toolchain uninstall` instead", - ); - }) -} From 085b7a943963ef125842151c65a944787c0b8fd3 Mon Sep 17 00:00:00 2001 From: rami3l Date: Sat, 20 Jan 2024 16:39:08 +0800 Subject: [PATCH 03/13] refactor(cli): rewrite `rustup (toolchain|update|(un)?install)` with `clap-derive` --- src/cli/rustup_mode.rs | 397 +++++++++--------- src/toolchain/names.rs | 9 - ...omponent_cmd_add_cmd_help_flag_stdout.toml | 2 +- ...onent_cmd_remove_cmd_help_flag_stdout.toml | 2 +- .../cli-ui/rustup/rustup_help_cmd_stdout.toml | 6 +- .../rustup/rustup_help_flag_stdout.toml | 6 +- .../rustup/rustup_only_options_stdout.toml | 8 +- ...hain_cmd_install_cmd_help_flag_stdout.toml | 28 +- ...olchain_cmd_link_cmd_help_flag_stdout.toml | 8 +- ...in_cmd_uninstall_cmd_help_flag_stdout.toml | 4 +- .../rustup_up_cmd_help_flag_stdout.toml | 4 +- .../rustup_update_cmd_help_flag_stdout.toml | 4 +- .../rustup_upgrade_cmd_help_flag_stdout.toml | 4 +- tests/suite/cli_self_upd.rs | 2 +- tests/suite/cli_v2.rs | 2 +- 15 files changed, 245 insertions(+), 241 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 53df09f312..08e36ba525 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -6,7 +6,7 @@ use std::str::FromStr; use anyhow::{anyhow, Error, Result}; use clap::{ builder::{EnumValueParser, PossibleValue, PossibleValuesParser}, - Arg, ArgAction, ArgGroup, ArgMatches, Command, FromArgMatches as _, Parser, Subcommand, + Arg, ArgAction, ArgGroup, ArgMatches, Args, Command, FromArgMatches as _, Parser, Subcommand, ValueEnum, }; use clap_complete::Shell; @@ -37,11 +37,10 @@ use crate::{ toolchain::{ distributable::DistributableToolchain, names::{ - custom_toolchain_name_parser, maybe_resolvable_toolchainame_parser, - partial_toolchain_desc_parser, resolvable_local_toolchainame_parser, - resolvable_toolchainame_parser, CustomToolchainName, LocalToolchainName, - MaybeResolvableToolchainName, ResolvableLocalToolchainName, ResolvableToolchainName, - ToolchainName, + maybe_resolvable_toolchainame_parser, partial_toolchain_desc_parser, + resolvable_local_toolchainame_parser, resolvable_toolchainame_parser, + CustomToolchainName, LocalToolchainName, MaybeResolvableToolchainName, + ResolvableLocalToolchainName, ResolvableToolchainName, ToolchainName, }, toolchain::Toolchain, }, @@ -80,7 +79,22 @@ struct Rustup { } #[derive(Debug, Subcommand)] +#[command(name = "rustup", bin_name = "rustup[EXE]")] enum RustupSubcmd { + /// Update Rust toolchains + #[command(hide = true, after_help = INSTALL_HELP)] + Install { + #[command(flatten)] + opts: UpdateOpts, + }, + + /// Uninstall Rust toolchains + #[command(hide = true)] + Uninstall { + #[command(flatten)] + opts: UninstallOpts, + }, + /// Show the active and installed toolchains or profiles #[command(after_help = SHOW_HELP)] Show { @@ -91,6 +105,35 @@ enum RustupSubcmd { #[command(subcommand)] subcmd: Option, }, + + /// Update Rust toolchains and rustup + #[command( + after_help = UPDATE_HELP, + aliases = ["upgrade", "up"], + )] + Update { + /// Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see `rustup help toolchain` + #[arg(num_args = 1..)] + toolchain: Vec, + + /// Don't perform self update when running the `rustup update` command + #[arg(long)] + no_self_update: bool, + + /// Force an update, even if some components are missing + #[arg(long)] + force: bool, + + /// Install toolchains that require an emulator. See https://github.com/rust-lang/rustup/wiki/Non-host-toolchains + #[arg(long)] + force_non_host: bool, + }, + + /// Modify or query the installed toolchains + Toolchain { + #[command(subcommand)] + subcmd: ToolchainSubcmd, + }, } #[derive(Debug, Subcommand)] @@ -110,9 +153,97 @@ enum ShowSubcmd { Profile, } +#[derive(Debug, Subcommand)] +#[command( + arg_required_else_help = true, + subcommand_required = true, + after_help = TOOLCHAIN_HELP, +)] +enum ToolchainSubcmd { + /// List installed toolchains + List { + /// Enable verbose output with toolchain information + #[arg(short, long)] + verbose: bool, + }, + + /// Install or update a given toolchain + #[command(aliases = ["update", "add"] )] + Install { + #[command(flatten)] + opts: UpdateOpts, + }, + + /// Uninstall a toolchain + #[command(alias = "remove")] + Uninstall { + #[command(flatten)] + opts: UninstallOpts, + }, + + /// Create a custom toolchain by symlinking to a directory + #[command(after_help = TOOLCHAIN_LINK_HELP)] + Link { + /// Custom toolchain name + toolchain: CustomToolchainName, + + /// Path to the directory + path: PathBuf, + }, +} + +#[derive(Debug, Default, Args)] +struct UpdateOpts { + #[arg( + required = true, + help = OFFICIAL_TOOLCHAIN_ARG_HELP, + num_args = 1.., + )] + toolchain: Vec, + + #[arg(long, value_parser = PossibleValuesParser::new(Profile::names()))] + profile: Option, + + /// Add specific components on installation + #[arg(short, long, value_delimiter = ',', num_args = 1..)] + component: Vec, + + /// Add specific targets on installation + #[arg(short, long, value_delimiter = ',', num_args = 1..)] + target: Vec, + + /// Don't perform self update when running the `rustup toolchain install` command + #[arg(long)] + no_self_update: bool, + + /// Force an update, even if some components are missing + #[arg(long)] + force: bool, + + /// Allow rustup to downgrade the toolchain to satisfy your component choice + #[arg(long)] + allow_downgrade: bool, + + /// Install toolchains that require an emulator. See https://github.com/rust-lang/rustup/wiki/Non-host-toolchains + #[arg(long)] + force_non_host: bool, +} + +#[derive(Debug, Default, Args)] +struct UninstallOpts { + #[arg( + help = RESOLVABLE_TOOLCHAIN_ARG_HELP, + required = true, + num_args = 1.., + )] + toolchain: Vec, +} + impl Rustup { fn dispatch(self, cfg: &mut Cfg) -> Result { match self.subcmd { + RustupSubcmd::Install { opts } => update(cfg, opts), + RustupSubcmd::Uninstall { opts } => toolchain_remove(cfg, opts), RustupSubcmd::Show { verbose, subcmd } => match subcmd { None => handle_epipe(show(cfg, verbose)), Some(ShowSubcmd::ActiveToolchain { verbose }) => { @@ -121,6 +252,27 @@ impl Rustup { Some(ShowSubcmd::Home) => handle_epipe(show_rustup_home(cfg)), Some(ShowSubcmd::Profile) => handle_epipe(show_profile(cfg)), }, + RustupSubcmd::Update { + toolchain, + no_self_update, + force, + force_non_host, + } => update( + cfg, + UpdateOpts { + toolchain, + no_self_update, + force, + force_non_host, + ..UpdateOpts::default() + }, + ), + RustupSubcmd::Toolchain { subcmd } => match subcmd { + ToolchainSubcmd::Install { opts } => update(cfg, opts), + ToolchainSubcmd::List { verbose } => handle_epipe(toolchain_list(cfg, verbose)), + ToolchainSubcmd::Link { toolchain, path } => toolchain_link(cfg, &toolchain, &path), + ToolchainSubcmd::Uninstall { opts } => toolchain_remove(cfg, opts), + }, } } } @@ -197,22 +349,11 @@ pub fn main() -> Result { Ok(match matches.subcommand() { Some(s) => match s { ("dump-testament", _) => common::dump_testament()?, - ("show", _) => Rustup::from_arg_matches(&matches)?.dispatch(cfg)?, - ("install", m) => update(cfg, m)?, - ("update", m) => update(cfg, m)?, + ("show" | "update" | "install" | "uninstall" | "toolchain", _) => { + Rustup::from_arg_matches(&matches)?.dispatch(cfg)? + } ("check", _) => check_updates(cfg)?, - ("uninstall", m) => toolchain_remove(cfg, m)?, ("default", m) => default_(cfg, m)?, - ("toolchain", c) => match c.subcommand() { - Some(s) => match s { - ("install", m) => update(cfg, m)?, - ("list", m) => handle_epipe(toolchain_list(cfg, m))?, - ("link", m) => toolchain_link(cfg, m)?, - ("uninstall", m) => toolchain_remove(cfg, m)?, - _ => unreachable!(), - }, - None => unreachable!(), - }, ("target", c) => match c.subcommand() { Some(s) => match s { ("list", m) => handle_epipe(target_list(cfg, m))?, @@ -327,112 +468,7 @@ pub(crate) fn cli() -> Command { Arg::new("toolchain") .help(MAYBE_RESOLVABLE_TOOLCHAIN_ARG_HELP) .required(false) - .value_parser(maybe_resolvable_toolchainame_parser) - ), - ) - .subcommand( - Command::new("toolchain") - .about("Modify or query the installed toolchains") - .after_help(TOOLCHAIN_HELP) - .subcommand_required(true) - .arg_required_else_help(true) - .subcommand( - Command::new("list") - .about("List installed toolchains") - .arg( - verbose_arg("Enable verbose output with toolchain information"), - ), - ) - .subcommand( - Command::new("install") - .about("Install or update a given toolchain") - .aliases(["update", "add"]) - .arg( - Arg::new("toolchain") - .help(OFFICIAL_TOOLCHAIN_ARG_HELP) - .required(true) - .value_parser( partial_toolchain_desc_parser) - .num_args(1..) - ) - .arg( - Arg::new("profile") - .long("profile") - .value_parser(PossibleValuesParser::new(Profile::names())) - .num_args(1), - ) - .arg( - Arg::new("components") - .help("Add specific components on installation") - .long("component") - .short('c') - .num_args(1..) - .use_value_delimiter(true) - .action(ArgAction::Append), - ) - .arg( - Arg::new("targets") - .help("Add specific targets on installation") - .long("target") - .short('t') - .num_args(1..) - .use_value_delimiter(true) - .action(ArgAction::Append), - ) - .arg( - Arg::new("no-self-update") - .help( - "Don't perform self update when running the\ - `rustup toolchain install` command", - ) - .long("no-self-update") - .action(ArgAction::SetTrue) - ) - .arg( - Arg::new("force") - .help("Force an update, even if some components are missing") - .long("force") - .action(ArgAction::SetTrue) - ) - .arg( - Arg::new("allow-downgrade") - .help("Allow rustup to downgrade the toolchain to satisfy your component choice") - .long("allow-downgrade") - .action(ArgAction::SetTrue) - ) - .arg( - Arg::new("force-non-host") - .help("Install toolchains that require an emulator. See https://github.com/rust-lang/rustup/wiki/Non-host-toolchains") - .long("force-non-host") - .action(ArgAction::SetTrue) - ), - ) - .subcommand( - Command::new("uninstall") - .about("Uninstall a toolchain") - .alias("remove") - .arg( - Arg::new("toolchain") - .help(RESOLVABLE_TOOLCHAIN_ARG_HELP) - .required(true) - .value_parser(resolvable_toolchainame_parser) - .num_args(1..) - ), - ) - .subcommand( - Command::new("link") - .about("Create a custom toolchain by symlinking to a directory") - .after_help(TOOLCHAIN_LINK_HELP) - .arg( - Arg::new("toolchain") - .help("Custom toolchain name") - .required(true) - .value_parser(custom_toolchain_name_parser), - ) - .arg( - Arg::new("path") - .help("Path to the directory") - .required(true), - ), + .value_parser(maybe_resolvable_toolchainame_parser), ), ) .subcommand( @@ -454,22 +490,17 @@ pub(crate) fn cli() -> Command { Arg::new("installed") .long("installed") .help("List only installed targets") - .action(ArgAction::SetTrue) + .action(ArgAction::SetTrue), ), ) .subcommand( Command::new("add") .about("Add a target to a Rust toolchain") .alias("install") - .arg( - Arg::new("target") - .required(true) - .num_args(1..) - .help( - "List of targets to install; \ - \"all\" installs all available targets" - ) - ) + .arg(Arg::new("target").required(true).num_args(1..).help( + "List of targets to install; \ + \"all\" installs all available targets", + )) .arg( Arg::new("toolchain") .help(OFFICIAL_TOOLCHAIN_ARG_HELP) @@ -484,9 +515,9 @@ pub(crate) fn cli() -> Command { .alias("uninstall") .arg( Arg::new("target") - .help("List of targets to uninstall") - .required(true) - .num_args(1..) + .help("List of targets to uninstall") + .required(true) + .num_args(1..), ) .arg( Arg::new("toolchain") @@ -516,14 +547,13 @@ pub(crate) fn cli() -> Command { Arg::new("installed") .long("installed") .help("List only installed components") - .action(ArgAction::SetTrue) + .action(ArgAction::SetTrue), ), ) .subcommand( Command::new("add") .about("Add a component to a Rust toolchain") - .arg(Arg::new("component").required(true) - .num_args(1..)) + .arg(Arg::new("component").required(true).num_args(1..)) .arg( Arg::new("toolchain") .help(OFFICIAL_TOOLCHAIN_ARG_HELP) @@ -531,17 +561,12 @@ pub(crate) fn cli() -> Command { .num_args(1) .value_parser(partial_toolchain_desc_parser), ) - .arg( - Arg::new("target") - .long("target") - .num_args(1) - ), + .arg(Arg::new("target").long("target").num_args(1)), ) .subcommand( Command::new("remove") .about("Remove a component from a Rust toolchain") - .arg(Arg::new("component").required(true) - .num_args(1..)) + .arg(Arg::new("component").required(true).num_args(1..)) .arg( Arg::new("toolchain") .help(OFFICIAL_TOOLCHAIN_ARG_HELP) @@ -549,11 +574,7 @@ pub(crate) fn cli() -> Command { .num_args(1) .value_parser(partial_toolchain_desc_parser), ) - .arg( - Arg::new("target") - .long("target") - .num_args(1) - ), + .arg(Arg::new("target").long("target").num_args(1)), ), ) .subcommand( @@ -562,9 +583,7 @@ pub(crate) fn cli() -> Command { .after_help(OVERRIDE_HELP) .subcommand_required(true) .arg_required_else_help(true) - .subcommand( - Command::new("list").about("List directory toolchain overrides"), - ) + .subcommand(Command::new("list").about("List directory toolchain overrides")) .subcommand( Command::new("set") .about("Set the override toolchain for a directory") @@ -669,7 +688,12 @@ pub(crate) fn cli() -> Command { .args( &DOCS_DATA .iter() - .map(|&(name, help_msg, _)| Arg::new(name).long(name).help(help_msg).action(ArgAction::SetTrue)) + .map(|&(name, help_msg, _)| { + Arg::new(name) + .long(name) + .help(help_msg) + .action(ArgAction::SetTrue) + }) .collect::>(), ), ); @@ -859,7 +883,7 @@ fn check_updates(cfg: &Cfg) -> Result { Ok(utils::ExitCode(0)) } -fn update(cfg: &mut Cfg, m: &ArgMatches) -> Result { +fn update(cfg: &mut Cfg, opts: UpdateOpts) -> Result { common::warn_if_host_is_emulated(); let self_update_mode = cfg.get_self_update_mode()?; // Priority: no-self-update feature > self_update_mode > no-self-update args. @@ -868,9 +892,9 @@ fn update(cfg: &mut Cfg, m: &ArgMatches) -> Result { // and has **no** no-self-update parameter. let self_update = !self_update::NEVER_SELF_UPDATE && self_update_mode == SelfUpdateMode::Enable - && !m.get_flag("no-self-update"); - let forced = m.get_flag("force-non-host"); - if let Ok(Some(p)) = m.try_get_one::("profile") { + && !opts.no_self_update; + let forced = opts.force_non_host; + if let Some(p) = &opts.profile { let p = Profile::from_str(p)?; cfg.set_profile_override(p); } @@ -878,8 +902,9 @@ fn update(cfg: &mut Cfg, m: &ArgMatches) -> Result { if cfg.get_profile()? == Profile::Complete { warn!("{}", common::WARN_COMPLETE_PROFILE); } - if let Ok(Some(names)) = m.try_get_many::("toolchain") { - for name in names.map(|n| n.to_owned()) { + let names = opts.toolchain; + if !names.is_empty() { + for name in names { // This needs another pass to fix it all up if name.has_triple() { let host_arch = TargetTriple::from_host_or_build(); @@ -896,20 +921,11 @@ fn update(cfg: &mut Cfg, m: &ArgMatches) -> Result { } let desc = name.resolve(&cfg.get_default_host_triple()?)?; - let components: Vec<_> = m - .try_get_many::("components") - .ok() - .flatten() - .map_or_else(Vec::new, |v| v.map(|s| &**s).collect()); - let targets: Vec<_> = m - .try_get_many::("targets") - .ok() - .flatten() - .map_or_else(Vec::new, |v| v.map(|s| &**s).collect()); - - let force = m.get_flag("force"); - let allow_downgrade = - matches!(m.try_get_one::("allow-downgrade"), Ok(Some(true))); + let components = opts.component.iter().map(|s| &**s).collect::>(); + let targets = opts.target.iter().map(|s| &**s).collect::>(); + + let force = opts.force; + let allow_downgrade = opts.allow_downgrade; let profile = cfg.get_profile()?; let status = match crate::toolchain::distributable::DistributableToolchain::new( cfg, @@ -946,7 +962,7 @@ fn update(cfg: &mut Cfg, m: &ArgMatches) -> Result { common::self_update(|| Ok(utils::ExitCode(0)))?; } } else { - common::update_all_channels(cfg, self_update, m.get_flag("force"))?; + common::update_all_channels(cfg, self_update, opts.force)?; info!("cleaning up downloads & tmp directories"); utils::delete_dir_contents_following_links(&cfg.download_dir); cfg.tmp_cx.clean(); @@ -1318,25 +1334,22 @@ fn explicit_or_dir_toolchain2( } } -fn toolchain_list(cfg: &Cfg, m: &ArgMatches) -> Result { - common::list_toolchains(cfg, m.get_flag("verbose")) +fn toolchain_list(cfg: &Cfg, verbose: bool) -> Result { + common::list_toolchains(cfg, verbose) } -fn toolchain_link(cfg: &Cfg, m: &ArgMatches) -> Result { - let toolchain = m.get_one::("toolchain").unwrap(); - let path = m.get_one::("path").unwrap(); +fn toolchain_link( + cfg: &Cfg, + toolchain: &CustomToolchainName, + path: &Path, +) -> Result { cfg.ensure_toolchains_dir()?; - crate::toolchain::custom::CustomToolchain::install_from_dir( - cfg, - Path::new(path), - toolchain, - true, - )?; + crate::toolchain::custom::CustomToolchain::install_from_dir(cfg, path, toolchain, true)?; Ok(utils::ExitCode(0)) } -fn toolchain_remove(cfg: &mut Cfg, m: &ArgMatches) -> Result { - for toolchain_name in m.get_many::("toolchain").unwrap() { +fn toolchain_remove(cfg: &mut Cfg, opts: UninstallOpts) -> Result { + for toolchain_name in &opts.toolchain { let toolchain_name = toolchain_name.resolve(&cfg.get_default_host_triple()?)?; Toolchain::ensure_removed(cfg, (&toolchain_name).into())?; } diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index f472ac1db8..431d3fb8a6 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -458,15 +458,6 @@ impl Display for CustomToolchainName { } } -/// Thunk to avoid -/// = note: `fn(&'2 str) -> Result>::Error> {>::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 custom_toolchain_name_parser( - value: &str, -) -> Result { - CustomToolchainName::try_from(value) -} - /// An toolchain specified just via its path. Relative paths enable arbitrary /// code execution in a rust dir, so as a partial mitigation is limited to /// absolute paths. diff --git a/tests/suite/cli-ui/rustup/rustup_component_cmd_add_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_component_cmd_add_cmd_help_flag_stdout.toml index e7c25e2bac..10643c0603 100644 --- a/tests/suite/cli-ui/rustup/rustup_component_cmd_add_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_component_cmd_add_cmd_help_flag_stdout.toml @@ -1,5 +1,5 @@ bin.name = "rustup" -args = ["component","add","--help"] +args = ["component", "add", "--help"] stdout = """ ... Add a component to a Rust toolchain diff --git a/tests/suite/cli-ui/rustup/rustup_component_cmd_remove_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_component_cmd_remove_cmd_help_flag_stdout.toml index 226db0a7ea..1bb7c4042f 100644 --- a/tests/suite/cli-ui/rustup/rustup_component_cmd_remove_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_component_cmd_remove_cmd_help_flag_stdout.toml @@ -1,5 +1,5 @@ bin.name = "rustup" -args = ["component","remove","--help"] +args = ["component", "remove", "--help"] stdout = """ ... Remove a component from a Rust toolchain diff --git a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml index eba5f967c7..59638a6931 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml @@ -9,11 +9,8 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - show Show the active and installed toolchains or profiles - update Update Rust toolchains and rustup check Check for updates to Rust toolchains and rustup default Set the default toolchain - toolchain Modify or query the installed toolchains target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify toolchain overrides for directories @@ -24,6 +21,9 @@ 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 + toolchain Modify or query the installed toolchains 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 index d1b11958f9..e0c413a77f 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml @@ -9,11 +9,8 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - show Show the active and installed toolchains or profiles - update Update Rust toolchains and rustup check Check for updates to Rust toolchains and rustup default Set the default toolchain - toolchain Modify or query the installed toolchains target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify toolchain overrides for directories @@ -24,6 +21,9 @@ 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 + toolchain Modify or query the installed toolchains help Print this message or the help of the given subcommand(s) Arguments: diff --git a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml index 6eacafa20d..32c6ef3845 100644 --- a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml @@ -6,14 +6,11 @@ rustup [..] The Rust toolchain installer -Usage: rustup [OPTIONS] [+toolchain] [COMMAND] +Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - show Show the active and installed toolchains or profiles - update Update Rust toolchains and rustup check Check for updates to Rust toolchains and rustup default Set the default toolchain - toolchain Modify or query the installed toolchains target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify toolchain overrides for directories @@ -24,6 +21,9 @@ 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 + toolchain Modify or query the installed toolchains help Print this message or the help of the given subcommand(s) Arguments: diff --git a/tests/suite/cli-ui/rustup/rustup_toolchain_cmd_install_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_toolchain_cmd_install_cmd_help_flag_stdout.toml index e02a1e4bd7..e986981ed9 100644 --- a/tests/suite/cli-ui/rustup/rustup_toolchain_cmd_install_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_toolchain_cmd_install_cmd_help_flag_stdout.toml @@ -1,26 +1,26 @@ bin.name = "rustup" -args = ["toolchain","install","--help"] +args = ["toolchain", "install", "--help"] stdout = """ ... Install or update a given toolchain -Usage: rustup[EXE] toolchain install [OPTIONS] ... +Usage: rustup[EXE] toolchain install [OPTIONS] ... Arguments: - ... Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see + ... Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see `rustup help toolchain` Options: - --profile [possible values: minimal, default, complete] - -c, --component ... Add specific components on installation - -t, --target ... Add specific targets on installation - --no-self-update Don't perform self update when running the`rustup toolchain - install` command - --force Force an update, even if some components are missing - --allow-downgrade Allow rustup to downgrade the toolchain to satisfy your component - choice - --force-non-host Install toolchains that require an emulator. See - https://github.com/rust-lang/rustup/wiki/Non-host-toolchains - -h, --help Print help + --profile [possible values: minimal, default, complete] + -c, --component ... Add specific components on installation + -t, --target ... Add specific targets on installation + --no-self-update Don't perform self update when running the `rustup toolchain + install` command + --force Force an update, even if some components are missing + --allow-downgrade Allow rustup to downgrade the toolchain to satisfy your component + choice + --force-non-host Install toolchains that require an emulator. See + https://github.com/rust-lang/rustup/wiki/Non-host-toolchains + -h, --help Print help """ stderr = "" diff --git a/tests/suite/cli-ui/rustup/rustup_toolchain_cmd_link_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_toolchain_cmd_link_cmd_help_flag_stdout.toml index 77433e818e..e0f084f02c 100644 --- a/tests/suite/cli-ui/rustup/rustup_toolchain_cmd_link_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_toolchain_cmd_link_cmd_help_flag_stdout.toml @@ -1,14 +1,14 @@ bin.name = "rustup" -args = ["toolchain","link","--help"] +args = ["toolchain", "link", "--help"] stdout = """ ... Create a custom toolchain by symlinking to a directory -Usage: rustup[EXE] toolchain link +Usage: rustup[EXE] toolchain link Arguments: - Custom toolchain name - Path to the directory + Custom toolchain name + Path to the directory Options: -h, --help Print help diff --git a/tests/suite/cli-ui/rustup/rustup_toolchain_cmd_uninstall_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_toolchain_cmd_uninstall_cmd_help_flag_stdout.toml index 9776b801ff..d79a2a2015 100644 --- a/tests/suite/cli-ui/rustup/rustup_toolchain_cmd_uninstall_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_toolchain_cmd_uninstall_cmd_help_flag_stdout.toml @@ -4,10 +4,10 @@ stdout = """ ... Uninstall a toolchain -Usage: rustup[EXE] toolchain uninstall ... +Usage: rustup[EXE] toolchain uninstall ... Arguments: - ... Toolchain name, such as 'stable', 'nightly', '1.8.0', or a custom toolchain name. + ... Toolchain name, such as 'stable', 'nightly', '1.8.0', or a custom toolchain name. For more information see `rustup help toolchain` Options: diff --git a/tests/suite/cli-ui/rustup/rustup_up_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_up_cmd_help_flag_stdout.toml index cfe1cdb5f7..58aa50511e 100644 --- a/tests/suite/cli-ui/rustup/rustup_up_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_up_cmd_help_flag_stdout.toml @@ -4,10 +4,10 @@ stdout = """ ... Update Rust toolchains and rustup -Usage: rustup[EXE] update [OPTIONS] [toolchain]... +Usage: rustup[EXE] update [OPTIONS] [TOOLCHAIN]... Arguments: - [toolchain]... Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see + [TOOLCHAIN]... Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see `rustup help toolchain` Options: diff --git a/tests/suite/cli-ui/rustup/rustup_update_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_update_cmd_help_flag_stdout.toml index cdebda35ca..59294d8dca 100644 --- a/tests/suite/cli-ui/rustup/rustup_update_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_update_cmd_help_flag_stdout.toml @@ -4,10 +4,10 @@ stdout = """ ... Update Rust toolchains and rustup -Usage: rustup[EXE] update [OPTIONS] [toolchain]... +Usage: rustup[EXE] update [OPTIONS] [TOOLCHAIN]... Arguments: - [toolchain]... Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see + [TOOLCHAIN]... Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see `rustup help toolchain` Options: diff --git a/tests/suite/cli-ui/rustup/rustup_upgrade_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_upgrade_cmd_help_flag_stdout.toml index 11559534b4..94d10f9f44 100644 --- a/tests/suite/cli-ui/rustup/rustup_upgrade_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_upgrade_cmd_help_flag_stdout.toml @@ -4,10 +4,10 @@ stdout = """ ... Update Rust toolchains and rustup -Usage: rustup[EXE] update [OPTIONS] [toolchain]... +Usage: rustup[EXE] update [OPTIONS] [TOOLCHAIN]... Arguments: - [toolchain]... Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see + [TOOLCHAIN]... Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see `rustup help toolchain` Options: diff --git a/tests/suite/cli_self_upd.rs b/tests/suite/cli_self_upd.rs index bf24c9f347..45d6b93481 100644 --- a/tests/suite/cli_self_upd.rs +++ b/tests/suite/cli_self_upd.rs @@ -398,7 +398,7 @@ fn update_bogus_version() { config.expect_ok(&["rustup-init", "-y", "--no-modify-path"]); config.expect_err( &["rustup", "update", "1.0.0-alpha"], - "invalid value '1.0.0-alpha' for '[toolchain]...': invalid toolchain name: '1.0.0-alpha'", + "invalid value '1.0.0-alpha' for '[TOOLCHAIN]...': invalid toolchain name: '1.0.0-alpha'", ); }); } diff --git a/tests/suite/cli_v2.rs b/tests/suite/cli_v2.rs index 0a38d3ee44..0573e04beb 100644 --- a/tests/suite/cli_v2.rs +++ b/tests/suite/cli_v2.rs @@ -797,7 +797,7 @@ fn cannot_add_empty_named_custom_toolchain() { let path = path.to_string_lossy(); config.expect_err( &["rustup", "toolchain", "link", "", &path], - "invalid value '' for '': invalid toolchain name ''", + "invalid value '' for '': invalid toolchain name ''", ); }); } From 32f8f00141fa7007f8a8a918d5be7ecdcfbb7f87 Mon Sep 17 00:00:00 2001 From: rami3l Date: Sun, 21 Jan 2024 19:44:13 +0800 Subject: [PATCH 04/13] refactor(cli): rewrite `rustup (check|default)` with `clap-derive` --- src/cli/rustup_mode.rs | 45 +++++++++---------- src/toolchain/names.rs | 9 ---- .../rustup_default_cmd_help_flag_stdout.toml | 6 +-- .../cli-ui/rustup/rustup_help_cmd_stdout.toml | 4 +- .../rustup/rustup_help_flag_stdout.toml | 4 +- .../rustup/rustup_only_options_stdout.toml | 4 +- 6 files changed, 31 insertions(+), 41 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 08e36ba525..79858ee236 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -37,10 +37,10 @@ use crate::{ toolchain::{ distributable::DistributableToolchain, names::{ - maybe_resolvable_toolchainame_parser, partial_toolchain_desc_parser, - resolvable_local_toolchainame_parser, resolvable_toolchainame_parser, - CustomToolchainName, LocalToolchainName, MaybeResolvableToolchainName, - ResolvableLocalToolchainName, ResolvableToolchainName, ToolchainName, + partial_toolchain_desc_parser, resolvable_local_toolchainame_parser, + resolvable_toolchainame_parser, CustomToolchainName, LocalToolchainName, + MaybeResolvableToolchainName, ResolvableLocalToolchainName, ResolvableToolchainName, + ToolchainName, }, toolchain::Toolchain, }, @@ -129,6 +129,16 @@ enum RustupSubcmd { force_non_host: bool, }, + /// Check for updates to Rust toolchains and rustup + Check, + + /// Set the default toolchain + #[command(after_help = DEFAULT_HELP)] + Default { + #[arg(help = MAYBE_RESOLVABLE_TOOLCHAIN_ARG_HELP)] + toolchain: Option, + }, + /// Modify or query the installed toolchains Toolchain { #[command(subcommand)] @@ -273,6 +283,8 @@ impl Rustup { ToolchainSubcmd::Link { toolchain, path } => toolchain_link(cfg, &toolchain, &path), ToolchainSubcmd::Uninstall { opts } => toolchain_remove(cfg, opts), }, + RustupSubcmd::Check => check_updates(cfg), + RustupSubcmd::Default { toolchain } => default_(cfg, toolchain), } } } @@ -349,11 +361,10 @@ pub fn main() -> Result { Ok(match matches.subcommand() { Some(s) => match s { ("dump-testament", _) => common::dump_testament()?, - ("show" | "update" | "install" | "uninstall" | "toolchain", _) => { - Rustup::from_arg_matches(&matches)?.dispatch(cfg)? - } - ("check", _) => check_updates(cfg)?, - ("default", m) => default_(cfg, m)?, + ( + "show" | "update" | "install" | "uninstall" | "toolchain" | "check" | "default", + _, + ) => Rustup::from_arg_matches(&matches)?.dispatch(cfg)?, ("target", c) => match c.subcommand() { Some(s) => match s { ("list", m) => handle_epipe(target_list(cfg, m))?, @@ -459,18 +470,6 @@ pub(crate) fn cli() -> Command { .about("Dump information about the build") .hide(true), // Not for users, only CI ) - .subcommand(Command::new("check").about("Check for updates to Rust toolchains and rustup")) - .subcommand( - Command::new("default") - .about("Set the default toolchain") - .after_help(DEFAULT_HELP) - .arg( - Arg::new("toolchain") - .help(MAYBE_RESOLVABLE_TOOLCHAIN_ARG_HELP) - .required(false) - .value_parser(maybe_resolvable_toolchainame_parser), - ), - ) .subcommand( Command::new("target") .about("Modify a toolchain's supported targets") @@ -797,10 +796,10 @@ fn maybe_upgrade_data(cfg: &Cfg, m: &ArgMatches) -> Result { } } -fn default_(cfg: &Cfg, m: &ArgMatches) -> Result { +fn default_(cfg: &Cfg, toolchain: Option) -> Result { common::warn_if_host_is_emulated(); - if let Some(toolchain) = m.get_one::("toolchain") { + if let Some(toolchain) = toolchain { match toolchain.to_owned() { MaybeResolvableToolchainName::None => { cfg.set_default(None)?; diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index 431d3fb8a6..bc444e47a5 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -229,15 +229,6 @@ impl Display for MaybeResolvableToolchainName { } } -/// Thunk to avoid errors like -/// = note: `fn(&'2 str) -> Result>::Error> {>::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 maybe_resolvable_toolchainame_parser( - value: &str, -) -> Result { - MaybeResolvableToolchainName::try_from(value) -} - /// ResolvableToolchainName + none, for overriding default-has-a-value /// situations in the CLI with an official toolchain name or none #[derive(Debug, Clone)] diff --git a/tests/suite/cli-ui/rustup/rustup_default_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_default_cmd_help_flag_stdout.toml index cd85c265b7..0d9c0a5ce7 100644 --- a/tests/suite/cli-ui/rustup/rustup_default_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_default_cmd_help_flag_stdout.toml @@ -1,13 +1,13 @@ bin.name = "rustup" -args = ["default","--help"] +args = ["default", "--help"] stdout = """ ... Set the default toolchain -Usage: rustup[EXE] default [toolchain] +Usage: rustup[EXE] default [TOOLCHAIN] Arguments: - [toolchain] 'none', a toolchain name, such as 'stable', 'nightly', '1.8.0', or a custom toolchain + [TOOLCHAIN] 'none', a toolchain name, such as 'stable', 'nightly', '1.8.0', or a custom toolchain name. For more information see `rustup help toolchain` Options: diff --git a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml index 59638a6931..2f0555de10 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml @@ -9,8 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - check Check for updates to Rust toolchains and rustup - default Set the default toolchain target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify toolchain overrides for directories @@ -23,6 +21,8 @@ Commands: 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 + default Set the default toolchain toolchain Modify or query the installed toolchains help Print this message or the help of the given subcommand(s) diff --git a/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml index e0c413a77f..2819847c64 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml @@ -9,8 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - check Check for updates to Rust toolchains and rustup - default Set the default toolchain target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify toolchain overrides for directories @@ -23,6 +21,8 @@ Commands: 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 + default Set the default toolchain toolchain Modify or query the installed toolchains help Print this message or the help of the given subcommand(s) diff --git a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml index 32c6ef3845..c468a588a5 100644 --- a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml @@ -9,8 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - check Check for updates to Rust toolchains and rustup - default Set the default toolchain target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify toolchain overrides for directories @@ -23,6 +21,8 @@ Commands: 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 + default Set the default toolchain toolchain Modify or query the installed toolchains help Print this message or the help of the given subcommand(s) From 3b257b4efcf5d48e9a68e5c6671c228a8e75437b Mon Sep 17 00:00:00 2001 From: rami3l Date: Sun, 21 Jan 2024 20:48:15 +0800 Subject: [PATCH 05/13] refactor(cli): rewrite `rustup target` with `clap-derive` --- src/cli/rustup_mode.rs | 177 +++++++++--------- .../cli-ui/rustup/rustup_help_cmd_stdout.toml | 2 +- .../rustup/rustup_help_flag_stdout.toml | 2 +- .../rustup/rustup_only_options_stdout.toml | 2 +- ...p_target_cmd_add_cmd_help_flag_stdout.toml | 8 +- ..._target_cmd_list_cmd_help_flag_stdout.toml | 4 +- ...arget_cmd_remove_cmd_help_flag_stdout.toml | 8 +- 7 files changed, 103 insertions(+), 100 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 79858ee236..e74501f236 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -144,6 +144,12 @@ enum RustupSubcmd { #[command(subcommand)] subcmd: ToolchainSubcmd, }, + + /// Modify a toolchain's supported targets + Target { + #[command(subcommand)] + subcmd: TargetSubcmd, + }, } #[derive(Debug, Subcommand)] @@ -249,6 +255,45 @@ struct UninstallOpts { toolchain: Vec, } +#[derive(Debug, Subcommand)] +#[command(arg_required_else_help = true, subcommand_required = true)] +enum TargetSubcmd { + /// List installed and available targets + List { + #[arg( + long, + help = OFFICIAL_TOOLCHAIN_ARG_HELP, + )] + toolchain: Option, + + /// List only installed targets + #[arg(long)] + installed: bool, + }, + + /// Add a target to a Rust toolchain + #[command(alias = "install")] + Add { + /// List of targets to install; "all" installs all available targets + #[arg(required = true, num_args = 1..)] + target: Vec, + + #[arg(long, help = OFFICIAL_TOOLCHAIN_ARG_HELP)] + toolchain: Option, + }, + + /// Remove a target from a Rust toolchain + #[command(alias = "uninstall")] + Remove { + /// List of targets to uninstall + #[arg(required = true, num_args = 1..)] + target: Vec, + + #[arg(long, help = OFFICIAL_TOOLCHAIN_ARG_HELP)] + toolchain: Option, + }, +} + impl Rustup { fn dispatch(self, cfg: &mut Cfg) -> Result { match self.subcmd { @@ -285,6 +330,14 @@ impl Rustup { }, RustupSubcmd::Check => check_updates(cfg), RustupSubcmd::Default { toolchain } => default_(cfg, toolchain), + RustupSubcmd::Target { subcmd } => match subcmd { + TargetSubcmd::List { + toolchain, + installed, + } => handle_epipe(target_list(cfg, toolchain, installed)), + TargetSubcmd::Add { target, toolchain } => target_add(cfg, target, toolchain), + TargetSubcmd::Remove { target, toolchain } => target_remove(cfg, target, toolchain), + }, } } } @@ -362,18 +415,10 @@ pub fn main() -> Result { Some(s) => match s { ("dump-testament", _) => common::dump_testament()?, ( - "show" | "update" | "install" | "uninstall" | "toolchain" | "check" | "default", + "show" | "update" | "install" | "uninstall" | "toolchain" | "check" | "default" + | "target", _, ) => Rustup::from_arg_matches(&matches)?.dispatch(cfg)?, - ("target", c) => match c.subcommand() { - Some(s) => match s { - ("list", m) => handle_epipe(target_list(cfg, m))?, - ("add", m) => target_add(cfg, m)?, - ("remove", m) => target_remove(cfg, m)?, - _ => unreachable!(), - }, - None => unreachable!(), - }, ("component", c) => match c.subcommand() { Some(s) => match s { ("list", m) => handle_epipe(component_list(cfg, m))?, @@ -470,63 +515,6 @@ pub(crate) fn cli() -> Command { .about("Dump information about the build") .hide(true), // Not for users, only CI ) - .subcommand( - Command::new("target") - .about("Modify a toolchain's supported targets") - .subcommand_required(true) - .arg_required_else_help(true) - .subcommand( - Command::new("list") - .about("List installed and available targets") - .arg( - Arg::new("toolchain") - .help(OFFICIAL_TOOLCHAIN_ARG_HELP) - .long("toolchain") - .value_parser(partial_toolchain_desc_parser) - .num_args(1), - ) - .arg( - Arg::new("installed") - .long("installed") - .help("List only installed targets") - .action(ArgAction::SetTrue), - ), - ) - .subcommand( - Command::new("add") - .about("Add a target to a Rust toolchain") - .alias("install") - .arg(Arg::new("target").required(true).num_args(1..).help( - "List of targets to install; \ - \"all\" installs all available targets", - )) - .arg( - Arg::new("toolchain") - .help(OFFICIAL_TOOLCHAIN_ARG_HELP) - .long("toolchain") - .num_args(1) - .value_parser(partial_toolchain_desc_parser), - ), - ) - .subcommand( - Command::new("remove") - .about("Remove a target from a Rust toolchain") - .alias("uninstall") - .arg( - Arg::new("target") - .help("List of targets to uninstall") - .required(true) - .num_args(1..), - ) - .arg( - Arg::new("toolchain") - .help(OFFICIAL_TOOLCHAIN_ARG_HELP) - .long("toolchain") - .num_args(1) - .value_parser(partial_toolchain_desc_parser), - ), - ), - ) .subcommand( Command::new("component") .about("Modify a toolchain's installed components") @@ -1176,15 +1164,23 @@ fn show_rustup_home(cfg: &Cfg) -> Result { Ok(utils::ExitCode(0)) } -fn target_list(cfg: &Cfg, m: &ArgMatches) -> Result { - let toolchain = explicit_desc_or_dir_toolchain(cfg, m)?; +fn target_list( + cfg: &Cfg, + toolchain: Option, + installed_only: bool, +) -> Result { + let toolchain = explicit_desc_or_dir_toolchain(cfg, toolchain)?; // downcasting required because the toolchain files can name any toolchain let distributable = (&toolchain).try_into()?; - common::list_targets(distributable, m.get_flag("installed")) + common::list_targets(distributable, installed_only) } -fn target_add(cfg: &Cfg, m: &ArgMatches) -> Result { - let toolchain = explicit_desc_or_dir_toolchain(cfg, m)?; +fn target_add( + cfg: &Cfg, + mut targets: Vec, + toolchain: Option, +) -> Result { + let toolchain = explicit_desc_or_dir_toolchain(cfg, toolchain)?; // XXX: long term move this error to cli ? the normal .into doesn't work // because Result here is the wrong sort and expression type ascription // isn't a feature yet. @@ -1192,11 +1188,6 @@ fn target_add(cfg: &Cfg, m: &ArgMatches) -> Result { // custom toolchains. let distributable = DistributableToolchain::try_from(&toolchain)?; let components = distributable.components()?; - let mut targets: Vec<_> = m - .get_many::("target") - .unwrap() - .map(ToOwned::to_owned) - .collect(); if targets.contains(&"all".to_string()) { if targets.len() != 1 { @@ -1234,12 +1225,16 @@ fn target_add(cfg: &Cfg, m: &ArgMatches) -> Result { Ok(utils::ExitCode(0)) } -fn target_remove(cfg: &Cfg, m: &ArgMatches) -> Result { - let toolchain = explicit_desc_or_dir_toolchain(cfg, m)?; +fn target_remove( + cfg: &Cfg, + targets: Vec, + toolchain: Option, +) -> Result { + let toolchain = explicit_desc_or_dir_toolchain(cfg, toolchain)?; let distributable = DistributableToolchain::try_from(&toolchain)?; - for target in m.get_many::("target").unwrap() { - let target = TargetTriple::new(target); + for target in targets { + let target = TargetTriple::new(&target); let default_target = cfg.get_default_host_triple()?; if target == default_target { warn!("after removing the default host target, proc-macros and build scripts might no longer build"); @@ -1266,7 +1261,7 @@ fn target_remove(cfg: &Cfg, m: &ArgMatches) -> Result { } fn component_list(cfg: &Cfg, m: &ArgMatches) -> Result { - let toolchain = explicit_desc_or_dir_toolchain(cfg, m)?; + let toolchain = explicit_desc_or_dir_toolchain_old(cfg, m)?; // downcasting required because the toolchain files can name any toolchain let distributable = (&toolchain).try_into()?; common::list_components(distributable, m.get_flag("installed"))?; @@ -1274,7 +1269,7 @@ fn component_list(cfg: &Cfg, m: &ArgMatches) -> Result { } fn component_add(cfg: &Cfg, m: &ArgMatches) -> Result { - let toolchain = explicit_desc_or_dir_toolchain(cfg, m)?; + let toolchain = explicit_desc_or_dir_toolchain_old(cfg, m)?; let distributable = DistributableToolchain::try_from(&toolchain)?; let target = get_target(m, &distributable); @@ -1294,7 +1289,7 @@ fn get_target(m: &ArgMatches, distributable: &DistributableToolchain<'_>) -> Opt } fn component_remove(cfg: &Cfg, m: &ArgMatches) -> Result { - let toolchain = explicit_desc_or_dir_toolchain(cfg, m)?; + let toolchain = explicit_desc_or_dir_toolchain_old(cfg, m)?; let distributable = DistributableToolchain::try_from(&toolchain)?; let target = get_target(m, &distributable); @@ -1308,13 +1303,21 @@ fn component_remove(cfg: &Cfg, m: &ArgMatches) -> Result { // Make *sure* only to use this for a subcommand whose "toolchain" argument // has .value_parser(partial_toolchain_desc_parser), or it will panic. -fn explicit_desc_or_dir_toolchain<'a>(cfg: &'a Cfg, m: &ArgMatches) -> Result> { +// FIXME: Delete this. +fn explicit_desc_or_dir_toolchain_old<'a>(cfg: &'a Cfg, m: &ArgMatches) -> Result> { let toolchain = m .get_one::("toolchain") .map(Into::into); explicit_or_dir_toolchain2(cfg, toolchain) } +fn explicit_desc_or_dir_toolchain( + cfg: &Cfg, + toolchain: Option, +) -> Result> { + explicit_or_dir_toolchain2(cfg, toolchain.map(|it| (&it).into())) +} + fn explicit_or_dir_toolchain2( cfg: &Cfg, toolchain: Option, @@ -1458,7 +1461,7 @@ const DOCS_DATA: &[(&str, &str, &str)] = &[ ]; fn doc(cfg: &Cfg, m: &ArgMatches) -> Result { - let toolchain = explicit_desc_or_dir_toolchain(cfg, m)?; + let toolchain = explicit_desc_or_dir_toolchain_old(cfg, m)?; if let Ok(distributable) = DistributableToolchain::try_from(&toolchain) { if let [_] = distributable @@ -1522,7 +1525,7 @@ fn man(cfg: &Cfg, m: &ArgMatches) -> Result { let command = m.get_one::("command").unwrap(); - let toolchain = explicit_desc_or_dir_toolchain(cfg, m)?; + let toolchain = explicit_desc_or_dir_toolchain_old(cfg, m)?; let mut path = toolchain.path().to_path_buf(); path.push("share"); path.push("man"); diff --git a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml index 2f0555de10..61b18dd2f5 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml @@ -9,7 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify toolchain overrides for directories run Run a command with an environment configured for a given toolchain @@ -24,6 +23,7 @@ Commands: check Check for updates to Rust toolchains and rustup default Set the default toolchain toolchain Modify or query the installed toolchains + target Modify a toolchain's supported targets 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 index 2819847c64..84fc97041c 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml @@ -9,7 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify toolchain overrides for directories run Run a command with an environment configured for a given toolchain @@ -24,6 +23,7 @@ Commands: check Check for updates to Rust toolchains and rustup default Set the default toolchain toolchain Modify or query the installed toolchains + target Modify a toolchain's supported targets help Print this message or the help of the given subcommand(s) Arguments: diff --git a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml index c468a588a5..1ea9049048 100644 --- a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml @@ -9,7 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify toolchain overrides for directories run Run a command with an environment configured for a given toolchain @@ -24,6 +23,7 @@ Commands: check Check for updates to Rust toolchains and rustup default Set the default toolchain toolchain Modify or query the installed toolchains + target Modify a toolchain's supported targets help Print this message or the help of the given subcommand(s) Arguments: diff --git a/tests/suite/cli-ui/rustup/rustup_target_cmd_add_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_target_cmd_add_cmd_help_flag_stdout.toml index c7b55dc104..302c4a06e9 100644 --- a/tests/suite/cli-ui/rustup/rustup_target_cmd_add_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_target_cmd_add_cmd_help_flag_stdout.toml @@ -1,16 +1,16 @@ bin.name = "rustup" -args = ["target","add","--help"] +args = ["target", "add", "--help"] stdout = """ ... Add a target to a Rust toolchain -Usage: rustup[EXE] target add [OPTIONS] ... +Usage: rustup[EXE] target add [OPTIONS] ... Arguments: - ... List of targets to install; \"all\" installs all available targets + ... List of targets to install; \"all\" installs all available targets Options: - --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more + --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_target_cmd_list_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_target_cmd_list_cmd_help_flag_stdout.toml index 6457da2ae7..091a0acf9f 100644 --- a/tests/suite/cli-ui/rustup/rustup_target_cmd_list_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_target_cmd_list_cmd_help_flag_stdout.toml @@ -1,5 +1,5 @@ bin.name = "rustup" -args = ["target","list","--help"] +args = ["target", "list", "--help"] stdout = """ ... List installed and available targets @@ -7,7 +7,7 @@ List installed and available targets Usage: rustup[EXE] target list [OPTIONS] Options: - --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more + --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see `rustup help toolchain` --installed List only installed targets -h, --help Print help diff --git a/tests/suite/cli-ui/rustup/rustup_target_cmd_remove_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_target_cmd_remove_cmd_help_flag_stdout.toml index 990f21ba8e..64cdd2ab99 100644 --- a/tests/suite/cli-ui/rustup/rustup_target_cmd_remove_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_target_cmd_remove_cmd_help_flag_stdout.toml @@ -1,16 +1,16 @@ bin.name = "rustup" -args = ["target","remove","--help"] +args = ["target", "remove", "--help"] stdout = """ ... Remove a target from a Rust toolchain -Usage: rustup[EXE] target remove [OPTIONS] ... +Usage: rustup[EXE] target remove [OPTIONS] ... Arguments: - ... List of targets to uninstall + ... List of targets to uninstall Options: - --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more + --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see `rustup help toolchain` -h, --help Print help """ From 16e3cd588a4e6d85de5fae732f626cdafce1e9e0 Mon Sep 17 00:00:00 2001 From: rami3l Date: Mon, 22 Jan 2024 10:52:13 +0800 Subject: [PATCH 06/13] refactor(cli): rewrite `rustup component` with `clap-derive` --- src/cli/rustup_mode.rs | 164 ++++++++++-------- ...omponent_cmd_add_cmd_help_flag_stdout.toml | 8 +- ...mponent_cmd_list_cmd_help_flag_stdout.toml | 4 +- ...onent_cmd_remove_cmd_help_flag_stdout.toml | 8 +- .../cli-ui/rustup/rustup_help_cmd_stdout.toml | 2 +- .../rustup/rustup_help_flag_stdout.toml | 2 +- .../rustup/rustup_only_options_stdout.toml | 2 +- 7 files changed, 104 insertions(+), 86 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index e74501f236..df11e9b747 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -150,6 +150,12 @@ enum RustupSubcmd { #[command(subcommand)] subcmd: TargetSubcmd, }, + + /// Modify a toolchain's installed components + Component { + #[command(subcommand)] + subcmd: ComponentSubcmd, + }, } #[derive(Debug, Subcommand)] @@ -294,6 +300,44 @@ enum TargetSubcmd { }, } +#[derive(Debug, Subcommand)] +#[command(arg_required_else_help = true, subcommand_required = true)] +enum ComponentSubcmd { + /// List installed and available components + List { + #[arg(long, help = OFFICIAL_TOOLCHAIN_ARG_HELP)] + toolchain: Option, + + /// List only installed components + #[arg(long)] + installed: bool, + }, + + /// Add a component to a Rust toolchain + Add { + #[arg(required = true, num_args = 1..)] + component: Vec, + + #[arg(long, help = OFFICIAL_TOOLCHAIN_ARG_HELP)] + toolchain: Option, + + #[arg(long)] + target: Option, + }, + + /// Remove a component from a Rust toolchain + Remove { + #[arg(required = true, num_args = 1..)] + component: Vec, + + #[arg(long, help = OFFICIAL_TOOLCHAIN_ARG_HELP)] + toolchain: Option, + + #[arg(long)] + target: Option, + }, +} + impl Rustup { fn dispatch(self, cfg: &mut Cfg) -> Result { match self.subcmd { @@ -338,6 +382,22 @@ impl Rustup { TargetSubcmd::Add { target, toolchain } => target_add(cfg, target, toolchain), TargetSubcmd::Remove { target, toolchain } => target_remove(cfg, target, toolchain), }, + RustupSubcmd::Component { subcmd } => match subcmd { + ComponentSubcmd::List { + toolchain, + installed, + } => handle_epipe(component_list(cfg, toolchain, installed)), + ComponentSubcmd::Add { + component, + toolchain, + target, + } => component_add(cfg, component, toolchain, target.as_deref()), + ComponentSubcmd::Remove { + component, + toolchain, + target, + } => component_remove(cfg, component, toolchain, target.as_deref()), + }, } } } @@ -416,18 +476,9 @@ pub fn main() -> Result { ("dump-testament", _) => common::dump_testament()?, ( "show" | "update" | "install" | "uninstall" | "toolchain" | "check" | "default" - | "target", + | "target" | "component", _, ) => Rustup::from_arg_matches(&matches)?.dispatch(cfg)?, - ("component", c) => match c.subcommand() { - Some(s) => match s { - ("list", m) => handle_epipe(component_list(cfg, m))?, - ("add", m) => component_add(cfg, m)?, - ("remove", m) => component_remove(cfg, m)?, - _ => unreachable!(), - }, - None => unreachable!(), - }, ("override", c) => match c.subcommand() { Some(s) => match s { ("list", _) => handle_epipe(common::list_overrides(cfg))?, @@ -515,55 +566,6 @@ pub(crate) fn cli() -> Command { .about("Dump information about the build") .hide(true), // Not for users, only CI ) - .subcommand( - Command::new("component") - .about("Modify a toolchain's installed components") - .subcommand_required(true) - .arg_required_else_help(true) - .subcommand( - Command::new("list") - .about("List installed and available components") - .arg( - Arg::new("toolchain") - .help(OFFICIAL_TOOLCHAIN_ARG_HELP) - .long("toolchain") - .num_args(1) - .value_parser(partial_toolchain_desc_parser), - ) - .arg( - Arg::new("installed") - .long("installed") - .help("List only installed components") - .action(ArgAction::SetTrue), - ), - ) - .subcommand( - Command::new("add") - .about("Add a component to a Rust toolchain") - .arg(Arg::new("component").required(true).num_args(1..)) - .arg( - Arg::new("toolchain") - .help(OFFICIAL_TOOLCHAIN_ARG_HELP) - .long("toolchain") - .num_args(1) - .value_parser(partial_toolchain_desc_parser), - ) - .arg(Arg::new("target").long("target").num_args(1)), - ) - .subcommand( - Command::new("remove") - .about("Remove a component from a Rust toolchain") - .arg(Arg::new("component").required(true).num_args(1..)) - .arg( - Arg::new("toolchain") - .help(OFFICIAL_TOOLCHAIN_ARG_HELP) - .long("toolchain") - .num_args(1) - .value_parser(partial_toolchain_desc_parser), - ) - .arg(Arg::new("target").long("target").num_args(1)), - ), - ) .subcommand( Command::new("override") .about("Modify toolchain overrides for directories") @@ -1260,20 +1262,29 @@ fn target_remove( Ok(utils::ExitCode(0)) } -fn component_list(cfg: &Cfg, m: &ArgMatches) -> Result { - let toolchain = explicit_desc_or_dir_toolchain_old(cfg, m)?; +fn component_list( + cfg: &Cfg, + toolchain: Option, + installed_only: bool, +) -> Result { + let toolchain = explicit_desc_or_dir_toolchain(cfg, toolchain)?; // downcasting required because the toolchain files can name any toolchain let distributable = (&toolchain).try_into()?; - common::list_components(distributable, m.get_flag("installed"))?; + common::list_components(distributable, installed_only)?; Ok(utils::ExitCode(0)) } -fn component_add(cfg: &Cfg, m: &ArgMatches) -> Result { - let toolchain = explicit_desc_or_dir_toolchain_old(cfg, m)?; +fn component_add( + cfg: &Cfg, + components: Vec, + toolchain: Option, + target: Option<&str>, +) -> Result { + let toolchain = explicit_desc_or_dir_toolchain(cfg, toolchain)?; let distributable = DistributableToolchain::try_from(&toolchain)?; - let target = get_target(m, &distributable); + let target = get_target(target, &distributable); - for component in m.get_many::("component").unwrap() { + for component in &components { let new_component = Component::try_new(component, &distributable, target.as_ref())?; distributable.add_component(new_component)?; } @@ -1281,19 +1292,26 @@ fn component_add(cfg: &Cfg, m: &ArgMatches) -> Result { Ok(utils::ExitCode(0)) } -fn get_target(m: &ArgMatches, distributable: &DistributableToolchain<'_>) -> Option { - m.get_one::("target") - .map(|s| &**s) +fn get_target( + target: Option<&str>, + distributable: &DistributableToolchain<'_>, +) -> Option { + target .map(TargetTriple::new) .or_else(|| Some(distributable.desc().target.clone())) } -fn component_remove(cfg: &Cfg, m: &ArgMatches) -> Result { - let toolchain = explicit_desc_or_dir_toolchain_old(cfg, m)?; +fn component_remove( + cfg: &Cfg, + components: Vec, + toolchain: Option, + target: Option<&str>, +) -> Result { + let toolchain = explicit_desc_or_dir_toolchain(cfg, toolchain)?; let distributable = DistributableToolchain::try_from(&toolchain)?; - let target = get_target(m, &distributable); + let target = get_target(target, &distributable); - for component in m.get_many::("component").unwrap() { + for component in &components { let new_component = Component::try_new(component, &distributable, target.as_ref())?; distributable.remove_component(new_component)?; } diff --git a/tests/suite/cli-ui/rustup/rustup_component_cmd_add_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_component_cmd_add_cmd_help_flag_stdout.toml index 10643c0603..e40c683361 100644 --- a/tests/suite/cli-ui/rustup/rustup_component_cmd_add_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_component_cmd_add_cmd_help_flag_stdout.toml @@ -4,15 +4,15 @@ stdout = """ ... Add a component to a Rust toolchain -Usage: rustup[EXE] component add [OPTIONS] ... +Usage: rustup[EXE] component add [OPTIONS] ... Arguments: - ... + ... Options: - --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more + --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see `rustup help toolchain` - --target + --target -h, --help Print help """ stderr = "" diff --git a/tests/suite/cli-ui/rustup/rustup_component_cmd_list_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_component_cmd_list_cmd_help_flag_stdout.toml index b4f5d20398..26e8628e5e 100644 --- a/tests/suite/cli-ui/rustup/rustup_component_cmd_list_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_component_cmd_list_cmd_help_flag_stdout.toml @@ -1,5 +1,5 @@ bin.name = "rustup" -args = ["component","list","--help"] +args = ["component", "list", "--help"] stdout = """ ... List installed and available components @@ -7,7 +7,7 @@ List installed and available components Usage: rustup[EXE] component list [OPTIONS] Options: - --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more + --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see `rustup help toolchain` --installed List only installed components -h, --help Print help diff --git a/tests/suite/cli-ui/rustup/rustup_component_cmd_remove_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_component_cmd_remove_cmd_help_flag_stdout.toml index 1bb7c4042f..a2f5a2980d 100644 --- a/tests/suite/cli-ui/rustup/rustup_component_cmd_remove_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_component_cmd_remove_cmd_help_flag_stdout.toml @@ -4,15 +4,15 @@ stdout = """ ... Remove a component from a Rust toolchain -Usage: rustup[EXE] component remove [OPTIONS] ... +Usage: rustup[EXE] component remove [OPTIONS] ... Arguments: - ... + ... Options: - --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more + --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see `rustup help toolchain` - --target + --target -h, --help Print help """ stderr = "" diff --git a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml index 61b18dd2f5..c6f199d7a7 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml @@ -9,7 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - component Modify a toolchain's installed components override Modify toolchain overrides for directories run Run a command with an environment configured for a given toolchain which Display which binary will be run for a given command @@ -24,6 +23,7 @@ Commands: default Set the default toolchain toolchain Modify or query the installed toolchains target Modify a toolchain's supported targets + component Modify a toolchain's installed components 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 index 84fc97041c..9c25c1aa0e 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml @@ -9,7 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - component Modify a toolchain's installed components override Modify toolchain overrides for directories run Run a command with an environment configured for a given toolchain which Display which binary will be run for a given command @@ -24,6 +23,7 @@ Commands: default Set the default toolchain toolchain Modify or query the installed toolchains target Modify a toolchain's supported targets + component Modify a toolchain's installed components help Print this message or the help of the given subcommand(s) Arguments: diff --git a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml index 1ea9049048..a1409ee330 100644 --- a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml @@ -9,7 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - component Modify a toolchain's installed components override Modify toolchain overrides for directories run Run a command with an environment configured for a given toolchain which Display which binary will be run for a given command @@ -24,6 +23,7 @@ Commands: default Set the default toolchain toolchain Modify or query the installed toolchains target Modify a toolchain's supported targets + component Modify a toolchain's installed components help Print this message or the help of the given subcommand(s) Arguments: From a465b1f90e91a1ce5789592605a11851421f2d09 Mon Sep 17 00:00:00 2001 From: rami3l Date: Mon, 22 Jan 2024 11:41:47 +0800 Subject: [PATCH 07/13] refactor(cli): rewrite `rustup override` with `clap-derive` --- src/cli/rustup_mode.rs | 140 +++++++++--------- .../cli-ui/rustup/rustup_help_cmd_stdout.toml | 2 +- .../rustup/rustup_help_flag_stdout.toml | 2 +- .../rustup/rustup_only_options_stdout.toml | 2 +- ...override_cmd_add_cmd_help_flag_stdout.toml | 8 +- ...rride_cmd_remove_cmd_help_flag_stdout.toml | 4 +- ...override_cmd_set_cmd_help_flag_stdout.toml | 8 +- ...erride_cmd_unset_cmd_help_flag_stdout.toml | 4 +- 8 files changed, 83 insertions(+), 87 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index df11e9b747..80eb2e8795 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -156,6 +156,12 @@ enum RustupSubcmd { #[command(subcommand)] subcmd: ComponentSubcmd, }, + + /// Modify toolchain overrides for directories + Override { + #[command(subcommand)] + subcmd: OverrideSubcmd, + }, } #[derive(Debug, Subcommand)] @@ -338,6 +344,40 @@ enum ComponentSubcmd { }, } +#[derive(Debug, Subcommand)] +#[command( + after_help = OVERRIDE_HELP, + arg_required_else_help = true, + subcommand_required = true, +)] +enum OverrideSubcmd { + /// List directory toolchain overrides + List, + + /// Set the override toolchain for a directory + #[command(alias = "add")] + Set { + #[arg(help = RESOLVABLE_TOOLCHAIN_ARG_HELP)] + toolchain: ResolvableToolchainName, + + /// Path to the directory + #[arg(long)] + path: Option, + }, + + /// Remove the override toolchain for a directory + #[command(alias = "remove", after_help = OVERRIDE_UNSET_HELP)] + Unset { + /// Path to the directory + #[arg(long)] + path: Option, + + /// Remove override toolchain for all nonexistent directories + #[arg(long)] + nonexistent: bool, + }, +} + impl Rustup { fn dispatch(self, cfg: &mut Cfg) -> Result { match self.subcmd { @@ -398,6 +438,15 @@ impl Rustup { target, } => component_remove(cfg, component, toolchain, target.as_deref()), }, + RustupSubcmd::Override { subcmd } => match subcmd { + OverrideSubcmd::List => handle_epipe(common::list_overrides(cfg)), + OverrideSubcmd::Set { toolchain, path } => { + override_add(cfg, toolchain, path.as_deref()) + } + OverrideSubcmd::Unset { path, nonexistent } => { + override_remove(cfg, path.as_deref(), nonexistent) + } + }, } } } @@ -476,18 +525,9 @@ pub fn main() -> Result { ("dump-testament", _) => common::dump_testament()?, ( "show" | "update" | "install" | "uninstall" | "toolchain" | "check" | "default" - | "target" | "component", + | "target" | "component" | "override", _, ) => Rustup::from_arg_matches(&matches)?.dispatch(cfg)?, - ("override", c) => match c.subcommand() { - Some(s) => match s { - ("list", _) => handle_epipe(common::list_overrides(cfg))?, - ("set", m) => override_add(cfg, m)?, - ("unset", m) => override_remove(cfg, m)?, - _ => unreachable!(), - }, - None => unreachable!(), - }, ("run", m) => run(cfg, m)?, ("which", m) => which(cfg, m)?, ("doc", m) => doc(cfg, m)?, @@ -566,50 +606,6 @@ pub(crate) fn cli() -> Command { .about("Dump information about the build") .hide(true), // Not for users, only CI ) - .subcommand( - Command::new("override") - .about("Modify toolchain overrides for directories") - .after_help(OVERRIDE_HELP) - .subcommand_required(true) - .arg_required_else_help(true) - .subcommand(Command::new("list").about("List directory toolchain overrides")) - .subcommand( - Command::new("set") - .about("Set the override toolchain for a directory") - .alias("add") - .arg( - Arg::new("toolchain") - .help(RESOLVABLE_TOOLCHAIN_ARG_HELP) - .required(true) - .num_args(1) - .value_parser(resolvable_toolchainame_parser), - ) - .arg( - Arg::new("path") - .long("path") - .num_args(1) - .help("Path to the directory"), - ), - ) - .subcommand( - Command::new("unset") - .about("Remove the override toolchain for a directory") - .after_help(OVERRIDE_UNSET_HELP) - .alias("remove") - .arg( - Arg::new("path") - .long("path") - .num_args(1) - .help("Path to the directory"), - ) - .arg( - Arg::new("nonexistent") - .long("nonexistent") - .help("Remove override toolchain for all nonexistent directories") - .action(ArgAction::SetTrue), - ), - ), - ) .subcommand( Command::new("run") .about("Run a command with an environment configured for a given toolchain") @@ -1376,11 +1372,14 @@ fn toolchain_remove(cfg: &mut Cfg, opts: UninstallOpts) -> Result Result { - let toolchain_name = m.get_one::("toolchain").unwrap(); - let toolchain_name = toolchain_name.resolve(&cfg.get_default_host_triple()?)?; +fn override_add( + cfg: &Cfg, + toolchain: ResolvableToolchainName, + path: Option<&Path>, +) -> Result { + let toolchain_name = toolchain.resolve(&cfg.get_default_host_triple()?)?; - let path = if let Some(path) = m.get_one::("path") { + let path = if let Some(path) = path { PathBuf::from(path) } else { utils::current_dir()? @@ -1415,17 +1414,14 @@ fn override_add(cfg: &Cfg, m: &ArgMatches) -> Result { Ok(utils::ExitCode(0)) } -fn override_remove(cfg: &Cfg, m: &ArgMatches) -> Result { - let paths = if m.get_flag("nonexistent") { +fn override_remove(cfg: &Cfg, path: Option<&Path>, nonexistent: bool) -> Result { + let paths = if nonexistent { let list: Vec<_> = cfg.settings_file.with(|s| { Ok(s.overrides .iter() .filter_map(|(k, _)| { - if Path::new(k).is_dir() { - None - } else { - Some(k.clone()) - } + let path = Path::new(k); + (!path.is_dir()).then(|| path.to_owned()) }) .collect()) })?; @@ -1433,21 +1429,21 @@ fn override_remove(cfg: &Cfg, m: &ArgMatches) -> Result { info!("no nonexistent paths detected"); } list - } else if let Some(path) = m.get_one::("path") { + } else if let Some(path) = path { vec![path.to_owned()] } else { - vec![utils::current_dir()?.to_str().unwrap().to_string()] + vec![utils::current_dir()?] }; - for path in paths { + for p in &paths { if cfg .settings_file - .with_mut(|s| Ok(s.remove_override(Path::new(&path), cfg.notify_handler.as_ref())))? + .with_mut(|s| Ok(s.remove_override(p, cfg.notify_handler.as_ref())))? { - info!("override toolchain for '{}' removed", path); + info!("override toolchain for '{}' removed", p.display()); } else { - info!("no override toolchain for '{}'", path); - if m.get_one::("path").is_none() && !m.get_flag("nonexistent") { + info!("no override toolchain for '{}'", p.display()); + if path.is_none() && !nonexistent { info!( "you may use `--path ` option to remove override toolchain \ for a specific path" diff --git a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml index c6f199d7a7..34ec8113e2 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml @@ -9,7 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - override Modify toolchain overrides for directories 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 @@ -24,6 +23,7 @@ Commands: toolchain Modify or query the installed toolchains target Modify a toolchain's supported targets component Modify a toolchain's installed components + override Modify toolchain overrides for directories 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 index 9c25c1aa0e..71631bfbde 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml @@ -9,7 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - override Modify toolchain overrides for directories 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 @@ -24,6 +23,7 @@ Commands: toolchain Modify or query the installed toolchains target Modify a toolchain's supported targets component Modify a toolchain's installed components + override Modify toolchain overrides for directories help Print this message or the help of the given subcommand(s) Arguments: diff --git a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml index a1409ee330..0676d20a84 100644 --- a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml @@ -9,7 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - override Modify toolchain overrides for directories 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 @@ -24,6 +23,7 @@ Commands: toolchain Modify or query the installed toolchains target Modify a toolchain's supported targets component Modify a toolchain's installed components + override Modify toolchain overrides for directories help Print this message or the help of the given subcommand(s) Arguments: diff --git a/tests/suite/cli-ui/rustup/rustup_override_cmd_add_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_override_cmd_add_cmd_help_flag_stdout.toml index bf4b218daa..d48e6e2a86 100644 --- a/tests/suite/cli-ui/rustup/rustup_override_cmd_add_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_override_cmd_add_cmd_help_flag_stdout.toml @@ -1,17 +1,17 @@ bin.name = "rustup" -args = ["override","add","--help"] +args = ["override", "add", "--help"] stdout = """ ... Set the override toolchain for a directory -Usage: rustup[EXE] override set [OPTIONS] +Usage: rustup[EXE] override set [OPTIONS] Arguments: - Toolchain name, such as 'stable', 'nightly', '1.8.0', or a custom toolchain name. For + Toolchain name, such as 'stable', 'nightly', '1.8.0', or a custom toolchain name. For more information see `rustup help toolchain` Options: - --path Path to the directory + --path Path to the directory -h, --help Print help """ stderr = "" diff --git a/tests/suite/cli-ui/rustup/rustup_override_cmd_remove_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_override_cmd_remove_cmd_help_flag_stdout.toml index 6af8bfc960..6d12a93427 100644 --- a/tests/suite/cli-ui/rustup/rustup_override_cmd_remove_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_override_cmd_remove_cmd_help_flag_stdout.toml @@ -1,5 +1,5 @@ bin.name = "rustup" -args = ["override","unset","--help"] +args = ["override", "remove", "--help"] stdout = """ ... Remove the override toolchain for a directory @@ -7,7 +7,7 @@ Remove the override toolchain for a directory Usage: rustup[EXE] override unset [OPTIONS] Options: - --path Path to the directory + --path Path to the directory --nonexistent Remove override toolchain for all nonexistent directories -h, --help Print help diff --git a/tests/suite/cli-ui/rustup/rustup_override_cmd_set_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_override_cmd_set_cmd_help_flag_stdout.toml index 462d4a3d46..58c7bccb62 100644 --- a/tests/suite/cli-ui/rustup/rustup_override_cmd_set_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_override_cmd_set_cmd_help_flag_stdout.toml @@ -1,17 +1,17 @@ bin.name = "rustup" -args = ["override","set","--help"] +args = ["override", "set", "--help"] stdout = """ ... Set the override toolchain for a directory -Usage: rustup[EXE] override set [OPTIONS] +Usage: rustup[EXE] override set [OPTIONS] Arguments: - Toolchain name, such as 'stable', 'nightly', '1.8.0', or a custom toolchain name. For + Toolchain name, such as 'stable', 'nightly', '1.8.0', or a custom toolchain name. For more information see `rustup help toolchain` Options: - --path Path to the directory + --path Path to the directory -h, --help Print help """ stderr = "" diff --git a/tests/suite/cli-ui/rustup/rustup_override_cmd_unset_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_override_cmd_unset_cmd_help_flag_stdout.toml index 6af8bfc960..4a1a455c76 100644 --- a/tests/suite/cli-ui/rustup/rustup_override_cmd_unset_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_override_cmd_unset_cmd_help_flag_stdout.toml @@ -1,5 +1,5 @@ bin.name = "rustup" -args = ["override","unset","--help"] +args = ["override", "unset", "--help"] stdout = """ ... Remove the override toolchain for a directory @@ -7,7 +7,7 @@ Remove the override toolchain for a directory Usage: rustup[EXE] override unset [OPTIONS] Options: - --path Path to the directory + --path Path to the directory --nonexistent Remove override toolchain for all nonexistent directories -h, --help Print help From 655a15a64a5869eb33679701d9c15744a4fa9d66 Mon Sep 17 00:00:00 2001 From: rami3l Date: Mon, 22 Jan 2024 14:46:50 +0800 Subject: [PATCH 08/13] refactor(cli): rewrite `rustup (run|which|dump-testament)` with `clap-derive` --- src/cli/rustup_mode.rs | 110 ++++++++---------- src/toolchain/names.rs | 15 --- .../cli-ui/rustup/rustup_help_cmd_stdout.toml | 4 +- .../rustup/rustup_help_flag_stdout.toml | 4 +- .../rustup/rustup_only_options_stdout.toml | 4 +- .../rustup_run_cmd_help_flag_stdout.toml | 6 +- .../rustup_which_cmd_help_flag_stdout.toml | 6 +- 7 files changed, 62 insertions(+), 87 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 80eb2e8795..649575e092 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -37,8 +37,7 @@ use crate::{ toolchain::{ distributable::DistributableToolchain, names::{ - partial_toolchain_desc_parser, resolvable_local_toolchainame_parser, - resolvable_toolchainame_parser, CustomToolchainName, LocalToolchainName, + partial_toolchain_desc_parser, CustomToolchainName, LocalToolchainName, MaybeResolvableToolchainName, ResolvableLocalToolchainName, ResolvableToolchainName, ToolchainName, }, @@ -95,6 +94,10 @@ enum RustupSubcmd { opts: UninstallOpts, }, + /// Dump information about the build + #[command(hide = true)] + DumpTestament, + /// Show the active and installed toolchains or profiles #[command(after_help = SHOW_HELP)] Show { @@ -162,6 +165,28 @@ enum RustupSubcmd { #[command(subcommand)] subcmd: OverrideSubcmd, }, + + /// Run a command with an environment configured for a given toolchain + #[command(after_help = RUN_HELP, trailing_var_arg = true)] + Run { + #[arg(help = RESOLVABLE_LOCAL_TOOLCHAIN_ARG_HELP)] + toolchain: ResolvableLocalToolchainName, + + #[arg(required = true, num_args = 1.., use_value_delimiter = false)] + command: Vec, + + /// Install the requested toolchain if needed + #[arg(long)] + install: bool, + }, + + /// Display which binary will be run for a given command + Which { + command: String, + + #[arg(long, help = RESOLVABLE_TOOLCHAIN_ARG_HELP)] + toolchain: Option, + }, } #[derive(Debug, Subcommand)] @@ -381,6 +406,7 @@ enum OverrideSubcmd { impl Rustup { fn dispatch(self, cfg: &mut Cfg) -> Result { match self.subcmd { + RustupSubcmd::DumpTestament => common::dump_testament(), RustupSubcmd::Install { opts } => update(cfg, opts), RustupSubcmd::Uninstall { opts } => toolchain_remove(cfg, opts), RustupSubcmd::Show { verbose, subcmd } => match subcmd { @@ -447,6 +473,12 @@ impl Rustup { override_remove(cfg, path.as_deref(), nonexistent) } }, + RustupSubcmd::Run { + toolchain, + command, + install, + } => run(cfg, toolchain, command, install), + RustupSubcmd::Which { command, toolchain } => which(cfg, &command, toolchain), } } } @@ -522,14 +554,11 @@ pub fn main() -> Result { Ok(match matches.subcommand() { Some(s) => match s { - ("dump-testament", _) => common::dump_testament()?, ( - "show" | "update" | "install" | "uninstall" | "toolchain" | "check" | "default" - | "target" | "component" | "override", + "dump-testament" | "show" | "update" | "install" | "uninstall" | "toolchain" + | "check" | "default" | "target" | "component" | "override" | "run" | "which", _, ) => Rustup::from_arg_matches(&matches)?.dispatch(cfg)?, - ("run", m) => run(cfg, m)?, - ("which", m) => which(cfg, m)?, ("doc", m) => doc(cfg, m)?, #[cfg(not(windows))] ("man", m) => man(cfg, m)?, @@ -601,48 +630,6 @@ pub(crate) fn cli() -> Command { } }), ) - .subcommand( - Command::new("dump-testament") - .about("Dump information about the build") - .hide(true), // Not for users, only CI - ) - .subcommand( - Command::new("run") - .about("Run a command with an environment configured for a given toolchain") - .after_help(RUN_HELP) - .trailing_var_arg(true) - .arg( - Arg::new("toolchain") - .help(RESOLVABLE_LOCAL_TOOLCHAIN_ARG_HELP) - .required(true) - .num_args(1) - .value_parser(resolvable_local_toolchainame_parser), - ) - .arg( - Arg::new("command") - .required(true) - .num_args(1..) - .use_value_delimiter(false), - ) - .arg( - Arg::new("install") - .help("Install the requested toolchain if needed") - .long("install") - .action(ArgAction::SetTrue), - ), - ) - .subcommand( - Command::new("which") - .about("Display which binary will be run for a given command") - .arg(Arg::new("command").required(true)) - .arg( - Arg::new("toolchain") - .help(RESOLVABLE_TOOLCHAIN_ARG_HELP) - .long("toolchain") - .num_args(1) - .value_parser(resolvable_toolchainame_parser), - ), - ) .subcommand( Command::new("doc") .alias("docs") @@ -965,22 +952,25 @@ fn update(cfg: &mut Cfg, opts: UpdateOpts) -> Result { Ok(utils::ExitCode(0)) } -fn run(cfg: &Cfg, m: &ArgMatches) -> Result { - let toolchain = m - .get_one::("toolchain") - .unwrap(); - let args = m.get_many::("command").unwrap(); - let args: Vec<_> = args.collect(); +fn run( + cfg: &Cfg, + toolchain: ResolvableLocalToolchainName, + command: Vec, + install: bool, +) -> Result { let toolchain = toolchain.resolve(&cfg.get_default_host_triple()?)?; - let cmd = cfg.create_command_for_toolchain(&toolchain, m.get_flag("install"), args[0])?; + let cmd = cfg.create_command_for_toolchain(&toolchain, install, &command[0])?; - let code = command::run_command_for_dir(cmd, args[0], &args[1..])?; + let code = command::run_command_for_dir(cmd, &command[0], &command[1..])?; Ok(code) } -fn which(cfg: &Cfg, m: &ArgMatches) -> Result { - let binary = m.get_one::("command").unwrap(); - let binary_path = if let Some(toolchain) = m.get_one::("toolchain") { +fn which( + cfg: &Cfg, + binary: &str, + toolchain: Option, +) -> Result { + let binary_path = if let Some(toolchain) = toolchain { let desc = toolchain.resolve(&cfg.get_default_host_triple()?)?; Toolchain::new(cfg, desc.into())?.binary_file(binary) } else { diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index bc444e47a5..611b0c2c07 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -185,15 +185,6 @@ impl Display for ResolvableToolchainName { } } -/// Thunk to avoid errors like -/// = note: `fn(&'2 str) -> Result>::Error> {>::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 resolvable_toolchainame_parser( - value: &str, -) -> Result { - ResolvableToolchainName::try_from(value) -} - /// A toolchain name from user input. MaybeToolchainName accepts 'none' or a /// custom or resolvable official name. Possibly this should be an Option with a /// local trait for our needs. @@ -366,12 +357,6 @@ impl Display for ResolvableLocalToolchainName { } } -pub(crate) fn resolvable_local_toolchainame_parser( - value: &str, -) -> Result { - ResolvableLocalToolchainName::try_from(value) -} - /// LocalToolchainName can be used in calls to Cfg that alter configuration, /// like setting overrides, or that depend on configuration, like calculating /// the toolchain directory. It is not used to model the RUSTUP_TOOLCHAIN diff --git a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml index 34ec8113e2..afc83aa2bd 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml @@ -9,8 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] 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 ... self Modify the rustup installation @@ -24,6 +22,8 @@ Commands: target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify toolchain overrides for directories + run Run a command with an environment configured for a given toolchain + which Display which binary will be run for a given command 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 index 71631bfbde..095dad1c26 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml @@ -9,8 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] 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 ... self Modify the rustup installation @@ -24,6 +22,8 @@ Commands: target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify toolchain overrides for directories + run Run a command with an environment configured for a given toolchain + which Display which binary will be run for a given command help Print this message or the help of the given subcommand(s) Arguments: diff --git a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml index 0676d20a84..aa554d859c 100644 --- a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml @@ -9,8 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] 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 ... self Modify the rustup installation @@ -24,6 +22,8 @@ Commands: target Modify a toolchain's supported targets component Modify a toolchain's installed components override Modify toolchain overrides for directories + run Run a command with an environment configured for a given toolchain + which Display which binary will be run for a given command help Print this message or the help of the given subcommand(s) Arguments: diff --git a/tests/suite/cli-ui/rustup/rustup_run_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_run_cmd_help_flag_stdout.toml index 649ec2db53..d5cfcb74ae 100644 --- a/tests/suite/cli-ui/rustup/rustup_run_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_run_cmd_help_flag_stdout.toml @@ -5,12 +5,12 @@ stdout = """ ... Run a command with an environment configured for a given toolchain -Usage: rustup[EXE] run [OPTIONS] ... +Usage: rustup[EXE] run [OPTIONS] ... Arguments: - Toolchain name, such as 'stable', 'nightly', '1.8.0', or a custom toolchain name, or + Toolchain name, such as 'stable', 'nightly', '1.8.0', or a custom toolchain name, or an absolute path. For more information see `rustup help toolchain` - ... + ... Options: --install Install the requested toolchain if needed diff --git a/tests/suite/cli-ui/rustup/rustup_which_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_which_cmd_help_flag_stdout.toml index 4810dcbe27..77f0163398 100644 --- a/tests/suite/cli-ui/rustup/rustup_which_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_which_cmd_help_flag_stdout.toml @@ -5,13 +5,13 @@ stdout = """ ... Display which binary will be run for a given command -Usage: rustup[EXE] which [OPTIONS] +Usage: rustup[EXE] which [OPTIONS] Arguments: - + Options: - --toolchain Toolchain name, such as 'stable', 'nightly', '1.8.0', or a custom + --toolchain Toolchain name, such as 'stable', 'nightly', '1.8.0', or a custom toolchain name. For more information see `rustup help toolchain` -h, --help Print help """ From 3d0d9cded728619c5a609b9e0fa7073fc98c73a7 Mon Sep 17 00:00:00 2001 From: rami3l Date: Mon, 22 Jan 2024 16:24:58 +0800 Subject: [PATCH 09/13] refactor(cli): rewrite `rustup doc` with `clap-derive` --- src/cli/rustup_mode.rs | 160 ++++++++++-------- .../rustup_doc_cmd_help_flag_stdout.toml | 8 +- .../cli-ui/rustup/rustup_help_cmd_stdout.toml | 2 +- .../rustup/rustup_help_flag_stdout.toml | 2 +- .../rustup/rustup_only_options_stdout.toml | 2 +- 5 files changed, 98 insertions(+), 76 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 649575e092..e89537f1bb 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -6,8 +6,7 @@ use std::str::FromStr; use anyhow::{anyhow, Error, Result}; use clap::{ builder::{EnumValueParser, PossibleValue, PossibleValuesParser}, - Arg, ArgAction, ArgGroup, ArgMatches, Args, Command, FromArgMatches as _, Parser, Subcommand, - ValueEnum, + Arg, ArgAction, ArgMatches, Args, Command, FromArgMatches as _, Parser, Subcommand, ValueEnum, }; use clap_complete::Shell; use itertools::Itertools; @@ -187,6 +186,26 @@ enum RustupSubcmd { #[arg(long, help = RESOLVABLE_TOOLCHAIN_ARG_HELP)] toolchain: Option, }, + + /// Open the documentation for the current toolchain + #[command( + alias = "docs", + after_help = DOC_HELP, + )] + Doc { + /// Only print the path to the documentation + #[arg(long)] + path: bool, + + #[arg(long, help = OFFICIAL_TOOLCHAIN_ARG_HELP)] + toolchain: Option, + + #[arg(help = TOPIC_ARG_HELP)] + topic: Option, + + #[command(flatten)] + page: DocPage, + }, } #[derive(Debug, Subcommand)] @@ -479,6 +498,12 @@ impl Rustup { install, } => run(cfg, toolchain, command, install), RustupSubcmd::Which { command, toolchain } => which(cfg, &command, toolchain), + RustupSubcmd::Doc { + path, + toolchain, + topic, + page, + } => doc(cfg, path, toolchain, topic.as_deref(), &page), } } } @@ -556,10 +581,10 @@ pub fn main() -> Result { Some(s) => match s { ( "dump-testament" | "show" | "update" | "install" | "uninstall" | "toolchain" - | "check" | "default" | "target" | "component" | "override" | "run" | "which", + | "check" | "default" | "target" | "component" | "override" | "run" | "which" + | "doc", _, ) => Rustup::from_arg_matches(&matches)?.dispatch(cfg)?, - ("doc", m) => doc(cfg, m)?, #[cfg(not(windows))] ("man", m) => man(cfg, m)?, ("self", c) => match c.subcommand() { @@ -629,45 +654,6 @@ 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}"))) } }), - ) - .subcommand( - Command::new("doc") - .alias("docs") - .about("Open the documentation for the current toolchain") - .after_help(DOC_HELP) - .arg( - Arg::new("path") - .long("path") - .help("Only print the path to the documentation") - .action(ArgAction::SetTrue), - ) - .arg( - Arg::new("toolchain") - .help(OFFICIAL_TOOLCHAIN_ARG_HELP) - .long("toolchain") - .num_args(1) - .value_parser(partial_toolchain_desc_parser), - ) - .arg(Arg::new("topic").help(TOPIC_ARG_HELP)) - .group( - ArgGroup::new("page").args( - DOCS_DATA - .iter() - .map(|(name, _, _)| *name) - .collect::>(), - ), - ) - .args( - &DOCS_DATA - .iter() - .map(|&(name, help_msg, _)| { - Arg::new(name) - .long(name) - .help(help_msg) - .action(ArgAction::SetTrue) - }) - .collect::>(), - ), ); if cfg!(not(target_os = "windows")) { @@ -1444,28 +1430,67 @@ fn override_remove(cfg: &Cfg, path: Option<&Path>, nonexistent: bool) -> Result< Ok(utils::ExitCode(0)) } -const DOCS_DATA: &[(&str, &str, &str)] = &[ +macro_rules! docs_data { + ( + $( + $( #[$meta:meta] )* + ($ident:ident, $help:expr, $path:expr $(,)?) + ),+ $(,)? + ) => { + #[derive(Debug, Args)] + struct DocPage { + $( + #[doc = $help] + #[arg(long, group = "page")] + $( #[$meta] )* + $ident: bool, + )+ + } + + impl DocPage { + fn name(&self) -> Option<&'static str> { + Some(self.path()?.rsplit_once('/')?.0) + } + + fn path(&self) -> Option<&'static str> { + $( if self.$ident { return Some($path); } )+ + None + } + } + }; +} + +docs_data![ // flags can be used to open specific documents, e.g. `rustup doc --nomicon` // tuple elements: document name used as flag, help message, document index path - ("alloc", "The Rust core allocation and collections library", "alloc/index.html"), - ("book", "The Rust Programming Language book", "book/index.html"), - ("cargo", "The Cargo Book", "cargo/index.html"), - ("core", "The Rust Core Library", "core/index.html"), - ("edition-guide", "The Rust Edition Guide", "edition-guide/index.html"), - ("nomicon", "The Dark Arts of Advanced and Unsafe Rust Programming", "nomicon/index.html"), - ("proc_macro", "A support library for macro authors when defining new macros", "proc_macro/index.html"), - ("reference", "The Rust Reference", "reference/index.html"), - ("rust-by-example", "A collection of runnable examples that illustrate various Rust concepts and standard libraries", "rust-by-example/index.html"), - ("rustc", "The compiler for the Rust programming language", "rustc/index.html"), - ("rustdoc", "Documentation generator for Rust projects", "rustdoc/index.html"), - ("std", "Standard library API documentation", "std/index.html"), - ("test", "Support code for rustc's built in unit-test and micro-benchmarking framework", "test/index.html"), - ("unstable-book", "The Unstable Book", "unstable-book/index.html"), - ("embedded-book", "The Embedded Rust Book", "embedded-book/index.html"), + (alloc, "The Rust core allocation and collections library", "alloc/index.html"), + (book, "The Rust Programming Language book", "book/index.html"), + (cargo, "The Cargo Book", "cargo/index.html"), + (core, "The Rust Core Library", "core/index.html"), + (edition_guide, "The Rust Edition Guide", "edition-guide/index.html"), + (nomicon, "The Dark Arts of Advanced and Unsafe Rust Programming", "nomicon/index.html"), + + #[arg(long = "proc_macro")] + (proc_macro, "A support library for macro authors when defining new macros", "proc_macro/index.html"), + + (reference, "The Rust Reference", "reference/index.html"), + (rust_by_example, "A collection of runnable examples that illustrate various Rust concepts and standard libraries", "rust-by-example/index.html"), + (rustc, "The compiler for the Rust programming language", "rustc/index.html"), + (rustdoc, "Documentation generator for Rust projects", "rustdoc/index.html"), + (std, "Standard library API documentation", "std/index.html"), + (test, "Support code for rustc's built in unit-test and micro-benchmarking framework", "test/index.html"), + (unstable_book, "The Unstable Book", "unstable-book/index.html"), + (embedded_book, "The Embedded Rust Book", "embedded-book/index.html"), ]; -fn doc(cfg: &Cfg, m: &ArgMatches) -> Result { - let toolchain = explicit_desc_or_dir_toolchain_old(cfg, m)?; +fn doc( + cfg: &Cfg, + path_only: bool, + toolchain: Option, + mut topic: Option<&str>, + doc_page: &DocPage, +) -> Result { + let toolchain = explicit_desc_or_dir_toolchain(cfg, toolchain)?; if let Ok(distributable) = DistributableToolchain::try_from(&toolchain) { if let [_] = distributable @@ -1493,24 +1518,21 @@ fn doc(cfg: &Cfg, m: &ArgMatches) -> Result { }; let topical_path: PathBuf; - let mut doc_name = m.get_one::("topic").map(|s| s.as_str()); - let doc_url = if let Some(topic) = doc_name { + let doc_url = if let Some(topic) = topic { topical_path = topical_doc::local_path(&toolchain.doc_path("").unwrap(), topic)?; topical_path.to_str().unwrap() - } else if let Some((name, _, path)) = DOCS_DATA.iter().find(|(name, _, _)| m.get_flag(name)) { - doc_name = Some(name); - path } else { - "index.html" + topic = doc_page.name(); + doc_page.path().unwrap_or("index.html") }; - if m.get_flag("path") { + if path_only { let doc_path = toolchain.doc_path(doc_url)?; writeln!(process().stdout().lock(), "{}", doc_path.display())?; Ok(utils::ExitCode(0)) } else { - if let Some(name) = doc_name { + if let Some(name) = topic { writeln!( process().stderr().lock(), "Opening docs named `{name}` in your browser" diff --git a/tests/suite/cli-ui/rustup/rustup_doc_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_doc_cmd_help_flag_stdout.toml index d9698f2820..f4741b6d33 100644 --- a/tests/suite/cli-ui/rustup/rustup_doc_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_doc_cmd_help_flag_stdout.toml @@ -1,19 +1,19 @@ bin.name = "rustup" -args = ["doc","--help"] +args = ["doc", "--help"] stdout = """ ... Open the documentation for the current toolchain -Usage: rustup[EXE] doc [OPTIONS] [topic] +Usage: rustup[EXE] doc [OPTIONS] [TOPIC] Arguments: - [topic] Topic such as 'core', 'fn', 'usize', 'eprintln!', 'core::arch', 'alloc::format!', + [TOPIC] Topic such as 'core', 'fn', 'usize', 'eprintln!', 'core::arch', 'alloc::format!', 'std::fs', 'std::fs::read_dir', 'std::io::Bytes', 'std::iter::Sum', 'std::io::error::Result' etc... Options: --path Only print the path to the documentation - --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more + --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more information see `rustup help toolchain` --alloc The Rust core allocation and collections library --book The Rust Programming Language book diff --git a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml index afc83aa2bd..f278defd17 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml @@ -9,7 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - doc Open the documentation for the current toolchain ... self Modify the rustup installation set Alter rustup settings @@ -24,6 +23,7 @@ Commands: override Modify toolchain overrides for directories 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 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 index 095dad1c26..38a256fdfc 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml @@ -9,7 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - doc Open the documentation for the current toolchain ... self Modify the rustup installation set Alter rustup settings @@ -24,6 +23,7 @@ Commands: override Modify toolchain overrides for directories 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 help Print this message or the help of the given subcommand(s) Arguments: diff --git a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml index aa554d859c..052a314a1d 100644 --- a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml @@ -9,7 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - doc Open the documentation for the current toolchain ... self Modify the rustup installation set Alter rustup settings @@ -24,6 +23,7 @@ Commands: override Modify toolchain overrides for directories 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 help Print this message or the help of the given subcommand(s) Arguments: From 9682e96271a57c5b6cd3f3b8004ae26c38d31147 Mon Sep 17 00:00:00 2001 From: rami3l Date: Mon, 22 Jan 2024 19:39:21 +0800 Subject: [PATCH 10/13] refactor(cli): rewrite `rustup (man|completions)` with `clap-derive` --- src/cli/rustup_mode.rs | 99 +++++++------------ src/toolchain/names.rs | 9 -- ...stup_completions_cmd_help_flag_stdout.toml | 8 +- .../cli-ui/rustup/rustup_help_cmd_stdout.toml | 4 +- .../rustup/rustup_help_flag_stdout.toml | 4 +- .../rustup_man_cmd_help_flag_stdout.toml | 8 +- .../rustup/rustup_only_options_stdout.toml | 4 +- tests/suite/cli_misc.rs | 6 +- 8 files changed, 52 insertions(+), 90 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index e89537f1bb..57acc0a2da 100644 --- 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, + }, + + /// 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 { ( "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 { }, None => unreachable!(), }, - ("completions", c) => { - if let Some(&shell) = c.get_one::("shell") { - output_completion_script( - shell, - c.get_one::("command") - .copied() - .unwrap_or(CompletionCommand::Rustup), - )? - } else { - unreachable!() - } - } _ => unreachable!(), }, None => { @@ -626,7 +634,7 @@ pub fn main() -> Result { } 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::::new())) - .arg( - Arg::new("command") - .value_parser(EnumValueParser::::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> { - let toolchain = m - .get_one::("toolchain") - .map(Into::into); - explicit_or_dir_toolchain2(cfg, toolchain) -} - fn explicit_desc_or_dir_toolchain( cfg: &Cfg, toolchain: Option, @@ -1546,12 +1515,14 @@ fn doc( } #[cfg(not(windows))] -fn man(cfg: &Cfg, m: &ArgMatches) -> Result { +fn man( + cfg: &Cfg, + command: &str, + toolchain: Option, +) -> Result { use crate::currentprocess::varsource::VarSource; - let command = m.get_one::("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 index 611b0c2c07..5457071cbd 100644 --- 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>::Error> {>::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 { - value.parse::() -} - /// 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 index 63ac522164..d6dcb9cb45 100644 --- 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 [COMMAND] Arguments: - [shell] [possible values: bash, elvish, fish, powershell, zsh] - [command] [possible values: rustup, cargo] + [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 index f278defd17..a17088cbf3 100644 --- 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 index 38a256fdfc..22c3dcc902 100644 --- 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 index 5c94c51058..a35b59460c 100644 --- 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] +Usage: rustup[EXE] man [OPTIONS] Arguments: - + Options: - --toolchain Toolchain name, such as 'stable', 'nightly', or '1.8.0'. For more + --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 index 052a314a1d..4d3165c893 100644 --- 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 index efc9efe224..48ee5e6b64 100644 --- 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 ''"#, ); config.expect_err( &["rustup", "completions", "fake", "cargo"], - r#"error: invalid value 'fake' for '[shell]'"#, + r#"error: invalid value 'fake' for ''"#, ); }); } @@ -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]'"#, ); }); } From 0f66cffab0c908bef88d95c65d80ca80cbc4e932 Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 23 Jan 2024 10:38:08 +0800 Subject: [PATCH 11/13] refactor(cli): rewrite `rustup (self|set)` with `clap-derive` --- src/cli/rustup_mode.rs | 183 ++++++++---------- .../cli-ui/rustup/rustup_help_cmd_stdout.toml | 4 +- .../rustup/rustup_help_flag_stdout.toml | 4 +- .../rustup/rustup_only_options_stdout.toml | 4 +- .../rustup_self_cmd_help_flag_stdout.toml | 6 +- ...lf_cmd_uninstall_cmd_help_flag_stdout.toml | 4 +- ...md_upgrade-data _cmd_help_flag_stdout.toml | 4 +- ...auto-self-update_cmd_help_flag_stdout.toml | 6 +- ...cmd_default-host_cmd_help_flag_stdout.toml | 6 +- ..._set_cmd_profile_cmd_help_flag_stdout.toml | 6 +- 10 files changed, 104 insertions(+), 123 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 57acc0a2da..d71351d1c1 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -6,7 +6,7 @@ use std::str::FromStr; use anyhow::{anyhow, Error, Result}; use clap::{ builder::{PossibleValue, PossibleValuesParser}, - Arg, ArgAction, ArgMatches, Args, Command, FromArgMatches as _, Parser, Subcommand, ValueEnum, + Arg, ArgAction, Args, Command, FromArgMatches as _, Parser, Subcommand, ValueEnum, }; use clap_complete::Shell; use itertools::Itertools; @@ -215,6 +215,18 @@ enum RustupSubcmd { toolchain: Option, }, + /// Modify the rustup installation + Self_ { + #[command(subcommand)] + subcmd: SelfSubcmd, + }, + + /// Alter rustup settings + Set { + #[command(subcommand)] + subcmd: SetSubcmd, + }, + /// Generate tab-completion scripts for your shell #[command(after_help = COMPLETIONS_HELP, arg_required_else_help = true)] Completions { @@ -439,6 +451,51 @@ enum OverrideSubcmd { }, } +#[derive(Debug, Subcommand)] +#[command( + name = "self", + arg_required_else_help = true, + subcommand_required = true +)] +enum SelfSubcmd { + /// Download and install updates to rustup + Update, + + /// Uninstall rustup + Uninstall { + #[arg(short = 'y')] + no_prompt: bool, + }, + + /// Upgrade the internal data format + UpgradeData, +} + +#[derive(Debug, Subcommand)] +#[command(arg_required_else_help = true, subcommand_required = true)] +enum SetSubcmd { + /// The triple used to identify toolchains when not specified + DefaultHost { host_triple: String }, + + /// The default components installed with a toolchain + Profile { + #[arg( + default_value = Profile::default_name(), + value_parser = PossibleValuesParser::new(Profile::names()), + )] + profile_name: String, + }, + + /// The rustup auto self update mode + AutoSelfUpdate { + #[arg( + default_value = SelfUpdateMode::default_mode(), + value_parser = PossibleValuesParser::new(SelfUpdateMode::modes()), + )] + auto_self_update_mode: String, + }, +} + impl Rustup { fn dispatch(self, cfg: &mut Cfg) -> Result { match self.subcmd { @@ -523,6 +580,20 @@ impl Rustup { } => doc(cfg, path, toolchain, topic.as_deref(), &page), #[cfg(not(windows))] RustupSubcmd::Man { command, toolchain } => man(cfg, &command, toolchain), + RustupSubcmd::Self_ { subcmd } => match subcmd { + SelfSubcmd::Update => self_update::update(cfg), + SelfSubcmd::Uninstall { no_prompt } => self_update::uninstall(no_prompt), + SelfSubcmd::UpgradeData => upgrade_data(cfg), + }, + RustupSubcmd::Set { subcmd } => match subcmd { + SetSubcmd::DefaultHost { host_triple } => { + set_default_host_triple(cfg, &host_triple) + } + SetSubcmd::Profile { profile_name } => set_profile(cfg, &profile_name), + SetSubcmd::AutoSelfUpdate { + auto_self_update_mode, + } => set_auto_self_update(cfg, &auto_self_update_mode), + }, RustupSubcmd::Completions { shell, command } => { output_completion_script(shell, command) } @@ -593,39 +664,10 @@ pub fn main() -> Result { cfg.set_toolchain_override(t); } - if maybe_upgrade_data(cfg, &matches)? { - return Ok(utils::ExitCode(0)); - } - cfg.check_metadata_version()?; Ok(match matches.subcommand() { - Some(s) => match s { - ( - "dump-testament" | "show" | "update" | "install" | "uninstall" | "toolchain" - | "check" | "default" | "target" | "component" | "override" | "run" | "which" - | "doc" | "man" | "completions", - _, - ) => Rustup::from_arg_matches(&matches)?.dispatch(cfg)?, - ("self", c) => match c.subcommand() { - Some(s) => match s { - ("update", _) => self_update::update(cfg)?, - ("uninstall", m) => self_uninstall(m)?, - _ => unreachable!(), - }, - None => unreachable!(), - }, - ("set", c) => match c.subcommand() { - Some(s) => match s { - ("default-host", m) => set_default_host_triple(cfg, m)?, - ("profile", m) => set_profile(cfg, m)?, - ("auto-self-update", m) => set_auto_self_update(cfg, m)?, - _ => unreachable!(), - }, - None => unreachable!(), - }, - _ => unreachable!(), - }, + Some(_) => Rustup::from_arg_matches(&matches)?.dispatch(cfg)?, None => { eprintln!("{}", cli().render_long_help()); utils::ExitCode(1) @@ -662,54 +704,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}"))) } }), - ) - .subcommand( - Command::new("self") - .about("Modify the rustup installation") - .subcommand_required(true) - .arg_required_else_help(true) - .subcommand(Command::new("update").about("Download and install updates to rustup")) - .subcommand( - Command::new("uninstall") - .about("Uninstall rustup.") - .arg(Arg::new("no-prompt").short('y').action(ArgAction::SetTrue)), - ) - .subcommand( - Command::new("upgrade-data").about("Upgrade the internal data format."), - ), - ) - .subcommand( - Command::new("set") - .about("Alter rustup settings") - .subcommand_required(true) - .arg_required_else_help(true) - .subcommand( - Command::new("default-host") - .about("The triple used to identify toolchains when not specified") - .arg(Arg::new("host_triple").required(true)), - ) - .subcommand( - Command::new("profile") - .about("The default components installed with a toolchain") - .arg( - Arg::new("profile-name") - .required(true) - .value_parser(PossibleValuesParser::new(Profile::names())) - .default_value(Profile::default_name()), - ), - ) - .subcommand( - Command::new("auto-self-update") - .about("The rustup auto self update mode") - .arg( - Arg::new("auto-self-update-mode") - .required(true) - .value_parser(PossibleValuesParser::new(SelfUpdateMode::modes())) - .default_value(SelfUpdateMode::default_mode()), - ), - ), ); - RustupSubcmd::augment_subcommands(app) } @@ -721,17 +716,9 @@ fn verbose_arg(help: &'static str) -> Arg { .action(ArgAction::SetTrue) } -fn maybe_upgrade_data(cfg: &Cfg, m: &ArgMatches) -> Result { - match m.subcommand() { - Some(("self", c)) => match c.subcommand() { - Some(("upgrade-data", _)) => { - cfg.upgrade_data()?; - Ok(true) - } - _ => Ok(false), - }, - _ => Ok(false), - } +fn upgrade_data(cfg: &Cfg) -> Result { + cfg.upgrade_data()?; + Ok(utils::ExitCode(0)) } fn default_(cfg: &Cfg, toolchain: Option) -> Result { @@ -1541,23 +1528,17 @@ fn man( Ok(utils::ExitCode(0)) } -fn self_uninstall(m: &ArgMatches) -> Result { - let no_prompt = m.get_flag("no-prompt"); - - self_update::uninstall(no_prompt) -} - -fn set_default_host_triple(cfg: &Cfg, m: &ArgMatches) -> Result { - cfg.set_default_host_triple(m.get_one::("host_triple").unwrap())?; +fn set_default_host_triple(cfg: &Cfg, host_triple: &str) -> Result { + cfg.set_default_host_triple(host_triple)?; Ok(utils::ExitCode(0)) } -fn set_profile(cfg: &mut Cfg, m: &ArgMatches) -> Result { - cfg.set_profile(m.get_one::("profile-name").unwrap())?; +fn set_profile(cfg: &mut Cfg, profile: &str) -> Result { + cfg.set_profile(profile)?; Ok(utils::ExitCode(0)) } -fn set_auto_self_update(cfg: &mut Cfg, m: &ArgMatches) -> Result { +fn set_auto_self_update(cfg: &mut Cfg, auto_self_update_mode: &str) -> Result { if self_update::NEVER_SELF_UPDATE { let mut args = crate::process().args_os(); let arg0 = args.next().map(PathBuf::from); @@ -1567,7 +1548,7 @@ fn set_auto_self_update(cfg: &mut Cfg, m: &ArgMatches) -> Result("auto-self-update-mode").unwrap())?; + cfg.set_auto_self_update(auto_self_update_mode)?; Ok(utils::ExitCode(0)) } diff --git a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml index a17088cbf3..eba5f967c7 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml @@ -9,8 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - self Modify the rustup installation - set Alter rustup settings show Show the active and installed toolchains or profiles update Update Rust toolchains and rustup check Check for updates to Rust toolchains and rustup @@ -23,6 +21,8 @@ Commands: which Display which binary will be run for a given command doc Open the documentation for the current toolchain ... + self Modify the rustup installation + set Alter rustup settings completions Generate tab-completion scripts for your shell help Print this message or the help of the given subcommand(s) diff --git a/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml index 22c3dcc902..d1b11958f9 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml @@ -9,8 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - self Modify the rustup installation - set Alter rustup settings show Show the active and installed toolchains or profiles update Update Rust toolchains and rustup check Check for updates to Rust toolchains and rustup @@ -23,6 +21,8 @@ Commands: which Display which binary will be run for a given command doc Open the documentation for the current toolchain ... + self Modify the rustup installation + set Alter rustup settings completions Generate tab-completion scripts for your shell help Print this message or the help of the given subcommand(s) diff --git a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml index 4d3165c893..51718cdd3d 100644 --- a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml @@ -9,8 +9,6 @@ The Rust toolchain installer Usage: rustup[EXE] [OPTIONS] [+toolchain] [COMMAND] Commands: - self Modify the rustup installation - set Alter rustup settings show Show the active and installed toolchains or profiles update Update Rust toolchains and rustup check Check for updates to Rust toolchains and rustup @@ -23,6 +21,8 @@ Commands: which Display which binary will be run for a given command doc Open the documentation for the current toolchain ... + self Modify the rustup installation + set Alter rustup settings completions Generate tab-completion scripts for your shell help Print this message or the help of the given subcommand(s) diff --git a/tests/suite/cli-ui/rustup/rustup_self_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_self_cmd_help_flag_stdout.toml index 966d1c1917..b1cf564245 100644 --- a/tests/suite/cli-ui/rustup/rustup_self_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_self_cmd_help_flag_stdout.toml @@ -1,5 +1,5 @@ bin.name = "rustup" -args = ["self","--help"] +args = ["self", "--help"] stdout = """ ... Modify the rustup installation @@ -8,8 +8,8 @@ Usage: rustup[EXE] self Commands: update Download and install updates to rustup - uninstall Uninstall rustup. - upgrade-data Upgrade the internal data format. + uninstall Uninstall rustup + upgrade-data Upgrade the internal data format help Print this message or the help of the given subcommand(s) Options: diff --git a/tests/suite/cli-ui/rustup/rustup_self_cmd_uninstall_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_self_cmd_uninstall_cmd_help_flag_stdout.toml index d8bbc2ad57..1a420ff446 100644 --- a/tests/suite/cli-ui/rustup/rustup_self_cmd_uninstall_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_self_cmd_uninstall_cmd_help_flag_stdout.toml @@ -1,8 +1,8 @@ bin.name = "rustup" -args = ["self","uninstall","--help"] +args = ["self", "uninstall", "--help"] stdout = """ ... -Uninstall rustup. +Uninstall rustup Usage: rustup[EXE] self uninstall [OPTIONS] diff --git a/tests/suite/cli-ui/rustup/rustup_self_cmd_upgrade-data _cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_self_cmd_upgrade-data _cmd_help_flag_stdout.toml index 01f1c4bdd8..590616e25e 100644 --- a/tests/suite/cli-ui/rustup/rustup_self_cmd_upgrade-data _cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_self_cmd_upgrade-data _cmd_help_flag_stdout.toml @@ -1,8 +1,8 @@ bin.name = "rustup" -args = ["self","upgrade-data","--help"] +args = ["self", "upgrade-data", "--help"] stdout = """ ... -Upgrade the internal data format. +Upgrade the internal data format Usage: rustup[EXE] self upgrade-data diff --git a/tests/suite/cli-ui/rustup/rustup_set_cmd_auto-self-update_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_set_cmd_auto-self-update_cmd_help_flag_stdout.toml index 854c81ad69..021708b0cb 100644 --- a/tests/suite/cli-ui/rustup/rustup_set_cmd_auto-self-update_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_set_cmd_auto-self-update_cmd_help_flag_stdout.toml @@ -1,13 +1,13 @@ bin.name = "rustup" -args = ["set","auto-self-update","--help"] +args = ["set", "auto-self-update", "--help"] stdout = """ ... The rustup auto self update mode -Usage: rustup[EXE] set auto-self-update +Usage: rustup[EXE] set auto-self-update [AUTO_SELF_UPDATE_MODE] Arguments: - [default: enable] [possible values: enable, disable, check-only] + [AUTO_SELF_UPDATE_MODE] [default: enable] [possible values: enable, disable, check-only] Options: -h, --help Print help diff --git a/tests/suite/cli-ui/rustup/rustup_set_cmd_default-host_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_set_cmd_default-host_cmd_help_flag_stdout.toml index bdab25cd07..74fef3a8a0 100644 --- a/tests/suite/cli-ui/rustup/rustup_set_cmd_default-host_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_set_cmd_default-host_cmd_help_flag_stdout.toml @@ -1,13 +1,13 @@ bin.name = "rustup" -args = ["set","default-host","--help"] +args = ["set", "default-host", "--help"] stdout = """ ... The triple used to identify toolchains when not specified -Usage: rustup[EXE] set default-host +Usage: rustup[EXE] set default-host Arguments: - + Options: -h, --help Print help diff --git a/tests/suite/cli-ui/rustup/rustup_set_cmd_profile_cmd_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_set_cmd_profile_cmd_help_flag_stdout.toml index 9ef3527c4e..15939409dd 100644 --- a/tests/suite/cli-ui/rustup/rustup_set_cmd_profile_cmd_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_set_cmd_profile_cmd_help_flag_stdout.toml @@ -1,13 +1,13 @@ bin.name = "rustup" -args = ["set","profile","--help"] +args = ["set", "profile", "--help"] stdout = """ ... The default components installed with a toolchain -Usage: rustup[EXE] set profile +Usage: rustup[EXE] set profile [PROFILE_NAME] Arguments: - [default: default] [possible values: minimal, default, complete] + [PROFILE_NAME] [default: default] [possible values: minimal, default, complete] Options: -h, --help Print help From 1e2b5cdc4337bd47cd1ce862d6986f028de1c64d Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 23 Jan 2024 13:00:28 +0800 Subject: [PATCH 12/13] refactor(cli): rewrite `rustup` itself with `clap-derive` --- src/cli/rustup_mode.rs | 98 +++++++++---------- .../cli-ui/rustup/rustup_help_cmd_stdout.toml | 2 +- .../rustup/rustup_help_flag_stdout.toml | 2 +- .../rustup/rustup_only_options_stdout.toml | 2 +- 4 files changed, 47 insertions(+), 57 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index d71351d1c1..874aa7b0b0 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -6,7 +6,7 @@ use std::str::FromStr; use anyhow::{anyhow, Error, Result}; use clap::{ builder::{PossibleValue, PossibleValuesParser}, - Arg, ArgAction, Args, Command, FromArgMatches as _, Parser, Subcommand, ValueEnum, + Args, CommandFactory, Parser, Subcommand, ValueEnum, }; use clap_complete::Shell; use itertools::Itertools; @@ -64,15 +64,43 @@ fn handle_epipe(res: Result) -> Result { } } +/// The Rust toolchain installer #[derive(Debug, Parser)] #[command( name = "rustup", bin_name = "rustup[EXE]", version = common::version(), + before_help = format!("rustup {}", common::version()), + after_help = RUSTUP_HELP, )] struct Rustup { + /// Enable verbose output + #[arg(short, long)] + verbose: bool, + + /// Disable progress output + #[arg(short, long, conflicts_with = "verbose")] + quiet: bool, + + /// Release channel (e.g. +stable) or custom toolchain to set override + #[arg( + name = "+toolchain", + value_parser = plus_toolchain_value_parser, + )] + plus_toolchain: Option, + #[command(subcommand)] - subcmd: RustupSubcmd, + subcmd: Option, +} + +fn plus_toolchain_value_parser(s: &str) -> clap::error::Result { + use clap::{error::ErrorKind, Error}; + if let Some(stripped) = s.strip_prefix('+') { + ResolvableToolchainName::try_from(stripped) + .map_err(|e| Error::raw(ErrorKind::InvalidValue, e)) + } else { + 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}"))) + } } #[derive(Debug, Subcommand)] @@ -496,9 +524,9 @@ enum SetSubcmd { }, } -impl Rustup { +impl RustupSubcmd { fn dispatch(self, cfg: &mut Cfg) -> Result { - match self.subcmd { + match self { RustupSubcmd::DumpTestament => common::dump_testament(), RustupSubcmd::Install { opts } => update(cfg, opts), RustupSubcmd::Uninstall { opts } => toolchain_remove(cfg, opts), @@ -606,7 +634,7 @@ pub fn main() -> Result { self_update::cleanup_self_updater()?; use clap::error::ErrorKind::*; - let matches = match cli().try_get_matches_from(process().args_os()) { + let matches = match Rustup::try_parse_from(process().args_os()) { Ok(matches) => Ok(matches), Err(err) if err.kind() == DisplayHelp => { write!(process().stdout().lock(), "{err}")?; @@ -656,66 +684,23 @@ pub fn main() -> Result { Err(err) } }?; - let verbose = matches.get_flag("verbose"); - let quiet = matches.get_flag("quiet"); - let cfg = &mut common::set_globals(verbose, quiet)?; + let cfg = &mut common::set_globals(matches.verbose, matches.quiet)?; - if let Some(t) = matches.get_one::("+toolchain") { + if let Some(t) = &matches.plus_toolchain { cfg.set_toolchain_override(t); } cfg.check_metadata_version()?; - Ok(match matches.subcommand() { - Some(_) => Rustup::from_arg_matches(&matches)?.dispatch(cfg)?, + Ok(match matches.subcmd { + Some(subcmd) => subcmd.dispatch(cfg)?, None => { - eprintln!("{}", cli().render_long_help()); + eprintln!("{}", Rustup::command().render_long_help()); utils::ExitCode(1) } }) } -pub(crate) fn cli() -> Command { - let app = Command::new("rustup") - .version(common::version()) - .about("The Rust toolchain installer") - .before_help(format!("rustup {}", common::version())) - .after_help(RUSTUP_HELP) - .subcommand_required(false) - .arg( - verbose_arg("Enable verbose output"), - ) - .arg( - Arg::new("quiet") - .conflicts_with("verbose") - .help("Disable progress output") - .short('q') - .long("quiet") - .action(ArgAction::SetTrue), - ) - .arg( - Arg::new("+toolchain") - .help("release channel (e.g. +stable) or custom toolchain to set override") - .value_parser(|s: &str| { - use clap::{Error, error::ErrorKind}; - if let Some(stripped) = s.strip_prefix('+') { - ResolvableToolchainName::try_from(stripped).map_err(|e| Error::raw(ErrorKind::InvalidValue, e)) - } else { - 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}"))) - } - }), - ); - RustupSubcmd::augment_subcommands(app) -} - -fn verbose_arg(help: &'static str) -> Arg { - Arg::new("verbose") - .help(help) - .short('v') - .long("verbose") - .action(ArgAction::SetTrue) -} - fn upgrade_data(cfg: &Cfg) -> Result { cfg.upgrade_data()?; Ok(utils::ExitCode(0)) @@ -1589,7 +1574,12 @@ impl fmt::Display for CompletionCommand { fn output_completion_script(shell: Shell, command: CompletionCommand) -> Result { match command { CompletionCommand::Rustup => { - clap_complete::generate(shell, &mut cli(), "rustup", &mut process().stdout().lock()); + clap_complete::generate( + shell, + &mut Rustup::command(), + "rustup", + &mut process().stdout().lock(), + ); } CompletionCommand::Cargo => { if let Shell::Zsh = shell { diff --git a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml index eba5f967c7..b6771fb5c3 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_cmd_stdout.toml @@ -27,7 +27,7 @@ Commands: help Print this message or the help of the given subcommand(s) Arguments: - [+toolchain] release channel (e.g. +stable) or custom toolchain to set override + [+toolchain] Release channel (e.g. +stable) or custom toolchain to set override Options: -v, --verbose Enable verbose output diff --git a/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml b/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml index d1b11958f9..4fa43f881b 100644 --- a/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_help_flag_stdout.toml @@ -27,7 +27,7 @@ Commands: help Print this message or the help of the given subcommand(s) Arguments: - [+toolchain] release channel (e.g. +stable) or custom toolchain to set override + [+toolchain] Release channel (e.g. +stable) or custom toolchain to set override Options: -v, --verbose Enable verbose output diff --git a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml index 51718cdd3d..e0d9a66f54 100644 --- a/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml +++ b/tests/suite/cli-ui/rustup/rustup_only_options_stdout.toml @@ -28,7 +28,7 @@ Commands: Arguments: [+toolchain] - release channel (e.g. +stable) or custom toolchain to set override + Release channel (e.g. +stable) or custom toolchain to set override Options: -v, --verbose From 986b302572d5f2563603dd1c5530d6ee8eb5bea8 Mon Sep 17 00:00:00 2001 From: rami3l Date: Fri, 10 May 2024 18:54:33 +0800 Subject: [PATCH 13/13] refactor(cli): hoist the `handle_epipe()` call out of the `match` --- src/cli/rustup_mode.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 874aa7b0b0..23178e856b 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -530,14 +530,14 @@ impl RustupSubcmd { RustupSubcmd::DumpTestament => common::dump_testament(), RustupSubcmd::Install { opts } => update(cfg, opts), RustupSubcmd::Uninstall { opts } => toolchain_remove(cfg, opts), - RustupSubcmd::Show { verbose, subcmd } => match subcmd { - None => handle_epipe(show(cfg, verbose)), + RustupSubcmd::Show { verbose, subcmd } => handle_epipe(match subcmd { + None => show(cfg, verbose), Some(ShowSubcmd::ActiveToolchain { verbose }) => { - handle_epipe(show_active_toolchain(cfg, verbose)) + show_active_toolchain(cfg, verbose) } - Some(ShowSubcmd::Home) => handle_epipe(show_rustup_home(cfg)), - Some(ShowSubcmd::Profile) => handle_epipe(show_profile(cfg)), - }, + Some(ShowSubcmd::Home) => show_rustup_home(cfg), + Some(ShowSubcmd::Profile) => show_profile(cfg), + }), RustupSubcmd::Update { toolchain, no_self_update,