Auto merge of #720 - kbknapp:issues-714,718, r=kbknapp

Issues 714,718

CHANGELOG.md

@@ -1,3 +1,23 @@
+<a name="v2.16.4"></a>
+### v2.16.4 (2016-10-31)
+
+
+#### Improvements
+
+* **Error Output:**  conflicting errors are now symetrical, meaning more consistent and less confusing ([3d37001d](https://github.com/kbknapp/clap-rs/commit/3d37001d1dc647d73cc597ff172f1072d4beb80d), closes [#718](https://github.com/kbknapp/clap-rs/issues/718))
+
+#### Documentation
+
+*   Fix typo in example `13a_enum_values_automatic` ([c22fbc07](https://github.com/kbknapp/clap-rs/commit/c22fbc07356e556ffb5d1a79ec04597d149b915e))
+* **README.md:**  fixes failing yaml example (#715) ([21fba9e6](https://github.com/kbknapp/clap-rs/commit/21fba9e6cd8c163012999cd0ce271ec8780c5695))
+
+#### Bug Fixes
+
+* **ZSH Completions:**  fixes bug that caused panic on subcommands with aliases ([5c70e1a0](https://github.com/kbknapp/clap-rs/commit/5c70e1a01bc977e44c10015d18bb8e215c32dfc8), closes [#714](https://github.com/kbknapp/clap-rs/issues/714))
+* **debug:**  fixes the debug feature (#716) ([6c11ccf4](https://github.com/kbknapp/clap-rs/commit/6c11ccf443d46258d51f7cda33fbcc81e7fe8e90))
+
+
+
 <a name="v2.16.3"></a>
 ### v2.16.3 (2016-10-28)
 

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 
 name = "clap"
-version = "2.16.3"
+version = "2.16.4"
 authors = ["Kevin K. <kbknapp@gmail.com>"]
 exclude = ["examples/*", "clap-test/*", "tests/*", "benches/*", "*.png", "clap-perf/*", "*.dot"]
 repository = "https://github.com/kbknapp/clap-rs.git"

README.md

@@ -41,6 +41,14 @@ Created by [gh-md-toc](https://github.com/ekalinin/github-markdown-toc)
 
 ## What's New
 
+Here's what's new in v2.16.4
+
+* Fixes bug that caused panic on subcommands with aliases
+* Conflicting argument errors are now symetrical, meaning more consistent and better usage suggestions
+* Fixes typo in example `13a_enum_values_automatic`
+* Fixes failing yaml example (#715)
+* Fixes the `debug` feature (#716)
+
 Here's the highlights for v2.16.3
 
 * Fixes a bug where the derived display order isn't propagated

src/app/macros.rs

@@ -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
+    }};
+}

src/app/parser.rs

@@ -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;");
+                debugln!("macro=build_err;name={}", $name);
-                let c_with = $me.blacklisted_from($name, &$matcher);
+                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()
+                } else if let Some(o) = $me.find_option($name) {
-                                                 .filter(|o| o.name == $name)
-                                                 .next() {
                    debugln!("It was an option...");
                     Error::argument_conflict(o, c_with, &*usg, self.color())
                 } else {
-                    match $me.positionals.values()
+                    match $me.find_positional($name) {
-                                            .filter(|p| p.name == $name)
-                                            .next() {
                         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);
             }
         }
@@ -1998,7 +1983,7 @@ impl<'a, 'b> Parser<'a, 'b>
         debugln!("Looking for sc...{}", sc);
         debugln!("Currently in Parser...{}", self.meta.bin_name.as_ref().unwrap());
         for s in self.subcommands.iter() {
-            if s.p.meta.bin_name.as_ref().unwrap_or(&String::new()) == sc {
+            if s.p.meta.bin_name.as_ref().unwrap_or(&String::new()) == sc || (s.p.meta.aliases.is_some() && s.p.meta.aliases.as_ref().unwrap().iter().any(|&(s,_)| s == sc.split(' ').rev().next().expect(INTERNAL_ERROR_MSG))) {
                 return Some(s);
             }
             if let Some(app) = s.p.find_subcommand(sc) {

src/args/any_arg.rs

@@ -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>>;

src/completions/macros.rs

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

src/completions/zsh.rs

@@ -4,6 +4,7 @@ use std::io::Write;
 use std::ascii::AsciiExt;
 
 // Internal
+use app::App;
 use app::parser::Parser;
 use args::{ArgSettings, AnyArg};
 use completions;
@@ -127,18 +128,26 @@ _{bin_name_underscore}_commands() {{
 fn subcommands_and_args_of(p: &Parser) -> String {
     debugln!("fn=subcommands_and_args_of;");
     let mut ret = vec![];
-
+    fn add_sc(sc: &App, n: &str, ret: &mut Vec<String>) {
-    // Firs the subcommands
-    for sc in p.subcommands() {
-        debugln!("iter;subcommand={}", sc.p.meta.name);
         let s = format!("\"{name}:{help}\" \\", 
-            name = sc.p.meta.name, 
+            name = n, 
             help = sc.p.meta.about.unwrap_or(""));
         if !s.is_empty() {
             ret.push(s);
         }
     }
 
+    // First the subcommands
+    for sc in p.subcommands() {
+        debugln!("iter;subcommand={}", sc.p.meta.name);
+        add_sc(sc, &sc.p.meta.name, &mut ret);
+        if let Some(ref v) = sc.p.meta.aliases {
+            for alias in v.iter().filter(|&&(_, vis)| vis).map(|&(n,_)| n) {
+                add_sc(sc, alias, &mut ret);
+            }
+        }
+    }
+
     // Then the positional args
     for arg in p.positionals() {
         debugln!("iter;arg={}", arg.name);

src/macros.rs

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

tests/conflicts.rs

@@ -5,10 +5,10 @@ include!("../clap-test.rs");
 
 use clap::{App, Arg, ErrorKind, ArgGroup};
 
-static CONFLICT_ERR: &'static str = "error: The argument '--flag' cannot be used with '-F'
+static CONFLICT_ERR: &'static str = "error: The argument '-F' cannot be used with '--flag'
 
 USAGE:
-    clap-test <positional> <positional2> -F --long-option-2 <option2>
+    clap-test <positional> <positional2> --flag --long-option-2 <option2>
 
 For more information try --help";