Finished color refactor

Cargo.toml

@@ -69,7 +69,7 @@ indexmap              = "1.0.1"
 strsim      = { version = "0.9.0",  optional = true }
 yaml-rust   = { version = "0.4.1",  optional = true }
 atty        = { version = "0.2",  optional = true }
-ansi_term   = { version = "0.12", optional = true }
+termcolor   = { version = "1.1", optional = true }
 vec_map     = { version = "0.8", optional = true }
 term_size   = { version = "1.0.0-beta1", optional = true }
 lazy_static = { version = "1", optional = true }
@@ -85,7 +85,7 @@ criterion = { git = "git://github.com/pksunkara/criterion.rs", version = "0.3" }
 default     = ["suggestions", "color", "vec_map", "derive", "std", "cargo"]
 std         = [] # support for no_std in a backwards-compatible way
 suggestions = ["strsim"]
-color       = ["ansi_term", "atty"]
+color       = ["atty", "termcolor"]
 wrap_help   = ["term_size", "textwrap/term_size"]
 derive      = ["clap_derive", "lazy_static"]
 yaml        = ["yaml-rust"]

clap_derive/tests/custom-string-parsers.rs

@@ -81,7 +81,10 @@ fn test_parse_hex() {
     );
 
     let err = HexOpt::try_parse_from(&["test", "-n", "gg"]).unwrap_err();
-    assert!(err.message.contains("invalid digit found in string"), err);
+    assert!(
+        err.to_string().contains("invalid digit found in string"),
+        err
+    );
 }
 
 fn custom_parser_1(_: &str) -> &'static str {

clap_derive/tests/non_literal_attributes.rs

@@ -147,5 +147,8 @@ fn test_parse_hex_function_path() {
     );
 
     let err = HexOpt::try_parse_from(&["test", "-n", "gg"]).unwrap_err();
-    assert!(err.message.contains("invalid digit found in string"), err);
+    assert!(
+        err.to_string().contains("invalid digit found in string"),
+        err
+    );
 }

clap_derive/tests/utils.rs

@@ -5,7 +5,7 @@
 // Accept and endure. Do not touch.
 #![allow(unused)]
 
