Repository: kbknapp/clap-rs
Pull request #1223 (2018-03-21 00:59:00)

V3 dev
Implements #1203 on v3

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/kbknapp/clap-rs/1223)
<!-- Reviewable:end -->



Patch:
diff --git a/src/app/parser.rs b/src/app/parser.rs
index 3d1dfe5c..c20a131e 100644
--- a/src/app/parser.rs
+++ b/src/app/parser.rs
@@ -487,7 +487,8 @@ where
             let low_index_mults = self.is_set(AS::LowIndexMultiplePositional)
                 && pos_counter == (self.positionals.len() - 1);
             let missing_pos = self.is_set(AS::AllowMissingPositional)
-                && pos_counter == (self.positionals.len() - 1);
+                && (pos_counter == (self.positionals.len() - 1) 
+                    && !self.is_set(AS::TrailingValues));
             debugln!(
                 "Parser::get_matches_with: Positional counter...{}",
                 pos_counter
@@ -522,7 +523,9 @@ where
                     debugln!("Parser::get_matches_with: Bumping the positional counter...");
                     pos_counter += 1;
                 }
-            } else if self.is_set(AS::ContainsLast) && self.is_set(AS::TrailingValues) {
+            } else if (self.is_set(AS::AllowMissingPositional) && self.is_set(AS::TrailingValues))
+                || (self.is_set(AS::ContainsLast) && self.is_set(AS::TrailingValues))
+            {
                 // Came to -- and one postional has .last(true) set, so we go immediately
                 // to the last (highest index) positional
                 debugln!("Parser::get_matches_with: .last(true) and --, setting last pos");
diff --git a/src/app/settings.rs b/src/app/settings.rs
index 368559a8..b2751e7a 100644
--- a/src/app/settings.rs
+++ b/src/app/settings.rs
@@ -219,7 +219,9 @@ pub enum AppSettings {
     /// [`AllowLeadingHyphen`]: ./enum.AppSettings.html#variant.AllowLeadingHyphen
     AllowNegativeNumbers,
 
-    /// Allows one to implement a CLI where the second to last positional argument is optional, but
+    /// Allows one to implement two styles of CLIs where positionals can be used out of order.
+    ///
+    /// The first example is a CLI where the second to last positional argument is optional, but
     /// the final positional argument is required. Such as `$ prog [optional] <required>` where one
     /// of the two following usages is allowed:
     ///
@@ -228,11 +230,46 @@ pub enum AppSettings {
     ///
     /// This would otherwise not be allowed. This is useful when `[optional]` has a default value.
     ///
-    /// **Note:** In addition to using this setting, the second positional argument *must* be
-    /// [required]
+    /// **Note:** when using this style of "missing positionals" the final positional *must* be
+    /// [required] if `--` will not be used to skip to the final positional argument.
+    ///
+    /// **Note:** This style also only allows a single positional argument to be "skipped" without
+    /// the use of `--`. To skip more than one, see the second example.
+    ///
+    /// The second example is when one wants to skip multiple optional positional arguments, and use
+    /// of the `--` operator is OK (but not required if all arguments will be specified anyways).
+    ///
+    /// For example, imagine a CLI which has three positional arguments `[foo] [bar] [baz]...` where
+    /// `baz` accepts multiple values (similar to man `ARGS...` style training arguments).
+    ///
+    /// With this setting the following invocations are posisble:
+    ///
+    /// * `$ prog foo bar baz1 baz2 baz3`
+    /// * `$ prog foo -- baz1 baz2 baz3`
+    /// * `$ prog -- baz1 baz2 baz3`
     ///
     /// # Examples
     ///
+    /// Style number one from above:
+    ///
+    /// ```rust
+    /// # use clap::{App, Arg, AppSettings};
+    /// // Assume there is an external subcommand named "subcmd"
+    /// let m = App::new("myprog")
+    ///     .setting(AppSettings::AllowMissingPositional)
+    ///     .arg(Arg::with_name("arg1"))
+    ///     .arg(Arg::with_name("arg2")
+    ///         .required(true))
+    ///     .get_matches_from(vec![
+    ///         "prog", "other"
+    ///     ]);
+    ///
+    /// assert_eq!(m.value_of("arg1"), None);
+    /// assert_eq!(m.value_of("arg2"), Some("other"));
+    /// ```
+    ///
+    /// Now the same example, but using a default value for the first optional positional argument
+    ///
     /// ```rust
     /// # use clap::{App, Arg, AppSettings};
     /// // Assume there is an external subcommand named "subcmd"
@@ -243,12 +280,49 @@ pub enum AppSettings {
     ///     .arg(Arg::with_name("arg2")
     ///         .required(true))
     ///     .get_matches_from(vec![
-    ///         "myprog", "other"
+    ///         "prog", "other"
     ///     ]);
     ///
     /// assert_eq!(m.value_of("arg1"), Some("something"));
     /// assert_eq!(m.value_of("arg2"), Some("other"));
     /// ```
+    /// Style number two from above:
+    ///
+    /// ```rust
+    /// # use clap::{App, Arg, AppSettings};
+    /// // Assume there is an external subcommand named "subcmd"
+    /// let m = App::new("myprog")
+    ///     .setting(AppSettings::AllowMissingPositional)
+    ///     .arg(Arg::with_name("foo"))
+    ///     .arg(Arg::with_name("bar"))
+    ///     .arg(Arg::with_name("baz").multiple(true))
+    ///     .get_matches_from(vec![
+    ///         "prog", "foo", "bar", "baz1", "baz2", "baz3"
+    ///     ]);
+    ///
+    /// assert_eq!(m.value_of("foo"), Some("foo"));
+    /// assert_eq!(m.value_of("bar"), Some("bar"));
+    /// assert_eq!(m.values_of("baz").unwrap().collect::<Vec<_>>(), &["baz1", "baz2", "baz3"]);
+    /// ```
+    ///
+    /// Now nofice if we don't specifiy `foo` or `baz` but use the `--` operator.
+    ///
+    /// ```rust
+    /// # use clap::{App, Arg, AppSettings};
+    /// // Assume there is an external subcommand named "subcmd"
+    /// let m = App::new("myprog")
+    ///     .setting(AppSettings::AllowMissingPositional)
+    ///     .arg(Arg::with_name("foo"))
+    ///     .arg(Arg::with_name("bar"))
+    ///     .arg(Arg::with_name("baz").multiple(true))
+    ///     .get_matches_from(vec![
+    ///         "prog", "--", "baz1", "baz2", "baz3"
+    ///     ]);
+    ///
+    /// assert_eq!(m.value_of("foo"), None);
+    /// assert_eq!(m.value_of("bar"), None);
+    /// assert_eq!(m.values_of("baz").unwrap().collect::<Vec<_>>(), &["baz1", "baz2", "baz3"]);
+    /// ```
     /// [required]: ./struct.Arg.html#method.required
     AllowMissingPositional,
 


Test patch:
diff --git a/tests/app_settings.rs b/tests/app_settings.rs
index 488abf1f..0a4cb62f 100644
--- a/tests/app_settings.rs
+++ b/tests/app_settings.rs
@@ -631,6 +631,91 @@ fn allow_missing_positional() {
     assert_eq!(m.value_of("dest"), Some("file"));
 }
 
+#[test]
+fn allow_missing_positional_no_default() {
+    let m = App::new("test")
+        .setting(AppSettings::AllowMissingPositional)
+        .arg(Arg::from_usage("[src] 'some file'"))
+        .arg_from_usage("<dest> 'some file'")
+        .get_matches_from_safe(vec!["test", "file"]);
+    assert!(m.is_ok(), "{:?}", m.unwrap_err().kind);
+    let m = m.unwrap();
+    assert_eq!(m.value_of("src"), None);
+    assert_eq!(m.value_of("dest"), Some("file"));
+}
+
+#[test]
+fn missing_positional_no_hyphen() {
+    let r = App::new("bench")
+        .setting(AppSettings::AllowMissingPositional)
+        .arg(Arg::from_usage("[BENCH] 'some bench'"))
+        .arg(Arg::from_usage("[ARGS]... 'some args'"))
+        .get_matches_from_safe(vec!["bench", "foo", "arg1", "arg2", "arg3"]);
+    assert!(r.is_ok(), "{:?}", r.unwrap_err().kind);
+
+    let m = r.unwrap();
+
+    let expected_bench = Some("foo");
+    let expected_args = vec!["arg1", "arg2", "arg3"];
+
+    assert_eq!(m.value_of("BENCH"), expected_bench);
+    assert_eq!(m.values_of("ARGS").unwrap().collect::<Vec<_>>(), &*expected_args);
+}
+
+#[test]
+fn missing_positional_hyphen() {
+    let r = App::new("bench")
+        .setting(AppSettings::AllowMissingPositional)
+        .arg(Arg::from_usage("[BENCH] 'some bench'"))
+        .arg(Arg::from_usage("[ARGS]... 'some args'"))
+        .get_matches_from_safe(vec!["bench", "--", "arg1", "arg2", "arg3"]);
+    assert!(r.is_ok(), "{:?}", r.unwrap_err().kind);
+
+    let m = r.unwrap();
+
+    let expected_bench = None;
+    let expected_args = vec!["arg1", "arg2", "arg3"];
+
+    assert_eq!(m.value_of("BENCH"), expected_bench);
+    assert_eq!(m.values_of("ARGS").unwrap().collect::<Vec<_>>(), &*expected_args);
+}
+
+#[test]
+fn missing_positional_hyphen_far_back() {
+    let r = App::new("bench")
+        .setting(AppSettings::AllowMissingPositional)
+        .arg(Arg::from_usage("[BENCH1] 'some bench'"))
+        .arg(Arg::from_usage("[BENCH2] 'some bench'"))
+        .arg(Arg::from_usage("[BENCH3] 'some bench'"))
+        .arg(Arg::from_usage("[ARGS]... 'some args'"))
+        .get_matches_from_safe(vec!["bench", "foo", "--", "arg1", "arg2", "arg3"]);
+    assert!(r.is_ok(), "{:?}", r.unwrap_err().kind);
+
+    let m = r.unwrap();
+
+    let expected_bench1 = Some("foo");
+    let expected_bench2 = None;
+    let expected_bench3 = None;
+    let expected_args = vec!["arg1", "arg2", "arg3"];
+
+    assert_eq!(m.value_of("BENCH1"), expected_bench1);
+    assert_eq!(m.value_of("BENCH2"), expected_bench2);
+    assert_eq!(m.value_of("BENCH3"), expected_bench3);
+    assert_eq!(m.values_of("ARGS").unwrap().collect::<Vec<_>>(), &*expected_args);
+}
+
+#[test]
+fn missing_positional_hyphen_req_error() {
+    let r = App::new("bench")
+        .setting(AppSettings::AllowMissingPositional)
+        .arg(Arg::from_usage("[BENCH1] 'some bench'"))
+        .arg(Arg::from_usage("<BENCH2> 'some bench'"))
+        .arg(Arg::from_usage("[ARGS]... 'some args'"))
+        .get_matches_from_safe(vec!["bench", "foo", "--", "arg1", "arg2", "arg3"]);
+    assert!(r.is_err());
+    assert_eq!(r.unwrap_err().kind, ErrorKind::MissingRequiredArgument);
+}
+
 #[test]
 fn issue_1066_allow_leading_hyphen_and_unknown_args() {
     let res = App::new("prog")