From 3a1caf859573e3a9e52afe358c0177c977f6ee86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Fri, 23 Aug 2024 16:10:44 +0200 Subject: [PATCH 01/12] Prevent line breaks --- src/options/theme.rs | 38 ++++++++++++-------------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/src/options/theme.rs b/src/options/theme.rs index bcf4ffd16..021314b58 100644 --- a/src/options/theme.rs +++ b/src/options/theme.rs @@ -148,6 +148,7 @@ mod tests { // TODO: Test influence of BAT_THEME env var. E.g. see utils::process::tests::FakeParentArgs. #[test] fn test_syntax_theme_selection() { + use Mode::*; #[derive(PartialEq)] enum Mode { Light, @@ -159,30 +160,15 @@ mod tests { expected_syntax_theme, expected_mode, ) in vec![ - (None, None, DEFAULT_DARK_SYNTAX_THEME, Mode::Dark), - (Some("GitHub"), None, "GitHub", Mode::Light), - (Some("GitHub"), None, "GitHub", Mode::Light), - ( - None, - Some(Mode::Light), - DEFAULT_LIGHT_SYNTAX_THEME, - Mode::Light, - ), - ( - None, - Some(Mode::Dark), - DEFAULT_DARK_SYNTAX_THEME, - Mode::Dark, - ), - ( - None, - Some(Mode::Light), - DEFAULT_LIGHT_SYNTAX_THEME, - Mode::Light, - ), - (None, Some(Mode::Light), "GitHub", Mode::Light), - (Some("none"), None, "none", Mode::Dark), - (Some("None"), Some(Mode::Light), "none", Mode::Light), + (None, None, DEFAULT_DARK_SYNTAX_THEME, Dark), + (Some("GitHub"), None, "GitHub", Light), + (Some("GitHub"), None, "GitHub", Light), + (None, Some(Light), DEFAULT_LIGHT_SYNTAX_THEME, Light), + (None, Some(Dark), DEFAULT_DARK_SYNTAX_THEME, Dark), + (None, Some(Light), DEFAULT_LIGHT_SYNTAX_THEME, Light), + (None, Some(Light), "GitHub", Light), + (Some("none"), None, "none", Dark), + (Some("None"), Some(Light), "none", Light), ] { let mut args = vec![]; if let Some(syntax_theme) = syntax_theme { @@ -198,10 +184,10 @@ mod tests { args.push("never"); } match mode { - Some(Mode::Light) => { + Some(Light) => { args.push("--light"); } - Some(Mode::Dark) => { + Some(Dark) => { args.push("--dark"); } None => {} From 09cd0297bf4a118cdad88b3f554d97c93969698e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Fri, 23 Aug 2024 16:11:34 +0200 Subject: [PATCH 02/12] De-duplicate test data --- src/options/theme.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/options/theme.rs b/src/options/theme.rs index 021314b58..92ff8d95a 100644 --- a/src/options/theme.rs +++ b/src/options/theme.rs @@ -162,11 +162,8 @@ mod tests { ) in vec![ (None, None, DEFAULT_DARK_SYNTAX_THEME, Dark), (Some("GitHub"), None, "GitHub", Light), - (Some("GitHub"), None, "GitHub", Light), - (None, Some(Light), DEFAULT_LIGHT_SYNTAX_THEME, Light), (None, Some(Dark), DEFAULT_DARK_SYNTAX_THEME, Dark), (None, Some(Light), DEFAULT_LIGHT_SYNTAX_THEME, Light), - (None, Some(Light), "GitHub", Light), (Some("none"), None, "none", Dark), (Some("None"), Some(Light), "none", Light), ] { From a3a8e8d902976f912c195849f749cbb75b17d735 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Fri, 23 Aug 2024 16:16:08 +0200 Subject: [PATCH 03/12] Do fallback outside switch --- src/options/theme.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/options/theme.rs b/src/options/theme.rs index 92ff8d95a..388a51675 100644 --- a/src/options/theme.rs +++ b/src/options/theme.rs @@ -88,15 +88,12 @@ fn get_is_light_mode_and_syntax_theme_name( bat_theme_env_var: Option<&String>, light_mode: bool, ) -> (bool, String) { - match (theme_arg, bat_theme_env_var, light_mode) { - (None, None, false) => (false, DEFAULT_DARK_SYNTAX_THEME.to_string()), - (Some(theme_name), _, false) => (is_light_syntax_theme(theme_name), theme_name.to_string()), - (None, Some(theme_name), false) => { - (is_light_syntax_theme(theme_name), theme_name.to_string()) - } - (None, None, true) => (true, DEFAULT_LIGHT_SYNTAX_THEME.to_string()), - (Some(theme_name), _, is_light_mode) => (is_light_mode, theme_name.to_string()), - (None, Some(theme_name), is_light_mode) => (is_light_mode, theme_name.to_string()), + let theme_arg = theme_arg.or(bat_theme_env_var); + match (theme_arg, light_mode) { + (None, false) => (false, DEFAULT_DARK_SYNTAX_THEME.to_string()), + (Some(theme_name), false) => (is_light_syntax_theme(theme_name), theme_name.to_string()), + (None, true) => (true, DEFAULT_LIGHT_SYNTAX_THEME.to_string()), + (Some(theme_name), is_light_mode) => (is_light_mode, theme_name.to_string()), } } From aabe39a6bcc0ea99e92000f3f872f91a1b8ca49c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Fri, 23 Aug 2024 16:36:41 +0200 Subject: [PATCH 04/12] Add test for dark theme --- src/options/theme.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/options/theme.rs b/src/options/theme.rs index 388a51675..554d5b0f4 100644 --- a/src/options/theme.rs +++ b/src/options/theme.rs @@ -159,6 +159,7 @@ mod tests { ) in vec![ (None, None, DEFAULT_DARK_SYNTAX_THEME, Dark), (Some("GitHub"), None, "GitHub", Light), + (Some("Nord"), None, "Nord", Dark), (None, Some(Dark), DEFAULT_DARK_SYNTAX_THEME, Dark), (None, Some(Light), DEFAULT_LIGHT_SYNTAX_THEME, Light), (Some("none"), None, "none", Dark), From 82593db7d6d23f967aa0af6491b85366c7a6f0a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Fri, 23 Aug 2024 16:43:29 +0200 Subject: [PATCH 05/12] Add missing test combinations --- src/options/theme.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/options/theme.rs b/src/options/theme.rs index 554d5b0f4..642243e09 100644 --- a/src/options/theme.rs +++ b/src/options/theme.rs @@ -162,7 +162,12 @@ mod tests { (Some("Nord"), None, "Nord", Dark), (None, Some(Dark), DEFAULT_DARK_SYNTAX_THEME, Dark), (None, Some(Light), DEFAULT_LIGHT_SYNTAX_THEME, Light), + (Some("GitHub"), Some(Light), "GitHub", Light), + (Some("GitHub"), Some(Dark), "GitHub", Light), // TODO: This should be Dark. + (Some("Nord"), Some(Light), "Nord", Light), + (Some("Nord"), Some(Dark), "Nord", Dark), (Some("none"), None, "none", Dark), + (Some("none"), Some(Dark), "none", Dark), (Some("None"), Some(Light), "none", Light), ] { let mut args = vec![]; From 90aec591ed6be6c766f8a128b2a7f5ca8d332235 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Fri, 23 Aug 2024 17:02:15 +0200 Subject: [PATCH 06/12] Allow --dark to override a light syntax theme --- src/options/theme.rs | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/options/theme.rs b/src/options/theme.rs index 642243e09..6ce9604a5 100644 --- a/src/options/theme.rs +++ b/src/options/theme.rs @@ -86,21 +86,23 @@ fn is_no_syntax_highlighting_syntax_theme_name(theme_name: &str) -> bool { fn get_is_light_mode_and_syntax_theme_name( theme_arg: Option<&String>, bat_theme_env_var: Option<&String>, - light_mode: bool, + light_mode: Option, ) -> (bool, String) { let theme_arg = theme_arg.or(bat_theme_env_var); match (theme_arg, light_mode) { - (None, false) => (false, DEFAULT_DARK_SYNTAX_THEME.to_string()), - (Some(theme_name), false) => (is_light_syntax_theme(theme_name), theme_name.to_string()), - (None, true) => (true, DEFAULT_LIGHT_SYNTAX_THEME.to_string()), - (Some(theme_name), is_light_mode) => (is_light_mode, theme_name.to_string()), + (Some(theme_name), None) => (is_light_syntax_theme(theme_name), theme_name.to_string()), + (Some(theme_name), Some(is_light_mode)) => (is_light_mode, theme_name.to_string()), + (None, None | Some(false)) => (false, DEFAULT_DARK_SYNTAX_THEME.to_string()), + (None, Some(true)) => (true, DEFAULT_LIGHT_SYNTAX_THEME.to_string()), } } -fn get_is_light(opt: &cli::Opt) -> bool { - get_is_light_opt(opt) - .or_else(|| should_detect_dark_light(opt).then(detect_light_mode)) - .unwrap_or_default() +fn get_is_light(opt: &cli::Opt) -> Option { + get_is_light_opt(opt).or_else(|| { + should_detect_dark_light(opt) + .then(detect_light_mode) + .flatten() + }) } fn get_is_light_opt(opt: &cli::Opt) -> Option { @@ -123,19 +125,18 @@ fn should_detect_dark_light(opt: &cli::Opt) -> bool { } #[cfg(not(test))] -fn detect_light_mode() -> bool { +fn detect_light_mode() -> Option { use terminal_colorsaurus::{color_scheme, ColorScheme, QueryOptions}; - color_scheme(QueryOptions::default()).unwrap_or_default() == ColorScheme::Light + color_scheme(QueryOptions::default()) + .ok() + .map(|color_scheme| color_scheme == ColorScheme::Light) } #[cfg(test)] -fn detect_light_mode() -> bool { - LIGHT_MODE_IN_TESTS +fn detect_light_mode() -> Option { + None } -#[cfg(test)] -pub(crate) const LIGHT_MODE_IN_TESTS: bool = false; - #[cfg(test)] mod tests { use super::*; @@ -163,7 +164,7 @@ mod tests { (None, Some(Dark), DEFAULT_DARK_SYNTAX_THEME, Dark), (None, Some(Light), DEFAULT_LIGHT_SYNTAX_THEME, Light), (Some("GitHub"), Some(Light), "GitHub", Light), - (Some("GitHub"), Some(Dark), "GitHub", Light), // TODO: This should be Dark. + (Some("GitHub"), Some(Dark), "GitHub", Dark), (Some("Nord"), Some(Light), "Nord", Light), (Some("Nord"), Some(Dark), "Nord", Dark), (Some("none"), None, "none", Dark), From 53f50c8cc562abfadfb1790b19f655806dff1bd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Fri, 6 Sep 2024 11:51:03 +0200 Subject: [PATCH 07/12] Merge get_is_light* functions together --- src/options/theme.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/options/theme.rs b/src/options/theme.rs index 6ce9604a5..6b9dbdcd9 100644 --- a/src/options/theme.rs +++ b/src/options/theme.rs @@ -98,20 +98,14 @@ fn get_is_light_mode_and_syntax_theme_name( } fn get_is_light(opt: &cli::Opt) -> Option { - get_is_light_opt(opt).or_else(|| { - should_detect_dark_light(opt) - .then(detect_light_mode) - .flatten() - }) -} - -fn get_is_light_opt(opt: &cli::Opt) -> Option { if opt.light { Some(true) } else if opt.dark { Some(false) } else { - None + should_detect_dark_light(opt) + .then(detect_light_mode) + .flatten() } } From 4dce6e42ea0a8f8979b050026093630e1aa7f4b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Fri, 6 Sep 2024 11:54:48 +0200 Subject: [PATCH 08/12] Make `use` top-level --- src/options/theme.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/options/theme.rs b/src/options/theme.rs index 6b9dbdcd9..60588d62a 100644 --- a/src/options/theme.rs +++ b/src/options/theme.rs @@ -10,6 +10,8 @@ use std::io::{stdout, IsTerminal}; /// default is selected. use bat; use bat::assets::HighlightingAssets; +#[cfg(not(test))] +use terminal_colorsaurus::{color_scheme, ColorScheme, QueryOptions}; use crate::cli::{self, DetectDarkLight}; @@ -120,7 +122,6 @@ fn should_detect_dark_light(opt: &cli::Opt) -> bool { #[cfg(not(test))] fn detect_light_mode() -> Option { - use terminal_colorsaurus::{color_scheme, ColorScheme, QueryOptions}; color_scheme(QueryOptions::default()) .ok() .map(|color_scheme| color_scheme == ColorScheme::Light) From 1cc7a0d10fd505f5a60991d24ed7aebef7e410dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Fri, 6 Sep 2024 11:55:46 +0200 Subject: [PATCH 09/12] Make docs proper module-level docs --- src/options/theme.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/options/theme.rs b/src/options/theme.rs index 60588d62a..a2cd35e46 100644 --- a/src/options/theme.rs +++ b/src/options/theme.rs @@ -1,13 +1,16 @@ +//! Delta doesn't have a formal concept of a "theme". What it has is +//! +//! * (a) the choice of syntax-highlighting theme +//! * (b) the choice of light-background-mode vs dark-background-mode, which determine certain +//! default color choices +//! +//! This module sets those options. If the light/dark background mode choice is not made explicitly +//! by the user, it is determined by the classification of the syntax theme into light-background +//! vs dark-background syntax themes. If the user didn't choose a syntax theme, a dark-background +//! default is selected. + use std::io::{stdout, IsTerminal}; -/// Delta doesn't have a formal concept of a "theme". What it has is -/// (a) the choice of syntax-highlighting theme -/// (b) the choice of light-background-mode vs dark-background-mode, which determine certain -/// default color choices -/// This module sets those options. If the light/dark background mode choice is not made explicitly -/// by the user, it is determined by the classification of the syntax theme into light-background -/// vs dark-background syntax themes. If the user didn't choose a syntax theme, a dark-background -/// default is selected. use bat; use bat::assets::HighlightingAssets; #[cfg(not(test))] From 4d506a1b3ac528f27064baa10db820fc223baef9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Fri, 6 Sep 2024 17:00:39 +0200 Subject: [PATCH 10/12] Replace "light mode" bool with a dedicated enum --- src/cli.rs | 3 +- src/color.rs | 60 ++++++++++------- src/config.rs | 12 ++-- src/features/line_numbers.rs | 20 +++--- src/main.rs | 2 +- src/options/set.rs | 2 +- src/options/theme.rs | 93 +++++++++++++-------------- src/parse_styles.rs | 20 ++---- src/subcommands/show_syntax_themes.rs | 19 +++--- src/subcommands/show_themes.rs | 7 +- 10 files changed, 123 insertions(+), 115 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 011cdb56a..909bcf00f 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -11,6 +11,7 @@ use syntect::highlighting::Theme as SyntaxTheme; use syntect::parsing::SyntaxSet; use crate::ansi::{ANSI_SGR_BOLD, ANSI_SGR_RESET, ANSI_SGR_UNDERLINE}; +use crate::color::ColorMode; use crate::config::delta_unreachable; use crate::env::DeltaEnv; use crate::git_config::GitConfig; @@ -1180,7 +1181,7 @@ pub struct ComputedValues { pub background_color_extends_to_terminal_width: bool, pub decorations_width: Width, pub inspect_raw_lines: InspectRawLines, - pub is_light_mode: bool, + pub color_mode: ColorMode, pub paging_mode: PagingMode, pub syntax_set: SyntaxSet, pub syntax_theme: Option, diff --git a/src/color.rs b/src/color.rs index a4b96b586..fb368f987 100644 --- a/src/color.rs +++ b/src/color.rs @@ -8,6 +8,7 @@ use syntect::highlighting::Color as SyntectColor; use crate::fatal; use crate::git_config::GitConfig; use crate::utils; +use ColorMode::*; pub fn parse_color(s: &str, true_color: bool, git_config: Option<&GitConfig>) -> Option { if s == "normal" { @@ -105,39 +106,50 @@ fn ansi_16_color_number_to_name(n: u8) -> Option<&'static str> { None } -pub fn get_minus_background_color_default(is_light_mode: bool, is_true_color: bool) -> Color { - match (is_light_mode, is_true_color) { - (true, true) => LIGHT_THEME_MINUS_COLOR, - (true, false) => LIGHT_THEME_MINUS_COLOR_256, - (false, true) => DARK_THEME_MINUS_COLOR, - (false, false) => DARK_THEME_MINUS_COLOR_256, +/// The color mode determines some default color choices +/// such as the diff background color or the palette used for blame. +#[derive(Default, Clone, Copy, Debug, PartialEq, Eq)] +pub enum ColorMode { + #[default] + /// Dark background with light text. + Dark, + /// Light background with dark text. + Light, +} + +pub fn get_minus_background_color_default(mode: ColorMode, is_true_color: bool) -> Color { + match (mode, is_true_color) { + (Light, true) => LIGHT_THEME_MINUS_COLOR, + (Light, false) => LIGHT_THEME_MINUS_COLOR_256, + (Dark, true) => DARK_THEME_MINUS_COLOR, + (Dark, false) => DARK_THEME_MINUS_COLOR_256, } } -pub fn get_minus_emph_background_color_default(is_light_mode: bool, is_true_color: bool) -> Color { - match (is_light_mode, is_true_color) { - (true, true) => LIGHT_THEME_MINUS_EMPH_COLOR, - (true, false) => LIGHT_THEME_MINUS_EMPH_COLOR_256, - (false, true) => DARK_THEME_MINUS_EMPH_COLOR, - (false, false) => DARK_THEME_MINUS_EMPH_COLOR_256, +pub fn get_minus_emph_background_color_default(mode: ColorMode, is_true_color: bool) -> Color { + match (mode, is_true_color) { + (Light, true) => LIGHT_THEME_MINUS_EMPH_COLOR, + (Light, false) => LIGHT_THEME_MINUS_EMPH_COLOR_256, + (Dark, true) => DARK_THEME_MINUS_EMPH_COLOR, + (Dark, false) => DARK_THEME_MINUS_EMPH_COLOR_256, } } -pub fn get_plus_background_color_default(is_light_mode: bool, is_true_color: bool) -> Color { - match (is_light_mode, is_true_color) { - (true, true) => LIGHT_THEME_PLUS_COLOR, - (true, false) => LIGHT_THEME_PLUS_COLOR_256, - (false, true) => DARK_THEME_PLUS_COLOR, - (false, false) => DARK_THEME_PLUS_COLOR_256, +pub fn get_plus_background_color_default(mode: ColorMode, is_true_color: bool) -> Color { + match (mode, is_true_color) { + (Light, true) => LIGHT_THEME_PLUS_COLOR, + (Light, false) => LIGHT_THEME_PLUS_COLOR_256, + (Dark, true) => DARK_THEME_PLUS_COLOR, + (Dark, false) => DARK_THEME_PLUS_COLOR_256, } } -pub fn get_plus_emph_background_color_default(is_light_mode: bool, is_true_color: bool) -> Color { - match (is_light_mode, is_true_color) { - (true, true) => LIGHT_THEME_PLUS_EMPH_COLOR, - (true, false) => LIGHT_THEME_PLUS_EMPH_COLOR_256, - (false, true) => DARK_THEME_PLUS_EMPH_COLOR, - (false, false) => DARK_THEME_PLUS_EMPH_COLOR_256, +pub fn get_plus_emph_background_color_default(mode: ColorMode, is_true_color: bool) -> Color { + match (mode, is_true_color) { + (Light, true) => LIGHT_THEME_PLUS_EMPH_COLOR, + (Light, false) => LIGHT_THEME_PLUS_EMPH_COLOR_256, + (Dark, true) => DARK_THEME_PLUS_EMPH_COLOR, + (Dark, false) => DARK_THEME_PLUS_EMPH_COLOR_256, } } diff --git a/src/config.rs b/src/config.rs index 8d42574f2..86ff40a81 100644 --- a/src/config.rs +++ b/src/config.rs @@ -9,7 +9,7 @@ use syntect::parsing::SyntaxSet; use crate::ansi; use crate::cli; -use crate::color; +use crate::color::{self, ColorMode}; use crate::delta::State; use crate::fatal; use crate::features::navigate; @@ -214,7 +214,7 @@ impl From for Config { )); }); - let blame_palette = make_blame_palette(opt.blame_palette, opt.computed.is_light_mode); + let blame_palette = make_blame_palette(opt.blame_palette, opt.computed.color_mode); if blame_palette.is_empty() { fatal("Option 'blame-palette' must not be empty.") @@ -437,17 +437,17 @@ impl From for Config { } } -fn make_blame_palette(blame_palette: Option, is_light_mode: bool) -> Vec { - match (blame_palette, is_light_mode) { +fn make_blame_palette(blame_palette: Option, mode: ColorMode) -> Vec { + match (blame_palette, mode) { (Some(string), _) => string .split_whitespace() .map(|s| s.to_owned()) .collect::>(), - (None, true) => color::LIGHT_THEME_BLAME_PALETTE + (None, ColorMode::Light) => color::LIGHT_THEME_BLAME_PALETTE .iter() .map(|s| s.to_string()) .collect::>(), - (None, false) => color::DARK_THEME_BLAME_PALETTE + (None, ColorMode::Dark) => color::DARK_THEME_BLAME_PALETTE .iter() .map(|s| s.to_string()) .collect::>(), diff --git a/src/features/line_numbers.rs b/src/features/line_numbers.rs index 0c488000c..477fafd51 100644 --- a/src/features/line_numbers.rs +++ b/src/features/line_numbers.rs @@ -3,6 +3,7 @@ use std::cmp::max; use lazy_static::lazy_static; use regex::Regex; +use crate::color::ColorMode::*; use crate::config; use crate::delta::State; use crate::features::hyperlinks; @@ -38,26 +39,27 @@ pub fn make_feature() -> Vec<(String, OptionValueFunction)> { "line-numbers-minus-style", String, None, - opt => if opt.computed.is_light_mode { - "red".to_string() - } else { - "88".to_string() + opt => match opt.computed.color_mode { + Light => "red", + Dark => "88", } ), ( "line-numbers-zero-style", String, None, - opt => if opt.computed.is_light_mode {"#dddddd"} else {"#444444"} + opt => match opt.computed.color_mode { + Light => "#dddddd", + Dark => "#444444", + } ), ( "line-numbers-plus-style", String, None, - opt => if opt.computed.is_light_mode { - "green".to_string() - } else { - "28".to_string() + opt => match opt.computed.color_mode { + Light => "green", + Dark => "28", } ) ]) diff --git a/src/main.rs b/src/main.rs index 4855dcc5c..0b16c6e4d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -105,7 +105,7 @@ fn run_app() -> std::io::Result { Some(subcommands::show_themes::show_themes( opt.dark, opt.light, - opt.computed.is_light_mode, + opt.computed.color_mode, )) } else if opt.show_colors { Some(subcommands::show_colors::show_colors()) diff --git a/src/options/set.rs b/src/options/set.rs index f02cd4ed7..6d26d15df 100644 --- a/src/options/set.rs +++ b/src/options/set.rs @@ -237,7 +237,7 @@ pub fn set_options( // Setting ComputedValues set_widths_and_isatty(opt); set_true_color(opt); - theme::set__is_light_mode__syntax_theme__syntax_set(opt, assets); + theme::set__color_mode__syntax_theme__syntax_set(opt, assets); opt.computed.inspect_raw_lines = cli::InspectRawLines::from_str(&opt.inspect_raw_lines).unwrap(); opt.computed.paging_mode = parse_paging_mode(&opt.paging_mode); diff --git a/src/options/theme.rs b/src/options/theme.rs index a2cd35e46..474ace0f9 100644 --- a/src/options/theme.rs +++ b/src/options/theme.rs @@ -14,22 +14,20 @@ use std::io::{stdout, IsTerminal}; use bat; use bat::assets::HighlightingAssets; #[cfg(not(test))] -use terminal_colorsaurus::{color_scheme, ColorScheme, QueryOptions}; +use terminal_colorsaurus::{color_scheme, QueryOptions}; use crate::cli::{self, DetectDarkLight}; +use crate::color::{ColorMode, ColorMode::*}; #[allow(non_snake_case)] -pub fn set__is_light_mode__syntax_theme__syntax_set( - opt: &mut cli::Opt, - assets: HighlightingAssets, -) { +pub fn set__color_mode__syntax_theme__syntax_set(opt: &mut cli::Opt, assets: HighlightingAssets) { let syntax_theme_name_from_bat_theme = &opt.env.bat_theme; - let (is_light_mode, syntax_theme_name) = get_is_light_mode_and_syntax_theme_name( + let (color_mode, syntax_theme_name) = get_color_mode_and_syntax_theme_name( opt.syntax_theme.as_ref(), syntax_theme_name_from_bat_theme.as_ref(), - get_is_light(opt), + get_color_mode(opt), ); - opt.computed.is_light_mode = is_light_mode; + opt.computed.color_mode = color_mode; opt.computed.syntax_theme = if is_no_syntax_highlighting_syntax_theme_name(&syntax_theme_name) { None @@ -43,6 +41,14 @@ pub fn is_light_syntax_theme(theme: &str) -> bool { LIGHT_SYNTAX_THEMES.contains(&theme) || theme.to_lowercase().contains("light") } +pub fn color_mode_from_syntax_theme(theme: &str) -> ColorMode { + if is_light_syntax_theme(theme) { + ColorMode::Light + } else { + ColorMode::Dark + } +} + const LIGHT_SYNTAX_THEMES: [&str; 7] = [ "Catppuccin Latte", "GitHub", @@ -88,34 +94,34 @@ fn is_no_syntax_highlighting_syntax_theme_name(theme_name: &str) -> bool { /// | - | - | yes | default light/dark theme, light/dark mode | /// | some_theme | (IGNORED) | yes | some_theme, light/dark mode (even if some_theme conflicts with light/dark) | /// | - | BAT_THEME | yes | BAT_THEME, light/dark mode (even if BAT_THEME conflicts with light/dark) | -fn get_is_light_mode_and_syntax_theme_name( +fn get_color_mode_and_syntax_theme_name( theme_arg: Option<&String>, bat_theme_env_var: Option<&String>, - light_mode: Option, -) -> (bool, String) { + mode: Option, +) -> (ColorMode, String) { let theme_arg = theme_arg.or(bat_theme_env_var); - match (theme_arg, light_mode) { - (Some(theme_name), None) => (is_light_syntax_theme(theme_name), theme_name.to_string()), - (Some(theme_name), Some(is_light_mode)) => (is_light_mode, theme_name.to_string()), - (None, None | Some(false)) => (false, DEFAULT_DARK_SYNTAX_THEME.to_string()), - (None, Some(true)) => (true, DEFAULT_LIGHT_SYNTAX_THEME.to_string()), + match (theme_arg, mode) { + (Some(theme), None) => (color_mode_from_syntax_theme(theme), theme.to_string()), + (Some(theme), Some(mode)) => (mode, theme.to_string()), + (None, None | Some(Dark)) => (Dark, DEFAULT_DARK_SYNTAX_THEME.to_string()), + (None, Some(Light)) => (Light, DEFAULT_LIGHT_SYNTAX_THEME.to_string()), } } -fn get_is_light(opt: &cli::Opt) -> Option { +fn get_color_mode(opt: &cli::Opt) -> Option { if opt.light { - Some(true) + Some(Light) } else if opt.dark { - Some(false) + Some(Dark) + } else if should_detect_color_mode(opt) { + detect_color_mode() } else { - should_detect_dark_light(opt) - .then(detect_light_mode) - .flatten() + None } } /// See [`cli::Opt::detect_dark_light`] for a detailed explanation. -fn should_detect_dark_light(opt: &cli::Opt) -> bool { +fn should_detect_color_mode(opt: &cli::Opt) -> bool { match opt.detect_dark_light { DetectDarkLight::Auto => opt.color_only || stdout().is_terminal(), DetectDarkLight::Always => true, @@ -124,14 +130,23 @@ fn should_detect_dark_light(opt: &cli::Opt) -> bool { } #[cfg(not(test))] -fn detect_light_mode() -> Option { +fn detect_color_mode() -> Option { color_scheme(QueryOptions::default()) .ok() - .map(|color_scheme| color_scheme == ColorScheme::Light) + .map(ColorMode::from) +} + +impl From for ColorMode { + fn from(value: terminal_colorsaurus::ColorScheme) -> Self { + match value { + terminal_colorsaurus::ColorScheme::Dark => ColorMode::Dark, + terminal_colorsaurus::ColorScheme::Light => ColorMode::Light, + } + } } #[cfg(test)] -fn detect_light_mode() -> Option { +fn detect_color_mode() -> Option { None } @@ -144,12 +159,6 @@ mod tests { // TODO: Test influence of BAT_THEME env var. E.g. see utils::process::tests::FakeParentArgs. #[test] fn test_syntax_theme_selection() { - use Mode::*; - #[derive(PartialEq)] - enum Mode { - Light, - Dark, - } for ( syntax_theme, mode, // (--light, --dark) @@ -210,31 +219,19 @@ mod tests { } assert_eq!( config.minus_style.ansi_term_style.background.unwrap(), - color::get_minus_background_color_default( - expected_mode == Mode::Light, - is_true_color - ) + color::get_minus_background_color_default(expected_mode, is_true_color) ); assert_eq!( config.minus_emph_style.ansi_term_style.background.unwrap(), - color::get_minus_emph_background_color_default( - expected_mode == Mode::Light, - is_true_color - ) + color::get_minus_emph_background_color_default(expected_mode, is_true_color) ); assert_eq!( config.plus_style.ansi_term_style.background.unwrap(), - color::get_plus_background_color_default( - expected_mode == Mode::Light, - is_true_color - ) + color::get_plus_background_color_default(expected_mode, is_true_color) ); assert_eq!( config.plus_emph_style.ansi_term_style.background.unwrap(), - color::get_plus_emph_background_color_default( - expected_mode == Mode::Light, - is_true_color - ) + color::get_plus_emph_background_color_default(expected_mode, is_true_color) ); } } diff --git a/src/parse_styles.rs b/src/parse_styles.rs index f1611c8fb..4eb621803 100644 --- a/src/parse_styles.rs +++ b/src/parse_styles.rs @@ -115,15 +115,14 @@ fn parse_as_reference_to_git_config(style_string: &str, opt: &cli::Opt) -> Style } fn make_hunk_styles<'a>(opt: &'a cli::Opt, styles: &'a mut HashMap<&str, StyleReference>) { - let is_light_mode = opt.computed.is_light_mode; + let color_mode = opt.computed.color_mode; let true_color = opt.computed.true_color; let minus_style = style_from_str( &opt.minus_style, Some(Style::from_colors( None, Some(color::get_minus_background_color_default( - is_light_mode, - true_color, + color_mode, true_color, )), )), None, @@ -136,8 +135,7 @@ fn make_hunk_styles<'a>(opt: &'a cli::Opt, styles: &'a mut HashMap<&str, StyleRe Some(Style::from_colors( None, Some(color::get_minus_emph_background_color_default( - is_light_mode, - true_color, + color_mode, true_color, )), )), None, @@ -160,8 +158,7 @@ fn make_hunk_styles<'a>(opt: &'a cli::Opt, styles: &'a mut HashMap<&str, StyleRe Some(Style::from_colors( None, Some(color::get_minus_background_color_default( - is_light_mode, - true_color, + color_mode, true_color, )), )), None, @@ -176,8 +173,7 @@ fn make_hunk_styles<'a>(opt: &'a cli::Opt, styles: &'a mut HashMap<&str, StyleRe Some(Style::from_colors( None, Some(color::get_plus_background_color_default( - is_light_mode, - true_color, + color_mode, true_color, )), )), None, @@ -190,8 +186,7 @@ fn make_hunk_styles<'a>(opt: &'a cli::Opt, styles: &'a mut HashMap<&str, StyleRe Some(Style::from_colors( None, Some(color::get_plus_emph_background_color_default( - is_light_mode, - true_color, + color_mode, true_color, )), )), None, @@ -214,8 +209,7 @@ fn make_hunk_styles<'a>(opt: &'a cli::Opt, styles: &'a mut HashMap<&str, StyleRe Some(Style::from_colors( None, Some(color::get_plus_background_color_default( - is_light_mode, - true_color, + color_mode, true_color, )), )), None, diff --git a/src/subcommands/show_syntax_themes.rs b/src/subcommands/show_syntax_themes.rs index 6af8acd18..27bbcf3eb 100644 --- a/src/subcommands/show_syntax_themes.rs +++ b/src/subcommands/show_syntax_themes.rs @@ -1,8 +1,9 @@ use crate::cli; +use crate::color::{ColorMode, ColorMode::*}; use crate::config; use crate::delta; use crate::env::DeltaEnv; -use crate::options::theme::is_light_syntax_theme; +use crate::options::theme::color_mode_from_syntax_theme; use crate::utils; use crate::utils::bat::output::{OutputType, PagingMode}; use clap::Parser; @@ -41,19 +42,19 @@ pub fn show_syntax_themes() -> std::io::Result<()> { let opt = make_opt(); if !(opt.dark || opt.light) { - _show_syntax_themes(opt, false, &mut writer, stdin_data.as_ref())?; - _show_syntax_themes(make_opt(), true, &mut writer, stdin_data.as_ref())?; + _show_syntax_themes(opt, Dark, &mut writer, stdin_data.as_ref())?; + _show_syntax_themes(make_opt(), Light, &mut writer, stdin_data.as_ref())?; } else if opt.light { - _show_syntax_themes(opt, true, &mut writer, stdin_data.as_ref())?; + _show_syntax_themes(opt, Light, &mut writer, stdin_data.as_ref())?; } else { - _show_syntax_themes(opt, false, &mut writer, stdin_data.as_ref())? + _show_syntax_themes(opt, Dark, &mut writer, stdin_data.as_ref())? }; Ok(()) } fn _show_syntax_themes( mut opt: cli::Opt, - is_light_mode: bool, + color_mode: ColorMode, writer: &mut dyn Write, stdin: Option<&Vec>, ) -> std::io::Result<()> { @@ -80,14 +81,14 @@ index f38589a..0f1bb83 100644 } }; - opt.computed.is_light_mode = is_light_mode; + opt.computed.color_mode = color_mode; let mut config = config::Config::from(opt); let title_style = ansi_term::Style::new().bold(); let assets = utils::bat::assets::load_highlighting_assets(); for syntax_theme in assets .themes() - .filter(|t| is_light_syntax_theme(t) == is_light_mode) + .filter(|t| color_mode_from_syntax_theme(t) == color_mode) { writeln!( writer, @@ -121,7 +122,7 @@ mod tests { let opt = integration_test_utils::make_options_from_args(&[]); let mut writer = Cursor::new(vec![0; 1024]); - _show_syntax_themes(opt, true, &mut writer, None).unwrap(); + _show_syntax_themes(opt, Light, &mut writer, None).unwrap(); let mut s = String::new(); writer.rewind().unwrap(); writer.read_to_string(&mut s).unwrap(); diff --git a/src/subcommands/show_themes.rs b/src/subcommands/show_themes.rs index 70987fc71..dd6311f7a 100644 --- a/src/subcommands/show_themes.rs +++ b/src/subcommands/show_themes.rs @@ -1,6 +1,7 @@ use std::io::{self, ErrorKind, IsTerminal, Read}; use crate::cli; +use crate::color::ColorMode; use crate::config; use crate::delta; use crate::env::DeltaEnv; @@ -8,7 +9,7 @@ use crate::git_config; use crate::options::get::get_themes; use crate::utils::bat::output::{OutputType, PagingMode}; -pub fn show_themes(dark: bool, light: bool, computed_theme_is_light: bool) -> std::io::Result<()> { +pub fn show_themes(dark: bool, light: bool, color_mode: ColorMode) -> std::io::Result<()> { use std::io::BufReader; use bytelines::ByteLines; @@ -58,8 +59,8 @@ pub fn show_themes(dark: bool, light: bool, computed_theme_is_light: bool) -> st let is_light_theme = opt.light; let config = config::Config::from(opt); - if (!computed_theme_is_light && is_dark_theme) - || (computed_theme_is_light && is_light_theme) + if (color_mode == ColorMode::Dark && is_dark_theme) + || (color_mode == ColorMode::Light && is_light_theme) || (dark && light) { writeln!(writer, "\n\nTheme: {}\n", title_style.paint(theme))?; From cd6daadd6cc4afe4c97829c38c38180453319a91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Sat, 7 Sep 2024 23:33:30 +0200 Subject: [PATCH 11/12] Remove fallback to bat theme env var The fallback is already handled in `set_options` --- src/options/theme.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/options/theme.rs b/src/options/theme.rs index 474ace0f9..975c633ff 100644 --- a/src/options/theme.rs +++ b/src/options/theme.rs @@ -21,12 +21,8 @@ use crate::color::{ColorMode, ColorMode::*}; #[allow(non_snake_case)] pub fn set__color_mode__syntax_theme__syntax_set(opt: &mut cli::Opt, assets: HighlightingAssets) { - let syntax_theme_name_from_bat_theme = &opt.env.bat_theme; - let (color_mode, syntax_theme_name) = get_color_mode_and_syntax_theme_name( - opt.syntax_theme.as_ref(), - syntax_theme_name_from_bat_theme.as_ref(), - get_color_mode(opt), - ); + let (color_mode, syntax_theme_name) = + get_color_mode_and_syntax_theme_name(opt.syntax_theme.as_ref(), get_color_mode(opt)); opt.computed.color_mode = color_mode; opt.computed.syntax_theme = if is_no_syntax_highlighting_syntax_theme_name(&syntax_theme_name) { @@ -95,12 +91,10 @@ fn is_no_syntax_highlighting_syntax_theme_name(theme_name: &str) -> bool { /// | some_theme | (IGNORED) | yes | some_theme, light/dark mode (even if some_theme conflicts with light/dark) | /// | - | BAT_THEME | yes | BAT_THEME, light/dark mode (even if BAT_THEME conflicts with light/dark) | fn get_color_mode_and_syntax_theme_name( - theme_arg: Option<&String>, - bat_theme_env_var: Option<&String>, + syntax_theme: Option<&String>, mode: Option, ) -> (ColorMode, String) { - let theme_arg = theme_arg.or(bat_theme_env_var); - match (theme_arg, mode) { + match (syntax_theme, mode) { (Some(theme), None) => (color_mode_from_syntax_theme(theme), theme.to_string()), (Some(theme), Some(mode)) => (mode, theme.to_string()), (None, None | Some(Dark)) => (Dark, DEFAULT_DARK_SYNTAX_THEME.to_string()), From b2d5ebf8ca082cb4fcbe1b1909caf25daf0a0a07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tau=20G=C3=A4rtli?= Date: Sun, 8 Sep 2024 16:12:40 +0200 Subject: [PATCH 12/12] Consolidate doc comments --- src/options/theme.rs | 46 ++++++++++++-------------------------------- 1 file changed, 12 insertions(+), 34 deletions(-) diff --git a/src/options/theme.rs b/src/options/theme.rs index 975c633ff..215f49d3d 100644 --- a/src/options/theme.rs +++ b/src/options/theme.rs @@ -1,13 +1,17 @@ //! Delta doesn't have a formal concept of a "theme". What it has is //! -//! * (a) the choice of syntax-highlighting theme -//! * (b) the choice of light-background-mode vs dark-background-mode, which determine certain -//! default color choices +//! 1. The choice of "theme". This is the language syntax highlighting theme; you have to make this +//! choice when using `bat` also. +//! 2. The choice of "light vs dark mode". This determines whether the background colors should be +//! chosen for a light or dark terminal background. (`bat` has no equivalent.) //! -//! This module sets those options. If the light/dark background mode choice is not made explicitly -//! by the user, it is determined by the classification of the syntax theme into light-background -//! vs dark-background syntax themes. If the user didn't choose a syntax theme, a dark-background -//! default is selected. +//! Basically: +//! 1. The theme is specified by the `--syntax-theme` option. If this isn't supplied then it is specified +//! by the `BAT_THEME` environment variable. +//! 2. Light vs dark mode is specified by the `--light` or `--dark` options. If these aren't +//! supplied then it detected from the terminal. If this fails it is inferred from the chosen theme. +//! +//! In the absence of other factors, the default assumes a dark terminal background. use std::io::{stdout, IsTerminal}; @@ -62,34 +66,8 @@ fn is_no_syntax_highlighting_syntax_theme_name(theme_name: &str) -> bool { theme_name.to_lowercase() == "none" } -/// Return a (theme_name, is_light_mode) tuple. +/// Return a (theme_name, color_mode) tuple. /// theme_name == None in return value means syntax highlighting is disabled. -/// -/// There are two types of color choices that have to be made: - -/// 1. The choice of "theme". This is the language syntax highlighting theme; you have to make this -/// choice when using `bat` also. -/// 2. The choice of "light vs dark mode". This determines whether the background colors should be -/// chosen for a light or dark terminal background. (`bat` has no equivalent.) -/// -/// Basically: -/// 1. The theme is specified by the `--syntax-theme` option. If this isn't supplied then it is specified -/// by the `BAT_THEME` environment variable. -/// 2. Light vs dark mode is specified by the `--light` or `--dark` options. If these aren't -/// supplied then it is inferred from the chosen theme. -/// -/// In the absence of other factors, the default assumes a dark terminal background. -/// -/// Specifically, the rules are as follows: -/// -/// | --theme | $BAT_THEME | --light/--dark | Behavior | -/// |------------|------------|----------------|----------------------------------------------------------------------------| -/// | - | - | - | default dark theme, dark mode | -/// | some_theme | (IGNORED) | - | some_theme with light/dark mode inferred accordingly | -/// | - | BAT_THEME | - | BAT_THEME, with light/dark mode inferred accordingly | -/// | - | - | yes | default light/dark theme, light/dark mode | -/// | some_theme | (IGNORED) | yes | some_theme, light/dark mode (even if some_theme conflicts with light/dark) | -/// | - | BAT_THEME | yes | BAT_THEME, light/dark mode (even if BAT_THEME conflicts with light/dark) | fn get_color_mode_and_syntax_theme_name( syntax_theme: Option<&String>, mode: Option,