Merge pull request #4796 from epage/lex

fix(lex): Allow reporting errors for non-UTF8 longs
clap-rs · Mar 28, 2023 · 6696513 · 6696513
2 parents a916daa + ea4dada
commit 6696513
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 21 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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")));
 }