Merge pull request #463 from kbknapp/issue-456

Issue 456 and 461

CHANGELOG.md

@@ -1,3 +1,13 @@
+<a name="v2.2.2"></a>
+### v2.2.2 (2016-03-27)
+
+
+#### Bug Fixes
+
+* **Help Message:**  fixes bug with wrapping in the middle of a unicode sequence ([05365ddc](https://github.com/kbknapp/clap-rs/commit/05365ddcc252e4b49e7a75e199d6001a430bd84d), closes [#456](https://github.com/kbknapp/clap-rs/issues/456))
+* **Usage Strings:**  fixes small bug where -- would appear needlessly in usage strings ([6933b849](https://github.com/kbknapp/clap-rs/commit/6933b8491c2a7e28cdb61b47dcf10caf33c2f78a), closes [#461](https://github.com/kbknapp/clap-rs/issues/461))
+
+
 <a name="2.2.1"></a>
 ### 2.2.1 (2016-03-16)
 

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 
 name = "clap"
-version = "2.2.1"
+version = "2.2.2"
 authors = ["Kevin K. <kbknapp@gmail.com>"]
 exclude = ["examples/*", "clap-tests/*", "tests/*", "benches/*", "*.png", "clap-perf/*"]
 description = "A simple to use, efficient, and full featured  Command Line Argument Parser"
@@ -19,13 +19,14 @@ ansi_term = { version = "~0.7.2",  optional = true }
 strsim    = { version = "~0.4.0",  optional = true }
 yaml-rust = { version = "~0.3",    optional = true }
 clippy    = { version = "=0.0.55", optional = true }
+unicode-width = { version = "~0.1.3",  optional = true }
 
 [features]
 default     = ["suggestions", "color", "wrap_help"]
 suggestions = ["strsim"]
 color       = ["ansi_term"]
 yaml        = ["yaml-rust"]
-wrap_help   = ["libc"]
+wrap_help   = ["libc", "unicode-width"]
 lints       = ["clippy", "nightly"]
 nightly     = [] # for building with nightly and unstable features
 unstable    = [] # for building with unstable features on stable Rust

README.md

@@ -457,7 +457,7 @@ The following is a list of optional `clap` features:
 
 * **"suggestions"**: Turns on the `Did you mean '--myoption' ?` feature for when users make typos. (builds dependency `strsim`)
 * **"color"**: Turns on colored error messages. This feature only works on non-Windows OSs. (builds dependency `ansi-term`)
-* **"wrap_help"**: Automatically detects terminal width and wraps long help text lines with proper indentation alignment (builds dependency `libc`)
+* **"wrap_help"**: Automatically detects terminal width and wraps long help text lines with proper indentation alignment (builds dependency `libc` and 'unicode-width')
 * **"lints"**: This is **not** included by default and should only be used while developing to run basic lints against changes. This can only be used on Rust nightly. (builds dependency `clippy`)
 * **"debug"**: This is **not** included by default and should only be used while developing to display debugging information.
 * **"yaml"**: This is **not** included by default. Enables building CLIs from YAML documents. (builds dependency `yaml-rust`)

src/app/parser.rs

@@ -341,19 +341,19 @@ impl<'a, 'b> Parser<'a, 'b> where 'a: 'b {
     }
 
     pub fn has_flags(&self) -> bool {
-        self.flags.is_empty()
+        !self.flags.is_empty()
     }
 
     pub fn has_opts(&self) -> bool {
-        self.opts.is_empty()
+        !self.opts.is_empty()
     }
 
     pub fn has_positionals(&self) -> bool {
-        self.positionals.is_empty()
+        !self.positionals.is_empty()
     }
 
     pub fn has_subcommands(&self) -> bool {
-        self.subcommands.is_empty()
+        !self.subcommands.is_empty()
     }
 
     pub fn is_set(&self, s: AppSettings) -> bool {
@@ -1318,13 +1318,14 @@ impl<'a, 'b> Parser<'a, 'b> where 'a: 'b {
                                  .iter()
                                  .fold(String::new(), |a, s| a + &format!(" {}", s)[..]);
 
-            if !self.has_flags() && !self.is_set(AppSettings::UnifiedHelpMessage) {
+            if self.has_flags() && !self.is_set(AppSettings::UnifiedHelpMessage) {
                 usage.push_str(" [FLAGS]");
             } else {
                 usage.push_str(" [OPTIONS]");
             }
-            if !self.is_set(AppSettings::UnifiedHelpMessage) && !self.has_opts() &&
+            if !self.is_set(AppSettings::UnifiedHelpMessage) 
-               self.opts.iter().any(|a| !a.settings.is_set(ArgSettings::Required)) {
+                && self.has_opts() 
+                && self.opts.iter().any(|a| !a.settings.is_set(ArgSettings::Required)) {
                 usage.push_str(" [OPTIONS]");
             }
 
@@ -1332,22 +1333,22 @@ impl<'a, 'b> Parser<'a, 'b> where 'a: 'b {
 
             // places a '--' in the usage string if there are args and options
             // supporting multiple values
-            if !self.has_positionals()
+            if self.has_positionals()
-                && (self.opts.iter().any(|a| a.settings.is_set(ArgSettings::Multiple))
+                && self.opts.iter().any(|a| a.settings.is_set(ArgSettings::Multiple))
-                    || self.positionals.values().any(|a| a.settings.is_set(ArgSettings::Multiple)))
+                   // || self.positionals.values().any(|a| a.settings.is_set(ArgSettings::Multiple)))
-                && !self.opts.iter().any(|a| a.settings.is_set(ArgSettings::Required))
+                && self.positionals.values().any(|a| !a.settings.is_set(ArgSettings::Required))
-                && self.has_subcommands() {
+                && !self.has_subcommands() {
                 usage.push_str(" [--]")
             }
-            if !self.has_positionals()
+            if self.has_positionals()
                 && self.positionals.values().any(|a| !a.settings.is_set(ArgSettings::Required)) {
                 usage.push_str(" [ARGS]");
             }
 
 
-            if !self.has_subcommands() && !self.is_set(AppSettings::SubcommandRequired) {
+            if self.has_subcommands() && !self.is_set(AppSettings::SubcommandRequired) {
                 usage.push_str(" [SUBCOMMAND]");
-            } else if self.is_set(AppSettings::SubcommandRequired) && !self.has_subcommands() {
+            } else if self.is_set(AppSettings::SubcommandRequired) && self.has_subcommands() {
                 usage.push_str(" <SUBCOMMAND>");
             }
         } else {
@@ -1417,10 +1418,10 @@ impl<'a, 'b> Parser<'a, 'b> where 'a: 'b {
 
         try!(write!(w, "\n{}", self.create_usage(&[])));
 
-        let flags = !self.has_flags();
+        let flags = self.has_flags();
-        let pos = !self.has_positionals();
+        let pos = self.has_positionals();
-        let opts = !self.has_opts();
+        let opts = self.has_opts();
-        let subcmds = !self.has_subcommands();
+        let subcmds = self.has_subcommands();
         let unified_help = self.is_set(AppSettings::UnifiedHelpMessage);
 
         let mut longest_flag = 0;

src/args/help_writer.rs

@@ -1,9 +1,23 @@
 use std::io;
 use std::fmt::Display;
 
+#[cfg(all(feature = "wrap_help", not(target_os = "windows")))]
+use unicode_width::UnicodeWidthStr;
+
 use args::AnyArg;
 use args::settings::ArgSettings;
 use term;
+use strext::_StrExt;
+
+#[cfg(any(not(feature = "wrap_help"), target_os = "windows"))]
+fn str_width(s: &str) -> usize {
+    s.len()
+}
+
+#[cfg(all(feature = "wrap_help", not(target_os = "windows")))]
+fn str_width(s: &str) -> usize {
+    UnicodeWidthStr::width(s)
+}
 
 const TAB: &'static str = "    ";
 
@@ -139,7 +153,7 @@ impl<'a, 'n, 'e, A> HelpWriter<'a, A> where A: AnyArg<'n, 'e> + Display {
         // determine if our help fits or needs to wrap
         let width = self.term_w.unwrap_or(0);
         debugln!("Term width...{}", width);
-        let too_long = self.term_w.is_some() && (spcs + h.len() + spec_vals.len() >= width);
+        let too_long = self.term_w.is_some() && (spcs + str_width(h) + str_width(&*spec_vals) >= width);
         debugln!("Too long...{:?}", too_long);
 
         // Is help on next line, if so newline + 2x tab
@@ -153,13 +167,13 @@ impl<'a, 'n, 'e, A> HelpWriter<'a, A> where A: AnyArg<'n, 'e> + Display {
             help.push_str(h);
             help.push_str(&*spec_vals);
             debugln!("help: {}", help);
-            debugln!("help len: {}", help.len());
+            debugln!("help width: {}", str_width(help));
             // Determine how many newlines we need to insert
             let avail_chars = width - spcs;
             debugln!("Usable space: {}", avail_chars);
             let longest_w = {
                 let mut lw = 0;
-                for l in help.split(' ').map(|s| s.len()) {
+                for l in help.split(' ').map(|s| str_width(s)) {
                     if l > lw {
                         lw = l;
                     }
@@ -167,7 +181,7 @@ impl<'a, 'n, 'e, A> HelpWriter<'a, A> where A: AnyArg<'n, 'e> + Display {
                 lw
             };
             debugln!("Longest word...{}", longest_w);
-            debug!("Enough space...");
+            debug!("Enough space to wrap...");
             if longest_w < avail_chars {
                 sdebugln!("Yes");
                 let mut indices = vec![];
@@ -182,13 +196,13 @@ impl<'a, 'n, 'e, A> HelpWriter<'a, A> where A: AnyArg<'n, 'e> + Display {
                     debugln!("Adding idx: {}", idx);
                     debugln!("At {}: {:?}", idx, help.chars().nth(idx));
                     indices.push(idx);
-                    if &help[idx..].len() <= &avail_chars {
+                    if str_width(&help[idx..]) <= avail_chars {
                         break;
                     }
                 }
                 for (i, idx) in indices.iter().enumerate() {
                     debugln!("iter;i={},idx={}", i, idx);
-                    let j = idx+(2*i);
+                    let j = idx + (2 * i);
                     debugln!("removing: {}", j);
                     debugln!("at {}: {:?}", j, help.chars().nth(j));
                     help.remove(j);
@@ -252,15 +266,20 @@ impl<'a, 'n, 'e, A> HelpWriter<'a, A> where A: AnyArg<'n, 'e> + Display {
     }
 }
 
-fn find_idx_of_space(full: &str, start: usize) -> usize {
+fn find_idx_of_space(full: &str, mut start: usize) -> usize {
     debugln!("fn=find_idx_of_space;");
-    let haystack = &full[..start];
+    let haystack = if full._is_char_boundary(start) {
+        &full[..start]
+    } else {
+        while !full._is_char_boundary(start) { start -= 1; }
+        &full[..start]
+    };
     debugln!("haystack: {}", haystack);
     for (i, c) in haystack.chars().rev().enumerate() {
         debugln!("iter;c={},i={}", c, i);
         if c == ' ' {
-            debugln!("Found space returning start-i...{}", start - (i+1));
+            debugln!("Found space returning start-i...{}", start - (i + 1));
-            return start - (i+1);
+            return start - (i + 1);
         }
     }
     0

src/lib.rs

@@ -406,8 +406,10 @@ extern crate strsim;
 extern crate ansi_term;
 #[cfg(feature = "yaml")]
 extern crate yaml_rust;
-#[cfg(feature = "wrap_help")]
+#[cfg(all(feature = "wrap_help", not(target_os = "windows")))]
 extern crate libc;
+#[cfg(all(feature = "wrap_help", not(target_os = "windows")))]
+extern crate unicode_width;
 #[macro_use]
 extern crate bitflags;
 extern crate vec_map;
@@ -429,6 +431,7 @@ mod suggestions;
 mod errors;
 mod osstringext;
 mod term;
+mod strext;
 
 const INTERNAL_ERROR_MSG: &'static str = "Fatal internal error. Please consider filing a bug \
                                           report at https://github.com/kbknapp/clap-rs/issues";

src/strext.rs

@@ -0,0 +1,14 @@
+pub trait _StrExt {
+    fn _is_char_boundary(&self, index: usize) -> bool;
+}
+
+impl _StrExt for str {
+    #[inline]
+    fn _is_char_boundary(&self, index: usize) -> bool {
+        if index == self.len() { return true; }
+        match self.as_bytes().get(index) {
+            None => false,
+            Some(&b) => b < 128 || b >= 192,
+        }
+    }
+}