Repository: kbknapp/clap-rs
Pull request #845 (2017-02-08 21:06:18)

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 -->

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

Coverage increased (+0.4%) to 91.689% when pulling **ba6a6c2ed3c1ae290e01d7b276ba55e2de2366a6 on mgeisler:textwrap** into **92919f5f67f8e6163d5f349fcbc93ebda1f77d58 on kbknapp:master**.

kbknapp: Thanks for taking the time to do this, but I do have a few questions/concerns before this is merged:

 * Does `text_wrap` compile on Stable 1.11.0? If not, it would be a somewhat breaking change for `clap` to include it, as it increases the minimum version of Rust. According to `clap`s compatibility policy, this is allowed, but the minor must be bumped (no big deal), but the key peice is as of this writing it *must* compile on stable 1.13.0 (Stable - 2 releases). If that can't be done, utililzing this feature needs to be done with the `unstable` cargo feature which includes all previous functionality if that feature is omitted.
 * How does `text_wrap` handle the whitespace alignment? I haven't dug into the source or documenation yet, so forgive me if it's listed clearly. I want to ensure `clap` isn't unknowingly taking a step back if we lose whitespace alignment, i.e.

```
    -f, --flag    some long text that needs to
                  be wrapped. Notice the
                  whitespace after each
                  newline
```

To be clear, I'm not saying this unmergable, just we need to ensure all the bases are covered first :smile:
mgeisler: Hi Kevin, thanks for the comments!

> Does text_wrap compile on Stable 1.11.0?

I'm very new to Rust so I didn't make any conscious choice about not supporting that version :-) Looking at this a bit indicates that it's the `quote` crate that is causing problems on Rust 1.11.0 -- it's being pulled in via the `hyphenation` crate.

I've tested with `hyphenation` version 0.5.0 and that version compiles all the way back to Rust 1.8.0. So I think I'll simply use that version -- the results seems to be the same, except that performance is worse (this affects only the words for which hyphenation is attempted, i.e., the first word that overflow a line). I didn't do anything to enable hyphenation in `clap-rs` -- but it could be a cool option to have, I think.

> How does text_wrap handle the whitespace alignment? I haven't dug into the source or documenation yet, so forgive me if it's listed clearly.

It probably isn't listed very clearly... :-) It is very simple currently: it splits the string on whitespace and proceeds to add each word into lines, one by one. So

    "foo    bar  \t  baz"

is treated just like `"foo bar baz"` with regard to wrapping.

From your example, I think you might be talking about an initial/subsequent indentation, though -- the string of whitespace on the second to last line? That kind of formatting is not supported yet, but I would like to have it: https://github.com/mgeisler/textwrap/issues/26.

It doesn't seem to be necessary, though, since I'm just replacing the `wrap_help` function which also didn't do anything with indentation. When `textwrap` can do nice indented wrapping, then I think one can simplify the `clap-rs` code further by letting `textwrap` format an option completely.
mgeisler: Because all whitespace (including tabs, newlines, etc) is treated as a single space, #617 will reintroduced if this PR were to be merged -- as mentioned, I added a test for the issue and the test passes before and fails after this PR.
kbknapp: Ah ok, I understand now, I was under the impression `textwrap` would be handling the indentation as well. It makes no difference to me if it's handled in `textwrap` or `clap` so long as it's handled performantly 😉 

> I didn't do anything to enable hyphenation in clap-rs -- but it could be a cool option to have, I think.

Perhaps. I have mixed feelings on hyphenation, it's subjective but I find it harder to read when not required. A good use for it, IMO would be when the terminal size is small, or there isn't much room for wrapping on whole word boundaries. I would be good with hyphenation happening in those cases, but I think enabling it wholesale would be a bit much.

Also, if hyphenation isn't enabled, it'd nice to be able opt-out of pulling in related deps that aren't used. I don't know if this can be done in `textwrap`s setup via cargo features, or not? Similar to how `clap` lets you opt out of `color` or `wrap_help` or `suggestions` to avoid pulling in deps frivilously.

