Skip to content

Commit

Permalink
Merge pull request #4796 from epage/lex
Browse files Browse the repository at this point in the history
fix(lex): Allow reporting errors for non-UTF8 longs
  • Loading branch information
epage authored Mar 28, 2023
2 parents a916daa + ea4dada commit 6696513
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 21 deletions.
10 changes: 9 additions & 1 deletion clap_builder/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(),
Expand Down
31 changes: 18 additions & 13 deletions clap_complete/src/dynamic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions clap_lex/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
//! }
//! _ => {
Expand Down Expand Up @@ -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() {
Expand All @@ -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))
}

Expand Down
6 changes: 3 additions & 3 deletions clap_lex/tests/parsed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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("")));
}

Expand All @@ -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")));
}

Expand Down

0 comments on commit 6696513

Please sign in to comment.