Skip to content

Commit

Permalink
Auto merge of #845 - mgeisler:textwrap, r=kbknapp
Browse files Browse the repository at this point in the history
Use my textwrap crate for wrapping help texts

This PR replaces the `wrap_help` function with a call to my [textwrap][1] crate which does word wrapping using a simpler and faster algorithm. I've extended the `05_ripgrep` benchmarks with a benchmark that tests generating the help text -- I figured that ripgrep would be a good example since it has lots of long strings in the help output. Comparing before and after looks like this on my machine:

```
 name              before ns/iter  after ns/iter  diff ns/iter   diff %  speedup
 build_app_long    22,101          21,099               -1,002   -4.53%   x 1.05
 build_app_short   22,138          21,205                 -933   -4.21%   x 1.04
 build_help_long   514,265         284,467            -229,798  -44.68%   x 1.81
 build_help_short  85,720          85,693                  -27   -0.03%   x 1.00
 parse_clean       23,471          22,859                 -612   -2.61%   x 1.03
 parse_complex     29,535          28,919                 -616   -2.09%   x 1.02
 parse_lots        422,815         414,577              -8,238   -1.95%   x 1.02
```

I've also added a test for #617 to ensure that the output described there is kept unchanged. That is, textwrap will now by default keep horizontal whitespace when wrapping text and this allows you to do rudimentary formatting in the help text by manually inserting whitespace.

[1]: https://crates.io/crates/textwrap

<!-- 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/845)
<!-- Reviewable:end -->
  • Loading branch information
homu committed May 29, 2017
2 parents 821929b + 12282e1 commit 2f4d3fc
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 39 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ 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
textwrap = "0.6.0"
strsim = { version = "0.6.0", optional = true }
ansi_term = { version = "0.9.0", optional = true }
term_size = { version = "0.3.0", optional = true }
Expand Down
23 changes: 23 additions & 0 deletions benches/05_ripgrep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,26 @@ use clap::{App, AppSettings, Arg, ArgSettings};

use test::Bencher;
use std::collections::HashMap;
use std::io::Cursor;

#[bench]
fn build_app_short(b: &mut Bencher) { b.iter(|| app_short()); }

#[bench]
fn build_app_long(b: &mut Bencher) { b.iter(|| app_long()); }

#[bench]
fn build_help_short(b: &mut Bencher) {
let app = app_short();
b.iter(|| build_help(&app));
}

#[bench]
fn build_help_long(b: &mut Bencher) {
let app = app_long();
b.iter(|| build_help(&app));
}

#[bench]
fn parse_clean(b: &mut Bencher) { b.iter(|| app_short().get_matches_from(vec!["rg", "pat"])); }

Expand Down Expand Up @@ -304,6 +317,16 @@ pub fn app_short() -> App<'static, 'static> { app(false, |k| USAGES[k].short) }
/// Build a clap application with long help strings.
pub fn app_long() -> App<'static, 'static> { app(true, |k| USAGES[k].long) }


/// Build the help text of an application.
fn build_help(app: &App) -> String {
let mut buf = Cursor::new(Vec::with_capacity(50));
app.write_help(&mut buf).unwrap();
let content = buf.into_inner();
String::from_utf8(content).unwrap()
}


/// Build a clap application parameterized by usage strings.
///
/// The function given should take a clap argument name and return a help
Expand Down
46 changes: 7 additions & 39 deletions src/app/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use app::usage;
use unicode_width::UnicodeWidthStr;
#[cfg(feature = "wrap_help")]
use term_size;
use unicode_segmentation::UnicodeSegmentation;
use textwrap;
use vec_map::VecMap;

#[cfg(not(feature = "wrap_help"))]
Expand Down Expand Up @@ -955,45 +955,13 @@ impl<'a> Help<'a> {
}

fn wrap_help(help: &mut String, longest_w: usize, avail_chars: usize) {
debugln!("Help::wrap_help: longest_w={}, avail_chars={}",
longest_w,
avail_chars);
debug!("Help::wrap_help: Enough space to wrap...");
// Keep previous behavior of not wrapping at all if one of the
// words would overflow the line.
if longest_w < avail_chars {
sdebugln!("Yes");
let mut prev_space = 0;
let mut j = 0;
for (idx, g) in (&*help.clone()).grapheme_indices(true) {
debugln!("Help::wrap_help:iter: idx={}, g={}", idx, g);
if g == "\n" {
debugln!("Help::wrap_help:iter: Newline found...");
debugln!("Help::wrap_help:iter: Still space...{:?}",
str_width(&help[j..idx]) < avail_chars);
if str_width(&help[j..idx]) < avail_chars {
j = idx;
continue;
}
} else if g != " " {
if idx != help.len() - 1 || str_width(&help[j..idx]) < avail_chars {
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 {
debugln!("Help::wrap_help:iter: Space found with room...");
prev_space = idx;
continue;
}
debugln!("Help::wrap_help:iter: Adding Newline...");
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 + 1]);
help.remove(j);
help.insert(j, '\n');
prev_space = idx;
}
} else {
sdebugln!("No");
*help = help.lines()
.map(|line| textwrap::fill(line, avail_chars))
.collect::<Vec<String>>()
.join("\n");
}
}

Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,7 @@ extern crate bitflags;
extern crate vec_map;
#[cfg(feature = "wrap_help")]
extern crate term_size;
extern crate textwrap;
extern crate unicode_segmentation;
#[cfg(feature = "color")]
extern crate atty;
Expand Down
28 changes: 28 additions & 0 deletions tests/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,22 @@ FLAGS:
(Defaults to something)
-V, --version Prints version information";

static WRAPPING_NEWLINE_CHARS: &'static str = "ctest 0.1
USAGE:
ctest [mode]
FLAGS:
-h, --help Prints help information
-V, --version Prints version information
ARGS:
<mode> x, max, maximum 20 characters, contains
symbols.
l, long Copy-friendly, 14
characters, contains symbols.
m, med, medium Copy-friendly, 8
characters, contains symbols.";

static ISSUE_688: &'static str = "ctest 0.1
Expand Down Expand Up @@ -663,6 +679,18 @@ fn final_word_wrapping() {
assert!(test::compare_output(app, "ctest --help", FINAL_WORD_WRAPPING, false));
}

#[test]
fn wrapping_newline_chars() {
let app = App::new("ctest")
.version("0.1")
.set_term_width(60)
.arg(Arg::with_name("mode")
.help("x, max, maximum 20 characters, contains symbols.{n}\
l, long Copy-friendly, 14 characters, contains symbols.{n}\
m, med, medium Copy-friendly, 8 characters, contains symbols.{n}"));
assert!(test::compare_output(app, "ctest --help", WRAPPING_NEWLINE_CHARS, false));
}

#[test]
fn old_newline_chars() {
let app = App::new("ctest")
Expand Down

0 comments on commit 2f4d3fc

Please sign in to comment.