From 3ca0947c166b4f8525752255e3a4fa6565eb9689 Mon Sep 17 00:00:00 2001
From: Kevin K <kbknapp@gmail.com>
Date: Mon, 9 May 2016 15:54:10 -0400
Subject: [PATCH] fix(Usage Strings): now properly dedups args that are also in
 groups

For example, if an arg is part of a required group, it will only appear
in the group usage string, and not in both the group as well as the arg
by itself.

Imagine a group containing two args, `arg1` and `--arg2`

OLD:
    `myprog <arg1> <arg1|--arg2>`

NEW:
    `myprog <arg1|--arg2>`

Closes #498
---
 src/app/macros.rs                  | 20 ++++++++----------
 src/app/parser.rs                  | 16 +++++++++++++++
 src/args/arg_builder/positional.rs | 33 +++++++++---------------------
 3 files changed, 35 insertions(+), 34 deletions(-)

diff --git a/src/app/macros.rs b/src/app/macros.rs
index f52b91b6bde..56b612f0fdb 100644
--- a/src/app/macros.rs
+++ b/src/app/macros.rs
@@ -77,21 +77,19 @@ macro_rules! _handle_group_reqs{
         debugln!("macro=_handle_group_reqs!;");
         for grp in $me.groups.values() {
             let mut found = false;
-            for name in &grp.args {
-                if name == &$arg.name() {
-                    vec_remove!($me.required, name);
-                    if let Some(ref reqs) = grp.requires {
-                        $me.required.extend(reqs);
-                    }
-                    if let Some(ref bl) = grp.conflicts {
-                        $me.blacklist.extend(bl);
-                    }
-                    found = true; // What if arg is in more than one group with different reqs?
-                    break;
+            if grp.args.contains(&$arg.name()) {
+                vec_remove!($me.required, &$arg.name());
+                if let Some(ref reqs) = grp.requires {
+                    $me.required.extend(reqs);
                 }
+                if let Some(ref bl) = grp.conflicts {
+                    $me.blacklist.extend(bl);
+                }
+                found = true; // What if arg is in more than one group with different reqs?
             }
             if found {
                 vec_remove_all!($me.required, &grp.args);
+                debugln!("Adding args from group to blacklist...{:?}", grp.args);
                 $me.blacklist.extend(&grp.args);
                 vec_remove!($me.blacklist, &$arg.name());
             }
diff --git a/src/app/parser.rs b/src/app/parser.rs
index 768f8dc8d02..e369a7c32ae 100644
--- a/src/app/parser.rs
+++ b/src/app/parser.rs
@@ -299,6 +299,13 @@ impl<'a, 'b> Parser<'a, 'b>
         c_pos.dedup();
         c_flags.dedup();
         c_opt.dedup();
+        grps.dedup();
+        let mut args_in_groups = vec![];
+        for g in grps.iter() {
+            for a in self.arg_names_in_group(g).into_iter() {
+                args_in_groups.push(a);
+            }
+        }
 
         let mut pmap = BTreeMap::new();
         for p in c_pos.into_iter() {
@@ -306,10 +313,19 @@ impl<'a, 'b> Parser<'a, 'b>
                 continue;
             }
             if let Some(p) = self.positionals.values().filter(|x| &x.name == &p).next() {
+                if args_in_groups.contains(&p.name) {
+                    continue;
+                }
                 pmap.insert(p.index, p.to_string());
             }
         }
+        debugln!("args_in_groups={:?}", args_in_groups);
         for (_, s) in pmap {
+            if !args_in_groups.is_empty() {
+                if args_in_groups.contains(&&*s) {
+                    continue;
+                }
+            }
             ret_val.push_back(s);
         }
         macro_rules! write_arg {
diff --git a/src/args/arg_builder/positional.rs b/src/args/arg_builder/positional.rs
index 2d67bf9d342..5a265991c93 100644
--- a/src/args/arg_builder/positional.rs
+++ b/src/args/arg_builder/positional.rs
@@ -112,29 +112,16 @@ impl<'n, 'e> PosBuilder<'n, 'e> {
 
 impl<'n, 'e> Display for PosBuilder<'n, 'e> {
     fn fmt(&self, f: &mut Formatter) -> Result {
-        // if self.settings.is_set(ArgSettings::Required) {
-            if let Some(ref names) = self.val_names {
-                try!(write!(f,
-                            "{}",
-                            names.values()
-                                 .map(|n| format!("<{}>", n))
-                                 .collect::<Vec<_>>()
-                                 .join(" ")));
-            } else {
-                try!(write!(f, "<{}>", self.name));
-            }
-        // } else {
-        //     if let Some(ref names) = self.val_names {
-        //         try!(write!(f,
-        //                     "{}",
-        //                     names.values()
-        //                          .map(|n| format!("[{}]", n))
-        //                          .collect::<Vec<_>>()
-        //                          .join(" ")));
-        //     } else {
-        //         try!(write!(f, "[{}]", self.name));
-        //     }
-        // }
+        if let Some(ref names) = self.val_names {
+            try!(write!(f,
+                        "{}",
+                        names.values()
+                             .map(|n| format!("<{}>", n))
+                             .collect::<Vec<_>>()
+                             .join(" ")));
+        } else {
+            try!(write!(f, "<{}>", self.name));
+        }
         if self.settings.is_set(ArgSettings::Multiple) && self.val_names.is_none() {
             try!(write!(f, "..."));
         }