Skip to content

Commit

Permalink
Auto merge of #832 - mgeisler:fix-wrapping-bugs, r=kbknapp
Browse files Browse the repository at this point in the history
Fix wrapping bugs

I've been working towards integrating my [textwrap][1] crate and along the way, I found some small problems in the existing `wrap_help` function in clap. I basically added new code that calls both `textwrap::fill` and `wrap_help` and panicked on any difference:
```
fn wrap_help(help: &mut String, longest_w: usize, avail_chars: usize) {
    let input = help.clone();
    let mut old = help.clone();
    old_wrap_help(&mut old, longest_w, avail_chars);

    let mut wrapped = String::with_capacity(help.len());
    for (i, line) in help.lines().enumerate() {
        if i > 0 {
            wrapped.push('\n');
        }
        wrapped.push_str(&textwrap::fill(line, avail_chars));
    }

    // TODO: move up, This keeps old behavior of not wrapping at all
    // if one of the words would overflow the line
    if longest_w < avail_chars {
        *help = wrapped;
    }

    if *old != *help {
        println!("********************************");
        println!("longest_w: {}, avail_chars: {}", longest_w, avail_chars);
        println!("help: {:3} bytes: {:?}", input.len(), input);
        println!("old:  {:3} bytes: {:?}", old.len(), old);
        println!("new:  {:3} bytes: {:?}", help.len(), help);
        println!("********************************");
        panic!("bad wrap");
    }
}

fn old_wrap_help(help: &mut String, longest_w: usize, avail_chars: usize) {
    // ... as before
```

This PR fixes two small problems discovered this way, one of which became #828.

[1]: https://crates.io/crates/textwrap
  • Loading branch information
homu committed Jan 30, 2017
2 parents 72d7bf3 + 564c5f0 commit e9e01e1
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 6 deletions.
17 changes: 15 additions & 2 deletions src/app/help.rs
Expand Up @@ -948,7 +948,7 @@ fn wrap_help(help: &mut String, longest_w: usize, avail_chars: usize) {
continue;
}
debugln!("Help::wrap_help:iter: Reached the end of the line and we're over...");
} else if str_width(&help[j..idx]) < avail_chars {
} else if str_width(&help[j..idx]) <= avail_chars {
debugln!("Help::wrap_help:iter: Space found with room...");
prev_space = idx;
continue;
Expand All @@ -957,11 +957,24 @@ fn wrap_help(help: &mut String, longest_w: usize, avail_chars: usize) {
j = prev_space;
debugln!("Help::wrap_help:iter: prev_space={}, j={}", prev_space, j);
debugln!("Help::wrap_help:iter: Removing...{}", j);
debugln!("Help::wrap_help:iter: Char at {}...{}", j, &help[j..j]);
debugln!("Help::wrap_help:iter: Char at {}: {:?}", j, &help[j..j+1]);
help.remove(j);
help.insert(j, '\n');
prev_space = idx;
}
} else {
sdebugln!("No");
}
}

#[cfg(test)]
mod test {
use super::wrap_help;

#[test]
fn wrap_help_last_word() {
let mut help = String::from("foo bar baz");
wrap_help(&mut help, 3, 5);
assert_eq!(help, "foo\nbar\nbaz");
}
}
27 changes: 23 additions & 4 deletions tests/help.rs
Expand Up @@ -113,9 +113,8 @@ OPTIONS:
latte, cappuccino, espresso), tea, and other
hot beverages. Some coffeehouses also serve
cold beverages such as iced coffee and iced
tea. Many cafés also serve some type of
food, such as light snacks, muffins, or
pastries.";
tea. Many cafés also serve some type of food,
such as light snacks, muffins, or pastries.";

static ISSUE_626_PANIC: &'static str = "ctest 0.1
Expand Down Expand Up @@ -149,6 +148,20 @@ OPTIONS:
-c, --cafe <FILE> A coffeehouse, coffee shop, or café.
-p, --pos <VAL> Some vals [values: fast, slow]";

static FINAL_WORD_WRAPPING: &'static str = "ctest 0.1
USAGE:
ctest
FLAGS:
-h, --help
Prints help
information
-V, --version
Prints
version
information";

static OLD_NEWLINE_CHARS: &'static str = "ctest 0.1
USAGE:
Expand Down Expand Up @@ -410,6 +423,12 @@ fn issue_626_variable_panic() {
}
}

#[test]
fn final_word_wrapping() {
let app = App::new("ctest").version("0.1").set_term_width(24);
assert!(test::compare_output(app, "ctest --help", FINAL_WORD_WRAPPING, false));
}

#[test]
fn old_newline_chars() {
let app = App::new("ctest")
Expand Down Expand Up @@ -514,4 +533,4 @@ fn issue_777_wrap_all_things() {
.about("Show how the about text is not wrapped")
.set_term_width(35);
assert!(test::compare_output(app, "ctest --help", ISSUE_777, false));
}
}

0 comments on commit e9e01e1

Please sign in to comment.