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(parser): Ban long flags with literals #2619

Merged
merged 11 commits into from
Aug 1, 2021
14 changes: 14 additions & 0 deletions examples/05_flag_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ fn main() {
// also has a conflicts_with_all(Vec<&str>)
// and an exclusive(true)
)
.arg(
// Sometimes we might want to accept one of the following: `--pseudo-flag`, `--pseudo-flag=true`, `--pseudo-flag=false.`
// The following is the `pseudo-flag` pattern stated in https://github.com/clap-rs/clap/issues/1649#issuecomment-661274943
Arg::new("pseudo-flag") // Create a "pesudo-flag" with optional value
.possible_values(&["true", "false"]) // Limit that value to `true` of `false`
.long("pseudo-flag")
.min_values(0)
.max_values(1),
)
rami3l marked this conversation as resolved.
Show resolved Hide resolved
.arg("-c, --config=[FILE] 'sets a custom config file'")
.arg("<output> 'sets an output file'")
.get_matches();
Expand All @@ -38,6 +47,11 @@ fn main() {
println!("Awesomeness is turned on");
}

// Same thing with `pseudo-flag`
if matches.value_of("pseudo-flag") != Some("false") {
println!("Pseudo-flag is turned on");
}

rami3l marked this conversation as resolved.
Show resolved Hide resolved
// If we set the multiple option of a flag we can check how many times the user specified
//
// Note: if we did not specify the multiple option, and the user used "awesome" we would get
Expand Down
3 changes: 1 addition & 2 deletions src/build/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2485,8 +2485,7 @@ impl<'help> App<'help> {
{
debug!("App::_check_help_and_version: Building help subcommand");
self.subcommands.push(
App::new("help")
.about("Print this message or the help of the given subcommand(s)"),
App::new("help").about("Print this message or the help of the given subcommand(s)"),
rami3l marked this conversation as resolved.
Show resolved Hide resolved
);
}
}
Expand Down
33 changes: 30 additions & 3 deletions src/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1075,11 +1075,38 @@ impl<'help, 'app> Parser<'help, 'app> {
self.app.settings.set(AS::ValidArgFound);
self.seen.push(opt.id.clone());
if opt.is_set(ArgSettings::TakesValue) {
debug!(
"Parser::parse_long_arg: Found an opt with value '{:?}'",
&val
);
return self.parse_opt(&val, opt, matcher);
} else {
self.check_for_help_and_version_str(&arg)?;
return Ok(self.parse_flag(opt, matcher));
}
debug!("Parser::parse_long_arg: Found a flag");
if let Some(rest) = val {
debug!(
"Parser::parse_long_arg: Got invalid literal `{:?}`",
rest.to_os_string()
);
let used: Vec<Id> = matcher
.arg_names()
.filter(|&n| {
self.app.find(n).map_or(true, |a| {
!(a.is_set(ArgSettings::Hidden) || self.required.contains(&a.id))
})
})
.cloned()
.collect();

return Err(ClapError::too_many_values(
rest.to_string_lossy().into(),
opt,
Usage::new(self).create_usage_no_title(&used),
self.app.color(),
));
}
self.check_for_help_and_version_str(&arg)?;
debug!("Parser::parse_long_arg: Presence validated");
return Ok(self.parse_flag(opt, matcher));
}

if let Some(sc_name) = self.possible_long_flag_subcommand(&arg) {
Expand Down
18 changes: 18 additions & 0 deletions tests/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,24 @@ fn flag_using_long() {
assert!(m.is_present("color"));
}

#[cfg(feature = "suggestions")]
rami3l marked this conversation as resolved.
Show resolved Hide resolved
#[test]
fn flag_using_long_with_literals() {
use clap::ErrorKind;

let m = App::new("flag")
.args(&[Arg::from("--rainbow 'yet another flag'").multiple_occurrences(true)])
rami3l marked this conversation as resolved.
Show resolved Hide resolved
.try_get_matches_from(vec![
"",
"--rainbow",
"--rainbow",
"--rainbow=false",
"--rainbow=true",
rami3l marked this conversation as resolved.
Show resolved Hide resolved
]);
assert!(m.is_err(), "{:#?}", m.unwrap());
assert_eq!(m.unwrap_err().kind, ErrorKind::TooManyValues);
}

#[test]
fn flag_using_mixed() {
let m = App::new("flag")
Expand Down