> [..] #617 

I would like to maintain this abiltiy, so it'll be a blocker until `textwrap` can handle extra whitespace. I'm glad to hear it's on your mind though!

---

One last thing I forgot ask beforehand, does `textwrap` handled unicode boundaries? I've had issues before with homegrown wrapping solutions cutting unicode graphemes in half (current `clap` handles them correctly AFAIK).


kbknapp: Once this PR reaches a mergable state I'd like to do some comparison benchmarks too for reference
mgeisler: > A good use for it, IMO would be when the terminal size is small, or there isn't much room for wrapping on whole word boundaries. I would be good with hyphenation happening in those cases, but I think enabling it wholesale would be a bit much.

Yeah, that makes a lot of sense! The user would have to tell clap which hyphenation patterns to use in any case, so I don't think it should be enabled unconditionally.

>  I don't know if this can be done in textwraps setup via cargo features, or not?

Nope, there's no support for this at the moment -- but I think you're right that it would make sense to add such a feature flag. The hyphenation dependency is rather large and pulls in many other dependencies.

> I would like to maintain this abiltiy, so it'll be a blocker until textwrap can handle extra whitespace. I'm glad to hear it's on your mind though!

Very well, I have some ideas for how this could be supported -- I'll let you know when it's there. Feel free to close/park the PR until then.

> One last thing I forgot ask beforehand, does textwrap handled unicode boundaries? 

Yes, it should handle that fine -- mostly due to how it simply iterates over words from `split_whitespace()`. The word length is computed with the `unicode_width` crate, just like the old `wrap_help` function did.

I saw that there is a [`unicode_segmentation`][1] crate, but the way it splits things into "words" doesn't really match what I would expect -- some words can be adjacent in the input text. I don't know how it fares on languages like Japanese or Chinese, though -- with no whitespace, it will treat the entire text as one big word.

The next version of `textwrap` will have an option to break words longer than the available line width -- that might work okay for scripts where there are no whitespace between the characters.

