From 7de6df878238ca8e3d9723bb9650f7fe9470d8bd Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 16 Feb 2024 06:41:45 -0600 Subject: [PATCH 1/2] test(error): Show existing last behavior --- tests/builder/error.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/builder/error.rs b/tests/builder/error.rs index 833a614d265..e1f7baacbb9 100644 --- a/tests/builder/error.rs +++ b/tests/builder/error.rs @@ -150,6 +150,29 @@ For more information, try '--help'. assert_error(err, expected_kind, MESSAGE, true); } +#[test] +#[cfg(feature = "error-context")] +fn suggest_trailing_last() { + let cmd = Command::new("cargo") + .arg(arg!([TESTNAME]).last(true)) + .arg(arg!(--"ignore-rust-version")); + + let res = cmd.try_get_matches_from(["cargo", "--ignored"]); + assert!(res.is_err()); + let err = res.unwrap_err(); + let expected_kind = ErrorKind::UnknownArgument; + static MESSAGE: &str = "\ +error: unexpected argument '--ignored' found + + tip: a similar argument exists: '--ignore-rust-version' + +Usage: cargo --ignore-rust-version [-- ] + +For more information, try '--help'. +"; + assert_error(err, expected_kind, MESSAGE, true); +} + #[test] #[cfg(feature = "error-context")] fn trailing_already_in_use() { From 446328a8d3cdaac28884bf8fdfcc85f35c8b5134 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 16 Feb 2024 06:54:57 -0600 Subject: [PATCH 2/2] fix(error): Include -- in more cases Inspired by rust-lang/cargo#12494. Part of this is that our "did you mean" does prefix checks so it can be overly aggressive in providing suggestions. To avoid providing needless suggestions I limited this change to `last` / `trailing_var_arg` as those convey that `--` is more likely a valid suggestion. --- clap_builder/src/parser/parser.rs | 10 ++++++++-- tests/builder/error.rs | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/clap_builder/src/parser/parser.rs b/clap_builder/src/parser/parser.rs index c1642001619..eade3e613dc 100644 --- a/clap_builder/src/parser/parser.rs +++ b/clap_builder/src/parser/parser.rs @@ -1570,10 +1570,16 @@ impl<'cmd> Parser<'cmd> { .collect(); // `did_you_mean` is a lot more likely and should cause us to skip the `--` suggestion + // with the one exception being that the CLI is trying to capture arguments // // In theory, this is only called for `--long`s, so we don't need to check - let suggested_trailing_arg = - did_you_mean.is_none() && !trailing_values && self.cmd.has_positionals(); + let suggested_trailing_arg = (did_you_mean.is_none() + || self + .cmd + .get_positionals() + .any(|arg| arg.is_last_set() || arg.is_trailing_var_arg_set())) + && !trailing_values + && self.cmd.has_positionals(); ClapError::unknown_argument( self.cmd, format!("--{arg}"), diff --git a/tests/builder/error.rs b/tests/builder/error.rs index e1f7baacbb9..a2ea6694c26 100644 --- a/tests/builder/error.rs +++ b/tests/builder/error.rs @@ -165,6 +165,7 @@ fn suggest_trailing_last() { error: unexpected argument '--ignored' found tip: a similar argument exists: '--ignore-rust-version' + tip: to pass '--ignored' as a value, use '-- --ignored' Usage: cargo --ignore-rust-version [-- ]