From 8413c155d253b5ac1e914f8d5c0b036b76431b5c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 16 Aug 2023 14:50:08 -0500 Subject: [PATCH 1/2] feat(builder): Allow injecting known unknowns Fixes #4706 --- clap_builder/src/builder/mod.rs | 1 + clap_builder/src/builder/value_parser.rs | 69 ++++++++++++++++++++++++ tests/builder/error.rs | 60 ++++++++++++++++++++- 3 files changed, 129 insertions(+), 1 deletion(-) diff --git a/clap_builder/src/builder/mod.rs b/clap_builder/src/builder/mod.rs index ec7d6059bbd..ff3c6151d2a 100644 --- a/clap_builder/src/builder/mod.rs +++ b/clap_builder/src/builder/mod.rs @@ -59,6 +59,7 @@ pub use value_parser::RangedU64ValueParser; pub use value_parser::StringValueParser; pub use value_parser::TryMapValueParser; pub use value_parser::TypedValueParser; +pub use value_parser::UnknownArgumentValueParser; pub use value_parser::ValueParser; pub use value_parser::ValueParserFactory; pub use value_parser::_AnonymousValueParser; diff --git a/clap_builder/src/builder/value_parser.rs b/clap_builder/src/builder/value_parser.rs index c7c7e61eb5d..48711aa0752 100644 --- a/clap_builder/src/builder/value_parser.rs +++ b/clap_builder/src/builder/value_parser.rs @@ -1,6 +1,7 @@ use std::convert::TryInto; use std::ops::RangeBounds; +use crate::builder::Str; use crate::util::AnyValue; use crate::util::AnyValueId; @@ -2086,6 +2087,74 @@ where } } +/// When encountered, report [ErrorKind::UnknownArgument][crate::error::ErrorKind::UnknownArgument] +/// +/// Useful to help users migrate, either from old versions or similar tools. +/// +/// # Examples +/// +/// ```rust +/// # use clap_builder as clap; +/// # use clap::Command; +/// # use clap::Arg; +/// let cmd = Command::new("mycmd") +/// .args([ +/// Arg::new("current-dir") +/// .short('C'), +/// Arg::new("current-dir-unknown") +/// .long("cwd") +/// .aliases(["current-dir", "directory", "working-directory", "root"]) +/// .value_parser(clap::builder::UnknownArgumentValueParser::suggest("-C")) +/// .hide(true), +/// ]); +/// +/// // Use a supported version of the argument +/// let matches = cmd.clone().try_get_matches_from(["mycmd", "-C", ".."]).unwrap(); +/// assert!(matches.contains_id("current-dir")); +/// assert_eq!( +/// matches.get_many::("current-dir").unwrap_or_default().map(|v| v.as_str()).collect::>(), +/// vec![".."] +/// ); +/// +/// // Use one of the invalid versions +/// let err = cmd.try_get_matches_from(["mycmd", "--cwd", ".."]).unwrap_err(); +/// assert_eq!(err.kind(), clap::error::ErrorKind::UnknownArgument); +/// ``` +#[derive(Clone, Debug)] +pub struct UnknownArgumentValueParser { + arg: Str, +} + +impl UnknownArgumentValueParser { + /// Suggest an alternative argument + pub fn suggest(arg: impl Into) -> Self { + Self { arg: arg.into() } + } +} + +impl TypedValueParser for UnknownArgumentValueParser { + type Value = String; + + fn parse_ref( + &self, + cmd: &crate::Command, + arg: Option<&crate::Arg>, + _value: &std::ffi::OsStr, + ) -> Result { + let arg = match arg { + Some(arg) => arg.to_string(), + None => "..".to_owned(), + }; + Err(crate::Error::unknown_argument( + cmd, + arg, + Some((self.arg.as_str().to_owned(), None)), + false, + crate::output::Usage::new(cmd).create_usage_with_title(&[]), + )) + } +} + /// Register a type with [value_parser!][crate::value_parser!] /// /// # Example diff --git a/tests/builder/error.rs b/tests/builder/error.rs index c4ab3d2218f..a5972120c17 100644 --- a/tests/builder/error.rs +++ b/tests/builder/error.rs @@ -1,6 +1,6 @@ use super::utils; -use clap::{arg, error::Error, error::ErrorKind, value_parser, Arg, Command}; +use clap::{arg, builder::ArgAction, error::Error, error::ErrorKind, value_parser, Arg, Command}; #[track_caller] fn assert_error( @@ -209,3 +209,61 @@ For more information, try '--help'. "; assert_error(err, expected_kind, MESSAGE, true); } + +#[test] +#[cfg(feature = "error-context")] +#[cfg(feature = "suggestions")] +fn unknown_argument_option() { + let cmd = Command::new("test").args([ + Arg::new("current-dir").short('C'), + Arg::new("current-dir-unknown") + .long("cwd") + .aliases(["current-dir", "directory", "working-directory", "root"]) + .value_parser(clap::builder::UnknownArgumentValueParser::suggest("-C")) + .hide(true), + ]); + let res = cmd.try_get_matches_from(["test", "--cwd", ".."]); + assert!(res.is_err()); + let err = res.unwrap_err(); + let expected_kind = ErrorKind::UnknownArgument; + static MESSAGE: &str = "\ +error: unexpected argument '--cwd ' found + + tip: a similar argument exists: '---C' + +Usage: test [OPTIONS] + +For more information, try '--help'. +"; + assert_error(err, expected_kind, MESSAGE, true); +} + +#[test] +#[cfg(feature = "error-context")] +#[cfg(feature = "suggestions")] +fn unknown_argument_flag() { + let cmd = Command::new("test").args([ + Arg::new("ignore-rust-version").long("ignore-rust-version"), + Arg::new("libtest-ignore") + .long("ignored") + .action(ArgAction::SetTrue) + .value_parser(clap::builder::UnknownArgumentValueParser::suggest( + "-- --ignored", + )) + .hide(true), + ]); + let res = cmd.try_get_matches_from(["test", "--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: '---- --ignored' + +Usage: test [OPTIONS] + +For more information, try '--help'. +"; + assert_error(err, expected_kind, MESSAGE, true); +} From 9f65eb0c9aa1f157ce9a14888aab4d80a27bc391 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 16 Aug 2023 14:54:45 -0500 Subject: [PATCH 2/2] refactor(error): Give caller control over suggestion --- clap_builder/src/builder/value_parser.rs | 46 ++++++++++++++++++++---- clap_builder/src/error/mod.rs | 4 +-- clap_builder/src/parser/parser.rs | 1 + tests/builder/error.rs | 18 ++++++---- 4 files changed, 54 insertions(+), 15 deletions(-) diff --git a/clap_builder/src/builder/value_parser.rs b/clap_builder/src/builder/value_parser.rs index 48711aa0752..ab06ae5a432 100644 --- a/clap_builder/src/builder/value_parser.rs +++ b/clap_builder/src/builder/value_parser.rs @@ -2,6 +2,7 @@ use std::convert::TryInto; use std::ops::RangeBounds; use crate::builder::Str; +use crate::builder::StyledStr; use crate::util::AnyValue; use crate::util::AnyValueId; @@ -2104,7 +2105,7 @@ where /// Arg::new("current-dir-unknown") /// .long("cwd") /// .aliases(["current-dir", "directory", "working-directory", "root"]) -/// .value_parser(clap::builder::UnknownArgumentValueParser::suggest("-C")) +/// .value_parser(clap::builder::UnknownArgumentValueParser::suggest_arg("-C")) /// .hide(true), /// ]); /// @@ -2122,13 +2123,31 @@ where /// ``` #[derive(Clone, Debug)] pub struct UnknownArgumentValueParser { - arg: Str, + arg: Option, + suggestions: Vec, } impl UnknownArgumentValueParser { /// Suggest an alternative argument - pub fn suggest(arg: impl Into) -> Self { - Self { arg: arg.into() } + pub fn suggest_arg(arg: impl Into) -> Self { + Self { + arg: Some(arg.into()), + suggestions: Default::default(), + } + } + + /// Provide a general suggestion + pub fn suggest(text: impl Into) -> Self { + Self { + arg: Default::default(), + suggestions: vec![text.into()], + } + } + + /// Extend the suggestions + pub fn and_suggest(mut self, text: impl Into) -> Self { + self.suggestions.push(text.into()); + self } } @@ -2145,13 +2164,26 @@ impl TypedValueParser for UnknownArgumentValueParser { Some(arg) => arg.to_string(), None => "..".to_owned(), }; - Err(crate::Error::unknown_argument( + let err = crate::Error::unknown_argument( cmd, arg, - Some((self.arg.as_str().to_owned(), None)), + self.arg.as_ref().map(|s| (s.as_str().to_owned(), None)), false, crate::output::Usage::new(cmd).create_usage_with_title(&[]), - )) + ); + #[cfg(feature = "error-context")] + let err = { + debug_assert_eq!( + err.get(crate::error::ContextKind::Suggested), + None, + "Assuming `Error::unknown_argument` doesn't apply any `Suggested` so we can without caution" + ); + err.insert_context_unchecked( + crate::error::ContextKind::Suggested, + crate::error::ContextValue::StyledStrs(self.suggestions.clone()), + ) + }; + Err(err) } } diff --git a/clap_builder/src/error/mod.rs b/clap_builder/src/error/mod.rs index 772058a5fe9..af90e2756f1 100644 --- a/clap_builder/src/error/mod.rs +++ b/clap_builder/src/error/mod.rs @@ -718,7 +718,7 @@ impl Error { let mut styled_suggestion = StyledStr::new(); let _ = write!( styled_suggestion, - "'{}{sub} --{flag}{}' exists", + "'{}{sub} {flag}{}' exists", valid.render(), valid.render_reset() ); @@ -727,7 +727,7 @@ impl Error { Some((flag, None)) => { err = err.insert_context_unchecked( ContextKind::SuggestedArg, - ContextValue::String(format!("--{flag}")), + ContextValue::String(flag), ); } None => {} diff --git a/clap_builder/src/parser/parser.rs b/clap_builder/src/parser/parser.rs index d6d8e8da436..81eddc9fd61 100644 --- a/clap_builder/src/parser/parser.rs +++ b/clap_builder/src/parser/parser.rs @@ -1521,6 +1521,7 @@ impl<'cmd> Parser<'cmd> { self.start_custom_arg(matcher, arg, ValueSource::CommandLine); } } + let did_you_mean = did_you_mean.map(|(arg, cmd)| (format!("--{arg}"), cmd)); let required = self.cmd.required_graph(); let used: Vec = matcher diff --git a/tests/builder/error.rs b/tests/builder/error.rs index a5972120c17..bfdf623cb5b 100644 --- a/tests/builder/error.rs +++ b/tests/builder/error.rs @@ -219,7 +219,10 @@ fn unknown_argument_option() { Arg::new("current-dir-unknown") .long("cwd") .aliases(["current-dir", "directory", "working-directory", "root"]) - .value_parser(clap::builder::UnknownArgumentValueParser::suggest("-C")) + .value_parser( + clap::builder::UnknownArgumentValueParser::suggest_arg("-C") + .and_suggest("not much else to say"), + ) .hide(true), ]); let res = cmd.try_get_matches_from(["test", "--cwd", ".."]); @@ -229,7 +232,8 @@ fn unknown_argument_option() { static MESSAGE: &str = "\ error: unexpected argument '--cwd ' found - tip: a similar argument exists: '---C' + tip: a similar argument exists: '-C' + tip: not much else to say Usage: test [OPTIONS] @@ -247,9 +251,10 @@ fn unknown_argument_flag() { Arg::new("libtest-ignore") .long("ignored") .action(ArgAction::SetTrue) - .value_parser(clap::builder::UnknownArgumentValueParser::suggest( - "-- --ignored", - )) + .value_parser( + clap::builder::UnknownArgumentValueParser::suggest_arg("-- --ignored") + .and_suggest("not much else to say"), + ) .hide(true), ]); let res = cmd.try_get_matches_from(["test", "--ignored"]); @@ -259,7 +264,8 @@ fn unknown_argument_flag() { static MESSAGE: &str = "\ error: unexpected argument '--ignored' found - tip: a similar argument exists: '---- --ignored' + tip: a similar argument exists: '-- --ignored' + tip: not much else to say Usage: test [OPTIONS]