[1]: https://unicode-rs.github.io/unicode-segmentation/unicode_segmentation/index.html
homu: :umbrella: The latest upstream changes (presumably #867) made this pull request unmergeable. Please resolve the merge conflicts.
mgeisler: I've fixed mgeisler/textwrap#36, so one can now use textwrap without the automatic hyphenation. I'll see about the indentation issue next and then resolve the merge conflicts here.
mgeisler: Hi @kbknapp, I've finally updated this PR :-) With the newly released version 0.6.0 of textwrap, whitespace is new kept by default. This means the output in #617 is preserved.

I've also added a benchmark which shows that the new code is faster -- generating the large and complicated help text for `ripgrep` is now apparently about 45% faster than before.

In my Travis, I have tested textwrap back to Rust 1.8.0 and it works there when the optional `hyphenation` feature is not enabled: https://travis-ci.org/mgeisler/textwrap/.

I've updated the PR description above to match the new state. I took out the old cleanup commits from this PR -- I hope to submit them afterwards if this gets merged. Please let me know if there's anything missing!
coveralls: 
[![Coverage Status](https://coveralls.io/builds/11639336/badge)](https://coveralls.io/builds/11639336)

Coverage increased (+0.3%) to 92.121% when pulling **247809090f69e10b8bd7721cbde90f1f2ba8baff on mgeisler:textwrap** into **2923515a0aec80a7ebd4b1fc3430aad859bcda8a on kbknapp:master**.

mgeisler: I see a failed job in Travis -- the job that tests with Rust 1.11.0. I believe that is because of 
 this change in the unicode-segmentation crate: https://github.com/unicode-rs/unicode-segmentation/commit/aaf9da43ad854aaa776f27a48d001675772c0422#diff-dd3463da52dddbc4cbf510ba1b8c1fd2R715 which introduced the `?` operator. That commit is part of the just released unicode-segmentation 1.2.0: https://crates.io/crates/unicode-segmentation/1.2.0.

I think that build failure is unrelated to my change :-) The `?` operator became stable in Rust 1.13.0, so perhaps the minimum version could be bumped to that? Alternatively, one could change the dependency on unicode-segmentation to be `<1.2.0` or something like that. Thoughts?

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

Coverage increased (+0.3%) to 92.121% when pulling **247809090f69e10b8bd7721cbde90f1f2ba8baff on mgeisler:textwrap** into **2923515a0aec80a7ebd4b1fc3430aad859bcda8a on kbknapp:master**.

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

Coverage increased (+0.3%) to 92.121% when pulling **bf0b1579147274ab5c0b086cf0a8c5ed19ca40d6 on mgeisler:textwrap** into **2923515a0aec80a7ebd4b1fc3430aad859bcda8a on kbknapp:master**.

homu: :umbrella: The latest upstream changes (presumably dd4c41e) made this pull request unmergeable. Please resolve the merge conflicts.
coveralls: 
[![Coverage Status](https://coveralls.io/builds/11725008/badge)](https://coveralls.io/builds/11725008)

Changes Unknown when pulling **12282e10510754ff5f455e65d638648df3643ee9 on mgeisler:textwrap** into ** on kbknapp:master**.

mgeisler: Yay, with the unicode-segmentation version fixed, the build passed.

Patch:
diff --git a/.mention-bot b/.mention-bot
new file mode 100644
index 00000000..f339f59c
--- /dev/null
+++ b/.mention-bot
@@ -0,0 +1,9 @@
+{
+  "findPotentialReviewers": false, 
+  "alwaysNotifyForPaths": [
+    {
+      "name": "kbknapp", 
+      "files": ["**/*.rs", "**/*.md", "*"] 
+    }
+  ]
+}
diff --git a/Cargo.toml b/Cargo.toml
index 2c0a0dcc..c9c3ea85 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -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 }
diff --git a/README.md b/README.md
index e76e6060..2e91b05c 100644
--- a/README.md
+++ b/README.md
@@ -618,6 +618,28 @@ clap = "~2.19.0"
 
 This will cause *only* the patch version to be updated upon a `cargo update` call, and therefore cannot break due to new features, or bumped minimum versions of Rust.
 
+#### Warning about '~' Dependencies
+
+Using `~` can cause issues in certain circumstances.
+
+From @alexcrichton:
+
+Right now Cargo's version resolution is pretty naive, it's just a brute-force search of the solution space, returning the first resolvable graph. This also means that it currently won't terminate until it proves there is not possible resolvable graph. This leads to situations where workspaces with multiple binaries, for example, have two different dependencies such as:
+
+```toml
+
+# In one Cargo.toml
+[dependencies]
+clap = "~2.19.0"
+
+# In another Cargo.toml
+[dependencies]
+clap = "2.22"
+
+```
+
+This is inherently an unresolvable crate graph in Cargo right now. Cargo requires there's only one major version of a crate, and being in the same workspace these two crates must share a version. This is impossible in this location, though, as these version constraints cannot be met.
+
 #### Minimum Version of Rust
 
 `clap` will officially support current stable Rust, minus two releases, but may work with prior releases as well. For example, current stable Rust at the time of this writing is 1.13.0, meaning `clap` is guaranteed to compile with 1.11.0 and beyond.
diff --git a/benches/05_ripgrep.rs b/benches/05_ripgrep.rs
index 42a43c92..48c0819b 100644
--- a/benches/05_ripgrep.rs
+++ b/benches/05_ripgrep.rs
@@ -14,6 +14,7 @@ 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()); }
@@ -21,6 +22,18 @@ 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"])); }
 
@@ -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
diff --git a/src/app/help.rs b/src/app/help.rs
index f8ca3348..e2cb64b1 100644
--- a/src/app/help.rs
+++ b/src/app/help.rs
@@ -10,14 +10,14 @@ use app::{App, AppSettings};
 use app::parser::Parser;
 use args::{AnyArg, ArgSettings, DispOrder};
 use errors::{Error, Result as ClapResult};
-use fmt::{Format, Colorizer};
+use fmt::{Format, Colorizer, ColorizerOption};
 use app::usage;
 
 // Third Party
 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"))]
@@ -155,10 +155,10 @@ impl<'a> Help<'a> {
         let nlh = parser.is_set(AppSettings::NextLineHelp);
         let hide_v = parser.is_set(AppSettings::HidePossibleValuesInHelp);
         let color = parser.is_set(AppSettings::ColoredHelp);
-        let cizer = Colorizer {
+        let cizer = Colorizer::new(ColorizerOption {
             use_stderr: stderr,
             when: parser.color(),
-        };
+        });
         Self::new(w,
                   nlh,
                   hide_v,
@@ -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");
     }
 }
 
diff --git a/src/app/parser.rs b/src/app/parser.rs
index 0a2ac5ca..9f373fb6 100644
--- a/src/app/parser.rs
+++ b/src/app/parser.rs
@@ -500,7 +500,7 @@ impl<'a, 'b> Parser<'a, 'b>
         // but no 2)
         if let Some((idx, p)) = self.positionals.iter().rev().next() {
             assert!(!(idx != self.positionals.len()),
-                    "Found positional argument \"{}\" who's index is {} but there \
+                    "Found positional argument \"{}\" whose index is {} but there \
                           are only {} positional arguments defined",
                     p.b.name,
                     idx,
diff --git a/src/app/settings.rs b/src/app/settings.rs
index 6049ccba..987b0258 100644
--- a/src/app/settings.rs
+++ b/src/app/settings.rs
@@ -36,7 +36,7 @@ bitflags! {
         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 ARGS_NEGATE_SCS      = 1 << 31,
         const PROPAGATE_VALS_DOWN  = 1 << 32,
         const ALLOW_MISSING_POS    = 1 << 33,
         const TRAILING_VALUES      = 1 << 34,
@@ -528,7 +528,7 @@ pub enum AppSettings {
     /// This can be useful if there are many values, or they are explained elsewhere.
     HidePossibleValuesInHelp,
 
-    /// Tries to match unknown args to partial [`subcommands`] or their [aliases]. For example to 
+    /// Tries to match unknown args to partial [`subcommands`] or their [aliases]. For example to
     /// match a subcommand named `test`, one could use `t`, `te`, `tes`, and `test`.
     ///
     /// **NOTE:** The match *must not* be ambiguous at all in order to succeed. i.e. to match `te`
@@ -536,7 +536,7 @@ pub enum AppSettings {
     ///
     /// **CAUTION:** This setting can interfere with [positional/free arguments], take care when
     /// designing CLIs which allow inferred subcommands and have potential positional/free
-    /// arguments who's values could start with the same characters as subcommands. If this is the
+    /// arguments whose values could start with the same characters as subcommands. If this is the
     /// case, it's recommended to use settings such as [`AppSeettings::ArgsNegateSubcommands`] in
     /// conjuction with this setting.
     ///
diff --git a/src/app/validator.rs b/src/app/validator.rs
index 7e8ba468..582ed959 100644
--- a/src/app/validator.rs
+++ b/src/app/validator.rs
@@ -11,7 +11,7 @@ use errors::Result as ClapResult;
 use osstringext::OsStrExt2;
 use app::settings::AppSettings as AS;
 use app::parser::{Parser, ParseResult};
-use fmt::Colorizer;
+use fmt::{Colorizer, ColorizerOption};
 use app::usage;
 
 pub struct Validator<'a, 'b, 'z>(&'z mut Parser<'a, 'b>)
@@ -417,10 +417,10 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> {
 
     fn missing_required_error(&self, matcher: &ArgMatcher, extra: Option<&str>) -> ClapResult<()> {
         debugln!("Validator::missing_required_error: extra={:?}", extra);
-        let c = Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: self.0.color(),
-        };
+        });
         let mut reqs = self.0
             .required
             .iter()
@@ -453,4 +453,4 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> {
         self.validate_required_unless(a, matcher)
             .unwrap_or(false)
     }
-}
\ No newline at end of file
+}
diff --git a/src/errors.rs b/src/errors.rs
index 37a49537..1781067f 100644
--- a/src/errors.rs
+++ b/src/errors.rs
@@ -9,7 +9,7 @@ use std::result::Result as StdResult;
 
 // Internal
 use args::{FlagBuilder, AnyArg};
-use fmt;
+use fmt::{Colorizer, ColorizerOption, ColorWhen};
 use suggestions;
 
 /// Short hand for [`Result`] type
@@ -408,17 +408,17 @@ impl Error {
     pub fn argument_conflict<'a, 'b, A, O, U>(arg: &A,
                                               other: Option<O>,
                                               usage: U,
-                                              color: fmt::ColorWhen)
+                                              color: ColorWhen)
                                               -> Self
         where A: AnyArg<'a, 'b> + Display,
               O: Into<String>,
               U: Display
     {
         let mut v = vec![arg.name().to_owned()];
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} The argument '{}' cannot be used with {}\n\n\
                             {}\n\n\
@@ -444,14 +444,14 @@ impl Error {
     }
 
     #[doc(hidden)]
-    pub fn empty_value<'a, 'b, A, U>(arg: &A, usage: U, color: fmt::ColorWhen) -> Self
+    pub fn empty_value<'a, 'b, A, U>(arg: &A, usage: U, color: ColorWhen) -> Self
         where A: AnyArg<'a, 'b> + Display,
               U: Display
     {
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} The argument '{}' requires a value but none was supplied\
                             \n\n\
@@ -471,17 +471,17 @@ impl Error {
                                              good_vals: &[G],
                                              arg: &A,
                                              usage: U,
-                                             color: fmt::ColorWhen)
+                                             color: ColorWhen)
                                              -> Self
         where B: AsRef<str>,
               G: AsRef<str> + Display,
               A: AnyArg<'a, 'b> + Display,
               U: Display
     {
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         let suffix =
             suggestions::did_you_mean_suffix(bad_val.as_ref(),
                                              good_vals.iter(),
@@ -517,7 +517,7 @@ impl Error {
                                           did_you_mean: D,
                                           name: N,
                                           usage: U,
-                                          color: fmt::ColorWhen)
+                                          color: ColorWhen)
                                           -> Self
         where S: Into<String>,
               D: AsRef<str> + Display,
@@ -525,10 +525,10 @@ impl Error {
               U: Display
     {
         let s = subcmd.into();
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} The subcommand '{}' wasn't recognized\n\t\
                             Did you mean '{}'?\n\n\
@@ -550,15 +550,15 @@ impl Error {
     }
 
     #[doc(hidden)]
-    pub fn unrecognized_subcommand<S, N>(subcmd: S, name: N, color: fmt::ColorWhen) -> Self
+    pub fn unrecognized_subcommand<S, N>(subcmd: S, name: N, color: ColorWhen) -> Self
         where S: Into<String>,
               N: Display
     {
         let s = subcmd.into();
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} The subcommand '{}' wasn't recognized\n\n\
                             {}\n\t\
@@ -575,14 +575,14 @@ impl Error {
     }
 
     #[doc(hidden)]
-    pub fn missing_required_argument<R, U>(required: R, usage: U, color: fmt::ColorWhen) -> Self
+    pub fn missing_required_argument<R, U>(required: R, usage: U, color: ColorWhen) -> Self
         where R: Display,
               U: Display
     {
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} The following required arguments were not provided:{}\n\n\
                             {}\n\n\
@@ -597,14 +597,14 @@ impl Error {
     }
 
     #[doc(hidden)]
-    pub fn missing_subcommand<N, U>(name: N, usage: U, color: fmt::ColorWhen) -> Self
+    pub fn missing_subcommand<N, U>(name: N, usage: U, color: ColorWhen) -> Self
         where N: AsRef<str> + Display,
               U: Display
     {
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} '{}' requires a subcommand, but one was not provided\n\n\
                             {}\n\n\
@@ -620,13 +620,13 @@ impl Error {
 
 
     #[doc(hidden)]
-    pub fn invalid_utf8<U>(usage: U, color: fmt::ColorWhen) -> Self
+    pub fn invalid_utf8<U>(usage: U, color: ColorWhen) -> Self
         where U: Display
     {
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} Invalid UTF-8 was detected in one or more arguments\n\n\
                             {}\n\n\
@@ -643,17 +643,17 @@ impl Error {
     pub fn too_many_values<'a, 'b, V, A, U>(val: V,
                                             arg: &A,
                                             usage: U,
-                                            color: fmt::ColorWhen)
+                                            color: ColorWhen)
                                             -> Self
         where V: AsRef<str> + Display + ToOwned,
               A: AnyArg<'a, 'b> + Display,
               U: Display
     {
         let v = val.as_ref();
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} The value '{}' was provided to '{}', but it wasn't expecting \
                             any more values\n\n\
@@ -674,15 +674,15 @@ impl Error {
                                         min_vals: u64,
                                         curr_vals: usize,
                                         usage: U,
-                                        color: fmt::ColorWhen)
+                                        color: ColorWhen)
                                         -> Self
         where A: AnyArg<'a, 'b> + Display,
               U: Display
     {
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} The argument '{}' requires at least {} values, but only {} w{} \
                             provided\n\n\
