Repository: kbknapp/clap-rs
Pull request #980 (2017-06-08 02:30:37)

Suggest to use flag after a sub-command 
Hi @kbknapp,

I saw that  https://github.com/kbknapp/clap-rs/issues/927 was mentored, so I'd like to give it a try.
Here is a first attempt. The message is simpler than what to suggest in the issue, because I could not find out how to print this part from `did_you_mean_flag_suffix`:

```
        myapp --foo bar
              ^^^^^

```

By the way, I splitted `did_you_mean_suffix` into `did_you_mean_flag_suffix` and `did_you_mean_value_suffix`, is that ok?

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


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

Coverage increased (+0.06%) to 91.985% when pulling **4e0ae66034502a79dfcb40abd2afd52c80908329 on little-dude:flag** into **49f9dc166a20483643e6e08606b2956ee5e8521d on kbknapp:master**.

kbknapp: Wow @little-dude looks like you're on a roll! Great work! I'm re-building the AppVeyor build to see if it was just a fluke.
kbknapp: Also, would you mind re-doing the commit subject lines so they'll be picked up automatically by [clog](https://github.com/clog-tool/clog-cli)?

I'd suggest: `imp(Suggestions): suggests to use flag after subcommand when applicable` and `tests(Suggestions): update tests for subcommand suggestions`

