fix: fixes a bug where calling the help of a subcommand wasn't ignori…

…ng required args of parent commands Closes #789
clap-rs · Dec 28, 2016 · d3d34a2 · d3d34a2
1 parent eb1d79d
commit d3d34a2
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 53 deletions.
diff --git a/src/app/parser.rs b/src/app/parser.rs
@@ -817,10 +817,10 @@ impl<'a, 'b> Parser<'a, 'b>
                     subcmd_name = Some(arg_os.to_str().expect(INVALID_UTF8).to_owned());
                     break;
                 } else if let Some(cdate) =
-                              suggestions::did_you_mean(&*arg_os.to_string_lossy(),
-                                                        self.subcommands
-                                                            .iter()
-                                                            .map(|s| &s.p.meta.name)) {
+                    suggestions::did_you_mean(&*arg_os.to_string_lossy(),
+                                              self.subcommands
+                                                  .iter()
+                                                  .map(|s| &s.p.meta.name)) {
                     return Err(Error::invalid_subcommand(arg_os.to_string_lossy().into_owned(),
                                                          cdate,
                                                          self.meta
@@ -894,51 +894,13 @@ impl<'a, 'b> Parser<'a, 'b>
             }
         }
 
-        self.validate(needs_val_of, subcmd_name, matcher, it)
-    }
-
-    fn validate<I, T>(&mut self,
-                      needs_val_of: Option<&'a str>,
-                      subcmd_name: Option<String>,
-                      matcher: &mut ArgMatcher<'a>,
-                      it: &mut Peekable<I>)
-                      -> ClapResult<()>
-        where I: Iterator<Item = T>,
-              T: Into<OsString> + Clone
-    {
-        let mut reqs_validated = false;
-        if let Some(a) = needs_val_of {
-            debugln!("needs_val_of={:?}", a);
-            let o = find_by_name!(self, &a, opts, iter).expect(INTERNAL_ERROR_MSG);
-            try!(self.validate_required(matcher));
-            reqs_validated = true;
-            let should_err = if let Some(v) = matcher.0.args.get(&*o.b.name) {
-                v.vals.is_empty() && !(o.v.min_vals.is_some() && o.v.min_vals.unwrap() == 0)
-            } else {
-                true
-            };
-            if should_err {
-                return Err(Error::empty_value(o,
-                                              &*self.create_current_usage(matcher),
-                                              self.color()));
-            }
-        }
-
-        try!(self.add_defaults(matcher));
-        try!(self.validate_blacklist(matcher));
-        try!(self.validate_num_args(matcher));
-        matcher.usage(self.create_usage(&[]));
-
-        if !(self.settings.is_set(AppSettings::SubcommandsNegateReqs) && subcmd_name.is_some()) &&
-           !reqs_validated {
-            try!(self.validate_required(matcher));
-        }
-        if let Some(pos_sc_name) = subcmd_name {
+        if let Some(ref pos_sc_name) = subcmd_name {
             // is this is a real subcommand, or an alias
-            let sc_name = if self.subcommands.iter().any(|sc| sc.p.meta.name == pos_sc_name) {
-                pos_sc_name
+            if self.subcommands.iter().any(|sc| &sc.p.meta.name == pos_sc_name) {
+
+                try!(self.parse_subcommand(&*pos_sc_name, matcher, it));
             } else {
-                self.subcommands
+                let sc_name = &*self.subcommands
                     .iter()
                     .filter(|sc| sc.p.meta.aliases.is_some())
                     .filter_map(|sc| if sc.p
@@ -953,9 +915,9 @@ impl<'a, 'b> Parser<'a, 'b>
                         None
                     })
                     .next()
-                    .expect(INTERNAL_ERROR_MSG)
+                    .expect(INTERNAL_ERROR_MSG);
+                try!(self.parse_subcommand(sc_name, matcher, it));
             };
-            try!(self.parse_subcommand(sc_name, matcher, it));
         } else if self.is_set(AppSettings::SubcommandRequired) {
             let bn = self.meta.bin_name.as_ref().unwrap_or(&self.meta.name);
             return Err(Error::missing_subcommand(bn,
@@ -970,6 +932,42 @@ impl<'a, 'b> Parser<'a, 'b>
                 info: None,
             });
         }
+
+        self.validate(needs_val_of, subcmd_name, matcher)
+    }
+
+    fn validate(&mut self,
+                needs_val_of: Option<&'a str>,
+                subcmd_name: Option<String>,
+                matcher: &mut ArgMatcher<'a>)
+                -> ClapResult<()> {
+        let mut reqs_validated = false;
+        if let Some(a) = needs_val_of {
+            debugln!("needs_val_of={:?}", a);
+            let o = find_by_name!(self, &a, opts, iter).expect(INTERNAL_ERROR_MSG);
+            try!(self.validate_required(matcher));
+            reqs_validated = true;
+            let should_err = if let Some(v) = matcher.0.args.get(&*o.b.name) {
+                v.vals.is_empty() && !(o.v.min_vals.is_some() && o.v.min_vals.unwrap() == 0)
+            } else {
+                true
+            };
+            if should_err {
+                return Err(Error::empty_value(o,
+                                              &*self.create_current_usage(matcher),
+                                              self.color()));
+            }
+        }
+
+        try!(self.add_defaults(matcher));
+        try!(self.validate_blacklist(matcher));
+        try!(self.validate_num_args(matcher));
+        matcher.usage(self.create_usage(&[]));
+
+        if !(self.settings.is_set(AppSettings::SubcommandsNegateReqs) && subcmd_name.is_some()) &&
+           !reqs_validated {
+            try!(self.validate_required(matcher));
+        }
         if matcher.is_empty() && matcher.subcommand_name().is_none() &&
            self.is_set(AppSettings::ArgRequiredElseHelp) {
             let mut out = vec![];
@@ -1019,7 +1017,7 @@ impl<'a, 'b> Parser<'a, 'b>
     }
 
     fn parse_subcommand<I, T>(&mut self,
-                              sc_name: String,
+                              sc_name: &str,
                               matcher: &mut ArgMatcher<'a>,
                               it: &mut Peekable<I>)
                               -> ClapResult<()>
@@ -1750,7 +1748,7 @@ impl<'a, 'b> Parser<'a, 'b>
                     return true;
                 }
             }
-        } 
+        }
         if let Some(ru) = a.required_unless() {
             debugln!("Required unless found...{:?}", ru);
             let mut found_any = false;

diff --git a/src/usage_parser.rs b/src/usage_parser.rs
@@ -6,8 +6,6 @@ use INTERNAL_ERROR_MSG;
 use args::Arg;
 use args::settings::ArgSettings;
 
-type ParseResult = Result<(), ()>;
-
 #[derive(PartialEq, Debug)]
 enum UsageToken {
     Name,