diff --git a/src/bin/cargo/commands/bench.rs b/src/bin/cargo/commands/bench.rs index 9c36422c47d..971dc945648 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 8313851f3a7..f8ec0e2f946 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 69b4ee3802c..f0c1ecad061 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 332ac415b53..84545ae998e 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(msg); + self._arg(flag("keep-going", "").value_parser(value_parser).hide(true)) + } + fn arg_targets_all( self, lib: &'static str, @@ -431,7 +438,7 @@ pub trait ArgMatchesExt { } fn keep_going(&self) -> bool { - self.flag("keep-going") + self.maybe_flag("keep-going") } fn targets(&self) -> Vec { @@ -777,6 +784,8 @@ pub trait ArgMatchesExt { fn flag(&self, name: &str) -> bool; + fn maybe_flag(&self, name: &str) -> bool; + fn _value_of(&self, name: &str) -> Option<&str>; fn _values_of(&self, name: &str) -> Vec; @@ -797,6 +806,17 @@ impl<'a> ArgMatchesExt for ArgMatches { .unwrap_or(false) } + // This works around before an upstream fix in clap for `UnknownArgumentValueParser` accepting + // generics arguments. `flag()` cannot be used with `--keep-going` at this moment due to + // . + fn maybe_flag(&self, name: &str) -> bool { + self.try_get_one::(name) + .ok() + .flatten() + .copied() + .unwrap_or_default() + } + fn _value_of(&self, name: &str) -> Option<&str> { ignore_unknown(self.try_get_one::(name)).map(String::as_str) } diff --git a/tests/testsuite/bench.rs b/tests/testsuite/bench.rs index d773308c683..6b09d47eb5f 100644 --- a/tests/testsuite/bench.rs +++ b/tests/testsuite/bench.rs @@ -1681,11 +1681,16 @@ fn cargo_bench_no_keep_going() { p.cargo("bench --keep-going") .with_stderr( "\ -error: unexpected argument `--keep-going` found +error: unexpected argument '--keep-going' found - tip: to run as many benchmarks as possible without failing fast, use `--no-fail-fast`", + tip: use `--no-fail-fast` to run as many tests as possible regardless of failure + +Usage: cargo bench [OPTIONS] [BENCHNAME] [-- [args]...] + +For more information, try '--help'. +", ) - .with_status(101) + .with_status(1) .run(); } diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index c6ae4ce612f..758bfe09c58 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -4854,11 +4854,16 @@ fn cargo_test_no_keep_going() { p.cargo("test --keep-going") .with_stderr( "\ -error: unexpected argument `--keep-going` found +error: unexpected argument '--keep-going' found - tip: to run as many tests as possible without failing fast, use `--no-fail-fast`", + tip: use `--no-fail-fast` to run as many tests as possible regardless of failure + +Usage: cargo test [OPTIONS] [TESTNAME] [-- [args]...] + +For more information, try '--help'. +", ) - .with_status(101) + .with_status(1) .run(); }