Auto merge of #730 - kbknapp:issue-725-take2, r=kbknapp

Issue 725 take2

CHANGELOG.md

@@ -1,3 +1,13 @@
+<a name="v2.17.1"></a>
+### v2.17.1 (2016-11-02)
+
+
+#### Bug Fixes
+
+* **Low Index Multiples:**  fixes a bug where using low index multiples was propgated to subcommands ([33924e88](https://github.com/kbknapp/clap-rs/commit/33924e884461983c4e6b5ea1330fecc769a4ade7), closes [#725](https://github.com/kbknapp/clap-rs/issues/725))
+
+
+
 <a name="v2.17.0"></a>
 ## v2.17.0 (2016-11-01)
 

Cargo.toml

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

README.md

@@ -41,9 +41,13 @@ Created by [gh-md-toc](https://github.com/ekalinin/github-markdown-toc)
 
 ## What's New
 
+Here's what's new in v2.17.1
+
+* Fixes a bug where using low index multiples was propagated to subcommands
+
 Here's what's new in v2.17.0
 
-* Allows specifying the second to last positional argument as `multiple(true)` (i.e. things such as `mv <files>... <target>`) 
+* Allows specifying the second to last positional argument as `multiple(true)` (i.e. things such as `mv <files>... <target>`)
 * Adds an `App::get_name` and `App::get_bin_name`
 
 Here's what's new in v2.16.4

src/app/mod.rs

@@ -1322,8 +1322,6 @@ impl<'a, 'b> App<'a, 'b> {
         where I: IntoIterator<Item = T>,
               T: Into<OsString> + Clone
     {
-        // Verify all positional assertions pass
-        self.p.verify_positionals();
         // If there are global arguments, or settings we need to propgate them down to subcommands
         // before parsing incase we run into a subcommand
         self.p.propogate_globals();

src/app/parser.rs

@@ -542,7 +542,7 @@ impl<'a, 'b> Parser<'a, 'b>
                 "When using a positional argument with .multiple(true) that is *not the last* \
                 positional argument, the last positional argument (i.e the one with the highest \
                 index) *must* have .required(true) set.");
-            
+
             debug_assert!({
                 let num = self.positionals.len() - 1;
                 self.positionals.get(num).unwrap().is_set(ArgSettings::Multiple)
@@ -704,10 +704,13 @@ impl<'a, 'b> Parser<'a, 'b>
                                   it: &mut Peekable<I>)
                                   -> ClapResult<()>
         where I: Iterator<Item = T>,
-              T: Into<OsString> + Clone 
+              T: Into<OsString> + Clone
     {
         debugln!("fn=get_matches_with;");
-        // First we create the `--help` and `--version` arguments and add them if
+        // Verify all positional assertions pass
+        self.verify_positionals();
+
+        // Next we create the `--help` and `--version` arguments and add them if
         // necessary
         self.create_help_and_version();
 
@@ -719,7 +722,7 @@ impl<'a, 'b> Parser<'a, 'b>
             debugln!("Begin parsing '{:?}' ({:?})", arg_os, &*arg_os.as_bytes());
 
             // Is this a new argument, or values from a previous option?
-            let starts_new_arg = is_new_arg(&arg_os); 
+            let starts_new_arg = is_new_arg(&arg_os);
 
             // Has the user already passed '--'? Meaning only positional args follow
             if !self.trailing_vals {

tests/multiple_values.rs

@@ -1014,6 +1014,35 @@ fn low_index_positional_with_subcmd() {
     assert_eq!(m.value_of("target").unwrap(), "target");
 }
 
+#[test]
+fn low_index_positional_in_subcmd() {
+    let m = App::new("lip")
+        .subcommand(SubCommand::with_name("test")
+			.arg(Arg::with_name("files")
+				.index(1)
+				.required(true)
+				.multiple(true))
+			.arg(Arg::with_name("target")
+				.index(2)
+				.required(true)))
+        .get_matches_from_safe(vec![
+            "lip", "test",
+            "file1", "file2",
+            "file3", "target"
+        ]);
+
+    assert!(m.is_ok(), "{:?}", m.unwrap_err().kind);
+    let m = m.unwrap();
+	let sm = m.subcommand_matches("test").unwrap();
+
+    assert!(sm.is_present("files"));
+    assert_eq!(sm.occurrences_of("files"), 3);
+    assert!(sm.is_present("target"));
+    assert_eq!(sm.occurrences_of("target"), 1);
+    assert_eq!(sm.values_of("files").unwrap().collect::<Vec<_>>(), ["file1", "file2", "file3"]);
+    assert_eq!(sm.value_of("target").unwrap(), "target");
+}
+
 #[test]
 fn low_index_positional_with_option() {
     let m = App::new("lip")
@@ -1075,4 +1104,4 @@ fn low_index_positional_with_flag() {
     assert_eq!(m.values_of("files").unwrap().collect::<Vec<_>>(), ["file1", "file2", "file3"]);
     assert_eq!(m.value_of("target").unwrap(), "target");
     assert!(m.is_present("flg"));
-}
+}