imp(Error Output): conflicting errors are now symetrical, meaning more consistent and less confusing

Prior to this commit, conflicting error messages and the suggeseted usage would depend on whether
you defined the conflict on both arguments, or just one, and the order in which you specified the
conflicting arguments at runtime.

Now they are symetrical, meaning the suggestions from the error message are consistent, and it no
longer matters if you specify the conflict in one, or both arguments.

Closes #718
This commit is contained in:
Kevin K 2016-10-31 00:35:23 -04:00
parent 44f6b1edbf
commit 3d37001d1d
No known key found for this signature in database
GPG key ID: 17218E4B3692F01A
5 changed files with 150 additions and 78 deletions

View file

@ -27,7 +27,7 @@ macro_rules! arg_post_processing {
// Handle POSIX overrides
debug!("Is '{}' in overrides...", $arg.to_string());
if $me.overrides.contains(&$arg.name()) {
if let Some(ref name) = $me.overriden_from($arg.name(), $matcher) {
if let Some(ref name) = find_name_from!($me, &$arg.name(), overrides, $matcher) {
sdebugln!("Yes by {}", name);
$matcher.remove(name);
remove_overriden!($me, name);
@ -48,6 +48,32 @@ macro_rules! arg_post_processing {
debug!("Does '{}' have conflicts...", $arg.to_string());
if let Some(bl) = $arg.blacklist() {
sdebugln!("Yes");
for c in bl {
// Inject two-way conflicts
debug!("Has '{}' already been matched...", c);
if $matcher.contains(c) {
sdebugln!("Yes");
// find who blacklisted us...
$me.blacklist.push(&$arg.name);
// if let Some(f) = $me.find_flag_mut(c) {
// if let Some(ref mut bl) = f.blacklist {
// bl.push(&$arg.name);
// }
// } else if let Some(o) = $me.find_option_mut(c) {
// if let Some(ref mut bl) = o.blacklist {
// bl.push(&$arg.name);
// }
// } else if let Some(p) = $me.find_positional_mut(c) {
// if let Some(ref mut bl) = p.blacklist {
// bl.push(&$arg.name);
// }
// }
} else {
sdebugln!("No");
}
}
$me.blacklist.extend(bl);
vec_remove_all!($me.overrides, bl);
vec_remove_all!($me.required, bl);
@ -143,3 +169,63 @@ macro_rules! parse_positional {
}
};
}
macro_rules! find_from {
($_self:ident, $arg_name:expr, $from:ident, $matcher:expr) => {{
let mut ret = None;
for k in $matcher.arg_names() {
if let Some(f) = $_self.find_flag(k) {
if let Some(ref v) = f.$from {
if v.contains($arg_name) {
ret = Some(f.to_string());
}
}
}
if let Some(o) = $_self.find_option(k) {
if let Some(ref v) = o.$from {
if v.contains(&$arg_name) {
ret = Some(o.to_string());
}
}
}
if let Some(pos) = $_self.find_positional(k) {
if let Some(ref v) = pos.$from {
if v.contains($arg_name) {
ret = Some(pos.name.to_owned());
}
}
}
}
ret
}};
}
macro_rules! find_name_from {
($_self:ident, $arg_name:expr, $from:ident, $matcher:expr) => {{
let mut ret = None;
for k in $matcher.arg_names() {
if let Some(f) = $_self.find_flag(k) {
if let Some(ref v) = f.$from {
if v.contains($arg_name) {
ret = Some(f.name);
}
}
}
if let Some(o) = $_self.find_option(k) {
if let Some(ref v) = o.$from {
if v.contains(&$arg_name) {
ret = Some(o.name);
}
}
}
if let Some(pos) = $_self.find_positional(k) {
if let Some(ref v) = pos.$from {
if v.contains($arg_name) {
ret = Some(pos.name);
}
}
}
}
ret
}};
}

View file

@ -1007,59 +1007,6 @@ impl<'a, 'b> Parser<'a, 'b>
Ok(())
}
fn blacklisted_from(&self, name: &str, matcher: &ArgMatcher) -> Option<String> {
for k in matcher.arg_names() {
if let Some(f) = self.flags.iter().find(|f| &f.name == &k) {
if let Some(ref bl) = f.blacklist {
if bl.contains(&name) {
return Some(f.to_string());
}
}
}
if let Some(o) = self.opts.iter().find(|o| &o.name == &k) {
if let Some(ref bl) = o.blacklist {
if bl.contains(&name) {
return Some(o.to_string());
}
}
}
if let Some(pos) = self.positionals.values().find(|p| &p.name == &k) {
if let Some(ref bl) = pos.blacklist {
if bl.contains(&name) {
return Some(pos.name.to_owned());
}
}
}
}
None
}
fn overriden_from(&self, name: &str, matcher: &ArgMatcher) -> Option<&'a str> {
for k in matcher.arg_names() {
if let Some(f) = self.flags.iter().find(|f| &f.name == &k) {
if let Some(ref bl) = f.overrides {
if bl.contains(&name.into()) {
return Some(f.name);
}
}
}
if let Some(o) = self.opts.iter().find(|o| &o.name == &k) {
if let Some(ref bl) = o.overrides {
if bl.contains(&name.into()) {
return Some(o.name);
}
}
}
if let Some(pos) = self.positionals.values().find(|p| &p.name == &k) {
if let Some(ref bl) = pos.overrides {
if bl.contains(&name.into()) {
return Some(pos.name);
}
}
}
}
None
}
fn groups_for_arg(&self, name: &str) -> Option<Vec<&'a str>> {
debugln!("fn=groups_for_arg; name={}", name);
@ -1535,26 +1482,43 @@ impl<'a, 'b> Parser<'a, 'b>
}
fn validate_blacklist(&self, matcher: &mut ArgMatcher) -> ClapResult<()> {
debugln!("fn=validate_blacklist;");
debugln!("fn=validate_blacklist;blacklist={:?}", self.blacklist);
macro_rules! build_err {
($me:ident, $name:expr, $matcher:ident) => ({
debugln!("macro=build_err;");
let c_with = $me.blacklisted_from($name, &$matcher);
debugln!("macro=build_err;name={}", $name);
let mut c_with = find_from!($me, $name, blacklist, &$matcher);
c_with = if c_with.is_none() {
if let Some(aa) = $me.find_any_arg($name) {
if let Some(bl) = aa.blacklist() {
if let Some(arg_name) = bl.iter().find(|arg| $matcher.contains(arg)) {
if let Some(aa) = $me.find_any_arg(arg_name) {
Some(aa.to_string())
} else {
c_with
}
} else {
c_with
}
} else {
c_with
}
} else {
c_with
}
} else {
c_with
};
debugln!("'{:?}' conflicts with '{}'", c_with, $name);
$matcher.remove($name);
let usg = $me.create_current_usage($matcher);
if let Some(f) = $me.flags.iter().filter(|f| f.name == $name).next() {
if let Some(f) = $me.find_flag($name) {
debugln!("It was a flag...");
Error::argument_conflict(f, c_with, &*usg, self.color())
} else if let Some(o) = $me.opts.iter()
.filter(|o| o.name == $name)
.next() {
debugln!("It was an option...");
} else if let Some(o) = $me.find_option($name) {
debugln!("It was an option...");
Error::argument_conflict(o, c_with, &*usg, self.color())
} else {
match $me.positionals.values()
.filter(|p| p.name == $name)
.next() {
match $me.find_positional($name) {
Some(p) => {
debugln!("It was a positional...");
Error::argument_conflict(p, c_with, &*usg, self.color())
@ -1572,12 +1536,12 @@ impl<'a, 'b> Parser<'a, 'b>
debugln!("Checking arg '{}' in group...", n);
if matcher.contains(n) {
debugln!("matcher contains it...");
return Err(build_err!(self, n, matcher));
return Err(build_err!(self, &n, matcher));
}
}
} else if matcher.contains(name) {
debugln!("matcher contains it...");
return Err(build_err!(self, *name, matcher));
return Err(build_err!(self, name, matcher));
}
}
Ok(())
@ -1940,15 +1904,15 @@ impl<'a, 'b> Parser<'a, 'b>
Ok(())
}
pub fn flags(&self) -> Iter<FlagBuilder> {
pub fn flags(&self) -> Iter<FlagBuilder<'a, 'b>> {
self.flags.iter()
}
pub fn opts(&self) -> Iter<OptBuilder> {
pub fn opts(&self) -> Iter<OptBuilder<'a, 'b>> {
self.opts.iter()
}
pub fn positionals(&self) -> vec_map::Values<PosBuilder> {
pub fn positionals(&self) -> vec_map::Values<PosBuilder<'a, 'b>> {
self.positionals.values()
}
@ -1973,19 +1937,40 @@ impl<'a, 'b> Parser<'a, 'b>
}
}
pub fn find_arg(&self, arg: &str) -> Option<&AnyArg> {
pub fn find_any_arg(&self, arg: &str) -> Option<&AnyArg> {
if let Some(f) = self.find_flag(arg) {
return Some(f);
}
if let Some(o) = self.find_option(arg) {
return Some(o);
}
if let Some(p) = self.find_positional(arg) {
return Some(p);
}
None
}
fn find_flag(&self, name: &str) -> Option<&FlagBuilder<'a, 'b>> {
for f in self.flags() {
if f.name == arg {
if f.name == name || f.aliases.as_ref().unwrap_or(&vec![("",false)]).iter().any(|&(n,_)| n == name) {
return Some(f);
}
}
None
}
fn find_option(&self, name: &str) -> Option<&OptBuilder<'a, 'b>> {
for o in self.opts() {
if o.name == arg {
if o.name == name || o.aliases.as_ref().unwrap_or(&vec![("",false)]).iter().any(|&(n,_)| n == name) {
return Some(o);
}
}
None
}
fn find_positional(&self, name: &str) -> Option<&PosBuilder<'a, 'b>> {
for p in self.positionals() {
if p.name == arg {
if p.name == name {
return Some(p);
}
}

View file

@ -1,5 +1,6 @@
// Std
use std::rc::Rc;
use std::fmt as std_fmt;
// Third Party
use vec_map::VecMap;
@ -8,7 +9,7 @@ use vec_map::VecMap;
use args::settings::ArgSettings;
#[doc(hidden)]
pub trait AnyArg<'n, 'e> {
pub trait AnyArg<'n, 'e>: std_fmt::Display {
fn name(&self) -> &'n str;
fn overrides(&self) -> Option<&[&'e str]>;
fn aliases(&self) -> Option<Vec<&'e str>>;

View file

@ -13,7 +13,7 @@ macro_rules! get_zsh_arg_conflicts {
if let Some(conf_vec) = $arg.blacklist() {
let mut v = vec![];
for arg_name in conf_vec {
let arg = $p.find_arg(arg_name).expect($msg);
let arg = $p.find_any_arg(arg_name).expect($msg);
if let Some(s) = arg.short() {
v.push(format!("-{}", s));
}

View file

@ -647,7 +647,7 @@ macro_rules! write_nspaces {
// convenience macro for remove an item from a vec
macro_rules! vec_remove {
($vec:expr, $to_rem:expr) => {
debugln!("macro=vec_remove!;");
debugln!("macro=vec_remove!;to_rem={:?}", $to_rem);
for i in (0 .. $vec.len()).rev() {
let should_remove = &$vec[i] == $to_rem;
if should_remove { $vec.swap_remove(i); }
@ -658,7 +658,7 @@ macro_rules! vec_remove {
// convenience macro for remove an item from a vec
macro_rules! vec_remove_all {
($vec:expr, $to_rem:expr) => {
debugln!("macro=vec_remove!;");
debugln!("macro=vec_remove_all!;to_rem={:?}", $to_rem);
for i in (0 .. $vec.len()).rev() {
let should_remove = $to_rem.contains(&$vec[i]);
if should_remove { $vec.swap_remove(i); }