Merge #2845

2845: fix(help)!: Consoldiate color settings r=pksunkara a=epage



Co-authored-by: Ed Page <eopage@gmail.com>

CHANGELOG.md

@@ -15,6 +15,7 @@ TODO: `YamlLoader`
 
 * **Removed Settings**
   * `AppSettings::DisableVersionForSubcommands`
+  * `AppSettings::ColoredHelp`: we are now relying solely on the `color` feature flag and `AppSettings::Color(Auto|Always|Never)`
 
 <a name="v3.0.0-beta.4"></a>
 ## v3.0.0-beta.4 (2021-08-14)

README.md

@@ -144,7 +144,6 @@ use clap::{AppSettings, Parser};
 /// as do all doc strings on fields
 #[derive(Parser)]
 #[clap(version = "1.0", author = "Kevin K. <kbknapp@gmail.com>")]
-#[clap(setting = AppSettings::ColoredHelp)]
 struct Opts {
     /// Sets a custom config file. Could have been an Option<T> with no default too
     #[clap(short, long, default_value = "default.conf")]
@@ -473,7 +472,7 @@ Disabling optional features can decrease the binary size of `clap` and decrease
 * **std**: _Not Currently Used._ Placeholder for supporting `no_std` environments in a backwards compatible manner.
 * **derive**: Enables the custom derive (i.e. `#[derive(Parser)]`). Without this you must use one of the other methods of creating a `clap` CLI listed above. (builds dependency `clap_derive`)
 * **cargo**: Turns on macros that read values from `CARGO_*` environment variables.
-* **color**: Turns on colored error messages. You still have to turn on colored help by setting `AppSettings::ColoredHelp`. (builds dependency `termcolor`)
+* **color**: Turns on colored error messages. (builds dependency `termcolor`)
 * **env**: Turns on the usage of environment variables during parsing.
 * **suggestions**: Turns on the `Did you mean '--myoption'?` feature for when users make typos. (builds dependency `strsim`)
 * **unicode**: Turns on support for unicode characters in arguments and help messages. (builds dependency `textwrap`, `unicase`)

clap_derive/tests/non_literal_attributes.rs

@@ -19,7 +19,7 @@ pub const DISPLAY_ORDER: usize = 2;
 
 // Check if the global settings compile
 #[derive(Parser, Debug, PartialEq, Eq)]
-#[clap(global_setting = AppSettings::ColoredHelp)]
+#[clap(global_setting = AppSettings::UnifiedHelpMessage)]
 struct Opt {
     #[clap(
         long = "x",

src/build/app/mod.rs

@@ -1788,9 +1788,10 @@ impl<'help> App<'help> {
     /// [`--help` (long)]: Arg::long_about()
     pub fn print_help(&mut self) -> io::Result<()> {
         self._build();
+        let color = self.color();
 
         let p = Parser::new(self);
-        let mut c = Colorizer::new(false, p.color_help());
+        let mut c = Colorizer::new(false, color);
         Help::new(HelpWriter::Buffer(&mut c), &p, false).write_help()?;
         c.print()
     }
@@ -1814,9 +1815,10 @@ impl<'help> App<'help> {
     /// [`--help` (long)]: Arg::long_about()
     pub fn print_long_help(&mut self) -> io::Result<()> {
         self._build();
+        let color = self.color();
 
         let p = Parser::new(self);
-        let mut c = Colorizer::new(false, p.color_help());
+        let mut c = Colorizer::new(false, color);
         Help::new(HelpWriter::Buffer(&mut c), &p, true).write_help()?;
         c.print()
     }

src/build/app/settings.rs

@@ -80,8 +80,6 @@ impl_settings! { AppSettings, AppFlags,
         => Flags::ALLOW_NEG_NUMS,
     AllowMissingPositional("allowmissingpositional")
         => Flags::ALLOW_MISSING_POS,
-    ColoredHelp("coloredhelp")
-        => Flags::COLORED_HELP,
     ColorAlways("coloralways")
         => Flags::COLOR_ALWAYS,
     ColorAuto("colorauto")
@@ -493,24 +491,6 @@ pub enum AppSettings {
     /// ```
     SubcommandPrecedenceOverArg,
 
-    /// Uses colorized help messages.
-    ///
-    /// **NOTE:** Must be compiled with the `color` cargo feature
-    ///
-    /// # Platform Specific
-    ///
-    /// This setting only applies to Unix, Linux, and OSX (i.e. non-Windows platforms)
-    ///
-    /// # Examples
-    ///
-    /// ```no_run
-    /// # use clap::{App, Arg, AppSettings};
-    /// App::new("myprog")
-    ///     .setting(AppSettings::ColoredHelp)
-    ///     .get_matches();
-    /// ```
-    ColoredHelp,
-
     /// Enables colored output only when the output is going to a terminal or TTY.
     ///
     /// **NOTE:** This is the default behavior of `clap`.
@@ -1106,10 +1086,6 @@ mod test {
             "allownegativenumbers".parse::<AppSettings>().unwrap(),
             AppSettings::AllowNegativeNumbers
         );
-        assert_eq!(
-            "coloredhelp".parse::<AppSettings>().unwrap(),
-            AppSettings::ColoredHelp
-        );
         assert_eq!(
             "colorauto".parse::<AppSettings>().unwrap(),
             AppSettings::ColorAuto

src/build/app/tests.rs

@@ -13,21 +13,21 @@ fn propagate_version() {
 #[test]
 fn global_setting() {
     let mut app = App::new("test")
-        .global_setting(AppSettings::ColoredHelp)
+        .global_setting(AppSettings::UnifiedHelpMessage)
         .subcommand(App::new("subcmd"));
     app._propagate();
     assert!(app
         .subcommands
         .iter()
         .find(|s| s.name == "subcmd")
         .unwrap()
-        .is_set(AppSettings::ColoredHelp));
+        .is_set(AppSettings::UnifiedHelpMessage));
 }
 
 #[test]
 fn global_settings() {
     let mut app = App::new("test")
-        .global_setting(AppSettings::ColoredHelp)
+        .global_setting(AppSettings::UnifiedHelpMessage)
         .global_setting(AppSettings::TrailingVarArg)
         .subcommand(App::new("subcmd"));
     app._propagate();
@@ -36,7 +36,7 @@ fn global_settings() {
         .iter()
         .find(|s| s.name == "subcmd")
         .unwrap()
-        .is_set(AppSettings::ColoredHelp));
+        .is_set(AppSettings::UnifiedHelpMessage));
     assert!(app
         .subcommands
         .iter()

src/parse/parser.rs

@@ -19,7 +19,7 @@ use crate::{
     parse::features::suggestions,
     parse::{ArgMatcher, SubCommand},
     parse::{Validator, ValueType},
-    util::{termcolor::ColorChoice, ChildGraph, Id},
+    util::{ChildGraph, Id},
     INTERNAL_ERROR_MSG, INVALID_UTF8,
 };
 
@@ -827,17 +827,6 @@ impl<'help, 'app> Parser<'help, 'app> {
         )
     }
 
-    // Should we color the help?
-    pub(crate) fn color_help(&self) -> ColorChoice {
-        debug!("Parser::color_help");
-
-        if self.is_set(AS::ColoredHelp) {
-            self.app.color()
-        } else {
-            ColorChoice::Never
-        }
-    }
-
     // Checks if the arg matches a subcommand name, or any of its aliases (if defined)
     fn possible_subcommand(&self, arg_os: &RawOsStr, valid_arg_found: bool) -> Option<&str> {
         debug!("Parser::possible_subcommand: arg={:?}", arg_os);
@@ -1885,7 +1874,7 @@ impl<'help, 'app> Parser<'help, 'app> {
     }
 
     pub(crate) fn write_help_err(&self) -> ClapResult<Colorizer> {
-        let mut c = Colorizer::new(true, self.color_help());
+        let mut c = Colorizer::new(true, self.app.color());
         Help::new(HelpWriter::Buffer(&mut c), self, false).write_help()?;
         Ok(c)
     }
@@ -1897,7 +1886,7 @@ impl<'help, 'app> Parser<'help, 'app> {
         );
 
         use_long = use_long && self.use_long_help();
-        let mut c = Colorizer::new(false, self.color_help());
+        let mut c = Colorizer::new(false, self.app.color());
 
         match Help::new(HelpWriter::Buffer(&mut c), self, use_long).write_help() {
             Err(e) => e.into(),
@@ -1909,7 +1898,7 @@ impl<'help, 'app> Parser<'help, 'app> {
         debug!("Parser::version_err");
 
         let msg = self.app._render_version(use_long);
-        let mut c = Colorizer::new(false, self.color_help());
+        let mut c = Colorizer::new(false, self.app.color());
         c.none(msg);
         ClapError::new(c, ErrorKind::DisplayVersion)
     }