@@ -701,13 +701,13 @@ impl Error {
     }
 
     #[doc(hidden)]
-    pub fn value_validation<'a, 'b, A>(arg: Option<&A>, err: String, color: fmt::ColorWhen) -> Self
+    pub fn value_validation<'a, 'b, A>(arg: Option<&A>, err: String, color: ColorWhen) -> Self
         where A: AnyArg<'a, 'b> + Display
     {
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} Invalid value{}: {}",
                              c.error("error:"),
@@ -725,7 +725,7 @@ impl Error {
     #[doc(hidden)]
     pub fn value_validation_auto(err: String) -> Self {
         let n: Option<&FlagBuilder> = None; 
-        Error::value_validation(n, err, fmt::ColorWhen::Auto)
+        Error::value_validation(n, err, ColorWhen::Auto)
     }
 
     #[doc(hidden)]
@@ -734,16 +734,16 @@ impl Error {
                                                    curr_vals: usize,
                                                    suffix: S,
                                                    usage: U,
-                                                   color: fmt::ColorWhen)
+                                                   color: ColorWhen)
                                                    -> Self
         where A: AnyArg<'a, 'b> + Display,
               S: Display,
               U: Display
     {
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} The argument '{}' requires {} values, but {} w{} \
                             provided\n\n\
