From 91a2faad142231e3d2a91ae23c33558c4c10cf99 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 18 Aug 2023 10:54:41 +0100 Subject: [PATCH] refactor(arg): use `UnknownArgumentValueParser` to suggest arg for unknown args --- src/bin/cargo/commands/bench.rs | 12 +------- src/bin/cargo/commands/test.rs | 12 +------- src/bin/cargo/commands/vendor.rs | 48 +++++++++++-------------------- src/cargo/util/command_prelude.rs | 7 +++++ 4 files changed, 25 insertions(+), 54 deletions(-) diff --git a/src/bin/cargo/commands/bench.rs b/src/bin/cargo/commands/bench.rs index 9c36422c47db..971dc9456483 100644 --- a/src/bin/cargo/commands/bench.rs +++ b/src/bin/cargo/commands/bench.rs @@ -43,7 +43,7 @@ pub fn cli() -> Command { ) .arg_features() .arg_jobs() - .arg(flag("keep-going", "Use `--no-fail-fast` instead").hide(true)) // See rust-lang/cargo#11702 + .arg_unsupported_keep_going() .arg_profile("Build artifacts with the specified profile") .arg_target_triple("Build for the target triple") .arg_target_dir() @@ -56,16 +56,6 @@ pub fn cli() -> Command { pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let ws = args.workspace(config)?; - if args.keep_going() { - return Err(anyhow::format_err!( - "\ -unexpected argument `--keep-going` found - - tip: to run as many benchmarks as possible without failing fast, use `--no-fail-fast`" - ) - .into()); - } - let mut compile_opts = args.compile_options( config, CompileMode::Bench, diff --git a/src/bin/cargo/commands/test.rs b/src/bin/cargo/commands/test.rs index 8313851f3a73..f8ec0e2f9460 100644 --- a/src/bin/cargo/commands/test.rs +++ b/src/bin/cargo/commands/test.rs @@ -49,7 +49,7 @@ pub fn cli() -> Command { ) .arg_features() .arg_jobs() - .arg(flag("keep-going", "Use `--no-fail-fast` instead").hide(true)) // See rust-lang/cargo#11702 + .arg_unsupported_keep_going() .arg_release("Build artifacts in release mode, with optimizations") .arg_profile("Build artifacts with the specified profile") .arg_target_triple("Build for the target triple") @@ -66,16 +66,6 @@ pub fn cli() -> Command { pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { let ws = args.workspace(config)?; - if args.keep_going() { - return Err(anyhow::format_err!( - "\ -unexpected argument `--keep-going` found - - tip: to run as many tests as possible without failing fast, use `--no-fail-fast`" - ) - .into()); - } - let mut compile_opts = args.compile_options( config, CompileMode::Test, diff --git a/src/bin/cargo/commands/vendor.rs b/src/bin/cargo/commands/vendor.rs index 69b4ee3802c8..f0c1ecad061a 100644 --- a/src/bin/cargo/commands/vendor.rs +++ b/src/bin/cargo/commands/vendor.rs @@ -32,15 +32,27 @@ pub fn cli() -> Command { "versioned-dirs", "Always include version in subdir name", )) - .arg(flag("no-merge-sources", "Not supported").hide(true)) - .arg(flag("relative-path", "Not supported").hide(true)) - .arg(flag("only-git-deps", "Not supported").hide(true)) - .arg(flag("disallow-duplicates", "Not supported").hide(true)) + .arg(unsupported("no-merge-sources")) + .arg(unsupported("relative-path")) + .arg(unsupported("only-git-deps")) + .arg(unsupported("disallow-duplicates")) .arg_quiet() .arg_manifest_path() .after_help("Run `cargo help vendor` for more detailed information.\n") } +fn unsupported(name: &'static str) -> Arg { + // When we moved `cargo vendor` into Cargo itself we didn't stabilize a few + // flags, so try to provide a helpful error message in that case to ensure + // that users currently using the flag aren't tripped up. + let value_parser = clap::builder::UnknownArgumentValueParser::suggest("the crates.io `cargo vendor` command has been merged into Cargo") + .and_suggest(format!("and the flag `--{name}` isn't supported currently")) + .and_suggest("to continue using the flag, execute `cargo-vendor vendor ...`") + .and_suggest("to suggest this flag supported in Cargo, file an issue at "); + + flag(name, "").value_parser(value_parser).hide(true) +} + pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { // We're doing the vendoring operation ourselves, so we don't actually want // to respect any of the `source` configuration in Cargo itself. That's @@ -50,34 +62,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { config.values_mut()?.remove("source"); } - // When we moved `cargo vendor` into Cargo itself we didn't stabilize a few - // flags, so try to provide a helpful error message in that case to ensure - // that users currently using the flag aren't tripped up. - let crates_io_cargo_vendor_flag = if args.flag("no-merge-sources") { - Some("--no-merge-sources") - } else if args.flag("relative-path") { - Some("--relative-path") - } else if args.flag("only-git-deps") { - Some("--only-git-deps") - } else if args.flag("disallow-duplicates") { - Some("--disallow-duplicates") - } else { - None - }; - if let Some(flag) = crates_io_cargo_vendor_flag { - return Err(anyhow::format_err!( - "\ -the crates.io `cargo vendor` command has now been merged into Cargo itself -and does not support the flag `{}` currently; to continue using the flag you -can execute `cargo-vendor vendor ...`, and if you would like to see this flag -supported in Cargo itself please feel free to file an issue at -https://github.com/rust-lang/cargo/issues/new -", - flag - ) - .into()); - } - let ws = args.workspace(config)?; let path = args .get_one::("path") diff --git a/src/cargo/util/command_prelude.rs b/src/cargo/util/command_prelude.rs index 332ac415b53c..3f8f371363eb 100644 --- a/src/cargo/util/command_prelude.rs +++ b/src/cargo/util/command_prelude.rs @@ -14,6 +14,7 @@ use crate::util::{ use crate::CargoResult; use anyhow::bail; use cargo_util::paths; +use clap::builder::UnknownArgumentValueParser; use std::ffi::{OsStr, OsString}; use std::path::Path; use std::path::PathBuf; @@ -102,6 +103,12 @@ pub trait CommandExt: Sized { ) } + fn arg_unsupported_keep_going(self) -> Self { + let msg = "use `--no-fail-fast` to run as many tests as possible regardless of failure"; + let value_parser = UnknownArgumentValueParser::suggest_arg("--no-fail-fast").and_suggest(msg); + self._arg(flag("keep-going", "").value_parser(value_parser).hide(true)) + } + fn arg_targets_all( self, lib: &'static str,