Auto merge of #629 - kbknapp:issue-626, r=kbknapp

Issue 626

CHANGELOG.md

@@ -1,3 +1,13 @@
+<a name="v2.10.4"></a>
+### v2.10.4 (2016-08-25)
+
+
+#### Bug Fixes
+
+* **Help Wrapping:**  fixes a bug where help is wrapped incorrectly and causing a panic with some non-English characters ([d0b442c7](https://github.com/kbknapp/clap-rs/commit/d0b442c7beeecac9764406bc3bd171ced0b8825e), closes [#626](https://github.com/kbknapp/clap-rs/issues/626))
+
+
+
 <a name="v2.10.3"></a>
 ### v2.10.3 (2016-08-25)
 

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 
 name = "clap"
-version = "2.10.3"
+version = "2.10.4"
 authors = ["Kevin K. <kbknapp@gmail.com>"]
 exclude = ["examples/*", "clap-test/*", "tests/*", "benches/*", "*.png", "clap-perf/*", "*.dot"]
 description = "A simple to use, efficient, and full featured  Command Line Argument Parser"

README.md

@@ -39,6 +39,10 @@ Created by [gh-md-toc](https://github.com/ekalinin/github-markdown-toc)
 
 ## What's New
 
+Here's the highlights for v2.10.4
+
+* Fixes a bug where help is wrapped incorrectly and causing a panic with some non-English characters 
+
 Here's the highlights for v2.10.3
 
 * Fixes a bug with non-English characters in help text wrapping, where the character is stripped or causes a panic

src/app/help.rs

@@ -25,7 +25,7 @@ mod term_size {
 
 use unicode_width::UnicodeWidthStr;
 
-use strext::_StrExt;
+// use strext::_StrExt;
 
 fn str_width(s: &str) -> usize {
     UnicodeWidthStr::width(s)
@@ -336,7 +336,6 @@ impl<'a> Help<'a> {
         let width = self.term_w;
         debugln!("Term width...{}", width);
         let too_long = str_width(h) >= width;
-        debugln!("Too long...{:?}", too_long);
 
         debug!("Too long...");
         if too_long {
@@ -355,41 +354,7 @@ impl<'a> Help<'a> {
                 }
                 lw
             };
-            debugln!("Longest word...{}", longest_w);
+            wrap_help(&mut help, longest_w, width);
-            debug!("Enough space to wrap...");
-            if longest_w < width {
-                sdebugln!("Yes");
-                let mut indices = vec![];
-                let mut idx = 0;
-                loop {
-                    idx += width - 1;
-                    if idx >= help.len() {
-                        break;
-                    }
-                    // 'a' arbitrary non space char
-                    if help.chars().nth(idx).unwrap_or('a') != ' ' {
-                        idx = find_idx_of_space(&*help, idx);
-                    }
-                    debugln!("Adding idx: {}", idx);
-                    debugln!("At {}: {:?}", idx, help.chars().nth(idx));
-                    indices.push(idx);
-                    if str_width(&help[idx..]) <= width {
-                        break;
-                    }
-                }
-                for (i, idx) in indices.iter().enumerate() {
-                    debugln!("iter;i={},idx={}", i, idx);
-                    let j = idx + (2 * i);
-                    debugln!("removing: {}", j);
-                    debugln!("at {}: {:?}", j, help.chars().nth(j));
-                    help.remove(j);
-                    help.insert(j, '{');
-                    help.insert(j + 1, 'n');
-                    help.insert(j + 2, '}');
-                }
-            } else {
-                sdebugln!("No");
-            }
         } else {
             sdebugln!("No");
         }
@@ -427,7 +392,7 @@ impl<'a> Help<'a> {
         let width = self.term_w;
         debugln!("Term width...{}", width);
         let too_long = spcs + str_width(h) + str_width(&*spec_vals) >= width;
-        debugln!("Too long...{:?}", too_long);
+        debugln!("Spaces: {}", spcs);
 
         // Is help on next line, if so newline + 2x tab
         if self.next_line_help || arg.is_set(ArgSettings::NextLineHelp) {
@@ -435,7 +400,7 @@ impl<'a> Help<'a> {
         }
 
         debug!("Too long...");
-        if too_long {
+        if too_long && spcs <= width {
             sdebugln!("Yes");
             help.push_str(h);
             help.push_str(&*spec_vals);
@@ -453,35 +418,7 @@ impl<'a> Help<'a> {
                 }
                 lw
             };
-            debugln!("Longest word...{}", longest_w);
+            wrap_help(&mut help, longest_w, avail_chars);
-            debug!("Enough space to wrap...");
-            if longest_w < avail_chars {
-                sdebugln!("Yes");
-                let mut prev_space = 0;
-                let mut j = 0;
-                let mut i = 0;
-                for (idx, g) in (&*help.clone()).grapheme_indices(true) {
-                    debugln!("iter;idx={},g={}", idx, g);
-                    if g != " " { continue; }
-                    if str_width(&help[j..idx]) < avail_chars {
-                    debugln!("Still enough space...");
-                        prev_space = idx;
-                        continue;
-                    }
-                    debugln!("Adding Newline...");
-                    j = prev_space + (2 * i);
-                    debugln!("i={},prev_space={},j={}", i, prev_space, j);
-                    debugln!("removing: {}", j);
-                    debugln!("char at {}: {}", j, &help[j..j]);
-                    help.remove(j);
-                    help.insert(j, '{');
-                    help.insert(j + 1, 'n');
-                    help.insert(j + 2, '}');
-                    i += 1;
-                }
-            } else {
-                sdebugln!("No");
-            }
         } else {
             sdebugln!("No");
         }
@@ -946,24 +883,34 @@ impl<'a> Help<'a> {
     }
 }
 
-
+fn wrap_help(help: &mut String, longest_w: usize, avail_chars: usize) {
-fn find_idx_of_space(full: &str, mut start: usize) -> usize {
+    debugln!("fn=wrap_help;longest_w={},avail_chars={}", longest_w, avail_chars);
-    debugln!("fn=find_idx_of_space;");
+    debug!("Enough space to wrap...");
-    let haystack = if full._is_char_boundary(start) {
+    if longest_w < avail_chars {
-        &full[..start]
+        sdebugln!("Yes");
+        let mut prev_space = 0;
+        let mut j = 0;
+        let mut i = 0;
+        for (idx, g) in (&*help.clone()).grapheme_indices(true) {
+            debugln!("iter;idx={},g={}", idx, g);
+            if g != " " { continue; }
+            if str_width(&help[j..idx + (2 * i)]) < avail_chars {
+            debugln!("Still enough space...");
+                prev_space = idx;
+                continue;
+            }
+            debugln!("Adding Newline...");
+            j = prev_space + (2 * i);
+            debugln!("i={},prev_space={},j={}", i, prev_space, j);
+            debugln!("removing: {}", j);
+            debugln!("char at {}: {}", j, &help[j..j]);
+            help.remove(j);
+            help.insert(j, '{');
+            help.insert(j + 1, 'n');
+            help.insert(j + 2, '}');
+            i += 1;
+        }
     } else {
-        while !full._is_char_boundary(start) {
+        sdebugln!("No");
-            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));
-            return start - (i + 1);
-        }
     }
-    0
 }

tests/help.rs

@@ -93,12 +93,13 @@ FLAGS:
 OPTIONS:
     -c, --cafe <FILE>    A coffeehouse, coffee shop, or café is an
                          establishment which primarily serves hot
-                         coffee, related coffee beverages (e.g., café
+                         coffee, related coffee beverages (e.g.,
-                         latte, cappuccino, espresso), tea, and other
+                         café latte, cappuccino, espresso), tea,
-                         hot beverages. Some coffeehouses also serve cold
+                         and other hot beverages. Some
-                         beverages such as iced coffee and iced tea. Many
+                         coffeehouses also serve cold beverages
-                         cafés also serve some type of food, such as light
+                         such as iced coffee and iced tea. Many
-                         snacks, muffins, or pastries.";
+                         cafés also serve some type of food, such
+                         as light snacks, muffins, or pastries.";
 
 static ISSUE_626_PANIC: &'static str = "ctest 0.1
 
@@ -110,15 +111,19 @@ FLAGS:
     -V, --version    Prints version information
 
 OPTIONS:
-    -c, --cafe <FILE>    La culture du café est très
+    -c, --cafe <FILE>    La culture du café est
-                         développée dans de
+                         très développée dans de
-                         nombreux pays à climat chaud
+                         nombreux pays à climat
-                         d'Amérique, d'Afrique et
+                         chaud d'Amérique,
-                         d'Asie, dans des plantations qui
+                         d'Afrique et d'Asie,
-                         sont cultivées pour les marchés
+                         dans des plantations
-                         d'exportation. Le café est souvent
+                         qui sont cultivées pour
-                         une contribution majeure aux
+                         les marchés
-                         exportations des régions productrices.";
+                         d'exportation. Le café
+                         est souvent une
+                         contribution majeure
+                         aux exportations des
+                         régions productrices.";
 
 #[test]
 fn help_short() {
@@ -271,7 +276,7 @@ fn issue_626_unicode_cutoff() {
 fn issue_626_panic() {
     let app = App::new("ctest")
         .version("0.1")
-        .set_term_width(53)
+        .set_term_width(52)
         .arg(Arg::with_name("cafe")
            .short("c")
            .long("cafe")
@@ -282,3 +287,21 @@ fn issue_626_panic() {
            .takes_value(true));
     test::check_err_output(app, "ctest --help", ISSUE_626_PANIC, false);
 }
+
+#[test]
+fn issue_626_variable_panic() {
+    for i in 10..320 {
+        let _ = App::new("ctest")
+            .version("0.1")
+            .set_term_width(i)
+            .arg(Arg::with_name("cafe")
+               .short("c")
+               .long("cafe")
+               .value_name("FILE")
+               .help("La culture du café est très développée dans de nombreux pays à climat chaud d'Amérique, \
+               d'Afrique et d'Asie, dans des plantations qui sont cultivées pour les marchés d'exportation. \
+               Le café est souvent une contribution majeure aux exportations des régions productrices.")
+               .takes_value(true))
+            .get_matches_from_safe(vec!["ctest", "--help"]);
+    }
+}