@@ -762,14 +762,14 @@ impl Error {
     }
 
     #[doc(hidden)]
-    pub fn unexpected_multiple_usage<'a, 'b, A, U>(arg: &A, usage: U, color: fmt::ColorWhen) -> Self
+    pub fn unexpected_multiple_usage<'a, 'b, A, U>(arg: &A, usage: U, color: ColorWhen) -> Self
         where A: AnyArg<'a, 'b> + Display,
               U: Display
     {
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} The argument '{}' was provided more than once, but cannot \
                             be used multiple times\n\n\
@@ -788,16 +788,16 @@ impl Error {
     pub fn unknown_argument<A, U>(arg: A,
                                   did_you_mean: &str,
                                   usage: U,
-                                  color: fmt::ColorWhen)
+                                  color: ColorWhen)
                                   -> Self
         where A: Into<String>,
               U: Display
     {
         let a = arg.into();
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} Found argument '{}' which wasn't expected, or isn't valid in \
             this context{}\n\
@@ -818,11 +818,11 @@ impl Error {
     }
 
     #[doc(hidden)]
-    pub fn io_error(e: &Error, color: fmt::ColorWhen) -> Self {
-        let c = fmt::Colorizer {
+    pub fn io_error(e: &Error, color: ColorWhen) -> Self {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: color,
-        };
+        });
         Error {
             message: format!("{} {}", c.error("error:"), e.description()),
             kind: ErrorKind::Io,
@@ -835,10 +835,10 @@ impl Error {
         where A: Into<String>
     {
         let a = arg.into();
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
-            when: fmt::ColorWhen::Auto,
-        };
+            when: ColorWhen::Auto,
+        });
         Error {
             message: format!("{} The argument '{}' wasn't found",
                              c.error("error:"),
@@ -853,10 +853,10 @@ impl Error {
     /// This can be used in combination with `Error::exit` to exit your program
     /// with a custom error message.
     pub fn with_description(description: &str, kind: ErrorKind) -> Self {
-        let c = fmt::Colorizer {
+        let c = Colorizer::new(ColorizerOption {
             use_stderr: true,
-            when: fmt::ColorWhen::Auto,
-        };
+            when: ColorWhen::Auto,
+        });
         Error {
             message: format!("{} {}", c.error("error:"), description),
             kind: kind,
diff --git a/src/fmt.rs b/src/fmt.rs
index d6f2d3ed..11da7e49 100644
--- a/src/fmt.rs
+++ b/src/fmt.rs
@@ -7,6 +7,7 @@ use ansi_term::Colour::{Green, Red, Yellow};
 #[cfg(feature = "color")]
 use atty;
 use std::fmt;
+use std::env;
 
 #[doc(hidden)]
 #[derive(Debug, Copy, Clone, PartialEq)]
@@ -33,20 +34,24 @@ pub fn is_a_tty(_: bool) -> bool {
     false
 }
 
+pub fn is_term_dumb() -> bool { env::var("TERM").ok() == Some(String::from("dumb")) }
+
 #[doc(hidden)]
-pub struct Colorizer {
+pub struct ColorizerOption {
     pub use_stderr: bool,
     pub when: ColorWhen,
 }
 
+#[doc(hidden)]
+pub struct Colorizer {
+    use_stderr: bool,
+    when: ColorWhen,
+}
+
 macro_rules! color {
     ($_self:ident, $c:ident, $m:expr) => {
         match $_self.when {
-            ColorWhen::Auto => if is_a_tty($_self.use_stderr) {
-                Format::$c($m)
-            } else {
-                Format::None($m)
-            },
+            ColorWhen::Auto => Format::$c($m),
             ColorWhen::Always => Format::$c($m),
             ColorWhen::Never => Format::None($m),
         }
@@ -54,6 +59,19 @@ macro_rules! color {
 }
 
 impl Colorizer {
+    pub fn new(option: ColorizerOption) -> Colorizer {
+        let is_a_tty = is_a_tty(option.use_stderr);
+        let is_term_dumb = is_term_dumb();
+        Colorizer {
+            use_stderr: option.use_stderr,
+            when: if is_a_tty && !is_term_dumb {
+                option.when
+            } else {
+                ColorWhen::Never
+            },
+        }
+    }
+
     pub fn good<T>(&self, msg: T) -> Format<T>
         where T: fmt::Display + AsRef<str>
     {
@@ -85,10 +103,10 @@ impl Colorizer {
 
 impl Default for Colorizer {
     fn default() -> Self {
-        Colorizer {
+        Colorizer::new(ColorizerOption {
             use_stderr: true,
             when: ColorWhen::Auto,
-        }
+        })
     }
 }
 
diff --git a/src/lib.rs b/src/lib.rs
index 7c3310ca..2c212bf5 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -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;


Test patch:
diff --git a/tests/help.rs b/tests/help.rs
index 9bbb4fd3..b00ee048 100644
--- a/tests/help.rs
+++ b/tests/help.rs
@@ -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
 
@@ -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")