Patch:
diff --git a/.appveyor.yml b/.appveyor.yml
deleted file mode 100644
index 247a550d..00000000
--- a/.appveyor.yml
+++ /dev/null
@@ -1,17 +0,0 @@
-environment:
-  matrix:
-  - TARGET: x86_64-pc-windows-msvc
-  - TARGET: i686-pc-windows-msvc
-  - TARGET: x86_64-pc-windows-gnu
-  - TARGET: i686-pc-windows-gnu
-  RUST_BACKTRACE: full
-install:
-  - curl -sSf -o rustup-init.exe https://win.rustup.rs/
-  - rustup-init.exe -y --default-host %TARGET%
-  - set PATH=%PATH%;C:\Users\appveyor\.cargo\bin
-  - rustc -vV
-  - cargo -vV
-build: false
-test_script:
-  - cargo build --verbose --features yaml
-  - cargo test --verbose --features yaml
diff --git a/Cargo.toml b/Cargo.toml
index ceb927ab..c9c3ea85 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -16,7 +16,7 @@ A simple to use, efficient, and full featured  Command Line Argument Parser
 """
 
 [dependencies]
-bitflags              = "0.9"
+bitflags              = "0.8.0"
 vec_map               = "0.8"
 unicode-width         = "0.1.4"
 unicode-segmentation  = "~1.1.0" # 1.2.0 requires Rust 1.13.0
diff --git a/appveyor.yml b/appveyor.yml
new file mode 100644
index 00000000..a9ee3e85
--- /dev/null
+++ b/appveyor.yml
@@ -0,0 +1,12 @@
+install:
+  - ps: Start-FileDownload 'https://static.rust-lang.org/dist/rust-nightly-i686-pc-windows-gnu.exe'
+  - rust-nightly-i686-pc-windows-gnu.exe /VERYSILENT /NORESTART /DIR="C:\Program Files (x86)\Rust"
+  - SET PATH=%PATH%;C:\Program Files (x86)\Rust\bin
+  - rustc -V
+  - cargo -V
+
+build: false
+
+test_script:
+  - cargo build --features yaml
+  - cargo test --features yaml 
diff --git a/examples/22_stop_parsing_with_--.rs b/examples/22_stop_parsing_with_--.rs
deleted file mode 100644
index 1e0282fe..00000000
--- a/examples/22_stop_parsing_with_--.rs
+++ /dev/null
@@ -1,24 +0,0 @@
-extern crate clap;
-
-use clap::{App, Arg};
-
-/// myprog -f -p=bob -- sloppy slop slop
-fn main() {
-
-    let matches = App::new("myprog")
-        .arg(Arg::with_name("eff")
-            .short("f"))
-        .arg(Arg::with_name("pea")
-             .short("p")
-             .takes_value(true))
-        .arg(Arg::with_name("slop")
-             .multiple(true))
-        .get_matches();
-
-
-    println!("-f used: {:?}", matches.is_present("eff"));
-    println!("-p's value: {:?}", matches.value_of("pea"));
-    println!("'slops' values: {:?}", matches.values_of("slop").map(|vals| vals.collect::<Vec<_>>()));
-
-    // Continued program logic goes here...
-}
diff --git a/src/app/help.rs b/src/app/help.rs
index 29322db4..800272b4 100644
--- a/src/app/help.rs
+++ b/src/app/help.rs
@@ -187,7 +187,7 @@ impl<'a> Help<'a> {
                                    arg.is_set(ArgSettings::NextLineHelp)
                                }) {
             if arg.longest_filter() {
-                self.longest = cmp::max(self.longest, str_width(arg.to_string().as_str()));
+                self.longest = cmp::max(self.longest, arg.to_string().len());
             }
             arg_v.push(arg)
         }
@@ -220,7 +220,7 @@ impl<'a> Help<'a> {
         }) {
             if arg.longest_filter() {
                 debugln!("Help::write_args: Current Longest...{}", self.longest);
-                self.longest = cmp::max(self.longest, str_width(arg.to_string().as_str()));
+                self.longest = cmp::max(self.longest, arg.to_string().len());
                 debugln!("Help::write_args: New Longest...{}", self.longest);
             }
             let btm = ord_m.entry(arg.disp_ord()).or_insert(BTreeMap::new());
@@ -352,7 +352,7 @@ impl<'a> Help<'a> {
             debug!("Help::val: next_line...");
             if !(nlh || self.force_next_line) {
                 sdebugln!("No");
-                let self_len = str_width(arg.to_string().as_str());
+                let self_len = arg.to_string().len();
                 // subtract ourself
                 let mut spcs = self.longest - self_len;
                 // Since we're writing spaces from the tab point we first need to know if we
@@ -371,7 +371,7 @@ impl<'a> Help<'a> {
             }
         } else if !(nlh || self.force_next_line) {
             sdebugln!("No, and not next_line");
-            write_nspaces!(self.writer, self.longest + 4 - (str_width(arg.to_string().as_str())));
+            write_nspaces!(self.writer, self.longest + 4 - (arg.to_string().len()));
         } else {
             sdebugln!("No");
         }
@@ -582,8 +582,7 @@ impl<'a> Help<'a> {
             let btm = ord_m
                 .entry(sc.p.meta.disp_ord)
                 .or_insert(BTreeMap::new());
-            self.longest = cmp::max(self.longest, str_width(sc.p.meta.name.as_str()));
-            //self.longest = cmp::max(self.longest, sc.p.meta.name.len());
+            self.longest = cmp::max(self.longest, sc.p.meta.name.len());
             btm.insert(sc.p.meta.name.clone(), sc.clone());
         }
 
diff --git a/src/app/parser.rs b/src/app/parser.rs
index 9f373fb6..800e0828 100644
--- a/src/app/parser.rs
+++ b/src/app/parser.rs
@@ -1662,11 +1662,10 @@ impl<'a, 'b> Parser<'a, 'b>
     }
 
     fn did_you_mean_error(&self, arg: &str, matcher: &mut ArgMatcher<'a>) -> ClapResult<()> {
-        // Didn't match a flag or option...maybe it was a typo and close to one
+
+        // Didn't match a flag or option
         let suffix =
-            suggestions::did_you_mean_suffix(arg,
-                                             longs!(self),
-                                             suggestions::DidYouMeanMessageStyle::LongFlag);
+            suggestions::did_you_mean_flag_suffix(arg, longs!(self), &self.subcommands);
 
         // Add the arg to the matches to build a proper usage string
         if let Some(name) = suffix.1 {
diff --git a/src/app/settings.rs b/src/app/settings.rs
index d2f78f7c..987b0258 100644
--- a/src/app/settings.rs
+++ b/src/app/settings.rs
@@ -4,47 +4,47 @@ use std::str::FromStr;
 use std::ops::BitOr;
 
 bitflags! {
-    struct Flags: u64 {
-        const SC_NEGATE_REQS       = 1 << 0;
-        const SC_REQUIRED          = 1 << 1;
-        const A_REQUIRED_ELSE_HELP = 1 << 2;
-        const GLOBAL_VERSION       = 1 << 3;
-        const VERSIONLESS_SC       = 1 << 4;
-        const UNIFIED_HELP         = 1 << 5;
-        const WAIT_ON_ERROR        = 1 << 6;
-        const SC_REQUIRED_ELSE_HELP= 1 << 7;
-        const NEEDS_LONG_HELP      = 1 << 8;
-        const NEEDS_LONG_VERSION   = 1 << 9;
-        const NEEDS_SC_HELP        = 1 << 10;
-        const DISABLE_VERSION      = 1 << 11;
-        const HIDDEN               = 1 << 12;
-        const TRAILING_VARARG      = 1 << 13;
-        const NO_BIN_NAME          = 1 << 14;
-        const ALLOW_UNK_SC         = 1 << 15;
-        const UTF8_STRICT          = 1 << 16;
-        const UTF8_NONE            = 1 << 17;
-        const LEADING_HYPHEN       = 1 << 18;
-        const NO_POS_VALUES        = 1 << 19;
-        const NEXT_LINE_HELP       = 1 << 20;
-        const DERIVE_DISP_ORDER    = 1 << 21;
-        const COLORED_HELP         = 1 << 22;
-        const COLOR_ALWAYS         = 1 << 23;
-        const COLOR_AUTO           = 1 << 24;
-        const COLOR_NEVER          = 1 << 25;
-        const DONT_DELIM_TRAIL     = 1 << 26;
-        const ALLOW_NEG_NUMS       = 1 << 27;
-        const LOW_INDEX_MUL_POS    = 1 << 28;
-        const DISABLE_HELP_SC      = 1 << 29;
-        const DONT_COLLAPSE_ARGS   = 1 << 30;
-        const ARGS_NEGATE_SCS      = 1 << 31;
-        const PROPAGATE_VALS_DOWN  = 1 << 32;
-        const ALLOW_MISSING_POS    = 1 << 33;
-        const TRAILING_VALUES      = 1 << 34;
-        const VALID_NEG_NUM_FOUND  = 1 << 35;
-        const PROPOGATED           = 1 << 36;
-        const VALID_ARG_FOUND      = 1 << 37;
-        const INFER_SUBCOMMANDS    = 1 << 38;
-        const CONTAINS_LAST        = 1 << 39;
+    flags Flags: u64 {
+        const SC_NEGATE_REQS       = 1 << 0,
+        const SC_REQUIRED          = 1 << 1,
+        const A_REQUIRED_ELSE_HELP = 1 << 2,
+        const GLOBAL_VERSION       = 1 << 3,
+        const VERSIONLESS_SC       = 1 << 4,
+        const UNIFIED_HELP         = 1 << 5,
+        const WAIT_ON_ERROR        = 1 << 6,
+        const SC_REQUIRED_ELSE_HELP= 1 << 7,
+        const NEEDS_LONG_HELP      = 1 << 8,
+        const NEEDS_LONG_VERSION   = 1 << 9,
+        const NEEDS_SC_HELP        = 1 << 10,
+        const DISABLE_VERSION      = 1 << 11,
+        const HIDDEN               = 1 << 12,
+        const TRAILING_VARARG      = 1 << 13,
+        const NO_BIN_NAME          = 1 << 14,
+        const ALLOW_UNK_SC         = 1 << 15,
+        const UTF8_STRICT          = 1 << 16,
+        const UTF8_NONE            = 1 << 17,
+        const LEADING_HYPHEN       = 1 << 18,
+        const NO_POS_VALUES        = 1 << 19,
+        const NEXT_LINE_HELP       = 1 << 20,
+        const DERIVE_DISP_ORDER    = 1 << 21,
+        const COLORED_HELP         = 1 << 22,
+        const COLOR_ALWAYS         = 1 << 23,
+        const COLOR_AUTO           = 1 << 24,
+        const COLOR_NEVER          = 1 << 25,
+        const DONT_DELIM_TRAIL     = 1 << 26,
+        const ALLOW_NEG_NUMS       = 1 << 27,
+        const LOW_INDEX_MUL_POS    = 1 << 28,
+        const DISABLE_HELP_SC      = 1 << 29,
+        const DONT_COLLAPSE_ARGS   = 1 << 30,
+        const ARGS_NEGATE_SCS      = 1 << 31,
+        const PROPAGATE_VALS_DOWN  = 1 << 32,
+        const ALLOW_MISSING_POS    = 1 << 33,
+        const TRAILING_VALUES      = 1 << 34,
+        const VALID_NEG_NUM_FOUND  = 1 << 35,
+        const PROPOGATED           = 1 << 36,
+        const VALID_ARG_FOUND      = 1 << 37,
+        const INFER_SUBCOMMANDS    = 1 << 38,
+        const CONTAINS_LAST        = 1 << 39,
     }
 }
 
diff --git a/src/args/settings.rs b/src/args/settings.rs
index 55ab7428..347946ee 100644
--- a/src/args/settings.rs
+++ b/src/args/settings.rs
@@ -3,23 +3,23 @@ use std::ascii::AsciiExt;
 use std::str::FromStr;
 
 bitflags! {
-    struct Flags: u16 {
-        const REQUIRED         = 1 << 0;
-        const MULTIPLE         = 1 << 1;
-        const EMPTY_VALS       = 1 << 2;
-        const GLOBAL           = 1 << 3;
-        const HIDDEN           = 1 << 4;
-        const TAKES_VAL        = 1 << 5;
-        const USE_DELIM        = 1 << 6;
-        const NEXT_LINE_HELP   = 1 << 7;
-        const R_UNLESS_ALL     = 1 << 8;
-        const REQ_DELIM        = 1 << 9;
-        const DELIM_NOT_SET    = 1 << 10;
-        const HIDE_POS_VALS    = 1 << 11;
-        const ALLOW_TAC_VALS   = 1 << 12;
-        const REQUIRE_EQUALS   = 1 << 13;
-        const LAST             = 1 << 14;
-        const HIDE_DEFAULT_VAL = 1 << 15;
+    flags Flags: u16 {
+        const REQUIRED         = 1 << 0,
+        const MULTIPLE         = 1 << 1,
+        const EMPTY_VALS       = 1 << 2,
+        const GLOBAL           = 1 << 3,
+        const HIDDEN           = 1 << 4,
+        const TAKES_VAL        = 1 << 5,
+        const USE_DELIM        = 1 << 6,
+        const NEXT_LINE_HELP   = 1 << 7,
+        const R_UNLESS_ALL     = 1 << 8,
+        const REQ_DELIM        = 1 << 9,
+        const DELIM_NOT_SET    = 1 << 10,
+        const HIDE_POS_VALS    = 1 << 11,
+        const ALLOW_TAC_VALS   = 1 << 12,
+        const REQUIRE_EQUALS   = 1 << 13,
+        const LAST             = 1 << 14,
+        const HIDE_DEFAULT_VAL = 1 << 15,
     }
 }
 
diff --git a/src/errors.rs b/src/errors.rs
index 1781067f..bd1e7d55 100644
--- a/src/errors.rs
+++ b/src/errors.rs
@@ -483,9 +483,9 @@ impl Error {
             when: color,
         });
         let suffix =
-            suggestions::did_you_mean_suffix(bad_val.as_ref(),
-                                             good_vals.iter(),
-                                             suggestions::DidYouMeanMessageStyle::EnumValue);
+            suggestions::did_you_mean_value_suffix(
+                bad_val.as_ref(),
+                good_vals.iter());
 
         let mut sorted = vec![];
         for v in good_vals {
diff --git a/src/suggestions.rs b/src/suggestions.rs
index 5c7cade5..bf586def 100644
--- a/src/suggestions.rs
+++ b/src/suggestions.rs
@@ -1,3 +1,4 @@
+use app::App;
 // Third Party
 #[cfg(feature = "suggestions")]
 use strsim;
@@ -40,40 +41,46 @@ pub fn did_you_mean<'a, T: ?Sized, I>(_: &str, _: I) -> Option<&'a str>
 
 /// Returns a suffix that can be empty, or is the standard 'did you mean' phrase
 #[cfg_attr(feature = "lints", allow(needless_lifetimes))]
-pub fn did_you_mean_suffix<'z, T, I>(arg: &str,
-                                     values: I,
-                                     style: DidYouMeanMessageStyle)
+pub fn did_you_mean_flag_suffix<'z, T, I>(arg: &str, longs: I, subcommands: &'z [App])
                                      -> (String, Option<&'z str>)
     where T: AsRef<str> + 'z,
           I: IntoIterator<Item = &'z T>
 {
-    match did_you_mean(arg, values) {
+    match did_you_mean(arg, longs) {
         Some(candidate) => {
-            let mut suffix = "\n\tDid you mean ".to_owned();
-            match style {
-                DidYouMeanMessageStyle::LongFlag => {
-                    suffix.push_str(&Format::Good("--").to_string())
+            let suffix = format!("\n\tDid you mean {}{}?", Format::Good("--"), Format::Good(candidate));
+            return (suffix, Some(candidate))
+        }
+        None => {
+            for subcommand in subcommands {
+                let opts = subcommand.p.flags.iter().filter_map(|f| f.s.long).chain(
+                    subcommand.p.opts.iter().filter_map(|o| o.s.long));
+
+                if let Some(candidate) = did_you_mean(arg, opts) {
+                    let suffix = format!(
+                        "\n\tDid you mean to put '--{}' after the subcommand '{}'?",
+                        Format::Good(arg),
+                        Format::Good(candidate));
+                    return (suffix, Some(candidate));
                 }
-                DidYouMeanMessageStyle::EnumValue => suffix.push('\''),
-            }
-            suffix.push_str(&Format::Good(candidate).to_string()[..]);
-            if let DidYouMeanMessageStyle::EnumValue = style {
-                suffix.push('\'');
             }
-            suffix.push_str("?");
-            (suffix, Some(candidate))
         }
-        None => (String::new(), None),
     }
+    return (String::new(), None)
 }
 
-/// A helper to determine message formatting
-#[derive(Copy, Clone, Debug)]
-pub enum DidYouMeanMessageStyle {
-    /// Suggested value is a long flag
-    LongFlag,
-    /// Suggested value is one of various possible values
-    EnumValue,
+/// Returns a suffix that can be empty, or is the standard 'did you mean' phrase
+pub fn did_you_mean_value_suffix<'z, T, I>(arg: &str, values: I) -> (String, Option<&'z str>)
+    where T: AsRef<str> + 'z,
+          I: IntoIterator<Item = &'z T>
+{
+    match did_you_mean(arg, values) {
+        Some(candidate) => {
+            let suffix = format!("\n\tDid you mean '{}'?", Format::Good(candidate));
+            (suffix, Some(candidate))
+        }
+        None => (String::new(), None),
+    }
 }
 
 #[cfg(all(test, features = "suggestions"))]
@@ -96,15 +103,13 @@ mod test {
     fn suffix_long() {
         let p_vals = ["test", "possible", "values"];
         let suffix = "\n\tDid you mean \'--test\'?";
-        assert_eq!(did_you_mean_suffix("tst", p_vals.iter(), DidYouMeanMessageStyle::LongFlag),
-                   (suffix, Some("test")));
+        assert_eq!(did_you_mean_flag_suffix("tst", p_vals.iter(), []), (suffix, Some("test")));
     }
 
     #[test]
     fn suffix_enum() {
         let p_vals = ["test", "possible", "values"];
         let suffix = "\n\tDid you mean \'test\'?";
-        assert_eq!(did_you_mean_suffix("tst", p_vals.iter(), DidYouMeanMessageStyle::EnumValue),
-                   (suffix, Some("test")));
+        assert_eq!(did_you_mean_value_suffix("tst", p_vals.iter()), (suffix, Some("test")));
     }
 }


Test patch:
diff --git a/clap-test.rs b/clap-test.rs
index 9b218172..2f91b9e8 100644
--- a/clap-test.rs
+++ b/clap-test.rs
@@ -70,6 +70,7 @@ mod test {
                                     .version("0.1")
                                     .author("Kevin K. <kbknapp@gmail.com>")
                                     .arg_from_usage("-o --option [scoption]... 'tests options'")
+                                    .arg_from_usage("-s --subcmdarg [subcmdarg] 'tests other args'")
                                     .arg_from_usage("[scpositional] 'tests positionals'"))
     }
 }
diff --git a/tests/help.rs b/tests/help.rs
index b00ee048..ef354ca9 100644
--- a/tests/help.rs
+++ b/tests/help.rs
@@ -118,7 +118,8 @@ FLAGS:
     -V, --version    Prints version information
 
 OPTIONS:
-    -o, --option <scoption>...    tests options
+    -o, --option <scoption>...     tests options
+    -s, --subcmdarg <subcmdarg>    tests other args
 
 ARGS:
     <scpositional>    tests positionals";
diff --git a/tests/subcommands.rs b/tests/subcommands.rs
index 00636b02..78d39cde 100644
--- a/tests/subcommands.rs
+++ b/tests/subcommands.rs
@@ -42,6 +42,15 @@ USAGE:
 
 For more information try --help";
 
+#[cfg(feature = "suggestions")]
+static DYM2: &'static str = "error: Found argument '--subcmdarg' which wasn't expected, or isn't valid in this context
+\tDid you mean to put '--subcmdarg' after the subcommand 'subcmdarg'?
+
+USAGE:
+    clap-test [FLAGS] [OPTIONS] [ARGS] [SUBCOMMAND]
+
+For more information try --help";
+
 #[test]
 fn subcommand() {
     let m = App::new("test")
@@ -121,6 +130,7 @@ fn multiple_aliases() {
 #[cfg(feature="suggestions")]
 fn subcmd_did_you_mean_output() {
     assert!(test::compare_output(test::complex_app(), "clap-test subcm", DYM, true));
+    assert!(test::compare_output(test::complex_app(), "clap-test --subcmdarg foo", DYM2, true));
 }
 
 #[test]