Merge pull request #1070 from willmurphyscode/propagate-values-down

Fix 978: global args' values available to all subcommands

src/app/mod.rs

@@ -16,6 +16,7 @@ use std::path::Path;
 use std::process;
 use std::rc::Rc;
 use std::result::Result as StdResult;
+use std::collections::HashMap;
 
 // Third Party
 #[cfg(feature = "yaml")]
@@ -24,7 +25,7 @@ use yaml_rust::Yaml;
 // Internal
 use app::help::Help;
 use app::parser::Parser;
-use args::{AnyArg, Arg, ArgGroup, ArgMatcher, ArgMatches, ArgSettings};
+use args::{AnyArg, Arg, ArgGroup, ArgMatcher, ArgMatches, ArgSettings, MatchedArg, SubCommand};
 use errors::Result as ClapResult;
 pub use self::settings::AppSettings;
 use completions::Shell;
@@ -1620,13 +1621,61 @@ impl<'a, 'b> App<'a, 'b> {
         }
 
         if self.p.is_set(AppSettings::PropagateGlobalValuesDown) {
-            for a in &self.p.global_args {
-                matcher.propagate(a.b.name);
+            let global_arg_vec : Vec<&str> = (&self).p.global_args.iter().map(|ga| ga.b.name).collect();
+            let mut global_arg_to_value_map = HashMap::new();
+            matcher.get_global_values(&global_arg_vec, &mut global_arg_to_value_map);
+            if let Some(ref mut sc) = matcher.0.subcommand {
+                self.handle_subcommand_globals(sc, &mut global_arg_to_value_map, &global_arg_vec);
             }
         }
 
         Ok(matcher.into())
     }
+
+    fn handle_subcommand_globals(&self, subcommand : &mut Box<SubCommand<'a>>, arg_value_map: &mut HashMap<&'a str, Vec<OsString>>, global_arg_vec: &Vec<&'a str>) {
+        let empty_vec_reference = &vec![];
+        for global_arg in global_arg_vec.iter() {
+            let sma = (*subcommand).matches.args.entry(global_arg).or_insert_with(|| {
+                let vals = arg_value_map.get(global_arg).unwrap_or(empty_vec_reference);
+                let mut gma = MatchedArg::new();
+                gma.occurs += 1;
+                if !vals.is_empty() {
+                    gma.vals = vals.clone();
+                }
+                gma
+            });
+            if sma.vals.is_empty() {
+                let vals = arg_value_map.get(global_arg).unwrap_or(empty_vec_reference);
+                sma.vals = vals.clone();
+            } else {
+                arg_value_map.insert(global_arg, sma.vals.clone());
+            }
+        }
+        if let Some(ref mut inner_sub) = subcommand.matches.subcommand {
+            self.handle_subcommand_globals(inner_sub, arg_value_map, global_arg_vec);
+        }
+        self.fill_in_missing_globals(subcommand, arg_value_map, global_arg_vec);
+    }
+
+    fn fill_in_missing_globals(&self, subcommand : &mut Box<SubCommand<'a>>, arg_value_map: &mut HashMap<&'a str, Vec<OsString>>, global_arg_vec: &Vec<&'a str>) {
+        let empty_vec_reference = &vec![];
+        for global_arg in global_arg_vec.iter() {
+            let sma = (*subcommand).matches.args.entry(global_arg).or_insert_with(|| {
+                let vals = arg_value_map.get(global_arg).unwrap_or(empty_vec_reference);
+                let mut gma = MatchedArg::new();
+                gma.occurs += 1;
+                if !vals.is_empty() {
+                    gma.vals = vals.clone();
+                }
+                gma
+            });
+            if sma.vals.is_empty() {
+                let vals = arg_value_map.get(global_arg).unwrap_or(empty_vec_reference);
+                sma.vals = vals.clone();
+            }
+        }
+    }
+
 }
 
 #[cfg(feature = "yaml")]

src/args/arg_matcher.rs

@@ -1,6 +1,8 @@
 // Std
 use std::collections::hash_map::{Entry, Iter};
+use std::collections::HashMap;
 use std::ffi::OsStr;
+use std::ffi::OsString;
 use std::ops::Deref;
 use std::mem;
 
