From 6dd9d467ce99801383b69942c6d7621f310038a7 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 11 Oct 2021 09:01:13 -0500 Subject: [PATCH] fix(help)!: Consoldiate color settings A lot of users expected `color` feature flag and `ColorAuto` etc to control all colors. Having this extra flag around is easy to miss and adds to our overall settings bloat, making it harder to find settings people want. This completely removes it, rather than make it deprecated like functions in #2617, because there is extra work to mark things deprecated as Settings and we should decide on our strategy first before investing time in addressing that issue. Fixes #2806 --- CHANGELOG.md | 1 + README.md | 3 +-- clap_derive/tests/non_literal_attributes.rs | 2 +- src/build/app/mod.rs | 6 ++++-- src/build/app/settings.rs | 24 --------------------- src/build/app/tests.rs | 8 +++---- src/parse/parser.rs | 19 ++++------------ 7 files changed, 15 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cd3d734669f..fef42305b78 100644 --- a/CHANGELOG.md +++ b/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)` ## v3.0.0-beta.4 (2021-08-14) diff --git a/README.md b/README.md index 0a3ddd9fb19..ac617513b0a 100644 --- a/README.md +++ b/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. ")] -#[clap(setting = AppSettings::ColoredHelp)] struct Opts { /// Sets a custom config file. Could have been an Option 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`) diff --git a/clap_derive/tests/non_literal_attributes.rs b/clap_derive/tests/non_literal_attributes.rs index 7b29def9221..51d00331514 100644 --- a/clap_derive/tests/non_literal_attributes.rs +++ b/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", diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 29e0671ace1..20995986be1 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -1780,9 +1780,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() } @@ -1806,9 +1807,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() } diff --git a/src/build/app/settings.rs b/src/build/app/settings.rs index 5efd8c7ac6c..c462626d708 100644 --- a/src/build/app/settings.rs +++ b/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::().unwrap(), AppSettings::AllowNegativeNumbers ); - assert_eq!( - "coloredhelp".parse::().unwrap(), - AppSettings::ColoredHelp - ); assert_eq!( "colorauto".parse::().unwrap(), AppSettings::ColorAuto diff --git a/src/build/app/tests.rs b/src/build/app/tests.rs index 59639f97e8b..ddc14737ad8 100644 --- a/src/build/app/tests.rs +++ b/src/build/app/tests.rs @@ -13,7 +13,7 @@ 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 @@ -21,13 +21,13 @@ fn global_setting() { .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() diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 7a41b814191..143dc418fe3 100644 --- a/src/parse/parser.rs +++ b/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 { - 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) }