Skip to content

Commit

Permalink
fix(help)!: Consoldiate color settings
Browse files Browse the repository at this point in the history
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 clap-rs#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 clap-rs#2806
  • Loading branch information
epage committed Oct 11, 2021
1 parent d97c038 commit 6dd9d46
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 48 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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`)
Expand Down
2 changes: 1 addition & 1 deletion clap_derive/tests/non_literal_attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 4 additions & 2 deletions src/build/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand All @@ -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()
}
Expand Down
24 changes: 0 additions & 24 deletions src/build/app/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions src/build/app/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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()
Expand Down
19 changes: 4 additions & 15 deletions src/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)
}
Expand All @@ -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(),
Expand All @@ -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)
}
Expand Down

0 comments on commit 6dd9d46

Please sign in to comment.