@@ -20,31 +22,15 @@ impl<'a> Default for ArgMatcher<'a> {
 impl<'a> ArgMatcher<'a> {
     pub fn new() -> Self { ArgMatcher::default() }
 
-    pub fn propagate(&mut self, arg: &'a str) {
-        debugln!("ArgMatcher::propagate: arg={}", arg);
-        let vals: Vec<_> = if let Some(ma) = self.get(arg) {
-            ma.vals.clone()
-        } else {
-            debugln!("ArgMatcher::propagate: arg wasn't used");
-            return;
-        };
-        if let Some(ref mut sc) = self.0.subcommand {
-            {
-                let sma = (*sc).matches.args.entry(arg).or_insert_with(|| {
-                    let mut gma = MatchedArg::new();
-                    gma.occurs += 1;
-                    gma.vals = vals.clone();
-                    gma
-                });
-                if sma.vals.is_empty() {
-                    sma.vals = vals.clone();
-                }
-            }
-            let mut am = ArgMatcher(mem::replace(&mut sc.matches, ArgMatches::new()));
-            am.propagate(arg);
-            mem::swap(&mut am.0, &mut sc.matches);
-        } else {
-            debugln!("ArgMatcher::propagate: Subcommand wasn't used");
+    pub fn get_global_values(&mut self, global_arg_vec : &Vec<&'a str>, vals_map: &mut HashMap<&'a str, Vec<OsString>>) {
+        for global_arg in global_arg_vec.iter() {
+            let vals: Vec<_> = if let Some(ma) = self.get(global_arg) {
+                ma.vals.clone()
+            } else {
+                debugln!("ArgMatcher::propagate: arg wasn't used");
+                Vec::new()
+            };
+            vals_map.insert(global_arg, vals);
         }
     }
 

tests/propagate_globals_down.rs

@@ -0,0 +1,90 @@
+extern crate clap;
+extern crate regex;
+
+#[cfg(test)]
+mod tests {
+    include!("../clap-test.rs");
+    use clap;
+    use clap::{App, Arg, SubCommand, AppSettings};
+
+    fn setup_app_with_globals_and_subcommands<'a, 'b>() -> clap::App<'a, 'b> {
+        let global_arg = Arg::with_name("GLOBAL_ARG")
+            .long("global-arg")
+            .help(
+                "Specifies something needed by the subcommands",
+            )
+            .global(true)
+            .takes_value(true);
+        
+        let double_sub_command = SubCommand::with_name("outer")
+            .setting(AppSettings::PropagateGlobalValuesDown)
+            .subcommand(SubCommand::with_name("inner"));
+
+        App::new("myprog")
+            .global_setting(AppSettings::PropagateGlobalValuesDown)
+            .arg(global_arg)
+            .subcommand(double_sub_command)
+    }
+
+    fn first_subcommand_can_access_global(arg_vector : Vec<&str>) {
+        let matches = setup_app_with_globals_and_subcommands().get_matches_from(
+            arg_vector
+        );
+
+        let sub_match = matches.subcommand_matches("outer").expect("could not access subcommand");
+
+        assert_eq!(sub_match.value_of("GLOBAL_ARG").expect("subcommand could not access global arg"), 
+                    "some_value", "subcommand did not have expected value for global arg");
+
+    }
+
+    fn second_subcommand_can_access_global(arg_vector : Vec<&str>) {
+        let matches = setup_app_with_globals_and_subcommands().get_matches_from(
+            arg_vector
+        );
+
+        let sub_match = matches.subcommand_matches("outer").expect("could not access subcommand");
+        let sub_sub_match = sub_match.subcommand_matches("inner").expect("could not access inner sub");
+
+        assert_eq!(sub_sub_match.value_of("GLOBAL_ARG").expect("inner subcommand could not access global arg"), 
+                "some_value", "inner subcommand did not have expected value for global arg");
+    }
+
+    #[test]
+    fn subcommand_can_access_global_arg_if_global_arg_is_first() {
+        // currently passes
+        first_subcommand_can_access_global(vec!["myprog", "--global-arg", "some_value", "outer", "inner"]);
+    }
+
+    #[test]
+    fn subcommand_can_access_global_arg_if_global_arg_is_in_the_middle() {
+        // currently passes
+        first_subcommand_can_access_global(vec!["myprog", "outer",  "--global-arg", "some_value" ,"inner"]);
+    }
+
+    #[test]
+    fn first_subcommand_can_access_global_arg_if_global_arg_is_last() {
+        // currently fails - hypothesis - nothing propagates global args back up
+        first_subcommand_can_access_global(vec!["myprog", "outer", "inner", "--global-arg", "some_value"]);
+    }
+
+    #[test]
+    fn second_subcommand_can_access_global_arg_if_global_arg_is_first() {
+        // currently passes
+        second_subcommand_can_access_global(vec!["myprog", "--global-arg", "some_value", "outer", "inner"]);
+    }
+
+    #[test]
+    fn second_subcommand_can_access_global_arg_if_global_arg_is_in_the_middle() {
+        // currently fails - hypothesis: subcommands do not recursively propagate global args
+        second_subcommand_can_access_global(vec!["myprog", "outer",  "--global-arg", "some_value" ,"inner"]);
+    }
+
+    #[test]
+    fn second_subcommand_can_access_global_arg_if_global_arg_is_last() {
+        // currently passes
+        second_subcommand_can_access_global(vec!["myprog", "outer", "inner", "--global-arg", "some_value"]);
+    }
+}
+
+