From e792bfd9c1843458a9e7226e6986f648fcf74194 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 11 May 2024 21:30:30 +0900 Subject: [PATCH 1/4] ui: add helper function that sets up FormatterFactory I'm going to split FormatterFactory::prepare() which takes 3 bool arguments, and prepare() will be inlined there. --- cli/src/ui.rs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/cli/src/ui.rs b/cli/src/ui.rs index 0762ba2b62..2de79a18ba 100644 --- a/cli/src/ui.rs +++ b/cli/src/ui.rs @@ -231,6 +231,19 @@ fn use_color(choice: ColorChoice) -> bool { } } +fn prepare_formatter_factory( + config: &config::Config, + stdout: &Stdout, +) -> Result { + let color = color_setting(config); + let debug = debug_color(color); + let color = use_color(color); + // Sanitize ANSI escape codes if we're printing to a terminal. Doesn't affect + // ANSI escape codes that originate from the formatter itself. + let sanitize = stdout.is_terminal(); + FormatterFactory::prepare(config, debug, color, sanitize) +} + fn be_quiet(config: &config::Config) -> bool { config.get_bool("ui.quiet").unwrap_or_default() } @@ -258,13 +271,9 @@ fn pager_setting(config: &config::Config) -> Result Result { let color = color_setting(config); - let debug = debug_color(color); let color = use_color(color); let quiet = be_quiet(config); - // Sanitize ANSI escape codes if we're printing to a terminal. Doesn't affect - // ANSI escape codes that originate from the formatter itself. - let sanitize = io::stdout().is_terminal(); - let formatter_factory = FormatterFactory::prepare(config, debug, color, sanitize)?; + let formatter_factory = prepare_formatter_factory(config, &io::stdout())?; let progress_indicator = progress_indicator_setting(config); Ok(Ui { color, @@ -279,14 +288,12 @@ impl Ui { pub fn reset(&mut self, config: &config::Config) -> Result<(), CommandError> { let color = color_setting(config); - let debug = debug_color(color); self.color = use_color(color); self.quiet = be_quiet(config); self.paginate = pagination_setting(config)?; self.pager_cmd = pager_setting(config)?; self.progress_indicator = progress_indicator_setting(config); - let sanitize = io::stdout().is_terminal(); - self.formatter_factory = FormatterFactory::prepare(config, debug, self.color, sanitize)?; + self.formatter_factory = prepare_formatter_factory(config, &io::stdout())?; Ok(()) } From debb3bb6002c5ffffd9d827c4559792a11a44afb Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 11 May 2024 21:45:34 +0900 Subject: [PATCH 2/4] ui: forward .color() to FormatterFactory This helps eliminate use_color() function. --- cli/src/formatter.rs | 4 ++++ cli/src/ui.rs | 8 +------- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/cli/src/formatter.rs b/cli/src/formatter.rs index ecc0e2581c..bf36ca09de 100644 --- a/cli/src/formatter.rs +++ b/cli/src/formatter.rs @@ -165,6 +165,10 @@ impl FormatterFactory { } } } + + pub fn is_color(&self) -> bool { + matches!(self.kind, FormatterFactoryKind::Color { .. }) + } } pub struct PlainTextFormatter { diff --git a/cli/src/ui.rs b/cli/src/ui.rs index 2de79a18ba..7180aecd6e 100644 --- a/cli/src/ui.rs +++ b/cli/src/ui.rs @@ -162,7 +162,6 @@ impl Write for UiStderr<'_> { } pub struct Ui { - color: bool, quiet: bool, pager_cmd: CommandNameAndArgs, paginate: PaginationChoice, @@ -270,13 +269,10 @@ fn pager_setting(config: &config::Config) -> Result Result { - let color = color_setting(config); - let color = use_color(color); let quiet = be_quiet(config); let formatter_factory = prepare_formatter_factory(config, &io::stdout())?; let progress_indicator = progress_indicator_setting(config); Ok(Ui { - color, quiet, formatter_factory, pager_cmd: pager_setting(config)?, @@ -287,8 +283,6 @@ impl Ui { } pub fn reset(&mut self, config: &config::Config) -> Result<(), CommandError> { - let color = color_setting(config); - self.color = use_color(color); self.quiet = be_quiet(config); self.paginate = pagination_setting(config)?; self.pager_cmd = pager_setting(config)?; @@ -333,7 +327,7 @@ impl Ui { } pub fn color(&self) -> bool { - self.color + self.formatter_factory.is_color() } pub fn new_formatter<'output, W: Write + 'output>( From 4e722c56196a99ee99427b08a68122c366122469 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 11 May 2024 21:50:47 +0900 Subject: [PATCH 3/4] ui: inline debug_color() and use_color() --- cli/src/ui.rs | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/cli/src/ui.rs b/cli/src/ui.rs index 7180aecd6e..4d62b1a56e 100644 --- a/cli/src/ui.rs +++ b/cli/src/ui.rs @@ -217,29 +217,20 @@ fn color_setting(config: &config::Config) -> ColorChoice { .unwrap_or_default() } -fn debug_color(choice: ColorChoice) -> bool { - matches!(choice, ColorChoice::Debug) -} - -fn use_color(choice: ColorChoice) -> bool { - match choice { - ColorChoice::Always => true, - ColorChoice::Never => false, - ColorChoice::Debug => true, - ColorChoice::Auto => io::stdout().is_terminal(), - } -} - fn prepare_formatter_factory( config: &config::Config, stdout: &Stdout, ) -> Result { - let color = color_setting(config); - let debug = debug_color(color); - let color = use_color(color); + let terminal = stdout.is_terminal(); + let (color, debug) = match color_setting(config) { + ColorChoice::Always => (true, false), + ColorChoice::Never => (false, false), + ColorChoice::Debug => (true, true), + ColorChoice::Auto => (terminal, false), + }; // Sanitize ANSI escape codes if we're printing to a terminal. Doesn't affect // ANSI escape codes that originate from the formatter itself. - let sanitize = stdout.is_terminal(); + let sanitize = terminal; FormatterFactory::prepare(config, debug, color, sanitize) } From 6e205ea5517f10972aad6331deb11fafc1668a8b Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sat, 11 May 2024 21:42:06 +0900 Subject: [PATCH 4/4] ui: add per-type FormatterFactory constructors, inline selection This should be better than passing (bool, bool, bool) arguments. --- cli/src/formatter.rs | 27 +++++++++++++-------------- cli/src/ui.rs | 13 +++++++++---- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/cli/src/formatter.rs b/cli/src/formatter.rs index bf36ca09de..164e4a201d 100644 --- a/cli/src/formatter.rs +++ b/cli/src/formatter.rs @@ -136,20 +136,19 @@ enum FormatterFactoryKind { } impl FormatterFactory { - pub fn prepare( - config: &config::Config, - debug: bool, - color: bool, - sanitized: bool, - ) -> Result { - let kind = if color { - let rules = Arc::new(rules_from_config(config)?); - FormatterFactoryKind::Color { rules, debug } - } else if sanitized { - FormatterFactoryKind::Sanitized - } else { - FormatterFactoryKind::PlainText - }; + pub fn plain_text() -> Self { + let kind = FormatterFactoryKind::PlainText; + FormatterFactory { kind } + } + + pub fn sanitized() -> Self { + let kind = FormatterFactoryKind::Sanitized; + FormatterFactory { kind } + } + + pub fn color(config: &config::Config, debug: bool) -> Result { + let rules = Arc::new(rules_from_config(config)?); + let kind = FormatterFactoryKind::Color { rules, debug }; Ok(FormatterFactory { kind }) } diff --git a/cli/src/ui.rs b/cli/src/ui.rs index 4d62b1a56e..27db4d8007 100644 --- a/cli/src/ui.rs +++ b/cli/src/ui.rs @@ -228,10 +228,15 @@ fn prepare_formatter_factory( ColorChoice::Debug => (true, true), ColorChoice::Auto => (terminal, false), }; - // Sanitize ANSI escape codes if we're printing to a terminal. Doesn't affect - // ANSI escape codes that originate from the formatter itself. - let sanitize = terminal; - FormatterFactory::prepare(config, debug, color, sanitize) + if color { + FormatterFactory::color(config, debug) + } else if terminal { + // Sanitize ANSI escape codes if we're printing to a terminal. Doesn't + // affect ANSI escape codes that originate from the formatter itself. + Ok(FormatterFactory::sanitized()) + } else { + Ok(FormatterFactory::plain_text()) + } } fn be_quiet(config: &config::Config) -> bool {