From 3683e2c791f8b8fcfd25a958f2fc5eebde4a52ed Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 28 Sep 2022 16:10:02 -0500 Subject: [PATCH] fix(parser): Allow one-off self-overrides bat needed this. See also #4261 --- src/builder/debug_asserts.rs | 5 ----- src/parser/parser.rs | 12 +++++++++--- tests/builder/posix_compatible.rs | 26 +++++++++++++++++++++++--- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/src/builder/debug_asserts.rs b/src/builder/debug_asserts.rs index e40d9369e97..684e5a31d55 100644 --- a/src/builder/debug_asserts.rs +++ b/src/builder/debug_asserts.rs @@ -686,11 +686,6 @@ fn assert_arg(arg: &Arg) { "Argument '{}' cannot conflict with itself", arg.get_id(), ); - assert!( - !arg.overrides.iter().any(|x| *x == arg.id), - "Argument '{}' cannot override itself, its the default", - arg.get_id(), - ); assert_eq!( arg.get_action().takes_values(), diff --git a/src/parser/parser.rs b/src/parser/parser.rs index 24550db27cc..319d25c187e 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -1180,7 +1180,9 @@ impl<'cmd> Parser<'cmd> { self.cur_idx.set(self.cur_idx.get() + 1); debug!("Parser::react: cur_idx:={}", self.cur_idx.get()); } - if matcher.remove(&arg.id) && !self.cmd.is_args_override_self() { + if matcher.remove(&arg.id) + && !(self.cmd.is_args_override_self() || arg.overrides.contains(arg.get_id())) + { return Err(ClapError::argument_conflict( self.cmd, arg.to_string(), @@ -1221,7 +1223,9 @@ impl<'cmd> Parser<'cmd> { raw_vals }; - if matcher.remove(&arg.id) && !self.cmd.is_args_override_self() { + if matcher.remove(&arg.id) + && !(self.cmd.is_args_override_self() || arg.overrides.contains(arg.get_id())) + { return Err(ClapError::argument_conflict( self.cmd, arg.to_string(), @@ -1240,7 +1244,9 @@ impl<'cmd> Parser<'cmd> { raw_vals }; - if matcher.remove(&arg.id) && !self.cmd.is_args_override_self() { + if matcher.remove(&arg.id) + && !(self.cmd.is_args_override_self() || arg.overrides.contains(arg.get_id())) + { return Err(ClapError::argument_conflict( self.cmd, arg.to_string(), diff --git a/tests/builder/posix_compatible.rs b/tests/builder/posix_compatible.rs index c6e202aa8a8..f5e7739da98 100644 --- a/tests/builder/posix_compatible.rs +++ b/tests/builder/posix_compatible.rs @@ -1,16 +1,36 @@ use clap::{arg, error::ErrorKind, Arg, ArgAction, Command}; #[test] -#[should_panic = "Argument 'flag' cannot override itself"] fn flag_overrides_itself() { - Command::new("posix") + let res = Command::new("posix") .arg( arg!(--flag "some flag" ) .action(ArgAction::SetTrue) .overrides_with("flag"), ) - .build(); + .try_get_matches_from(vec!["", "--flag", "--flag"]); + assert!(res.is_ok(), "{}", res.unwrap_err()); + let m = res.unwrap(); + assert!(*m.get_one::("flag").expect("defaulted by clap")); +} + +#[test] +fn option_overrides_itself() { + let res = Command::new("posix") + .arg( + arg!(--opt "some option") + .required(false) + .overrides_with("opt"), + ) + .try_get_matches_from(vec!["", "--opt=some", "--opt=other"]); + assert!(res.is_ok(), "{}", res.unwrap_err()); + let m = res.unwrap(); + assert!(m.contains_id("opt")); + assert_eq!( + m.get_one::("opt").map(|v| v.as_str()), + Some("other") + ); } #[test]