imp(ArgMatcher): huge refactor and deduplication of code

Tons of code has been moved into functions, deduplicated, made much
easier to read, maintain, and understand. Comments still need to be
added, but that will happen shortly. Modules have also been moved around
to follow Rust conventions and best practices.

All functionality remains exactly the same

benches/03_complex.rs

@@ -212,7 +212,7 @@ fn parse_sc_positional(b: &mut Bencher) {
 
 #[bench]
 fn parse_complex1(b: &mut Bencher) {
-    b.iter(|| create_app!().get_matches_from(vec!["", "-ff", "-o", "option1", "arg1", "-O", "fast", "arg2", "--multvals", "one", "two", "three"]));
+    b.iter(|| create_app!().get_matches_from(vec!["", "-ff", "-o", "option1", "arg1", "-O", "fast", "arg2", "--multvals", "one", "two", "emacs"]));
 }
 
 #[bench]

clap-tests/run_tests.py

@@ -217,14 +217,14 @@ option NOT present
 positional present with value: too
 subcmd NOT present'''
 
-_mult_vals_more = '''error: The argument '--multvals' was supplied more than once, but does not support multiple values
+_mult_vals_more = '''error: The argument '--multvals <one> <two>' was supplied more than once, but does not support multiple occurrences
 
 USAGE:
 \tclaptests --multvals <one> <two>
 
 For more information try --help'''
 
-_mult_vals_few = '''error: The argument '--multvals <one> <two>' requires a value but none was supplied
+_mult_vals_few = '''error: The argument '--multvals <one> <two>' requires 2 values, but 1 was provided
 
 USAGE:
 \tclaptests --multvals <one> <two>

src/app/macros.rs

@@ -0,0 +1,130 @@
+macro_rules! remove_overriden {
+    ($me:ident, $name:expr) => ({
+        if let Some(ref o) = $me.opts.iter().filter(|o| &o.name == $name).next() {
+            if let Some(ref ora) = o.requires {
+                for a in ora {
+                    vec_remove!($me.required, a);
+                }
+            }
+            if let Some(ref ora) = o.blacklist {
+                for a in ora {
+                    vec_remove!($me.blacklist, a);
+                }
+            }
+            if let Some(ref ora) = o.overrides {
+                for a in ora {
+                    vec_remove!($me.overrides, a);
+                }
+            }
+        } else if let Some(ref o) = $me.flags.iter().filter(|f| &f.name == $name).next() {
+            if let Some(ref ora) = o.requires {
+                for a in ora {
+                    vec_remove!($me.required, a);
+                }
+            }
+            if let Some(ref ora) = o.blacklist {
+                for a in ora {
+                    vec_remove!($me.blacklist, a);
+                }
+            }
+            if let Some(ref ora) = o.overrides {
+                for a in ora {
+                    vec_remove!($me.overrides, a);
+                }
+            }
+        } else if let Some(p) = $me.positionals.values().filter(|p| &&p.name == &$name).next() {
+            if let Some(ref ora) = p.requires {
+                for a in ora {
+                    vec_remove!($me.required, a);
+                }
+            }
+            if let Some(ref ora) = p.blacklist {
+                for a in ora {
+                    vec_remove!($me.blacklist, a);
+                }
+            }
+            if let Some(ref ora) = p.overrides {
+                for a in ora {
+                    vec_remove!($me.overrides, a);
+                }
+            }
+        }
+    })
+}
+
+macro_rules! arg_post_processing(
+    ($me:ident, $arg:ident, $matcher:ident) => ({
+        use args::AnyArg;
+        // Handle POSIX overrides
+        if $me.overrides.contains(&$arg.name()) {
+            if let Some(ref name) = $me.overriden_from($arg.name(), $matcher) {
+                $matcher.remove(name);
+                remove_overriden!($me, name);
+            }
+        }
+        if let Some(or) = $arg.overrides() {
+            for pa in or {
+                $matcher.remove(pa);
+                remove_overriden!($me, pa);
+                $me.overrides.push(pa);
+                vec_remove!($me.required, pa);
+            }
+        }
+        // Handle conflicts
+        if let Some(bl) = $arg.blacklist() {
+            for name in bl {
+                $me.blacklist.push(name);
+                vec_remove!($me.overrides, name);
+                vec_remove!($me.required, name);
+            }
+        }
+
+        // Add all required args which aren't already found in matcher to the master
+        // list
+        if let Some(reqs) = $arg.requires() {
+            for n in reqs {
+                if $matcher.contains(n) {
+                    continue;
+                }
+
+                $me.required.push(n);
+            }
+        }
+
+        _handle_group_reqs!($me, $arg);
+    })
+);
+
+macro_rules! _handle_group_reqs{
+    ($me:ident, $arg:ident) => ({
+        use args::AnyArg;
+        for grp in $me.groups.values() {
+            let mut found = false;
+            for name in grp.args.iter() {
+                if name == &$arg.name() {
+                    vec_remove!($me.required, name);
+                    if let Some(ref reqs) = grp.requires {
+                        for r in reqs {
+                            $me.required.push(r);
+                        }
+                    }
+                    if let Some(ref bl) = grp.conflicts {
+                        for b in bl {
+                            $me.blacklist.push(b);
+                        }
+                    }
+                    found = true;
+                    break;
+                }
+            }
+            if found {
+                for name in grp.args.iter() {
+                    if name == &$arg.name() { continue }
+                    vec_remove!($me.required, name);
+
+                    $me.blacklist.push(name);
+                }
+            }
+        }
+    })
+}

src/args/any_arg.rs

@@ -1,8 +1,19 @@
+use std::rc::Rc;
+
 use args::settings::ArgSettings;
 
+#[doc(hidden)]
 pub trait AnyArg<'n> {
     fn name(&self) -> &'n str;
     fn overrides(&self) -> Option<&[&'n str]>;
-    fn is_set(&self, &ArgSettings) -> bool;
-    fn set(&mut self, &ArgSettings);
+    fn requires(&self) -> Option<&[&'n str]>;
+    fn blacklist(&self) -> Option<&[&'n str]>;
+    fn is_set(&self, ArgSettings) -> bool;
+    fn set(&mut self, ArgSettings);
+    fn has_switch(&self) -> bool;
+    fn max_vals(&self) -> Option<u8>;
+    fn min_vals(&self) -> Option<u8>;
+    fn num_vals(&self) -> Option<u8>;
+    fn possible_vals(&self) -> Option<&[&'n str]>;
+    fn validator(&self) -> Option<&Rc<Fn(String) -> Result<(), String>>>;
 }

src/args/arg.rs

@@ -98,6 +98,34 @@ pub struct Arg<'n, 'l, 'h, 'g, 'p, 'r> {
     pub hidden: bool,
 }
 
+impl<'n, 'l, 'h, 'g, 'p, 'r> Default for Arg<'n, 'l, 'h, 'g, 'p, 'r> {
+    fn default() -> Self {
+        Arg {
+            name: "",
+            short: None,
+            long: None,
+            help: None,
+            required: false,
+            takes_value: false,
+            multiple: false,
+            index: None,
+            possible_vals: None,
+            blacklist: None,
+            requires: None,
+            num_vals: None,
+            min_vals: None,
+            max_vals: None,
+            val_names: None,
+            group: None,
+            global: false,
+            empty_vals: true,
+            validator: None,
+            overrides: None,
+            hidden: false,
+        }
+    }
+}
+
 impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
     /// Creates a new instance of `Arg` using a unique string name.
     /// The name will be used by the library consumer to get information about
@@ -120,26 +148,7 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
     pub fn with_name(n: &'n str) -> Self {
         Arg {
             name: n,
-            short: None,
-            long: None,
-            help: None,
-            required: false,
-            takes_value: false,
-            multiple: false,
-            index: None,
-            possible_vals: None,
-            blacklist: None,
-            requires: None,
-            num_vals: None,
-            min_vals: None,
-            max_vals: None,
-            val_names: None,
-            group: None,
-            global: false,
-            empty_vals: true,
-            validator: None,
-            overrides: None,
-            hidden: false,
+            ..Default::default()
         }
     }
 
@@ -370,10 +379,6 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
             required: required,
             takes_value: takes_value,
             multiple: multiple,
-            index: None,
-            possible_vals: None,
-            blacklist: None,
-            requires: None,
             num_vals: if num_names > 1 {
                 Some(num_names)
             } else {
@@ -384,14 +389,7 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
             } else {
                 None
             },
-            max_vals: None,
-            min_vals: None,
-            group: None,
-            global: false,
-            empty_vals: true,
-            validator: None,
-            overrides: None,
-            hidden: false,
+            ..Default::default()
         }
     }
 

src/args/arg_builder/flag.rs

@@ -2,6 +2,8 @@
 use std::fmt::{Display, Formatter, Result};
 use std::convert::From;
 use std::io;
+use std::rc::Rc;
+use std::result::Result as StdResult;
 
 use Arg;
 use args::AnyArg;
@@ -170,21 +172,29 @@ impl<'n> Display for FlagBuilder<'n> {
 }
 
 impl<'n> AnyArg<'n> for FlagBuilder<'n> {
-    fn name(&self) -> &'n str {
-        self.name
-    }
+    fn name(&self) -> &'n str { self.name }
 
-    fn overrides(&self) -> Option<&[&'n str]> {
-        self.overrides.as_ref().map(|o| &o[..])
-    }
+    fn overrides(&self) -> Option<&[&'n str]> { self.overrides.as_ref().map(|o| &o[..]) }
 
-    fn is_set(&self, s: &ArgSettings) -> bool {
-        self.settings.is_set(s)
-    }
+    fn requires(&self) -> Option<&[&'n str]> { self.requires.as_ref().map(|o| &o[..]) }
 
-    fn set(&mut self, s: &ArgSettings) {
-        self.settings.set(s)
-    }
+    fn blacklist(&self) -> Option<&[&'n str]> { self.blacklist.as_ref().map(|o| &o[..]) }
+
+    fn is_set(&self, s: ArgSettings) -> bool { self.settings.is_set(&s) }
+
+    fn has_switch(&self) -> bool { true }
+
+    fn set(&mut self, s: ArgSettings) { self.settings.set(&s) }
+
+    fn max_vals(&self) -> Option<u8> { None }
+
+    fn num_vals(&self) -> Option<u8> { None }
+
+    fn possible_vals(&self) -> Option<&[&'n str]> { None }
+
+    fn validator(&self) -> Option<&Rc<Fn(String) -> StdResult<(), String>>> { None }
+
+    fn min_vals(&self) -> Option<u8> { None }
 }
 
 #[cfg(test)]

src/args/arg_builder/option.rs

@@ -4,7 +4,7 @@ use std::fmt::{Display, Formatter, Result};
 use std::result::Result as StdResult;
 use std::io;
 
-use args::{ArgMatches, Arg};
+use args::{AnyArg, ArgMatcher, Arg};
 use args::settings::{ArgFlags, ArgSettings};
 use errors::{ClapResult, error_builder};
 use app::App;
@@ -36,10 +36,10 @@ pub struct OptBuilder<'n> {
     pub settings: ArgFlags,
 }
 
-impl<'n> OptBuilder<'n> {
-    pub fn new(name: &'n str) -> Self {
+impl<'n> Default for OptBuilder<'n> {
+    fn default() -> Self {
         OptBuilder {
-            name: name,
+            name: "",
             short: None,
             long: None,
             help: None,
@@ -55,6 +55,14 @@ impl<'n> OptBuilder<'n> {
             settings: ArgFlags::new(),
         }
     }
+}
+impl<'n> OptBuilder<'n> {
+    pub fn new(name: &'n str) -> Self {
+        OptBuilder {
+            name: name,
+            ..Default::default()
+        }
+    }
 
     pub fn from_arg(a: &Arg<'n, 'n, 'n, 'n, 'n, 'n>, reqs: &mut Vec<&'n str>) -> Self {
         if a.short.is_none() && a.long.is_none() {
@@ -67,17 +75,12 @@ impl<'n> OptBuilder<'n> {
             name: a.name,
             short: a.short,
             long: a.long,
-            blacklist: None,
             help: a.help,
-            possible_vals: None,
             num_vals: a.num_vals,
             min_vals: a.min_vals,
             max_vals: a.max_vals,
             val_names: a.val_names.clone(),
-            requires: None,
-            validator: None,
-            overrides: None,
-            settings: ArgFlags::new(),
+            ..Default::default()
         };
         if a.multiple {
             ob.settings.set(&ArgSettings::Multiple);
@@ -192,26 +195,26 @@ impl<'n> OptBuilder<'n> {
 
     pub fn validate_value(&self,
                        val: &str,
-                       matches: &ArgMatches,
+                       matcher: &ArgMatcher,
                        app: &App)
                        -> ClapResult<()> {
         // Check the possible values
         if let Some(ref p_vals) = self.possible_vals {
             if !p_vals.contains(&val) {
-                let usage = try!(app.create_current_usage(matches));
+                let usage = try!(app.create_current_usage(matcher));
                 return Err(error_builder::InvalidValue(val, p_vals, &self.to_string(), &usage));
             }
         }
 
         // Check the required number of values
         if let Some(num) = self.num_vals {
-            if let Some(ref ma) = matches.args.get(self.name) {
+            if let Some(ref ma) = matcher.get(self.name) {
                 if let Some(ref vals) = ma.values {
                     if (vals.len() as u8) > num && !self.settings.is_set(&ArgSettings::Multiple) {
                         return Err(error_builder::TooManyValues(
                             val,
                             &self.to_string(),
-                            &*try!(app.create_current_usage(matches))));
+                            &*try!(app.create_current_usage(matcher))));
                     }
                 }
             }
@@ -219,10 +222,10 @@ impl<'n> OptBuilder<'n> {
 
         // if it's an empty value, and we don't allow that, report the error
         if !self.settings.is_set(&ArgSettings::EmptyValues) &&
-            matches.args.contains_key(self.name) &&
+            matcher.contains(self.name) &&
             val.is_empty() {
             return Err(error_builder::EmptyValue(&*self.to_string(),
-                                                 &*try!(app.create_current_usage(matches))));
+                                                 &*try!(app.create_current_usage(matcher))));
         }
 
         if let Some(ref vtor) = self.validator {
@@ -263,6 +266,54 @@ impl<'n> Display for OptBuilder<'n> {
     }
 }
 
+impl<'n> AnyArg<'n> for OptBuilder<'n> {
+    fn name(&self) -> &'n str {
+        self.name
+    }
+
+    fn overrides(&self) -> Option<&[&'n str]> {
+        self.overrides.as_ref().map(|o| &o[..])
+    }
+
+    fn requires(&self) -> Option<&[&'n str]> {
+        self.requires.as_ref().map(|o| &o[..])
+    }
+
+    fn blacklist(&self) -> Option<&[&'n str]> {
+        self.blacklist.as_ref().map(|o| &o[..])
+    }
+
+    fn is_set(&self, s: ArgSettings) -> bool {
+        self.settings.is_set(&s)
+    }
+
+    fn has_switch(&self) -> bool {
+        true
+    }
+
+    fn set(&mut self, s: ArgSettings) {
+        self.settings.set(&s)
+    }
+
+    fn max_vals(&self) -> Option<u8> {
+        self.max_vals
+    }
+    fn num_vals(&self) -> Option<u8> {
+        self.num_vals
+    }
+    fn possible_vals(&self) -> Option<&[&'n str]> {
+        self.possible_vals.as_ref().map(|o| &o[..])
+    }
+
+    fn validator(&self) -> Option<&Rc<Fn(String) -> StdResult<(), String>>> {
+        self.validator.as_ref()
+    }
+
+    fn min_vals(&self) -> Option<u8> {
+        self.min_vals
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::OptBuilder;

src/args/arg_builder/positional.rs

@@ -4,6 +4,7 @@ use std::rc::Rc;
 use std::io;
 
 use Arg;
+use args::AnyArg;
 use args::settings::{ArgFlags, ArgSettings};
 
 #[allow(missing_debug_implementations)]
@@ -181,6 +182,53 @@ impl<'n> Display for PosBuilder<'n> {
         Ok(())
     }
 }
+
+impl<'n> AnyArg<'n> for PosBuilder<'n> {
+    fn name(&self) -> &'n str {
+        self.name
+    }
+
+    fn overrides(&self) -> Option<&[&'n str]> {
+        self.overrides.as_ref().map(|o| &o[..])
+    }
+
+    fn requires(&self) -> Option<&[&'n str]> {
+        self.requires.as_ref().map(|o| &o[..])
+    }
+
+    fn blacklist(&self) -> Option<&[&'n str]> {
+        self.blacklist.as_ref().map(|o| &o[..])
+    }
+
+    fn is_set(&self, s: ArgSettings) -> bool {
+        self.settings.is_set(&s)
+    }
+
+    fn set(&mut self, s: ArgSettings) {
+        self.settings.set(&s)
+    }
+
+    fn has_switch(&self) -> bool {
+        false
+    }
+    fn max_vals(&self) -> Option<u8> {
+        self.max_vals
+    }
+    fn num_vals(&self) -> Option<u8> {
+        self.num_vals
+    }
+    fn possible_vals(&self) -> Option<&[&'n str]> {
+        self.possible_vals.as_ref().map(|o| &o[..])
+    }
+    fn validator(&self) -> Option<&Rc<Fn(String) -> StdResult<(), String>>> {
+        self.validator.as_ref()
+    }
+
+    fn min_vals(&self) -> Option<u8> {
+        self.min_vals
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::PosBuilder;

src/args/arg_matcher.rs

@@ -0,0 +1,97 @@
+use vec_map::VecMap;
+
+use args::{ArgMatches, MatchedArg, SubCommand};
+use std::collections::hash_map::{Entry, Keys, Iter};
+
+pub struct ArgMatcher<'ar>(ArgMatches<'ar, 'ar>);
+
+impl<'ar> ArgMatcher<'ar> {
+    pub fn new() -> Self {
+        ArgMatcher(ArgMatches::new())
+    }
+
+    pub fn get_mut(&mut self, arg: &str) -> Option<&mut MatchedArg> {
+        self.0.args.get_mut(arg)
+    }
+
+    pub fn get(&self, arg: &str) -> Option<&MatchedArg> {
+        self.0.args.get(arg)
+    }
+
+    pub fn remove(&mut self, arg: &str) {
+        self.0.args.remove(arg);
+    }
+
+    pub fn insert(&mut self, name: &'ar str) {
+        self.0.args.insert(name, MatchedArg::new());
+    }
+
+    pub fn contains(&self, arg: &str) -> bool {
+        self.0.args.contains_key(arg)
+    }
+
+    pub fn is_empty(&self) -> bool {
+        self.0.args.is_empty()
+    }
+
+    pub fn values_of(&self, arg: &str) -> Option<Vec<&str>> {
+        self.0.values_of(arg)
+    }
+
+    pub fn usage(&mut self, usage: String) {
+        self.0.usage = Some(usage);
+    }
+
+    pub fn arg_names(&self) -> Keys<&'ar str, MatchedArg> {
+        self.0.args.keys()
+    }
+
+    pub fn entry(&mut self, arg: &'ar str) -> Entry<&'ar str, MatchedArg> {
+        self.0.args.entry(arg)
+    }
+
+    pub fn subcommand(&mut self, sc: SubCommand<'ar, 'ar>) {
+        self.0.subcommand = Some(Box::new(sc));
+    }
+
+    pub fn subcommand_name(&self) -> Option<&str> {
+        self.0.subcommand_name()
+    }
+
+    pub fn iter(&self) -> Iter<&'ar str, MatchedArg> {
+        self.0.args.iter()
+    }
+
+    pub fn inc_occurrence_of(&mut self, arg: &'ar str) {
+        if let Some(a) = self.get_mut(arg) {
+            a.occurrences += 1;
+            return;
+        }
+        self.insert(arg);
+    }
+
+    pub fn inc_occurrences_of(&mut self, args: &[&'ar str]) {
+        for arg in args {
+            self.inc_occurrence_of(arg);
+        }
+    }
+
+    pub fn add_val_to(&mut self, arg: &'ar str, val: String) {
+        let ma = self.entry(arg).or_insert(MatchedArg {
+            // occurrences will be incremented on getting a value
+            occurrences: 0,
+            values: Some(VecMap::new()),
+        });
+        if let Some(ref mut vals) = ma.values {
+            let len = vals.len() + 1;
+            vals.insert(len, val);
+            ma.occurrences += 1;
+        }
+    }
+}
+
+impl<'ar> Into<ArgMatches<'ar, 'ar>> for ArgMatcher<'ar> {
+    fn into(self) -> ArgMatches<'ar, 'ar> {
+        self.0
+    }
+}

src/args/matched_arg.rs

@@ -1,12 +1,19 @@
-use std::collections::BTreeMap;
+use vec_map::VecMap;
 
 #[doc(hidden)]
 #[derive(Debug)]
 pub struct MatchedArg {
-    // #[doc(hidden)]
-    // pub name: String,
     #[doc(hidden)]
     pub occurrences: u8,
     #[doc(hidden)]
-    pub values: Option<BTreeMap<u8, String>>,
+    pub values: Option<VecMap<String>>,
+}
+
+impl MatchedArg {
+    pub fn new() -> Self {
+        MatchedArg {
+            occurrences: 1,
+            values: None
+        }
+    }
 }

src/args/mod.rs

@@ -1,5 +1,6 @@
 pub use self::arg::Arg;
 pub use self::arg_matches::ArgMatches;
+pub use self::arg_matcher::ArgMatcher;
 pub use self::subcommand::SubCommand;
 pub use self::arg_builder::{FlagBuilder, OptBuilder, PosBuilder};
 pub use self::matched_arg::MatchedArg;

src/errors.rs

@@ -281,7 +281,7 @@ pub mod error_builder {
     {
         ClapError {
             error: format!("{} The argument '{}' was supplied more than once, but does \
-                            not support multiple values\n\n\
+                            not support multiple occurrences\n\n\
                             {}\n\n\
                             For more information try {}",
                            Format::Error("error:"),

src/macros.rs

@@ -44,88 +44,7 @@ macro_rules! load_yaml {
     );
 }
 
-macro_rules! write_spaces {
-    ($num:expr, $w:ident) => ({
-        for _ in 0..$num {
-            try!(write!($w, " "));
-        }
-    })
-}
-
-// convenience macro for remove an item from a vec
-macro_rules! vec_remove {
-    ($vec:expr, $to_rem:ident) => {
-        {
-            let mut ix = None;
-            $vec.dedup();
-            for (i, val) in $vec.iter().enumerate() {
-                if &val == &$to_rem {
-                    ix = Some(i);
-                    break;
-                }
-            }
-            if let Some(i) = ix {
-                $vec.remove(i);
-            }
-        }
-    }
-}
-
-macro_rules! remove_overriden {
-    ($me:ident, $name:expr) => ({
-        if let Some(ref o) = $me.opts.iter().filter(|o| &o.name == $name).next() {
-            if let Some(ref ora) = o.requires {
-                for a in ora {
-                    vec_remove!($me.required, a);
-                }
-            }
-            if let Some(ref ora) = o.blacklist {
-                for a in ora {
-                    vec_remove!($me.blacklist, a);
-                }
-            }
-            if let Some(ref ora) = o.overrides {
-                for a in ora {
-                    vec_remove!($me.overrides, a);
-                }
-            }
-        } else if let Some(ref o) = $me.flags.iter().filter(|f| &f.name == $name).next() {
-            if let Some(ref ora) = o.requires {
-                for a in ora {
-                    vec_remove!($me.required, a);
-                }
-            }
-            if let Some(ref ora) = o.blacklist {
-                for a in ora {
-                    vec_remove!($me.blacklist, a);
-                }
-            }
-            if let Some(ref ora) = o.overrides {
-                for a in ora {
-                    vec_remove!($me.overrides, a);
-                }
-            }
-        } else if let Some(p) = $me.positionals.values().filter(|p| &&p.name == &$name).next() {
-            if let Some(ref ora) = p.requires {
-                for a in ora {
-                    vec_remove!($me.required, a);
-                }
-            }
-            if let Some(ref ora) = p.blacklist {
-                for a in ora {
-                    vec_remove!($me.blacklist, a);
-                }
-            }
-            if let Some(ref ora) = p.overrides {
-                for a in ora {
-                    vec_remove!($me.overrides, a);
-                }
-            }
-        }
-    })
-}
-
-// De-duplication macro used in src/app.rs
+// used in src/args/arg_builder/option.rs
 macro_rules! print_opt_help {
     ($opt:ident, $spc:expr, $w:ident) => {
         if let Some(h) = $opt.help {
@@ -153,38 +72,35 @@ macro_rules! print_opt_help {
     };
 }
 
-// De-duplication macro used in src/app.rs
-macro_rules! parse_group_reqs {
-    ($me:ident, $arg:ident) => {
-        for ag in $me.groups.values() {
-            let mut found = false;
-            for name in ag.args.iter() {
-                if name == &$arg.name {
-                    vec_remove!($me.required, name);
-                    if let Some(ref reqs) = ag.requires {
-                        for r in reqs {
-                            $me.required.push(r);
-                        }
-                    }
-                    if let Some(ref bl) = ag.conflicts {
-                        for b in bl {
-                            $me.blacklist.push(b);
-                        }
-                    }
-                    found = true;
+// Helper/deduplication macro for printing the correct number of spaces in help messages
+// used in:
+//    src/args/arg_builder/*.rs
+//    src/app/mod.rs
+macro_rules! write_spaces {
+    ($num:expr, $w:ident) => ({
+        for _ in 0..$num {
+            try!(write!($w, " "));
+        }
+    })
+}
+
+// convenience macro for remove an item from a vec
+macro_rules! vec_remove {
+    ($vec:expr, $to_rem:ident) => {
+        {
+            let mut ix = None;
+            $vec.dedup();
+            for (i, val) in $vec.iter().enumerate() {
+                if &val == &$to_rem {
+                    ix = Some(i);
                     break;
                 }
             }
-            if found {
-                for name in ag.args.iter() {
-                    if name == &$arg.name { continue }
-                    vec_remove!($me.required, name);
-
-                    $me.blacklist.push(name);
-                }
+            if let Some(i) = ix {
+                $vec.remove(i);
             }
         }
-    };
+    }
 }
 
 // Thanks to bluss and flan3002 in #rust IRC

tests/app_settings.rs

@@ -43,12 +43,11 @@ fn arg_required_else_help() {
     let result = App::new("arg_required")
         .setting(AppSettings::ArgRequiredElseHelp)
         .arg(Arg::with_name("test")
-               .required(true)
                .index(1))
         .get_matches_from_safe(vec![""]);
     assert!(result.is_err());
     let err = result.err().unwrap();
-    assert_eq!(err.error_type, ClapErrorType::MissingRequiredArgument);
+    assert_eq!(err.error_type, ClapErrorType::MissingArgumentOrSubcommand);
 }
 
 #[test]
@@ -109,4 +108,4 @@ fn app_settings_fromstr() {
     assert_eq!("subcommandrequiredelsehelp".parse::<AppSettings>().ok().unwrap(), AppSettings::SubcommandRequiredElseHelp);
     assert_eq!("hidden".parse::<AppSettings>().ok().unwrap(), AppSettings::Hidden);
     assert!("hahahaha".parse::<AppSettings>().is_err());
-}
+}

tests/groups.rs

@@ -26,6 +26,10 @@ fn group_single_value() {
         .get_matches_from(vec!["", "-c", "blue"]);
     assert!(m.is_present("grp"));
     assert_eq!(m.value_of("grp").unwrap(), "blue");
+}
+
+#[test]
+fn group_single_flag() {
     let m = App::new("group")
         .args_from_usage("-f, --flag 'some flag'
                           -c, --color [color] 'some option'")
@@ -34,6 +38,10 @@ fn group_single_value() {
         .get_matches_from(vec!["", "-f"]);
     assert!(m.is_present("grp"));
     assert!(m.value_of("grp").is_none());
+}
+
+#[test]
+fn group_empty() {
     let m = App::new("group")
         .args_from_usage("-f, --flag 'some flag'
                           -c, --color [color] 'some option'")
@@ -44,6 +52,20 @@ fn group_single_value() {
     assert!(m.value_of("grp").is_none());
 }
 
+#[test]
+fn group_reqired_flags_empty() {
+    let result = App::new("group")
+        .args_from_usage("-f, --flag 'some flag'
+                          -c, --color 'some option'")
+        .arg_group(ArgGroup::with_name("grp")
+            .required(true)
+            .add_all(&["flag", "color"]))
+        .get_matches_from_safe(vec![""]);
+    assert!(result.is_err());
+    let err = result.err().unwrap();
+    assert_eq!(err.error_type, ClapErrorType::MissingRequiredArgument);
+}
+
 #[test]
 fn group_multi_value_single_arg() {
     let m = App::new("group")

tests/posix_compatible.rs

@@ -150,15 +150,15 @@ fn conflict_overriden_4() {
 }
 
 #[test]
-fn require_overriden() {
+fn pos_required_overridden_by_flag() {
     let result = App::new("require_overriden")
-        .arg(Arg::with_name("flag")
+        .arg(Arg::with_name("pos")
             .index(1)
             .required(true))
-        .arg(Arg::from_usage("-c, --color 'other flag'")
-            .mutually_overrides_with("flag"))
-        .get_matches_from_safe(vec!["", "flag", "-c"]);
-    assert!(result.is_ok());
+        .arg(Arg::from_usage("-c, --color 'some flag'")
+            .mutually_overrides_with("pos"))
+        .get_matches_from_safe(vec!["", "test", "-c"]);
+    assert!(result.is_ok(), "{:?}", result.unwrap_err());
 }
 
 #[test]
@@ -200,4 +200,4 @@ fn require_overriden_4() {
     assert!(result.is_err());
     let err = result.err().unwrap();
     assert_eq!(err.error_type, ClapErrorType::MissingRequiredArgument);
-}
+}