From ea4dada1e4b0ac26e01207fb614f57355577af05 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Sat, 25 Mar 2023 03:42:04 -0500 Subject: [PATCH] fix(lex): Allow reporting errors for non-UTF8 longs --- clap_builder/src/parser/parser.rs | 10 +++++++++- clap_complete/src/dynamic.rs | 31 ++++++++++++++++++------------- clap_lex/src/lib.rs | 8 ++++---- clap_lex/tests/parsed.rs | 6 +++--- 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/clap_builder/src/parser/parser.rs b/clap_builder/src/parser/parser.rs index 79111015f9b..37d8e589e85 100644 --- a/clap_builder/src/parser/parser.rs +++ b/clap_builder/src/parser/parser.rs @@ -720,7 +720,7 @@ impl<'cmd> Parser<'cmd> { fn parse_long_arg( &mut self, matcher: &mut ArgMatcher, - long_arg: &str, + long_arg: Result<&str, &OsStr>, long_value: Option<&OsStr>, parse_state: &ParseState, pos_counter: usize, @@ -738,6 +738,14 @@ impl<'cmd> Parser<'cmd> { } debug!("Parser::parse_long_arg: Does it contain '='..."); + let long_arg = match long_arg { + Ok(long_arg) => long_arg, + Err(long_arg_os) => { + return Ok(ParseResult::NoMatchingArg { + arg: long_arg_os.to_string_lossy().into_owned(), + }) + } + }; if long_arg.is_empty() { debug_assert!( long_value.is_some(), diff --git a/clap_complete/src/dynamic.rs b/clap_complete/src/dynamic.rs index 6c0881c3557..76cede2a6c0 100644 --- a/clap_complete/src/dynamic.rs +++ b/clap_complete/src/dynamic.rs @@ -368,23 +368,28 @@ complete OPTIONS -F _clap_complete_NAME EXECUTABLES if !is_escaped { if let Some((flag, value)) = arg.to_long() { - if let Some(value) = value { - if let Some(arg) = cmd.get_arguments().find(|a| a.get_long() == Some(flag)) { + if let Ok(flag) = flag { + if let Some(value) = value { + if let Some(arg) = cmd.get_arguments().find(|a| a.get_long() == Some(flag)) + { + completions.extend( + complete_arg_value(value.to_str().ok_or(value), arg, current_dir) + .into_iter() + .map(|os| { + // HACK: Need better `OsStr` manipulation + format!("--{}={}", flag, os.to_string_lossy()).into() + }), + ) + } + } else { completions.extend( - complete_arg_value(value.to_str().ok_or(value), arg, current_dir) + crate::generator::utils::longs_and_visible_aliases(cmd) .into_iter() - .map(|os| { - // HACK: Need better `OsStr` manipulation - format!("--{}={}", flag, os.to_string_lossy()).into() + .filter_map(|f| { + f.starts_with(flag).then(|| format!("--{}", f).into()) }), - ) + ); } - } else { - completions.extend( - crate::generator::utils::longs_and_visible_aliases(cmd) - .into_iter() - .filter_map(|f| f.starts_with(flag).then(|| format!("--{}", f).into())), - ); } } else if arg.is_escape() || arg.is_stdio() || arg.is_empty() { // HACK: Assuming knowledge of is_escape / is_stdio diff --git a/clap_lex/src/lib.rs b/clap_lex/src/lib.rs index 333a99aa511..ea13e4baf86 100644 --- a/clap_lex/src/lib.rs +++ b/clap_lex/src/lib.rs @@ -65,13 +65,13 @@ //! args.paths.push(PathBuf::from("-")); //! } else if let Some((long, value)) = arg.to_long() { //! match long { -//! "verbose" => { +//! Ok("verbose") => { //! if let Some(value) = value { //! return Err(format!("`--verbose` does not take a value, got `{:?}`", value).into()); //! } //! args.verbosity += 1; //! } -//! "color" => { +//! Ok("color") => { //! args.color = Color::parse(value)?; //! } //! _ => { @@ -308,7 +308,7 @@ impl<'s> ParsedArg<'s> { } /// Treat as a long-flag - pub fn to_long(&self) -> Option<(&str, Option<&OsStr>)> { + pub fn to_long(&self) -> Option<(Result<&str, &OsStr>, Option<&OsStr>)> { let raw = self.inner; let remainder = raw.strip_prefix("--")?; if remainder.is_empty() { @@ -321,7 +321,7 @@ impl<'s> ParsedArg<'s> { } else { (remainder, None) }; - let flag = flag.to_str()?; + let flag = flag.to_str().ok_or(flag); Some((flag, value)) } diff --git a/clap_lex/tests/parsed.rs b/clap_lex/tests/parsed.rs index f1f36b38041..133974cb8fa 100644 --- a/clap_lex/tests/parsed.rs +++ b/clap_lex/tests/parsed.rs @@ -36,7 +36,7 @@ fn to_long_no_value() { assert!(next.is_long()); let (key, value) = next.to_long().unwrap(); - assert_eq!(key, "long"); + assert_eq!(key, Ok("long")); assert_eq!(value, None); } @@ -50,7 +50,7 @@ fn to_long_with_empty_value() { assert!(next.is_long()); let (key, value) = next.to_long().unwrap(); - assert_eq!(key, "long"); + assert_eq!(key, Ok("long")); assert_eq!(value, Some(OsStr::new(""))); } @@ -64,7 +64,7 @@ fn to_long_with_value() { assert!(next.is_long()); let (key, value) = next.to_long().unwrap(); - assert_eq!(key, "long"); + assert_eq!(key, Ok("long")); assert_eq!(value, Some(OsStr::new("hello"))); }