-use clap::IntoApp;
+use clap::{find_subcmd_mut, match_alias, IntoApp};
 
 pub fn get_help<T: IntoApp>() -> String {
     let mut output = Vec::new();
@@ -32,10 +32,12 @@ pub fn get_long_help<T: IntoApp>() -> String {
 }
 
 pub fn get_subcommand_long_help<T: IntoApp>(subcmd: &str) -> String {
-    let output = <T as IntoApp>::into_app()
-        .try_get_matches_from(vec!["test", subcmd, "--help"])
-        .expect_err("")
-        .message;
+    let mut output = Vec::new();
+    find_subcmd_mut!(<T as IntoApp>::into_app(), subcmd)
+        .unwrap()
+        .write_long_help(&mut output)
+        .unwrap();
+    let output = String::from_utf8(output).unwrap();
 
     eprintln!(
         "\n%%% SUBCOMMAND `{}` HELP %%%:=====\n{}\n=====\n",

clap_generate/src/generators/shells/bash.rs

@@ -178,7 +178,7 @@ fn option_details_for_path(app: &App, path: &str) -> String {
 }
 
 fn vals_for(o: &Arg) -> String {
-    debugln!("Bash::vals_for: o={}", o.name);
+    debugln!("Bash::vals_for: o={}", o.get_name());
 
     if let Some(ref vals) = o.get_possible_values() {
         format!("$(compgen -W \"{}\" -- ${{cur}})", vals.join(" "))

src/build/app/mod.rs

@@ -9,7 +9,7 @@ use std::collections::HashMap;
 use std::env;
 use std::ffi::OsString;
 use std::fmt;
-use std::io::{self, BufRead, BufWriter, Write};
+use std::io::{self, BufRead, Write};
 use std::path::Path;
 use std::process;
 
@@ -20,11 +20,10 @@ use yaml_rust::Yaml;
 // Internal
 use crate::build::{app::settings::AppFlags, Arg, ArgGroup, ArgSettings};
 use crate::mkeymap::MKeyMap;
-use crate::output::fmt::ColorWhen;
-use crate::output::{Help, Usage};
+use crate::output::{fmt::Colorizer, Help, HelpWriter, Usage};
 use crate::parse::errors::Result as ClapResult;
 use crate::parse::{ArgMatcher, ArgMatches, Input, Parser};
-use crate::util::{Id, Key};
+use crate::util::{termcolor::ColorChoice, Id, Key};
 use crate::INTERNAL_ERROR_MSG;
 
 // FIXME (@CreepySkeleton): some of this variants are never constructed
@@ -1087,13 +1086,14 @@ impl<'b> App<'b> {
     /// [`-h` (short)]: ./struct.Arg.html#method.help
     /// [`--help` (long)]: ./struct.Arg.html#method.long_help
     pub fn print_help(&mut self) -> ClapResult<()> {
-        // If there are global arguments, or settings we need to propagate them down to subcommands
-        // before parsing incase we run into a subcommand
         self._build();
 
-        let out = io::stdout();
-        let mut buf_w = BufWriter::new(out.lock());
-        self.write_help(&mut buf_w)
+        let p = Parser::new(self);
+        let mut c = Colorizer::new(false, p.color_help());
+
+        Help::new(HelpWriter::Buffer(&mut c), &p, true).write_help()?;
+
+        Ok(c.print()?)
     }
 
     /// Prints the full help message to [`io::stdout()`] using a [`BufWriter`] using the same
@@ -1114,13 +1114,14 @@ impl<'b> App<'b> {
     /// [`-h` (short)]: ./struct.Arg.html#method.help
     /// [`--help` (long)]: ./struct.Arg.html#method.long_help
     pub fn print_long_help(&mut self) -> ClapResult<()> {
-        // If there are global arguments, or settings we need to propagate them down to subcommands
-        // before parsing incase we run into a subcommand
         self._build();
 
-        let out = io::stdout();
-        let mut buf_w = BufWriter::new(out.lock());
-        self.write_long_help(&mut buf_w)
+        let p = Parser::new(self);
+        let mut c = Colorizer::new(false, p.color_help());
+
+        Help::new(HelpWriter::Buffer(&mut c), &p, true).write_help()?;
+
+        Ok(c.print()?)
     }
 
     /// Writes the full help message to the user to a [`io::Write`] object in the same method as if
@@ -1145,7 +1146,7 @@ impl<'b> App<'b> {
         self._build();
 
         let p = Parser::new(self);
-        Help::new(w, &p, false, false).write_help()
+        Help::new(HelpWriter::Normal(w), &p, false).write_help()
     }
 
     /// Writes the full help message to the user to a [`io::Write`] object in the same method as if
@@ -1170,7 +1171,7 @@ impl<'b> App<'b> {
         self._build();
 
         let p = Parser::new(self);
-        Help::new(w, &p, true, false).write_help()
+        Help::new(HelpWriter::Normal(w), &p, true).write_help()
     }
 
     /// Writes the version message to the user to a [`io::Write`] object as if the user ran `-V`.
@@ -1264,13 +1265,15 @@ impl<'b> App<'b> {
             .unwrap_or_else(|e| {
                 // Otherwise, write to stderr and exit
                 if e.use_stderr() {
-                    wlnerr!("{}", e.message);
+                    e.message.print().expect("Error writing Error to stderr");
+
                     if self.settings.is_set(AppSettings::WaitOnError) {
                         wlnerr!("\nPress [ENTER] / [RETURN] to continue...");
                         let mut s = String::new();
                         let i = io::stdin();
                         i.lock().read_line(&mut s).unwrap();
                     }
+
                     drop(e);
                     process::exit(2);
                 }
@@ -1294,7 +1297,7 @@ impl<'b> App<'b> {
     /// let matches = App::new("myprog")
     ///     // Args and options go here...
     ///     .try_get_matches()
-    ///     .unwrap_or_else( |e| e.exit() );
+    ///     .unwrap_or_else(|e| e.exit());
     /// ```
     /// [`env::args_os`]: https://doc.rust-lang.org/std/env/fn.args_os.html
     /// [`ErrorKind::HelpDisplayed`]: ./enum.ErrorKind.html#variant.HelpDisplayed
@@ -1338,13 +1341,15 @@ impl<'b> App<'b> {
         self.try_get_matches_from_mut(itr).unwrap_or_else(|e| {
             // Otherwise, write to stderr and exit
             if e.use_stderr() {
-                wlnerr!("{}", e.message);
+                e.message.print().expect("Error writing Error to stderr");
+
                 if self.settings.is_set(AppSettings::WaitOnError) {
                     wlnerr!("\nPress [ENTER] / [RETURN] to continue...");
                     let mut s = String::new();
                     let i = io::stdin();
                     i.lock().read_line(&mut s).unwrap();
                 }
+
                 drop(self);
                 drop(e);
                 process::exit(2);
@@ -1375,7 +1380,7 @@ impl<'b> App<'b> {
     /// let matches = App::new("myprog")
     ///     // Args and options go here...
     ///     .try_get_matches_from(arg_vec)
-    ///     .unwrap_or_else( |e| { panic!("An error occurs: {}", e) });
+    ///     .unwrap_or_else(|e| e.exit());
     /// ```
     /// [`App::get_matches_from`]: ./struct.App.html#method.get_matches_from
     /// [`App::try_get_matches`]: ./struct.App.html#method.try_get_matches
@@ -1411,7 +1416,7 @@ impl<'b> App<'b> {
     /// let mut app = App::new("myprog");
     ///     // Args and options go here...
     /// let matches = app.try_get_matches_from_mut(arg_vec)
-    ///     .unwrap_or_else( |e| { panic!("An error occurs: {}", e) });
+    ///     .unwrap_or_else(|e| e.exit());
     /// ```
     /// [`App`]: ./struct.App.html
     /// [`App::try_get_matches_from`]: ./struct.App.html#method.try_get_matches_from
@@ -1481,9 +1486,6 @@ impl<'b> App<'b> {
     pub fn _build(&mut self) {
         debugln!("App::_build;");
 
-        #[cfg(all(feature = "color", windows))]
-        let _ = ansi_term::enable_ansi_support();
-
         // Make sure all the globally set flags apply to us as well
         self.settings = self.settings | self.g_settings;
 
@@ -1896,19 +1898,20 @@ impl<'b> App<'b> {
         self.args.args.iter().find(|a| a.id == *arg_id)
     }
 
-    // Should we color the output? None=determined by output location, true=yes, false=no
-    pub(crate) fn color(&self) -> ColorWhen {
+    // Should we color the output?
+    pub(crate) fn color(&self) -> ColorChoice {
         debugln!("App::color;");
         debug!("App::color: Color setting...");
+
         if self.is_set(AppSettings::ColorNever) {
             sdebugln!("Never");
-            ColorWhen::Never
+            ColorChoice::Never
         } else if self.is_set(AppSettings::ColorAlways) {
             sdebugln!("Always");
-            ColorWhen::Always
+            ColorChoice::Always
         } else {
             sdebugln!("Auto");
-            ColorWhen::Auto
+            ColorChoice::Auto
         }
     }
 

src/macros.rs

@@ -752,19 +752,6 @@ mod debug_macros {
     }
 }
 
-// Helper/deduplication macro for printing the correct number of spaces in help messages
-// used in:
-//    src/args/arg_builder/*.rs
-//    src/app/mod.rs
-macro_rules! write_nspaces {
-    ($dst:expr, $num:expr) => {{
-        debugln!("write_spaces!: num={}", $num);
-        for _ in 0..$num {
-            $dst.write_all(b" ")?;
-        }
-    }};
-}
-
 #[macro_export]
 #[doc(hidden)]
 macro_rules! flags {
@@ -837,6 +824,7 @@ macro_rules! find_subcmd {
 macro_rules! find_subcmd_mut {
     ($app:expr, $sc:expr) => {{
         $app.get_subcommands_mut()
+            .iter_mut()
             .find(|a| match_alias!(a, $sc, a.get_name()))
     }};
 }