Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(lex): Allow reporting errors for non-UTF8 longs #4796

Merged
merged 1 commit into from Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion clap_builder/src/parser/parser.rs
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
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
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
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