Repository: clap-rs/clap
Pull request #4796 (2023-03-28 00:21:27)

fix(lex): Allow reporting errors for non-UTF8 longs
<!--
Thanks for helping out!

Please link the appropriate issue from your PR.

If you don't have an issue, we'd recommend starting with one first so the PR can focus on the
implementation (unless its an obvious bug or documentation fix that will have
little conversation).
-->



Patch:
diff --git a/clap_builder/src/parser/parser.rs b/clap_builder/src/parser/parser.rs
index 79111015..37d8e589 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 6c0881c3..76cede2a 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 333a99aa..ea13e4ba 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))
     }
 


Test patch:
diff --git a/clap_lex/tests/parsed.rs b/clap_lex/tests/parsed.rs
index f1f36b38..133974cb 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")));
 }