Repository: kbknapp/clap-rs
Pull request #961 (2017-05-15 22:37:23)

Issue 960, 949, 917, and 932


<!-- 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/961)
<!-- Reviewable:end -->


mention-bot: @kbknapp, thanks for your PR! By analyzing the history of the files in this pull request, we identified @hgrecco, @starkat99 and @tormol to be potential reviewers.
coveralls: 
[![Coverage Status](https://coveralls.io/builds/11537825/badge)](https://coveralls.io/builds/11537825)

Coverage increased (+0.007%) to 91.814% when pulling **f9b95d48e502b783f635261929a7f9c731f1aff0 on issue-960** into **34a7436dd366da1a046e00310e96357ccf2dd533 on master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/11538031/badge)](https://coveralls.io/builds/11538031)

Coverage increased (+0.007%) to 91.814% when pulling **f9b95d48e502b783f635261929a7f9c731f1aff0 on issue-960** into **34a7436dd366da1a046e00310e96357ccf2dd533 on master**.

kbknapp: @homu r+
homu: :pushpin: Commit f9b95d4 has been approved by `kbknapp`

<!-- @homu r=kbknapp f9b95d48e502b783f635261929a7f9c731f1aff0 -->
homu: :hourglass: Testing commit f9b95d4 with merge d3f7177...
homu: :boom: Test timed out

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5a08349b..4aa84440 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,21 @@
+<a name="v2.24.2"></a>
+### v2.24.2 (2017-05-15)
+
+
+#### Bug Fixes
+
+*   adds a debug assertion to ensure all args added to groups actually exist ([14f6b8f3](https://github.com/kbknapp/clap-rs/commit/14f6b8f3a2f6df73aeeec9c54a54909b1acfc158), closes [#917](https://github.com/kbknapp/clap-rs/issues/917))
+*   fixes a bug where args that allow values to start with a hyphen couldnt contain a double hyphen -- as a value ([ebf73a09](https://github.com/kbknapp/clap-rs/commit/ebf73a09db6f3c03c19cdd76b1ba6113930e1643), closes [#960](https://github.com/kbknapp/clap-rs/issues/960))
+*   fixes a bug where positional argument help text is misaligned ([54c16836](https://github.com/kbknapp/clap-rs/commit/54c16836dea4651806a2cfad53146a83fa3abf21))
+
+#### Documentation
+
+* **App::template:**  adds details about the necessity to use AppSettings::UnifiedHelpMessage when using {unified} tags in the help template ([cf569438](https://github.com/kbknapp/clap-rs/commit/cf569438f309c199800bb8e46c9f140187de69d7), closes [#949](https://github.com/kbknapp/clap-rs/issues/949))
+* **Arg::allow_hyphen_values:**  updates the docs to include warnings for allow_hyphen_values and multiple(true) used together ([ded5a2f1](https://github.com/kbknapp/clap-rs/commit/ded5a2f15474d4a5bd46a67b130ccb8b6781bd01))
+* **clap_app!:**  adds using the @group specifier to the macro docs ([fe85fcb1](https://github.com/kbknapp/clap-rs/commit/fe85fcb1772b61f13b20b7ea5290e2437a76190c), closes [#932](https://github.com/kbknapp/clap-rs/issues/932))
+
+
+
 <a name="v2.24.0"></a>
 ### v2.24.0 (2017-05-07)
 
diff --git a/Cargo.toml b/Cargo.toml
index 89b39a7b..5590a870 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 
 name = "clap"
-version = "2.24.1"
+version = "2.24.2"
 authors = ["Kevin K. <kbknapp@gmail.com>"]
 exclude = ["examples/*", "clap-test/*", "tests/*", "benches/*", "*.png", "clap-perf/*", "*.dot"]
 repository = "https://github.com/kbknapp/clap-rs.git"
diff --git a/README.md b/README.md
index ae9a8c7d..e76e6060 100644
--- a/README.md
+++ b/README.md
@@ -45,14 +45,20 @@ Created by [gh-md-toc](https://github.com/ekalinin/github-markdown-toc)
 
 ## What's New
 
-Here's the highlights for v2.24.1
+Here's the highlights for v2.24.2
+
+* fixes a bug where args that allow values to start with a hyphen couldnt contain a double hyphen -- as a value 
+* fixes a bug where positional argument help text is misaligned 
+* **App::template docs:**  adds details about the necessity to use AppSettings::UnifiedHelpMessage when using {unified} tags in the help template 
+* **Arg::allow_hyphen_values docs:**  updates the docs to include warnings for allow_hyphen_values and multiple(true) used together 
+* **clap_app! docs:**  adds using the @group specifier to the macro docs 
+* adds a debug assertion to ensure all args added to groups actually exist 
+
+Here's the highlights for v2.21.0 to v2.24.1
 
 * fixes a bug where args with last(true) and required(true) set were not being printed in the usage string 
 * fixes a bug that was printing the arg name, instead of value name when Arg::last(true) was used 
 * fixes a bug where flags were parsed as flags AND positional values when specific combinations of settings were used 
-
-Here's the highlights for v2.21.0 to v2.24.0
-
 * **README.md:**  fix some typos 
 * **Arg:**  add `default_value_os` 
 * **arg_matches.rs:**  Added a Default implementation for Values and OsValues iterators. 
diff --git a/src/app/mod.rs b/src/app/mod.rs
index 0c9f9aee..03721386 100644
--- a/src/app/mod.rs
+++ b/src/app/mod.rs
@@ -537,7 +537,9 @@ impl<'a, 'b> App<'a, 'b> {
     ///   * `{usage}`       - Automatically generated or given usage string.
     ///   * `{all-args}`    - Help for all arguments (options, flags, positionals arguments,
     ///                       and subcommands) including titles.
-    ///   * `{unified}`     - Unified help for options and flags.
+    ///   * `{unified}`     - Unified help for options and flags. Note, you must *also* set 
+    ///                       [`AppSettings::UnifiedHelpMessage`] to fully merge both options and 
+    ///                       flags, otherwise the ordering is "best effort"
     ///   * `{flags}`       - Help for flags.
     ///   * `{options}`     - Help for options.
     ///   * `{positionals}` - Help for positionals arguments.
@@ -559,6 +561,7 @@ impl<'a, 'b> App<'a, 'b> {
     /// [`App::about`]: ./struct.App.html#method.about
     /// [`App::after_help`]: ./struct.App.html#method.after_help
     /// [`App::before_help`]: ./struct.App.html#method.before_help
+    /// [`AppSettings::UnifiedHelpMessage`]: ./enum.AppSettings.html#variant.UnifiedHelpMessage
     pub fn template<S: Into<&'b str>>(mut self, s: S) -> Self {
         self.p.meta.template = Some(s.into());
         self
diff --git a/src/app/parser.rs b/src/app/parser.rs
index 84d999e7..0a2ac5ca 100644
--- a/src/app/parser.rs
+++ b/src/app/parser.rs
@@ -128,6 +128,49 @@ impl<'a, 'b> Parser<'a, 'b>
         self.gen_completions_to(for_shell, &mut file)
     }
 
+    #[inline]
+    fn app_debug_asserts(&mut self) -> bool {
+        assert!(self.verify_positionals());
+        let should_err = self.groups
+            .iter()
+            .all(|g| {
+                g.args
+                    .iter()
+                    .all(|arg| {
+                             (self.flags.iter().any(|f| &f.b.name == arg) ||
+                              self.opts.iter().any(|o| &o.b.name == arg) ||
+                              self.positionals.values().any(|p| &p.b.name == arg) ||
+                              self.groups.iter().any(|g| &g.name == arg))
+                         })
+            });
+        let g = self.groups
+            .iter()
+            .find(|g| {
+                g.args
+                    .iter()
+                    .any(|arg| {
+                             !(self.flags.iter().any(|f| &f.b.name == arg) ||
+                               self.opts.iter().any(|o| &o.b.name == arg) ||
+                               self.positionals.values().any(|p| &p.b.name == arg) ||
+                               self.groups.iter().any(|g| &g.name == arg))
+                         })
+            });
+        assert!(should_err,
+                "The group '{}' contains the arg '{}' that doesn't actually exist.",
+                g.unwrap().name,
+                g.unwrap()
+                    .args
+                    .iter()
+                    .find(|arg| {
+                              !(self.flags.iter().any(|f| &&f.b.name == arg) ||
+                                self.opts.iter().any(|o| &&o.b.name == arg) ||
+                                self.positionals.values().any(|p| &&p.b.name == arg) ||
+                                self.groups.iter().any(|g| &&g.name == arg))
+                          })
+                    .unwrap());
+        true
+    }
+
     #[inline]
     fn debug_asserts(&self, a: &Arg) -> bool {
         assert!(!arg_names!(self).any(|name| name == a.b.name),
@@ -417,9 +460,7 @@ impl<'a, 'b> Parser<'a, 'b>
         if self.flags.is_empty() {
             return false;
         }
-        self.flags
-            .iter()
-            .any(|f| !f.is_set(ArgSettings::Hidden))
+        self.flags.iter().any(|f| !f.is_set(ArgSettings::Hidden))
     }
 
     #[inline]
@@ -739,8 +780,10 @@ impl<'a, 'b> Parser<'a, 'b>
         // Is this a new argument, or values from a previous option?
         let mut ret = if arg_os.starts_with(b"--") {
             debugln!("Parser::is_new_arg: -- found");
-            if arg_os.len_() == 2 {
+            if arg_os.len_() == 2 && !arg_allows_tac {
                 return true; // We have to return true so override everything else
+            } else if arg_allows_tac {
+                return false;
             }
             true
         } else if arg_os.starts_with(b"-") {
@@ -769,7 +812,7 @@ impl<'a, 'b> Parser<'a, 'b>
     {
         debugln!("Parser::get_matches_with;");
         // Verify all positional assertions pass
-        debug_assert!(self.verify_positionals());
+        debug_assert!(self.app_debug_asserts());
         if self.positionals
                .values()
                .any(|a| {
@@ -800,7 +843,7 @@ impl<'a, 'b> Parser<'a, 'b>
             self.unset(AS::ValidNegNumFound);
             // Is this a new argument, or values from a previous option?
             let starts_new_arg = self.is_new_arg(&arg_os, needs_val_of);
-            if arg_os.starts_with(b"--") && arg_os.len_() == 2 {
+            if arg_os.starts_with(b"--") && arg_os.len_() == 2 && starts_new_arg {
                 debugln!("Parser::get_matches_with: setting TrailingVals=true");
                 self.set(AS::TrailingValues);
                 continue;
@@ -850,9 +893,6 @@ impl<'a, 'b> Parser<'a, 'b>
                             ParseResult::ValuesDone => continue,
                             _ => (),
                         }
-                        // if !(needs_val_of == ParseResult::Flag && self.is_set(AS::AllowLeadingHyphen)) {
-                        //     continue;
-                        // }
                     } else if arg_os.starts_with(b"-") && arg_os.len_() != 1 {
                         // Try to parse short args like normal, if AllowLeadingHyphen or
                         // AllowNegativeNumbers is set, parse_short_arg will *not* throw
@@ -871,7 +911,6 @@ impl<'a, 'b> Parser<'a, 'b>
                                         self.color()));
                                 }
                             }
-                            // ParseResult::MaybeHyphenValue => (),
                             ParseResult::Opt(..) |
                             ParseResult::Flag |
                             ParseResult::ValuesDone => continue,
@@ -1246,8 +1285,7 @@ impl<'a, 'b> Parser<'a, 'b>
             let arg = FlagBuilder {
                 b: Base {
                     name: "vclap_version",
-                    help: self.version_message
-                        .or(Some("Prints version information")),
+                    help: self.version_message.or(Some("Prints version information")),
                     ..Default::default()
                 },
                 s: Switched {
@@ -1263,7 +1301,7 @@ impl<'a, 'b> Parser<'a, 'b>
             debugln!("Parser::create_help_and_version: Building help");
             self.subcommands
                 .push(App::new("help")
-                    .about("Prints this message or the help of the given subcommand(s)"));
+                          .about("Prints this message or the help of the given subcommand(s)"));
         }
     }
 
@@ -1309,9 +1347,7 @@ impl<'a, 'b> Parser<'a, 'b>
     fn use_long_help(&self) -> bool {
         let ul = self.flags.iter().any(|f| f.b.long_help.is_some()) ||
                  self.opts.iter().any(|o| o.b.long_help.is_some()) ||
-                 self.positionals
-                     .values()
-                     .any(|p| p.b.long_help.is_some()) ||
+                 self.positionals.values().any(|p| p.b.long_help.is_some()) ||
                  self.subcommands
                      .iter()
                      .any(|s| s.p.meta.long_about.is_some());
diff --git a/src/args/arg.rs b/src/args/arg.rs
index c3cb531e..9e81911f 100644
--- a/src/args/arg.rs
+++ b/src/args/arg.rs
@@ -820,6 +820,11 @@ impl<'a, 'b> Arg<'a, 'b> {
 
     /// Allows values which start with a leading hyphen (`-`)
     ///
+    /// **WARNING**: Take caution when using this setting, combined with [`Arg::multiple(true)`] as
+    /// it this becomes ambigous `$ prog --arg -- -- val`. All three `--, --, val` will be values
+    /// when the user may have thought the second `--` would constitute the normal, "Only
+    /// positional args follow" idiom. To fix this, consider using [`Arg::number_of_values(1)`]
+    ///
     /// **WARNING**: When building your CLIs, consider the effects of allowing leading hyphens and
     /// the user passing in a value that matches a valid short. For example `prog -opt -F` where
     /// `-F` is supposed to be a value, yet `-F` is *also* a valid short for anther arg. Care should
@@ -867,6 +872,8 @@ impl<'a, 'b> Arg<'a, 'b> {
     /// assert_eq!(res.unwrap_err().kind, ErrorKind::UnknownArgument);
     /// ```
     /// [`Arg::allow_hyphen_values(true)`]: ./struct.Arg.html#method.allow_hyphen_values
+    /// [`Arg::multiple(true)`]: ./struct.Arg.html#method.multiple
+    /// [`Arg::number_of_values(1)`]: ./struct.Arg.html#method.number_of_values
     pub fn allow_hyphen_values(self, a: bool) -> Self {
         if a {
             self.set(ArgSettings::AllowLeadingHyphen)
diff --git a/src/macros.rs b/src/macros.rs
index 650042fb..853daaaa 100644
--- a/src/macros.rs
+++ b/src/macros.rs
@@ -549,6 +549,11 @@ macro_rules! app_from_crate {
 ///     (@arg CONFIG: -c --config +takes_value "Sets a custom config file")
 ///     (@arg INPUT: +required "Sets the input file to use")
 ///     (@arg debug: -d ... "Sets the level of debugging information")
+///     (@group difficulty =>
+///         (@arg hard: -h --hard "Sets hard mode")
+///         (@arg normal: -n --normal "Sets normal mode")
+///         (@arg easy: -e --easy "Sets easy mode")
+///     )
 ///     (@subcommand test =>
 ///         (about: "controls testing features")
 ///         (version: "1.3")


Test patch:
diff --git a/tests/groups.rs b/tests/groups.rs
index 7f7dd709..fa497bfb 100644
--- a/tests/groups.rs
+++ b/tests/groups.rs
@@ -41,6 +41,18 @@ fn required_group_missing_arg() {
     assert_eq!(err.kind, ErrorKind::MissingRequiredArgument);
 }
 
+#[test]
+#[should_panic]
+fn non_existing_arg() {
+    let _ = App::new("group")
+        .args_from_usage("-f, --flag 'some flag'
+                          -c, --color 'some other flag'")
+        .group(ArgGroup::with_name("req")
+            .args(&["flg", "color"])
+            .required(true))
+        .get_matches_from_safe(vec![""]);
+}
+
 #[test]
 fn group_single_value() {
     let m = App::new("group")
diff --git a/tests/opts.rs b/tests/opts.rs
index a7d2abe7..31f39512 100644
--- a/tests/opts.rs
+++ b/tests/opts.rs
@@ -28,6 +28,20 @@ fn require_equals_fail() {
     assert_eq!(res.unwrap_err().kind, ErrorKind::EmptyValue);
 }
 
+#[test]
+fn double_hyphen_as_value() {
+    let res = App::new("prog")
+        .arg(Arg::with_name("cfg")
+            .takes_value(true)
+            .allow_hyphen_values(true)
+            .long("config"))
+        .get_matches_from_safe(vec![
+            "prog", "--config", "--"
+        ]);
+    assert!(res.is_ok(), "{:?}", res);
+    assert_eq!(res.unwrap().value_of("cfg"), Some("--"));
+}
+
 #[test]
 fn require_equals_no_empty_values_fail() {
     let res = App::new("prog")
@@ -330,10 +344,10 @@ fn leading_hyphen_with_flag_before() {
 #[test]
 fn leading_hyphen_with_only_pos_follows() {
     let r = App::new("mvae")
-        .arg(Arg::from_usage("-o [opt]... 'some opt'").allow_hyphen_values(true))
+        .arg(Arg::from_usage("-o [opt]... 'some opt'").number_of_values(1).allow_hyphen_values(true))
         .arg_from_usage("[arg] 'some arg'")
         .get_matches_from_safe(vec!["", "-o", "-2", "--", "val"]);
-    assert!(r.is_ok());
+    assert!(r.is_ok(), "{:?}", r);
     let m = r.unwrap();
     assert!(m.is_present("o"));
     assert_eq!(m.values_of("o").unwrap().collect::<Vec<_>>(), &["-2"]);