From 46b622c50a4528cb9085989acbf854dd0a887773 Mon Sep 17 00:00:00 2001 From: AlexanderNZ Date: Sun, 26 Jul 2026 15:59:03 +1200 Subject: [PATCH 1/8] feat: custom color themes via [[theme.custom]] with inheritance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the fixed Theme enum with a ThemeSet built at startup from the built-ins plus any [[theme.custom]] entries in config.toml, so built-in and user-defined themes are the same thing and theme cycling treats them alike. Custom themes accept #RRGGBB hex or the 16 named ANSI colors. `inherits` lets a theme extend another and only restate what differs. Customs resolve in config order, so a theme can only inherit from one defined before it — that ordering requirement is what makes circular chains unrepresentable rather than something to detect. A custom sharing a built-in's name replaces it in place, keeping cycle order stable. `foreground` and `background` are broad-brush aliases, applied before the per-field overrides so specific fields still win. They deliberately skip every element whose job is to stand out — the cursor cell, current row and column, and search highlights — which keep the contrast their parent theme designed in. Setting `background` used to flatten current_row_bg onto it and make the cursor row invisible; the regression test covers that whole class, not just the one field. deny_unknown_fields catches typos like `forground`. The tradeoff is that a config using a field from a newer xleak fails on an older binary instead of degrading. Theme resolution happens in run_tui before the terminal is reconfigured, so an unresolvable `inherits` fails with a readable message and warnings aren't swallowed by the alternate screen. Co-Authored-By: Claude --- src/config.rs | 272 ++++++++++++++++++++++- src/tui/event.rs | 12 +- src/tui/rendering.rs | 13 +- src/tui/state.rs | 25 +-- src/tui/theme.rs | 512 +++++++++++++++++++++++++++++++++++++++---- 5 files changed, 766 insertions(+), 68 deletions(-) diff --git a/src/config.rs b/src/config.rs index dbfe447..ad366c0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,5 +1,6 @@ use anyhow::{Context, Result}; use crossterm::event::{KeyCode, KeyModifiers}; +use ratatui::style::Color; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::fs; @@ -21,6 +22,132 @@ pub struct Config { pub struct ThemeConfig { /// Default theme to use on startup pub default: String, + /// User-defined themes from `[[theme.custom]]` + #[serde(default, skip_serializing)] + pub custom: Vec, +} + +/// A user-defined theme from `[[theme.custom]]` in config.toml. +/// +/// Every color is optional: unset fields come from the theme named by +/// `inherits`, or from the built-in Default theme. `foreground` and `background` +/// are broad-brush aliases that the specific fields below override. +/// +/// `deny_unknown_fields` catches typos like `forground` instead of silently +/// ignoring them. The tradeoff is that a config using a field added by a newer +/// xleak will fail on an older binary rather than degrade. +#[derive(Debug, Clone, Default, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct CustomTheme { + pub name: String, + pub inherits: Option, + + // Broad-brush aliases + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub foreground: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub background: Option, + + // Cell type colors + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub string_fg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub number_fg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub bool_fg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub datetime_fg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub error_fg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub empty_fg: Option, + + // UI element colors + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub header_fg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub header_bg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub current_cell_fg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub current_cell_bg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub current_row_bg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub current_col_fg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub alternating_row_bg: Option, + + // Search colors + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub search_match_fg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub search_match_bg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub current_search_fg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub current_search_bg: Option, + + // Border and status bar + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub border_fg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub status_bar_fg: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub status_bar_bg: Option, +} + +/// Parse a config color: either `#RRGGBB` or one of the 16 ANSI color names. +/// +/// 3-digit hex shorthand (`#fff`) is deliberately not supported — accepting it +/// would mean guessing whether `#123` means `#112233` or `#010203`. +fn parse_color(s: &str) -> Option { + let s = s.trim(); + + if let Some(hex) = s.strip_prefix('#') { + if hex.len() != 6 { + return None; + } + return Some(Color::Rgb( + u8::from_str_radix(&hex[0..2], 16).ok()?, + u8::from_str_radix(&hex[2..4], 16).ok()?, + u8::from_str_radix(&hex[4..6], 16).ok()?, + )); + } + + match s.to_lowercase().replace([' ', '_', '-'], "").as_str() { + "black" => Some(Color::Black), + "red" => Some(Color::Red), + "green" => Some(Color::Green), + "yellow" => Some(Color::Yellow), + "blue" => Some(Color::Blue), + "magenta" => Some(Color::Magenta), + "cyan" => Some(Color::Cyan), + "gray" | "grey" => Some(Color::Gray), + "darkgray" | "darkgrey" => Some(Color::DarkGray), + "lightred" => Some(Color::LightRed), + "lightgreen" => Some(Color::LightGreen), + "lightyellow" => Some(Color::LightYellow), + "lightblue" => Some(Color::LightBlue), + "lightmagenta" => Some(Color::LightMagenta), + "lightcyan" => Some(Color::LightCyan), + "white" => Some(Color::White), + _ => None, + } +} + +fn deserialize_opt_color<'de, D>(deserializer: D) -> std::result::Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + match Option::::deserialize(deserializer)? { + None => Ok(None), + Some(s) => parse_color(&s).map(Some).ok_or_else(|| { + serde::de::Error::custom(format!( + "invalid color '{s}': expected \"#RRGGBB\" or a named color such as \"cyan\"" + )) + }), + } } /// UI configuration @@ -48,6 +175,7 @@ impl Default for ThemeConfig { fn default() -> Self { Self { default: "Default".to_string(), + custom: Vec::new(), } } } @@ -384,8 +512,8 @@ mod tests { #[test] fn test_theme_name_case_insensitive() { - // Theme config parsing stores the string as-is - // TuiState::parse_theme_name handles case-insensitive matching + // Theme config parsing stores the string as-is; + // theme::ThemeSet handles case-insensitive matching let config_str = "[theme]\ndefault = \"dracula\""; let config: Config = toml::from_str(config_str).unwrap(); assert_eq!(config.theme.default, "dracula"); @@ -578,4 +706,144 @@ page_up = "Ctrl+b" Some((KeyCode::Char('/'), KeyModifiers::empty())) ); } + + // ========================================================================= + // Color Parsing Tests + // ========================================================================= + + #[test] + fn test_parse_color_hex() { + assert_eq!(parse_color("#ff0000"), Some(Color::Rgb(255, 0, 0))); + assert_eq!(parse_color("#00ff00"), Some(Color::Rgb(0, 255, 0))); + assert_eq!(parse_color("#0000ff"), Some(Color::Rgb(0, 0, 255))); + assert_eq!(parse_color("#1a1b26"), Some(Color::Rgb(26, 27, 38))); + assert_eq!(parse_color("#1A1B26"), Some(Color::Rgb(26, 27, 38))); + assert_eq!(parse_color(" #1a1b26 "), Some(Color::Rgb(26, 27, 38))); + } + + #[test] + fn test_parse_color_named() { + assert_eq!(parse_color("red"), Some(Color::Red)); + assert_eq!(parse_color("cyan"), Some(Color::Cyan)); + assert_eq!(parse_color("White"), Some(Color::White)); + assert_eq!(parse_color("dark_gray"), Some(Color::DarkGray)); + assert_eq!(parse_color("DarkGray"), Some(Color::DarkGray)); + assert_eq!(parse_color("light-yellow"), Some(Color::LightYellow)); + assert_eq!(parse_color("grey"), Some(Color::Gray)); + } + + #[test] + fn test_parse_color_invalid() { + // 3-digit shorthand is deliberately unsupported. + assert_eq!(parse_color("#fff"), None); + assert_eq!(parse_color("#gggggg"), None); + assert_eq!(parse_color("#1a1b2"), None); + assert_eq!(parse_color("#1a1b267"), None); + assert_eq!(parse_color("notacolor"), None); + assert_eq!(parse_color(""), None); + } + + // ========================================================================= + // Custom Theme Config Tests + // ========================================================================= + + #[test] + fn test_custom_theme_parsing() { + // r## so the `"#` in the hex values doesn't terminate the raw string. + let config_str = r##" +[theme] +default = "tokyonight" + +[[theme.custom]] +name = "tokyonight" +inherits = "Dracula" +foreground = "#c0caf5" +background = "#1a1b26" +header_fg = "#7aa2f7" +border_fg = "cyan" +"##; + let config: Config = toml::from_str(config_str).unwrap(); + assert_eq!(config.theme.default, "tokyonight"); + assert_eq!(config.theme.custom.len(), 1); + + let t = &config.theme.custom[0]; + assert_eq!(t.name, "tokyonight"); + assert_eq!(t.inherits.as_deref(), Some("Dracula")); + assert_eq!(t.foreground, Some(Color::Rgb(192, 202, 245))); + assert_eq!(t.background, Some(Color::Rgb(26, 27, 38))); + assert_eq!(t.header_fg, Some(Color::Rgb(122, 162, 247))); + assert_eq!(t.border_fg, Some(Color::Cyan)); + assert_eq!(t.number_fg, None, "unset fields stay None to be inherited"); + } + + #[test] + fn test_multiple_custom_themes_keep_config_order() { + let config_str = r#" +[theme] +default = "Default" + +[[theme.custom]] +name = "first" + +[[theme.custom]] +name = "second" +"#; + let config: Config = toml::from_str(config_str).unwrap(); + let names: Vec<&str> = config + .theme + .custom + .iter() + .map(|t| t.name.as_str()) + .collect(); + assert_eq!(names, ["first", "second"]); + } + + #[test] + fn test_no_custom_themes_defaults_to_empty() { + let config: Config = toml::from_str("[theme]\ndefault = \"Nord\"").unwrap(); + assert!(config.theme.custom.is_empty()); + } + + #[test] + fn test_invalid_color_in_theme() { + let config_str = r#" +[theme] +default = "Default" + +[[theme.custom]] +name = "bad" +foreground = "notacolor" +"#; + let err = toml::from_str::(config_str).unwrap_err(); + assert!( + err.to_string().contains("notacolor"), + "error should name the bad value, got: {err}" + ); + } + + #[test] + fn test_unknown_field_rejected() { + // Typo protection: `forground` must not be silently ignored. + let config_str = r#" +[theme] +default = "Default" + +[[theme.custom]] +name = "typo" +forground = "red" +"#; + assert!(toml::from_str::(config_str).is_err()); + } + + #[test] + fn test_custom_theme_requires_name() { + let config_str = r#" +[theme] +default = "Default" + +[[theme.custom]] +foreground = "red" +"#; + assert!(toml::from_str::(config_str).is_err()); + } } diff --git a/src/tui/event.rs b/src/tui/event.rs index c462f6f..db771b9 100644 --- a/src/tui/event.rs +++ b/src/tui/event.rs @@ -10,6 +10,7 @@ use std::io; use std::time::Duration; use super::state::TuiState; +use super::theme::ThemeSet; impl TuiState { /// Check if a key press matches a configured action @@ -149,7 +150,7 @@ impl TuiState { self.show_help = true; self.help_scroll = 0; } else if self.key_matches(code, modifiers, "theme_toggle") { - self.current_theme = self.current_theme.next(); + self.themes.cycle(); } else if self.key_matches(code, modifiers, "search") { self.search_mode = true; self.clear_search(); @@ -236,6 +237,14 @@ pub fn run_tui( ); } + // Resolve themes before touching the terminal: a bad `inherits` should fail + // with a readable message, and warnings written after EnterAlternateScreen + // would be mangled by raw mode and then wiped by the first draw. + let (themes, warnings) = ThemeSet::resolve(&config.theme)?; + for warning in &warnings { + eprintln!("Warning: {warning}"); + } + // Restore the terminal before the panic message prints, so it lands on a // readable screen instead of vanishing into the alternate buffer. let original_hook = std::panic::take_hook(); @@ -258,6 +267,7 @@ pub fn run_tui( workbook, sheet_name, config, + themes, horizontal_scroll, no_header, no_column_id, diff --git a/src/tui/rendering.rs b/src/tui/rendering.rs index 5910288..b17b3e8 100644 --- a/src/tui/rendering.rs +++ b/src/tui/rendering.rs @@ -24,8 +24,9 @@ impl TuiState { format!("{current_cell_value} ") }; - let mut status_style = Style::default().fg(self.current_theme.colors().status_bar_fg); - if let Some(bg) = self.current_theme.colors().status_bar_bg { + let colors = self.themes.current(); + let mut status_style = Style::default().fg(colors.status_bar_fg); + if let Some(bg) = colors.status_bar_bg { status_style = status_style.bg(bg); } @@ -95,7 +96,7 @@ impl TuiState { // Cloned to avoid borrowing self while building rows. let headers = self.sheet_data.headers().to_vec(); - let colors = self.current_theme.colors(); + let colors = self.themes.current().clone(); let mut header_cells: Vec = Vec::new(); @@ -353,7 +354,7 @@ impl TuiState { format!(" {} | {} ", cell_addr, self.search_query), format!( "{} | t:theme /:search ?:help q:quit ", - self.current_theme.name() + self.themes.current_name() ), ) } else if let Some(idx) = self.current_match_index { @@ -362,7 +363,7 @@ impl TuiState { format!(" {} | {} ", match_info, cell_addr), format!( "{} | n:next N:prev Esc:clear ?:help q:quit ", - self.current_theme.name() + self.themes.current_name() ), ) } else { @@ -370,7 +371,7 @@ impl TuiState { format!(" {} ", cell_addr), format!( "{} | t:theme /:search ?:help q:quit ", - self.current_theme.name() + self.themes.current_name() ), ) }; diff --git a/src/tui/state.rs b/src/tui/state.rs index 9840455..1d3a546 100644 --- a/src/tui/state.rs +++ b/src/tui/state.rs @@ -5,7 +5,7 @@ use std::collections::{HashMap, HashSet}; use std::time::Instant; use super::clipboard::{self, CopyOutcome}; -use super::theme::Theme; +use super::theme::ThemeSet; /// Cached row data for lazy loading pub(crate) struct RowCache { @@ -158,7 +158,7 @@ pub struct TuiState { // Clipboard state pub copy_feedback: Option<(String, Instant)>, // Message and timestamp for copy feedback // Theme state - pub current_theme: Theme, // Current color theme + pub themes: ThemeSet, // Available themes and the active one // Config state pub config: crate::config::Config, // User configuration // No-header mode @@ -177,10 +177,17 @@ impl TuiState { pub const LAZY_LOADING_THRESHOLD: usize = 1000; pub const ROW_CACHE_SIZE: usize = 200; + /// `themes` is resolved by the caller rather than here, so that config + /// errors and warnings surface before the terminal enters the alternate + /// screen. + // The parameter list is over clippy's limit; the next commit replaces the + // trailing flags with a `TuiOptions` struct. + #[allow(clippy::too_many_arguments)] pub fn new( mut workbook: Workbook, initial_sheet_name: &str, config: &crate::config::Config, + themes: ThemeSet, horizontal_scroll: bool, no_header: bool, no_column_id: bool, @@ -233,7 +240,7 @@ impl TuiState { jump_mode: false, jump_input: String::new(), copy_feedback: None, - current_theme: Self::parse_theme_name(&config.theme.default), + themes, config: config.clone(), no_header, no_column_id, @@ -249,18 +256,6 @@ impl TuiState { Ok(state) } - /// Parse theme name from config string - pub fn parse_theme_name(name: &str) -> Theme { - match name.to_lowercase().as_str() { - "dracula" => Theme::Dracula, - "solarized dark" | "solarizeddark" => Theme::SolarizedDark, - "solarized light" | "solarizedlight" => Theme::SolarizedLight, - "github dark" | "githubdark" => Theme::GitHubDark, - "nord" => Theme::Nord, - _ => Theme::Default, - } - } - pub fn current_sheet_name(&self) -> &str { &self.sheet_names[self.current_sheet_index] } diff --git a/src/tui/theme.rs b/src/tui/theme.rs index 62aeb87..8f9de84 100644 --- a/src/tui/theme.rs +++ b/src/tui/theme.rs @@ -1,60 +1,214 @@ +use crate::config::{CustomTheme, ThemeConfig}; use crate::workbook::CellValue; +use anyhow::Result; use ratatui::style::Color; -/// Available themes -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum Theme { - Default, - Dracula, - SolarizedDark, - SolarizedLight, - GitHubDark, - Nord, +/// A color scheme with the name users refer to it by, in config and in the +/// status bar. Built-ins and `[[theme.custom]]` entries are the same shape, so +/// theme cycling doesn't care where a theme came from. +#[derive(Debug, Clone)] +pub struct NamedTheme { + pub name: String, + pub colors: ColorScheme, +} + +/// Theme names are matched loosely, so `"Solarized Dark"`, `"solarized_dark"`, +/// and `"solarizeddark"` all refer to the same theme. +fn normalized(name: &str) -> String { + name.trim().to_lowercase().replace(' ', "") +} + +/// Returns the built-in themes in cycle order. +pub fn builtin_themes() -> Vec { + vec![ + NamedTheme { + name: "Default".into(), + colors: ColorScheme::default_theme(), + }, + NamedTheme { + name: "Dracula".into(), + colors: ColorScheme::dracula(), + }, + NamedTheme { + name: "Solarized Dark".into(), + colors: ColorScheme::solarized_dark(), + }, + NamedTheme { + name: "Solarized Light".into(), + colors: ColorScheme::solarized_light(), + }, + NamedTheme { + name: "GitHub Dark".into(), + colors: ColorScheme::github_dark(), + }, + NamedTheme { + name: "Nord".into(), + colors: ColorScheme::nord(), + }, + ] +} + +/// Every theme available this run, plus which one is active. +/// +/// Invariants: `themes` is never empty (it always contains the built-ins) and +/// `current` is always a valid index, so `current()` cannot panic. +#[derive(Debug, Clone)] +pub struct ThemeSet { + themes: Vec, + current: usize, } -impl Theme { - /// Get all available themes - pub fn all() -> &'static [Theme] { - &[ - Theme::Default, - Theme::Dracula, - Theme::SolarizedDark, - Theme::SolarizedLight, - Theme::GitHubDark, - Theme::Nord, - ] +impl ThemeSet { + /// Build the theme list from config and select the startup theme. + /// + /// Fails only on an unresolvable `inherits`. An unknown `theme.default` is + /// recoverable, so it comes back as a warning string rather than an error — + /// callers print those *before* entering the alternate screen, where + /// stderr is still visible to the user. + pub fn resolve(config: &ThemeConfig) -> Result<(Self, Vec)> { + let themes = resolve_themes(&config.custom)?; + let mut warnings = Vec::new(); + + let current = match find_index(&themes, &config.default) { + Some(idx) => idx, + None => { + warnings.push(format!( + "theme '{}' not found, falling back to 'Default'", + config.default + )); + 0 + } + }; + + Ok((Self { themes, current }, warnings)) } - /// Get the next theme in the cycle - pub fn next(&self) -> Theme { - let themes = Self::all(); - let current_idx = themes.iter().position(|t| t == self).unwrap_or(0); - themes[(current_idx + 1) % themes.len()] + /// Colors for the active theme. + pub fn current(&self) -> &ColorScheme { + &self.themes[self.current].colors } - /// Get theme name for display - pub fn name(&self) -> &'static str { - match self { - Theme::Default => "Default", - Theme::Dracula => "Dracula", - Theme::SolarizedDark => "Solarized Dark", - Theme::SolarizedLight => "Solarized Light", - Theme::GitHubDark => "GitHub Dark", - Theme::Nord => "Nord", - } + /// Display name of the active theme. + pub fn current_name(&self) -> &str { + &self.themes[self.current].name + } + + /// Advance to the next theme, wrapping. Built-ins come first, then customs + /// in the order they appear in config. + pub fn cycle(&mut self) { + self.current = (self.current + 1) % self.themes.len(); } +} - /// Get the color scheme for this theme - pub fn colors(&self) -> ColorScheme { - match self { - Theme::Default => ColorScheme::default_theme(), - Theme::Dracula => ColorScheme::dracula(), - Theme::SolarizedDark => ColorScheme::solarized_dark(), - Theme::SolarizedLight => ColorScheme::solarized_light(), - Theme::GitHubDark => ColorScheme::github_dark(), - Theme::Nord => ColorScheme::nord(), +/// Case- and space-insensitive lookup of a theme by name. +fn find_index(themes: &[NamedTheme], name: &str) -> Option { + let needle = normalized(name); + themes.iter().position(|t| normalized(&t.name) == needle) +} + +/// Merge custom themes onto the built-ins. +/// +/// Customs are resolved in config order, so a theme can only inherit from one +/// defined before it. That ordering requirement is what makes circular +/// `inherits` chains unrepresentable rather than something we have to detect. +/// A custom sharing a built-in's name replaces it in place, keeping cycle order +/// stable. +pub fn resolve_themes(custom_themes: &[CustomTheme]) -> Result> { + let mut themes = builtin_themes(); + + for custom in custom_themes { + let colors = apply_custom_fields(resolve_base(&themes, custom)?, custom); + match find_index(&themes, &custom.name) { + Some(idx) => themes[idx].colors = colors, + None => themes.push(NamedTheme { + name: custom.name.clone(), + colors, + }), } } + + Ok(themes) +} + +/// The scheme a custom theme starts from before its own fields are applied. +fn resolve_base(themes: &[NamedTheme], custom: &CustomTheme) -> Result { + let Some(ref parent_name) = custom.inherits else { + return Ok(ColorScheme::default_theme()); + }; + + let idx = find_index(themes, parent_name).ok_or_else(|| { + anyhow::anyhow!( + "Theme '{}' referenced in 'inherits' not found. \ + If it's a custom theme, make sure it appears earlier in [[theme.custom]].", + parent_name + ) + })?; + + Ok(themes[idx].colors.clone()) +} + +/// Apply a custom theme's fields over a base scheme. +/// +/// The `foreground`/`background` aliases only touch elements meant to look +/// uniform. Anything whose job is to stand out — the cursor cell, the current +/// row and column, search highlights — is deliberately excluded so it keeps the +/// contrast the parent theme designed in. Users who want those changed set them +/// explicitly, which the per-field overrides below still allow. +fn apply_custom_fields(mut colors: ColorScheme, custom: &CustomTheme) -> ColorScheme { + if let Some(fg) = custom.foreground { + colors.string_fg = fg; + colors.number_fg = fg; + colors.bool_fg = fg; + colors.datetime_fg = fg; + colors.error_fg = fg; + colors.empty_fg = fg; + colors.header_fg = fg; + colors.border_fg = fg; + colors.status_bar_fg = fg; + } + if let Some(bg) = custom.background { + colors.header_bg = Some(bg); + colors.alternating_row_bg = Some(bg); + colors.status_bar_bg = Some(bg); + } + + macro_rules! apply { + ($field:ident) => { + if let Some(c) = custom.$field { + colors.$field = c; + } + }; + } + macro_rules! apply_opt { + ($field:ident) => { + if let Some(c) = custom.$field { + colors.$field = Some(c); + } + }; + } + + apply!(string_fg); + apply!(number_fg); + apply!(bool_fg); + apply!(datetime_fg); + apply!(error_fg); + apply!(empty_fg); + apply!(header_fg); + apply_opt!(header_bg); + apply!(current_cell_fg); + apply!(current_cell_bg); + apply!(current_row_bg); + apply!(current_col_fg); + apply_opt!(alternating_row_bg); + apply!(search_match_fg); + apply!(search_match_bg); + apply!(current_search_fg); + apply!(current_search_bg); + apply!(border_fg); + apply!(status_bar_fg); + apply_opt!(status_bar_bg); + + colors } /// Color scheme for the TUI @@ -300,3 +454,273 @@ impl ColorScheme { } } } + +#[cfg(test)] +mod tests { + use super::*; + + const BUILTIN_COUNT: usize = 6; + + /// Look a theme up by name so tests don't depend on cycle position. + fn colors_of<'a>(themes: &'a [NamedTheme], name: &str) -> &'a ColorScheme { + let idx = find_index(themes, name).unwrap_or_else(|| panic!("no theme named '{name}'")); + &themes[idx].colors + } + + fn custom(name: &str) -> CustomTheme { + CustomTheme { + name: name.into(), + ..Default::default() + } + } + + // ========================================================================= + // Theme Resolution + // ========================================================================= + + #[test] + fn resolve_themes_without_customs_returns_builtins() { + let themes = resolve_themes(&[]).unwrap(); + assert_eq!(themes.len(), BUILTIN_COUNT); + assert_eq!(themes[0].name, "Default"); + assert_eq!(themes[BUILTIN_COUNT - 1].name, "Nord"); + } + + #[test] + fn custom_replaces_builtin_of_same_name_in_place() { + let themes = resolve_themes(&[CustomTheme { + inherits: Some("Dracula".into()), + string_fg: Some(Color::Green), + ..custom("Dracula") + }]) + .unwrap(); + + // Replaced, not appended, so cycle order is unchanged. + assert_eq!(themes.len(), BUILTIN_COUNT); + assert_eq!(themes[1].name, "Dracula"); + + let c = colors_of(&themes, "Dracula"); + assert_eq!(c.string_fg, Color::Green); + // Untouched fields still come from Dracula, not from Default. + assert_eq!(c.number_fg, Color::Rgb(189, 147, 249)); + } + + #[test] + fn custom_with_new_name_is_appended() { + let themes = resolve_themes(&[custom("Brand New")]).unwrap(); + assert_eq!(themes.len(), BUILTIN_COUNT + 1); + assert_eq!(themes[BUILTIN_COUNT].name, "Brand New"); + } + + #[test] + fn custom_can_inherit_an_earlier_custom() { + let themes = resolve_themes(&[ + CustomTheme { + inherits: Some("Nord".into()), + string_fg: Some(Color::Rgb(1, 2, 3)), + ..custom("parent") + }, + CustomTheme { + inherits: Some("parent".into()), + number_fg: Some(Color::Rgb(4, 5, 6)), + ..custom("child") + }, + ]) + .unwrap(); + + let c = colors_of(&themes, "child"); + assert_eq!(c.string_fg, Color::Rgb(1, 2, 3), "inherited from parent"); + assert_eq!(c.number_fg, Color::Rgb(4, 5, 6), "own field"); + assert_eq!(c.bool_fg, Color::Rgb(180, 142, 173), "from Nord via parent"); + } + + #[test] + fn inherits_unknown_theme_is_an_error() { + let err = resolve_themes(&[CustomTheme { + inherits: Some("NonExistent".into()), + ..custom("Bad") + }]) + .unwrap_err(); + assert!(err.to_string().contains("not found")); + } + + #[test] + fn inheriting_a_later_custom_is_an_error() { + // Forward references are rejected; that's what rules out cycles. + let err = resolve_themes(&[ + CustomTheme { + inherits: Some("second".into()), + ..custom("first") + }, + custom("second"), + ]) + .unwrap_err(); + assert!(err.to_string().contains("appears earlier")); + } + + #[test] + fn theme_names_match_loosely() { + let themes = resolve_themes(&[CustomTheme { + inherits: Some("solarizeddark".into()), + ..custom("loose") + }]) + .unwrap(); + assert_eq!( + colors_of(&themes, "loose").number_fg, + Color::Rgb(38, 139, 210), + "'solarizeddark' should resolve to 'Solarized Dark'" + ); + } + + // ========================================================================= + // foreground / background aliases + // ========================================================================= + + #[test] + fn foreground_alias_sets_uniform_fields_and_yields_to_explicit_fields() { + let themes = resolve_themes(&[CustomTheme { + inherits: Some("Default".into()), + foreground: Some(Color::Blue), + string_fg: Some(Color::Red), + ..custom("AliasTest") + }]) + .unwrap(); + + let c = colors_of(&themes, "AliasTest"); + assert_eq!(c.number_fg, Color::Blue); + assert_eq!(c.header_fg, Color::Blue); + assert_eq!(c.border_fg, Color::Blue); + assert_eq!(c.status_bar_fg, Color::Blue); + assert_eq!(c.string_fg, Color::Red, "explicit field wins over alias"); + } + + #[test] + fn background_alias_sets_uniform_fields_and_yields_to_explicit_fields() { + let themes = resolve_themes(&[CustomTheme { + inherits: Some("Default".into()), + background: Some(Color::Rgb(10, 10, 10)), + current_row_bg: Some(Color::Rgb(30, 30, 30)), + ..custom("BgTest") + }]) + .unwrap(); + + let c = colors_of(&themes, "BgTest"); + assert_eq!(c.header_bg, Some(Color::Rgb(10, 10, 10))); + assert_eq!(c.alternating_row_bg, Some(Color::Rgb(10, 10, 10))); + assert_eq!(c.status_bar_bg, Some(Color::Rgb(10, 10, 10))); + assert_eq!(c.current_row_bg, Color::Rgb(30, 30, 30), "explicit wins"); + } + + /// Regression test for the alias bug found in review: setting only + /// `background` used to collapse `current_row_bg` and `current_cell_bg` onto + /// it, so the cursor row became invisible. Generalised to every element whose + /// job is to contrast against the bulk of the table. + #[test] + fn aliases_never_flatten_the_elements_that_provide_contrast() { + let fg = Color::Rgb(192, 202, 245); + let bg = Color::Rgb(26, 27, 38); + let themes = resolve_themes(&[CustomTheme { + inherits: Some("Default".into()), + foreground: Some(fg), + background: Some(bg), + ..custom("NavTest") + }]) + .unwrap(); + + let c = colors_of(&themes, "NavTest"); + let default = ColorScheme::default_theme(); + + for (label, got, inherited) in [ + ("current_row_bg", c.current_row_bg, default.current_row_bg), + ( + "current_cell_bg", + c.current_cell_bg, + default.current_cell_bg, + ), + ( + "current_cell_fg", + c.current_cell_fg, + default.current_cell_fg, + ), + ("current_col_fg", c.current_col_fg, default.current_col_fg), + ( + "search_match_fg", + c.search_match_fg, + default.search_match_fg, + ), + ( + "search_match_bg", + c.search_match_bg, + default.search_match_bg, + ), + ( + "current_search_fg", + c.current_search_fg, + default.current_search_fg, + ), + ( + "current_search_bg", + c.current_search_bg, + default.current_search_bg, + ), + ] { + assert_eq!( + got, inherited, + "{label} should inherit, not follow an alias" + ); + assert_ne!(got, fg, "{label} was flattened onto the foreground alias"); + assert_ne!(got, bg, "{label} was flattened onto the background alias"); + } + + // The specific collapse from the review: these must stay distinguishable. + assert_ne!(c.current_row_bg, c.current_cell_bg); + } + + // ========================================================================= + // ThemeSet + // ========================================================================= + + #[test] + fn resolve_selects_the_configured_default() { + let cfg = ThemeConfig { + default: "nord".into(), + custom: Vec::new(), + }; + let (set, warnings) = ThemeSet::resolve(&cfg).unwrap(); + assert_eq!(set.current_name(), "Nord"); + assert!(warnings.is_empty()); + } + + #[test] + fn resolve_warns_and_falls_back_when_default_is_unknown() { + let cfg = ThemeConfig { + default: "nope".into(), + custom: Vec::new(), + }; + let (set, warnings) = ThemeSet::resolve(&cfg).unwrap(); + assert_eq!(set.current_name(), "Default"); + assert_eq!(warnings.len(), 1); + assert!(warnings[0].contains("nope")); + } + + #[test] + fn cycle_visits_every_theme_and_wraps() { + let cfg = ThemeConfig { + default: "Default".into(), + custom: vec![custom("mine")], + }; + let (mut set, _) = ThemeSet::resolve(&cfg).unwrap(); + + let mut seen = vec![set.current_name().to_string()]; + for _ in 0..BUILTIN_COUNT { + set.cycle(); + seen.push(set.current_name().to_string()); + } + + // Built-ins first, then customs, then back to the start. + assert_eq!(seen.first().unwrap(), "Default"); + assert_eq!(seen[BUILTIN_COUNT], "mine", "customs cycle after built-ins"); + set.cycle(); + assert_eq!(set.current_name(), "Default", "wraps around"); + } +} From f273f318f64241fdb662d4cc49e5bf28995105a0 Mon Sep 17 00:00:00 2001 From: AlexanderNZ Date: Sun, 26 Jul 2026 16:05:57 +1200 Subject: [PATCH 2/8] refactor: replace positional booleans with TuiOptions struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group the four display flags (horizontal_scroll, no_header, no_column_id, no_row_id) into a TuiOptions struct in tui::mod, removing the #[allow(clippy::too_many_arguments)] on TuiState::new. Pure refactor — deliberately ordered after the feature commit so bgreenwell can drop it without unpicking any custom-theme logic. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/main.rs | 10 ++++++---- src/tui/event.rs | 17 +++-------------- src/tui/mod.rs | 17 +++++++++++++++++ src/tui/state.rs | 16 +++++++++------- 4 files changed, 35 insertions(+), 25 deletions(-) diff --git a/src/main.rs b/src/main.rs index 3aec32d..78a6a97 100644 --- a/src/main.rs +++ b/src/main.rs @@ -174,10 +174,12 @@ fn main() -> Result<()> { wb, &sheet_name, &config, - cli.horizontal_scroll, - cli.no_header, - cli.no_column_id, - cli.no_row_id, + &tui::TuiOptions { + horizontal_scroll: cli.horizontal_scroll, + no_header: cli.no_header, + no_column_id: cli.no_column_id, + no_row_id: cli.no_row_id, + }, )?; } else { // Load the sheet data for non-interactive modes diff --git a/src/tui/event.rs b/src/tui/event.rs index db771b9..138c83b 100644 --- a/src/tui/event.rs +++ b/src/tui/event.rs @@ -9,6 +9,7 @@ use ratatui::{Terminal, backend::CrosstermBackend}; use std::io; use std::time::Duration; +use super::TuiOptions; use super::state::TuiState; use super::theme::ThemeSet; @@ -223,10 +224,7 @@ pub fn run_tui( workbook: Workbook, sheet_name: &str, config: &crate::config::Config, - horizontal_scroll: bool, - no_header: bool, - no_column_id: bool, - no_row_id: bool, + options: &TuiOptions, ) -> Result<()> { use std::io::IsTerminal; if !io::stdout().is_terminal() { @@ -263,16 +261,7 @@ pub fn run_tui( let mut terminal = Terminal::new(backend).context("Failed to initialize terminal backend")?; // Create app state - let mut app = TuiState::new( - workbook, - sheet_name, - config, - themes, - horizontal_scroll, - no_header, - no_column_id, - no_row_id, - )?; + let mut app = TuiState::new(workbook, sheet_name, config, themes, options)?; // Main event loop run_event_loop(&mut terminal, &mut app) diff --git a/src/tui/mod.rs b/src/tui/mod.rs index 450bfe9..b344b40 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -6,6 +6,23 @@ mod theme; pub use event::run_tui; +/// Display options for a TUI session, all sourced from CLI flags. +/// +/// These are passed as one value rather than a run of positional booleans, +/// where `run_tui(wb, name, cfg, false, true, false, true)` says nothing about +/// which flag is which. +#[derive(Debug, Clone, Default)] +pub struct TuiOptions { + /// Auto-size columns and allow horizontal scrolling. + pub horizontal_scroll: bool, + /// Treat the first row as data rather than headers. + pub no_header: bool, + /// Hide the column-letter row (A, B, C, ...). + pub no_column_id: bool, + /// Hide the row-number column. + pub no_row_id: bool, +} + #[cfg(test)] mod tests { use super::state::TuiState; diff --git a/src/tui/state.rs b/src/tui/state.rs index 1d3a546..39f81cf 100644 --- a/src/tui/state.rs +++ b/src/tui/state.rs @@ -4,6 +4,7 @@ use anyhow::Result; use std::collections::{HashMap, HashSet}; use std::time::Instant; +use super::TuiOptions; use super::clipboard::{self, CopyOutcome}; use super::theme::ThemeSet; @@ -180,19 +181,20 @@ impl TuiState { /// `themes` is resolved by the caller rather than here, so that config /// errors and warnings surface before the terminal enters the alternate /// screen. - // The parameter list is over clippy's limit; the next commit replaces the - // trailing flags with a `TuiOptions` struct. - #[allow(clippy::too_many_arguments)] pub fn new( mut workbook: Workbook, initial_sheet_name: &str, config: &crate::config::Config, themes: ThemeSet, - horizontal_scroll: bool, - no_header: bool, - no_column_id: bool, - no_row_id: bool, + options: &TuiOptions, ) -> Result { + let &TuiOptions { + horizontal_scroll, + no_header, + no_column_id, + no_row_id, + } = options; + let sheet_names = workbook.sheet_names(); let current_sheet_index = sheet_names .iter() From bc135f28f24476796435c7aef1a386d19dac84ad Mon Sep 17 00:00:00 2001 From: AlexanderNZ Date: Sun, 26 Jul 2026 16:10:16 +1200 Subject: [PATCH 3/8] feat: add --theme flag for CLI theme override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add --theme (long-only; -t is taken by --table) to select the startup theme from the command line. Unknown names are a hard error listing available themes, while an unknown config default still falls back gracefully with a warning. Theme resolution moves from run_tui to main.rs so errors and warnings surface identically in interactive and non-interactive (--export) mode. Also fixes the help text ("6 built-in themes" → "available themes") now that custom themes join the cycle. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/cli.rs | 4 +++ src/main.rs | 28 +++++++++------ src/tui/event.rs | 14 +++----- src/tui/mod.rs | 3 ++ src/tui/rendering.rs | 2 +- src/tui/state.rs | 1 + src/tui/theme.rs | 86 ++++++++++++++++++++++++++++++++++++++------ tests/integration.rs | 28 +++++++++++++++ 8 files changed, 135 insertions(+), 31 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index bad150a..dfbd1ea 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -65,6 +65,10 @@ pub struct Cli { #[arg(long)] pub no_row_id: bool, + /// Color theme to use (e.g. Dracula, Nord, "Solarized Dark") + #[arg(long, value_name = "NAME")] + pub theme: Option, + /// Disable colored output (useful for piping) #[arg(long)] pub no_color: bool, diff --git a/src/main.rs b/src/main.rs index 78a6a97..dcf3ffe 100644 --- a/src/main.rs +++ b/src/main.rs @@ -167,20 +167,26 @@ fn main() -> Result<()> { sheet_names[0].clone() }; + let tui_options = tui::TuiOptions { + horizontal_scroll: cli.horizontal_scroll, + no_header: cli.no_header, + no_column_id: cli.no_column_id, + no_row_id: cli.no_row_id, + theme: cli.theme.clone(), + }; + + // Resolve themes before entering the TUI or exporting, so `--theme` and + // bad `inherits` errors surface identically in both modes. + let (themes, warnings) = + tui::resolve_themes_from_config(&config.theme, tui_options.theme.as_deref())?; + for warning in &warnings { + eprintln!("Warning: {warning}"); + } + // Display, export, or run TUI if cli.interactive { // Interactive TUI mode - pass the workbook so it can switch sheets - tui::run_tui( - wb, - &sheet_name, - &config, - &tui::TuiOptions { - horizontal_scroll: cli.horizontal_scroll, - no_header: cli.no_header, - no_column_id: cli.no_column_id, - no_row_id: cli.no_row_id, - }, - )?; + tui::run_tui(wb, &sheet_name, &config, themes, &tui_options)?; } else { // Load the sheet data for non-interactive modes let data = wb diff --git a/src/tui/event.rs b/src/tui/event.rs index 138c83b..1cfefe6 100644 --- a/src/tui/event.rs +++ b/src/tui/event.rs @@ -219,11 +219,15 @@ impl Drop for TerminalGuard { } } -/// Run the TUI application +/// Run the TUI application. +/// +/// `themes` is resolved by the caller so that `--theme` errors and warnings +/// surface before we enter raw mode (and identically in non-interactive mode). pub fn run_tui( workbook: Workbook, sheet_name: &str, config: &crate::config::Config, + themes: ThemeSet, options: &TuiOptions, ) -> Result<()> { use std::io::IsTerminal; @@ -235,14 +239,6 @@ pub fn run_tui( ); } - // Resolve themes before touching the terminal: a bad `inherits` should fail - // with a readable message, and warnings written after EnterAlternateScreen - // would be mangled by raw mode and then wiped by the first draw. - let (themes, warnings) = ThemeSet::resolve(&config.theme)?; - for warning in &warnings { - eprintln!("Warning: {warning}"); - } - // Restore the terminal before the panic message prints, so it lands on a // readable screen instead of vanishing into the alternate buffer. let original_hook = std::panic::take_hook(); diff --git a/src/tui/mod.rs b/src/tui/mod.rs index b344b40..42b994e 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -5,6 +5,7 @@ mod state; mod theme; pub use event::run_tui; +pub use theme::resolve_themes_from_config; /// Display options for a TUI session, all sourced from CLI flags. /// @@ -21,6 +22,8 @@ pub struct TuiOptions { pub no_column_id: bool, /// Hide the row-number column. pub no_row_id: bool, + /// Override the startup theme (`--theme`). + pub theme: Option, } #[cfg(test)] diff --git a/src/tui/rendering.rs b/src/tui/rendering.rs index b17b3e8..66f59a6 100644 --- a/src/tui/rendering.rs +++ b/src/tui/rendering.rs @@ -680,7 +680,7 @@ impl TuiState { Line::from(" Cell colors vary by type and current theme:"), Line::from(" • Numbers, strings, dates, booleans, errors each have distinct colors"), Line::from(" • Alternating row backgrounds improve readability"), - Line::from(" • Press 't' to cycle through 6 built-in themes"), + Line::from(" • Press 't' to cycle through available themes"), Line::from(""), Line::from(Span::styled( "STATUS BAR INFO", diff --git a/src/tui/state.rs b/src/tui/state.rs index 39f81cf..d5d72cb 100644 --- a/src/tui/state.rs +++ b/src/tui/state.rs @@ -193,6 +193,7 @@ impl TuiState { no_header, no_column_id, no_row_id, + .. } = options; let sheet_names = workbook.sheet_names(); diff --git a/src/tui/theme.rs b/src/tui/theme.rs index 8f9de84..ea1978f 100644 --- a/src/tui/theme.rs +++ b/src/tui/theme.rs @@ -61,20 +61,33 @@ pub struct ThemeSet { impl ThemeSet { /// Build the theme list from config and select the startup theme. /// - /// Fails only on an unresolvable `inherits`. An unknown `theme.default` is - /// recoverable, so it comes back as a warning string rather than an error — - /// callers print those *before* entering the alternate screen, where - /// stderr is still visible to the user. - pub fn resolve(config: &ThemeConfig) -> Result<(Self, Vec)> { + /// When `override_name` is `Some`, it takes precedence over the config + /// default and is a **hard error** if not found (it came from `--theme`, + /// so the user explicitly asked for it). An unknown config default is + /// recoverable, so it comes back as a warning instead. + pub fn resolve( + config: &ThemeConfig, + override_name: Option<&str>, + ) -> Result<(Self, Vec)> { let themes = resolve_themes(&config.custom)?; let mut warnings = Vec::new(); - let current = match find_index(&themes, &config.default) { + let startup_name = override_name.unwrap_or(&config.default); + + let current = match find_index(&themes, startup_name) { Some(idx) => idx, + None if override_name.is_some() => { + let available = Self::format_names(&themes); + anyhow::bail!( + "Unknown theme '{}'. Available themes: {}", + startup_name, + available + ); + } None => { warnings.push(format!( "theme '{}' not found, falling back to 'Default'", - config.default + startup_name )); 0 } @@ -83,6 +96,14 @@ impl ThemeSet { Ok((Self { themes, current }, warnings)) } + fn format_names(themes: &[NamedTheme]) -> String { + themes + .iter() + .map(|t| t.name.as_str()) + .collect::>() + .join(", ") + } + /// Colors for the active theme. pub fn current(&self) -> &ColorScheme { &self.themes[self.current].colors @@ -211,6 +232,15 @@ fn apply_custom_fields(mut colors: ColorScheme, custom: &CustomTheme) -> ColorSc colors } +/// Convenience wrapper for `main.rs`: resolve themes + select the startup +/// theme, with `--theme` override support. +pub fn resolve_themes_from_config( + config: &ThemeConfig, + override_name: Option<&str>, +) -> Result<(ThemeSet, Vec)> { + ThemeSet::resolve(config, override_name) +} + /// Color scheme for the TUI #[derive(Debug, Clone)] pub struct ColorScheme { @@ -686,7 +716,7 @@ mod tests { default: "nord".into(), custom: Vec::new(), }; - let (set, warnings) = ThemeSet::resolve(&cfg).unwrap(); + let (set, warnings) = ThemeSet::resolve(&cfg, None).unwrap(); assert_eq!(set.current_name(), "Nord"); assert!(warnings.is_empty()); } @@ -697,19 +727,55 @@ mod tests { default: "nope".into(), custom: Vec::new(), }; - let (set, warnings) = ThemeSet::resolve(&cfg).unwrap(); + let (set, warnings) = ThemeSet::resolve(&cfg, None).unwrap(); assert_eq!(set.current_name(), "Default"); assert_eq!(warnings.len(), 1); assert!(warnings[0].contains("nope")); } + #[test] + fn override_selects_theme_by_name() { + let cfg = ThemeConfig { + default: "Default".into(), + custom: Vec::new(), + }; + let (set, warnings) = ThemeSet::resolve(&cfg, Some("Dracula")).unwrap(); + assert_eq!(set.current_name(), "Dracula"); + assert!(warnings.is_empty()); + } + + #[test] + fn override_unknown_theme_is_a_hard_error() { + let cfg = ThemeConfig { + default: "Default".into(), + custom: Vec::new(), + }; + let err = ThemeSet::resolve(&cfg, Some("nope")).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("Unknown theme 'nope'"), "{msg}"); + assert!( + msg.contains("Default"), + "should list available themes: {msg}" + ); + } + + #[test] + fn override_beats_config_default() { + let cfg = ThemeConfig { + default: "Nord".into(), + custom: Vec::new(), + }; + let (set, _) = ThemeSet::resolve(&cfg, Some("Dracula")).unwrap(); + assert_eq!(set.current_name(), "Dracula"); + } + #[test] fn cycle_visits_every_theme_and_wraps() { let cfg = ThemeConfig { default: "Default".into(), custom: vec![custom("mine")], }; - let (mut set, _) = ThemeSet::resolve(&cfg).unwrap(); + let (mut set, _) = ThemeSet::resolve(&cfg, None).unwrap(); let mut seen = vec![set.current_name().to_string()]; for _ in 0..BUILTIN_COUNT { diff --git a/tests/integration.rs b/tests/integration.rs index 3a79736..4b0ad45 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -654,3 +654,31 @@ fn test_extensionless_utf8_csv_detected() { let _ = std::fs::remove_file(&path); } + +// ========================================================================= +// --theme flag +// ========================================================================= + +#[test] +fn test_theme_unknown_name_exits_nonzero() { + let fixture = format!("{FIXTURE_DIR}/test_comprehensive.xlsx"); + let output = run_xleak(&[&fixture, "--theme", "nope"]); + assert!(!output.status.success(), "should fail for unknown theme"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(stderr.contains("Unknown theme"), "stderr: {stderr}"); + assert!( + stderr.contains("Default"), + "should list available: {stderr}" + ); +} + +#[test] +fn test_theme_valid_with_export_succeeds() { + let fixture = format!("{FIXTURE_DIR}/test_comprehensive.xlsx"); + let output = run_xleak(&[&fixture, "--theme", "Nord", "--export", "csv"]); + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); +} From 2b0a26141c1e989fe5c9b524412c71201dddf1b1 Mon Sep 17 00:00:00 2001 From: AlexanderNZ Date: Sun, 26 Jul 2026 16:14:34 +1200 Subject: [PATCH 4/8] fix: inherit-by-name, scoped truecolor warning, shared normalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings addressed: Finding 3 — inherit-by-name: when `inherits` is absent, resolve_base now falls back to an existing theme with the same normalized name before defaulting to Default. `name = "Dracula"` + one field now inherits Dracula's palette rather than silently resetting 19 fields. Finding 4 — scoped truecolor warning: NamedTheme gains a `custom` flag so the RGB-without-truecolor warning only fires for user-defined themes. Every non-Default built-in uses Color::Rgb, so warning unconditionally would nag users with no custom config. Reports the actual COLORTERM value instead of assuming "not set". Finding 5 — shared normalization: promote the private `normalized()` to `utils::normalize_name` and use it from both `theme.rs` and `config.rs::parse_color`. Now `inherits = "solarized-dark"` resolves the same way the color name `light-yellow` always did. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/config.rs | 2 +- src/main.rs | 3 + src/tui/mod.rs | 1 + src/tui/theme.rs | 234 ++++++++++++++++++++++++++++++++++++++++------- src/utils.rs | 19 ++++ 5 files changed, 224 insertions(+), 35 deletions(-) diff --git a/src/config.rs b/src/config.rs index ad366c0..be208f0 100644 --- a/src/config.rs +++ b/src/config.rs @@ -115,7 +115,7 @@ fn parse_color(s: &str) -> Option { )); } - match s.to_lowercase().replace([' ', '_', '-'], "").as_str() { + match crate::utils::normalize_name(s).as_str() { "black" => Some(Color::Black), "red" => Some(Color::Red), "green" => Some(Color::Green), diff --git a/src/main.rs b/src/main.rs index dcf3ffe..fdf6f3d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -182,6 +182,9 @@ fn main() -> Result<()> { for warning in &warnings { eprintln!("Warning: {warning}"); } + if let Some(w) = tui::truecolor_warning(&themes, std::env::var("COLORTERM").ok().as_deref()) { + eprintln!("Warning: {w}"); + } // Display, export, or run TUI if cli.interactive { diff --git a/src/tui/mod.rs b/src/tui/mod.rs index 42b994e..f7aa595 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -6,6 +6,7 @@ mod theme; pub use event::run_tui; pub use theme::resolve_themes_from_config; +pub use theme::truecolor_warning; /// Display options for a TUI session, all sourced from CLI flags. /// diff --git a/src/tui/theme.rs b/src/tui/theme.rs index ea1978f..d6df3d9 100644 --- a/src/tui/theme.rs +++ b/src/tui/theme.rs @@ -10,42 +10,28 @@ use ratatui::style::Color; pub struct NamedTheme { pub name: String, pub colors: ColorScheme, + pub custom: bool, } -/// Theme names are matched loosely, so `"Solarized Dark"`, `"solarized_dark"`, -/// and `"solarizeddark"` all refer to the same theme. -fn normalized(name: &str) -> String { - name.trim().to_lowercase().replace(' ', "") -} +use crate::utils::normalize_name; /// Returns the built-in themes in cycle order. pub fn builtin_themes() -> Vec { - vec![ - NamedTheme { - name: "Default".into(), - colors: ColorScheme::default_theme(), - }, - NamedTheme { - name: "Dracula".into(), - colors: ColorScheme::dracula(), - }, - NamedTheme { - name: "Solarized Dark".into(), - colors: ColorScheme::solarized_dark(), - }, - NamedTheme { - name: "Solarized Light".into(), - colors: ColorScheme::solarized_light(), - }, - NamedTheme { - name: "GitHub Dark".into(), - colors: ColorScheme::github_dark(), - }, - NamedTheme { - name: "Nord".into(), - colors: ColorScheme::nord(), - }, + [ + ("Default", ColorScheme::default_theme()), + ("Dracula", ColorScheme::dracula()), + ("Solarized Dark", ColorScheme::solarized_dark()), + ("Solarized Light", ColorScheme::solarized_light()), + ("GitHub Dark", ColorScheme::github_dark()), + ("Nord", ColorScheme::nord()), ] + .into_iter() + .map(|(name, colors)| NamedTheme { + name: name.into(), + colors, + custom: false, + }) + .collect() } /// Every theme available this run, plus which one is active. @@ -123,8 +109,10 @@ impl ThemeSet { /// Case- and space-insensitive lookup of a theme by name. fn find_index(themes: &[NamedTheme], name: &str) -> Option { - let needle = normalized(name); - themes.iter().position(|t| normalized(&t.name) == needle) + let needle = normalize_name(name); + themes + .iter() + .position(|t| normalize_name(&t.name) == needle) } /// Merge custom themes onto the built-ins. @@ -140,10 +128,14 @@ pub fn resolve_themes(custom_themes: &[CustomTheme]) -> Result> for custom in custom_themes { let colors = apply_custom_fields(resolve_base(&themes, custom)?, custom); match find_index(&themes, &custom.name) { - Some(idx) => themes[idx].colors = colors, + Some(idx) => { + themes[idx].colors = colors; + themes[idx].custom = true; + } None => themes.push(NamedTheme { name: custom.name.clone(), colors, + custom: true, }), } } @@ -152,9 +144,16 @@ pub fn resolve_themes(custom_themes: &[CustomTheme]) -> Result> } /// The scheme a custom theme starts from before its own fields are applied. +/// +/// When `inherits` is absent, fall back to an existing theme with the same +/// name so that `name = "Dracula"` + one field inherits the built-in Dracula +/// palette rather than silently resetting every untouched field to Default. fn resolve_base(themes: &[NamedTheme], custom: &CustomTheme) -> Result { let Some(ref parent_name) = custom.inherits else { - return Ok(ColorScheme::default_theme()); + return Ok(match find_index(themes, &custom.name) { + Some(idx) => themes[idx].colors.clone(), + None => ColorScheme::default_theme(), + }); }; let idx = find_index(themes, parent_name).ok_or_else(|| { @@ -241,6 +240,30 @@ pub fn resolve_themes_from_config( ThemeSet::resolve(config, override_name) } +/// Check whether the active theme needs truecolor and warn if the terminal +/// doesn't advertise it. Only fires for custom themes — every non-Default +/// built-in uses `Color::Rgb`, so warning unconditionally would nag users +/// who never touched their config. +pub fn truecolor_warning(themes: &ThemeSet, colorterm: Option<&str>) -> Option { + let theme = &themes.themes[themes.current]; + if !theme.custom || !theme.colors.uses_rgb() { + return None; + } + match colorterm { + Some(v) if v.eq_ignore_ascii_case("truecolor") || v.eq_ignore_ascii_case("24bit") => None, + Some(v) => Some(format!( + "Theme '{}' uses RGB colors, but COLORTERM is '{}' (expected 'truecolor' or '24bit'). \ + Colors may not display correctly.", + theme.name, v + )), + None => Some(format!( + "Theme '{}' uses RGB colors, but COLORTERM is not set. \ + Colors may not display correctly.", + theme.name + )), + } +} + /// Color scheme for the TUI #[derive(Debug, Clone)] pub struct ColorScheme { @@ -472,6 +495,32 @@ impl ColorScheme { } } + /// Whether any field uses `Color::Rgb`, which requires 24-bit color support. + pub fn uses_rgb(&self) -> bool { + let all = [ + self.string_fg, + self.number_fg, + self.bool_fg, + self.datetime_fg, + self.error_fg, + self.empty_fg, + self.header_fg, + self.current_cell_fg, + self.current_cell_bg, + self.current_row_bg, + self.current_col_fg, + self.search_match_fg, + self.search_match_bg, + self.current_search_fg, + self.current_search_bg, + self.border_fg, + self.status_bar_fg, + ]; + let opts = [self.header_bg, self.alternating_row_bg, self.status_bar_bg]; + all.iter().any(|c| matches!(c, Color::Rgb(..))) + || opts.iter().any(|o| matches!(o, Some(Color::Rgb(..)))) + } + /// Get foreground color for a cell based on its value type pub fn cell_color(&self, cell: &CellValue) -> Color { match cell { @@ -602,6 +651,46 @@ mod tests { ); } + #[test] + fn override_without_inherits_falls_back_to_same_name() { + let themes = resolve_themes(&[CustomTheme { + string_fg: Some(Color::Green), + ..custom("Dracula") + }]) + .unwrap(); + + let c = colors_of(&themes, "Dracula"); + assert_eq!(c.string_fg, Color::Green, "explicit override applied"); + assert_eq!( + c.number_fg, + Color::Rgb(189, 147, 249), + "number_fg should come from built-in Dracula, not Default" + ); + } + + #[test] + fn theme_names_match_with_hyphens_and_underscores() { + let themes = resolve_themes(&[CustomTheme { + inherits: Some("solarized-dark".into()), + ..custom("hyphen") + }]) + .unwrap(); + assert_eq!( + colors_of(&themes, "hyphen").number_fg, + Color::Rgb(38, 139, 210), + ); + + let themes = resolve_themes(&[CustomTheme { + inherits: Some("github_dark".into()), + ..custom("underscore") + }]) + .unwrap(); + assert_eq!( + colors_of(&themes, "underscore").number_fg, + Color::Rgb(121, 192, 255), + ); + } + // ========================================================================= // foreground / background aliases // ========================================================================= @@ -789,4 +878,81 @@ mod tests { set.cycle(); assert_eq!(set.current_name(), "Default", "wraps around"); } + + // ========================================================================= + // Truecolor warning + // ========================================================================= + + #[test] + fn truecolor_warning_silent_for_builtins() { + let cfg = ThemeConfig { + default: "Nord".into(), + custom: Vec::new(), + }; + let (set, _) = ThemeSet::resolve(&cfg, None).unwrap(); + assert!(set.themes[set.current].colors.uses_rgb()); + assert!(truecolor_warning(&set, None).is_none()); + } + + #[test] + fn truecolor_warning_fires_for_custom_with_rgb() { + let cfg = ThemeConfig { + default: "mine".into(), + custom: vec![CustomTheme { + inherits: Some("Nord".into()), + string_fg: Some(Color::Rgb(1, 2, 3)), + ..custom("mine") + }], + }; + let (set, _) = ThemeSet::resolve(&cfg, None).unwrap(); + let w = truecolor_warning(&set, None); + assert!(w.is_some(), "should warn"); + assert!(w.as_ref().unwrap().contains("COLORTERM is not set")); + } + + #[test] + fn truecolor_warning_silent_when_colorterm_is_truecolor() { + let cfg = ThemeConfig { + default: "mine".into(), + custom: vec![CustomTheme { + inherits: Some("Nord".into()), + ..custom("mine") + }], + }; + let (set, _) = ThemeSet::resolve(&cfg, None).unwrap(); + assert!(truecolor_warning(&set, Some("truecolor")).is_none()); + assert!(truecolor_warning(&set, Some("24bit")).is_none()); + } + + #[test] + fn truecolor_warning_reports_actual_colorterm_value() { + let cfg = ThemeConfig { + default: "mine".into(), + custom: vec![CustomTheme { + inherits: Some("Nord".into()), + ..custom("mine") + }], + }; + let (set, _) = ThemeSet::resolve(&cfg, None).unwrap(); + let w = truecolor_warning(&set, Some("256color")).unwrap(); + assert!(w.contains("256color"), "should report actual value: {w}"); + } + + // ========================================================================= + // uses_rgb + // ========================================================================= + + #[test] + fn all_builtins_use_rgb() { + for t in builtin_themes() { + assert!(t.colors.uses_rgb(), "{} should use RGB", t.name); + } + } + + #[test] + fn ansi_only_scheme_does_not_use_rgb() { + let mut c = ColorScheme::default_theme(); + c.alternating_row_bg = None; + assert!(!c.uses_rgb()); + } } diff --git a/src/utils.rs b/src/utils.rs index db47190..ed5ddad 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -69,6 +69,14 @@ pub fn detect_file_type>(path: P) -> io::Result { Ok(FileType::Unknown) } +/// Normalize a name for case- and separator-insensitive matching. +/// +/// Used for both theme names and color names so that `"Solarized Dark"`, +/// `"solarized-dark"`, and `"solarized_dark"` all resolve the same way. +pub fn normalize_name(name: &str) -> String { + name.trim().to_lowercase().replace([' ', '_', '-'], "") +} + /// Strip control characters (Unicode `Cc`: C0, DEL, C1) except tab and /// newline, so untrusted spreadsheet content can't inject terminal escape /// sequences into non-interactive output (#59). Borrows when already clean. @@ -98,6 +106,17 @@ pub fn column_index_to_letters(col: usize) -> String { mod tests { use super::*; + #[test] + fn test_normalize_name() { + assert_eq!(normalize_name("Solarized Dark"), "solarizeddark"); + assert_eq!(normalize_name("solarized-dark"), "solarizeddark"); + assert_eq!(normalize_name("solarized_dark"), "solarizeddark"); + assert_eq!(normalize_name(" Nord "), "nord"); + assert_eq!(normalize_name("GitHub Dark"), "githubdark"); + assert_eq!(normalize_name("github_dark"), "githubdark"); + assert_eq!(normalize_name("light-yellow"), "lightyellow"); + } + #[test] fn test_sanitize_terminal_text() { // Escape sequences are stripped, including C1 controls. From adbceda9b30953060f0f26489cc22571c9c30a2e Mon Sep 17 00:00:00 2001 From: AlexanderNZ Date: Sun, 26 Jul 2026 16:31:19 +1200 Subject: [PATCH 5/8] docs: document custom themes in README, config example, and changelog Add Custom Themes section to README with config syntax, inheritance behaviour, and alias semantics. Update config.toml.example with the full list of per-field overrides and a commented-out example. Add the feature, --theme flag, and truecolor warning to CHANGELOG. Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 3 +++ README.md | 38 +++++++++++++++++++++++++++++++++++++- config.toml.example | 30 ++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e0bf64..6e84bab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- Custom color themes via `[[theme.custom]]` in config.toml with `#RRGGBB` hex and named color support, `inherits` for theme inheritance, and `foreground`/`background` broad-brush aliases ([#42](https://github.com/bgreenwell/xleak/issues/42)) +- `--theme ` CLI flag to select a theme at launch, overriding the configured default +- Truecolor warning on stderr when a custom theme uses RGB colors and `COLORTERM` doesn't advertise truecolor support ([#48](https://github.com/bgreenwell/xleak/issues/48)) - OSC 52 clipboard support: `c`/`C` now copy via OSC 52 (works over SSH) in addition to the system clipboard - CSV/TSV support: read and interactively view `.csv`/`.tsv` files as a single sheet (behind the default-on `csv` feature) - `--csv-delimiter` option to override the inferred CSV/TSV field delimiter diff --git a/README.md b/README.md index 2575509..f6f67c1 100644 --- a/README.md +++ b/README.md @@ -324,7 +324,7 @@ xleak --config /path/to/config.toml file.xlsx -i default = "Dracula" ``` -**Available themes:** +**Built-in themes:** - `"Default"` - Clean light theme with subtle colors - `"Dracula"` - Popular dark theme with purple accents - `"Solarized Dark"` - Precision colors for machines and people @@ -333,6 +333,28 @@ default = "Dracula" - `"Nord"` - Arctic, north-bluish color palette Press `t` in interactive mode to cycle through themes at runtime. +Use `--theme ` to select a theme from the command line. + +#### Custom Themes + +Define custom themes in `[[theme.custom]]` blocks. Colors can be `#RRGGBB` hex +or named colors (`red`, `cyan`, `light-yellow`, etc.). + +```toml +[[theme.custom]] +name = "tokyonight" +inherits = "Dracula" +foreground = "#c0caf5" +background = "#1a1b26" +header_fg = "#7aa2f7" +border_fg = "#565f89" +``` + +- `inherits` copies another theme's palette before applying your overrides. Without it, a custom theme sharing a built-in's name inherits that built-in; a new name inherits Default. +- `foreground` / `background` are broad-brush aliases — specific fields like `string_fg` override them. +- Aliases exclude cursor, search, and row-highlight colors so they keep contrast from the parent theme. +- Custom themes appear after built-ins in the `t` cycle. +- `deny_unknown_fields` is on: a typo like `forground` will error rather than be silently ignored. #### UI Settings @@ -482,6 +504,20 @@ jump = "Ctrl+j" default = "Nord" ``` +**Custom theme:** +```toml +[theme] +default = "tokyonight" + +[[theme.custom]] +name = "tokyonight" +inherits = "Dracula" +foreground = "#c0caf5" +background = "#1a1b26" +header_fg = "#7aa2f7" +border_fg = "#565f89" +``` + **VIM user:** ```toml [theme] diff --git a/config.toml.example b/config.toml.example index c19e4df..dfe8c97 100644 --- a/config.toml.example +++ b/config.toml.example @@ -32,6 +32,36 @@ # "Nord" - Arctic, north-bluish color palette default = "Default" +# Custom themes +# ============= +# Define custom themes with [[theme.custom]]. Colors can be "#RRGGBB" hex +# or named colors: black, red, green, yellow, blue, magenta, cyan, white, +# gray, dark-gray, light-red, light-green, light-yellow, light-blue, +# light-magenta, light-cyan. +# +# `inherits` copies another theme's palette before applying your overrides. +# Without it, a custom theme sharing a built-in's name inherits that built-in; +# a new name inherits Default. +# +# `foreground` and `background` are broad-brush aliases — specific fields +# override them. Cursor, search, and row-highlight colors are excluded from +# aliases so they keep contrast from the parent theme. +# +# [[theme.custom]] +# name = "tokyonight" +# inherits = "Dracula" +# foreground = "#c0caf5" +# background = "#1a1b26" +# header_fg = "#7aa2f7" +# border_fg = "#565f89" +# +# Per-field overrides (all optional): +# string_fg, number_fg, bool_fg, datetime_fg, error_fg, empty_fg, +# header_fg, header_bg, current_cell_fg, current_cell_bg, +# current_row_bg, current_col_fg, alternating_row_bg, +# search_match_fg, search_match_bg, current_search_fg, current_search_bg, +# border_fg, status_bar_fg, status_bar_bg + # ============================================================================= # UI SETTINGS # ============================================================================= From 7abed627b67caed87c5675904feed87383ce4384 Mon Sep 17 00:00:00 2001 From: AlexanderNZ Date: Sun, 26 Jul 2026 16:42:22 +1200 Subject: [PATCH 6/8] refactor: single color_field_table! drives CustomTheme, apply, and uses_rgb MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CustomTheme's 20 color fields, apply_custom_fields, and uses_rgb were three hand-maintained parallel lists — adding a ColorScheme field in one silently missed the others (exactly the class of bug behind the original alias review finding). A single color_field_table! macro in theme.rs now defines every customizable field with its kind (Color vs Option) and alias membership (fg/bg/none). Three consumer macros generate the struct fields, the apply logic, and the RGB check from that one table. Tradeoff: CustomTheme is now macro-generated, so it stops being greppable and drops out of rustdoc. Kept as the last commit so it can be dropped independently. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/config.rs | 83 ++++++++------------------- src/tui/mod.rs | 1 + src/tui/theme.rs | 142 ++++++++++++++++++++++------------------------- 3 files changed, 90 insertions(+), 136 deletions(-) diff --git a/src/config.rs b/src/config.rs index be208f0..81ab85b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -36,66 +36,31 @@ pub struct ThemeConfig { /// `deny_unknown_fields` catches typos like `forground` instead of silently /// ignoring them. The tradeoff is that a config using a field added by a newer /// xleak will fail on an older binary rather than degrade. -#[derive(Debug, Clone, Default, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct CustomTheme { - pub name: String, - pub inherits: Option, - - // Broad-brush aliases - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub foreground: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub background: Option, - - // Cell type colors - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub string_fg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub number_fg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub bool_fg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub datetime_fg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub error_fg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub empty_fg: Option, - - // UI element colors - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub header_fg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub header_bg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub current_cell_fg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub current_cell_bg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub current_row_bg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub current_col_fg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub alternating_row_bg: Option, - - // Search colors - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub search_match_fg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub search_match_bg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub current_search_fg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub current_search_bg: Option, - - // Border and status bar - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub border_fg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub status_bar_fg: Option, - #[serde(default, deserialize_with = "deserialize_opt_color")] - pub status_bar_bg: Option, +/// +/// The color fields are generated by `color_field_table!` in `tui/theme.rs` — +/// the same table that drives `apply_custom_fields` and `uses_rgb`, so adding +/// a field in one place can't silently miss the others. +macro_rules! define_custom_theme { + ( $( [$field:ident, $kind:ident, $alias:ident] )* ) => { + #[derive(Debug, Clone, Default, Deserialize)] + #[serde(deny_unknown_fields)] + pub struct CustomTheme { + pub name: String, + pub inherits: Option, + + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub foreground: Option, + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub background: Option, + + $( + #[serde(default, deserialize_with = "deserialize_opt_color")] + pub $field: Option, + )* + } + }; } +crate::tui::color_field_table!(define_custom_theme); /// Parse a config color: either `#RRGGBB` or one of the 16 ANSI color names. /// diff --git a/src/tui/mod.rs b/src/tui/mod.rs index f7aa595..620d3be 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -5,6 +5,7 @@ mod state; mod theme; pub use event::run_tui; +pub(crate) use theme::color_field_table; pub use theme::resolve_themes_from_config; pub use theme::truecolor_warning; diff --git a/src/tui/theme.rs b/src/tui/theme.rs index d6df3d9..cf5ca11 100644 --- a/src/tui/theme.rs +++ b/src/tui/theme.rs @@ -3,6 +3,43 @@ use crate::workbook::CellValue; use anyhow::Result; use ratatui::style::Color; +/// Single source of truth for the 20 customizable color fields. +/// +/// Each row is `[field_name, kind, alias]` where: +/// kind: `plain` = `Color` in ColorScheme, `opt` = `Option` +/// alias: `fg` = set by the `foreground` alias, `bg` = by `background`, `none` = excluded +/// +/// This table drives `CustomTheme` (config.rs), `apply_custom_fields`, and +/// `uses_rgb` so that adding a `ColorScheme` field in one place can't silently +/// miss the others. +macro_rules! color_field_table { + ($mac:ident) => { + $mac! { + [string_fg, plain, fg] + [number_fg, plain, fg] + [bool_fg, plain, fg] + [datetime_fg, plain, fg] + [error_fg, plain, fg] + [empty_fg, plain, fg] + [header_fg, plain, fg] + [header_bg, opt, bg] + [current_cell_fg, plain, none] + [current_cell_bg, plain, none] + [current_row_bg, plain, none] + [current_col_fg, plain, none] + [alternating_row_bg, opt, bg] + [search_match_fg, plain, none] + [search_match_bg, plain, none] + [current_search_fg, plain, none] + [current_search_bg, plain, none] + [border_fg, plain, fg] + [status_bar_fg, plain, fg] + [status_bar_bg, opt, bg] + } + }; +} +pub(crate) use color_field_table; + /// A color scheme with the name users refer to it by, in config and in the /// status bar. Built-ins and `[[theme.custom]]` entries are the same shape, so /// theme cycling doesn't care where a theme came from. @@ -169,65 +206,30 @@ fn resolve_base(themes: &[NamedTheme], custom: &CustomTheme) -> Result ColorScheme { - if let Some(fg) = custom.foreground { - colors.string_fg = fg; - colors.number_fg = fg; - colors.bool_fg = fg; - colors.datetime_fg = fg; - colors.error_fg = fg; - colors.empty_fg = fg; - colors.header_fg = fg; - colors.border_fg = fg; - colors.status_bar_fg = fg; - } - if let Some(bg) = custom.background { - colors.header_bg = Some(bg); - colors.alternating_row_bg = Some(bg); - colors.status_bar_bg = Some(bg); - } - - macro_rules! apply { - ($field:ident) => { - if let Some(c) = custom.$field { - colors.$field = c; + macro_rules! impl_apply { + ( $( [$field:ident, $kind:ident, $alias:ident] )* ) => { + // Broad-brush aliases first. + if let Some(fg) = custom.foreground { + $( impl_apply!(@fg colors, $field, fg, $alias); )* } - }; - } - macro_rules! apply_opt { - ($field:ident) => { - if let Some(c) = custom.$field { - colors.$field = Some(c); + if let Some(bg) = custom.background { + $( impl_apply!(@bg colors, $field, bg, $alias); )* } + // Per-field overrides on top. + $( impl_apply!(@field colors, custom, $field, $kind); )* }; - } - - apply!(string_fg); - apply!(number_fg); - apply!(bool_fg); - apply!(datetime_fg); - apply!(error_fg); - apply!(empty_fg); - apply!(header_fg); - apply_opt!(header_bg); - apply!(current_cell_fg); - apply!(current_cell_bg); - apply!(current_row_bg); - apply!(current_col_fg); - apply_opt!(alternating_row_bg); - apply!(search_match_fg); - apply!(search_match_bg); - apply!(current_search_fg); - apply!(current_search_bg); - apply!(border_fg); - apply!(status_bar_fg); - apply_opt!(status_bar_bg); - + (@fg $colors:ident, $field:ident, $val:ident, fg) => { $colors.$field = $val; }; + (@fg $colors:ident, $field:ident, $val:ident, $t:ident) => {}; + (@bg $colors:ident, $field:ident, $val:ident, bg) => { $colors.$field = Some($val); }; + (@bg $colors:ident, $field:ident, $val:ident, $t:ident) => {}; + (@field $c:ident, $cu:ident, $f:ident, plain) => { if let Some(v) = $cu.$f { $c.$f = v; } }; + (@field $c:ident, $cu:ident, $f:ident, opt) => { if let Some(v) = $cu.$f { $c.$f = Some(v); } }; + } + color_field_table!(impl_apply); colors } @@ -497,28 +499,14 @@ impl ColorScheme { /// Whether any field uses `Color::Rgb`, which requires 24-bit color support. pub fn uses_rgb(&self) -> bool { - let all = [ - self.string_fg, - self.number_fg, - self.bool_fg, - self.datetime_fg, - self.error_fg, - self.empty_fg, - self.header_fg, - self.current_cell_fg, - self.current_cell_bg, - self.current_row_bg, - self.current_col_fg, - self.search_match_fg, - self.search_match_bg, - self.current_search_fg, - self.current_search_bg, - self.border_fg, - self.status_bar_fg, - ]; - let opts = [self.header_bg, self.alternating_row_bg, self.status_bar_bg]; - all.iter().any(|c| matches!(c, Color::Rgb(..))) - || opts.iter().any(|o| matches!(o, Some(Color::Rgb(..)))) + macro_rules! impl_uses_rgb { + ( $( [$field:ident, $kind:ident, $alias:ident] )* ) => { + $( impl_uses_rgb!(@check self, $field, $kind) || )* false + }; + (@check $s:ident, $f:ident, plain) => { matches!($s.$f, Color::Rgb(..)) }; + (@check $s:ident, $f:ident, opt) => { matches!($s.$f, Some(Color::Rgb(..))) }; + } + color_field_table!(impl_uses_rgb) } /// Get foreground color for a cell based on its value type From 8f1a56698e7d776fe51d31b9571a085ac8553c0e Mon Sep 17 00:00:00 2001 From: AlexanderNZ Date: Wed, 12 Aug 2026 19:31:43 +1200 Subject: [PATCH 7/8] docs: softens truecolour warning and rediretion to #48 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e84bab..a1c8f69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Custom color themes via `[[theme.custom]]` in config.toml with `#RRGGBB` hex and named color support, `inherits` for theme inheritance, and `foreground`/`background` broad-brush aliases ([#42](https://github.com/bgreenwell/xleak/issues/42)) - `--theme ` CLI flag to select a theme at launch, overriding the configured default -- Truecolor warning on stderr when a custom theme uses RGB colors and `COLORTERM` doesn't advertise truecolor support ([#48](https://github.com/bgreenwell/xleak/issues/48)) +- Truecolor warning on stderr when launching the interactive TUI with a custom theme that uses RGB colors and `COLORTERM` doesn't advertise truecolor support. Built-in themes are not checked; graceful fallback for all themes is tracked in [#48](https://github.com/bgreenwell/xleak/issues/48) - OSC 52 clipboard support: `c`/`C` now copy via OSC 52 (works over SSH) in addition to the system clipboard - CSV/TSV support: read and interactively view `.csv`/`.tsv` files as a single sheet (behind the default-on `csv` feature) - `--csv-delimiter` option to override the inferred CSV/TSV field delimiter From 107ac9e5a0320db57279ace5cc271fb286dffa60 Mon Sep 17 00:00:00 2001 From: AlexanderNZ Date: Wed, 12 Aug 2026 19:32:33 +1200 Subject: [PATCH 8/8] fix: gates terminal-color warning on cli.interactive --- src/main.rs | 9 +++++---- tests/integration.rs | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/main.rs b/src/main.rs index fdf6f3d..7e37f1a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -182,12 +182,13 @@ fn main() -> Result<()> { for warning in &warnings { eprintln!("Warning: {warning}"); } - if let Some(w) = tui::truecolor_warning(&themes, std::env::var("COLORTERM").ok().as_deref()) { - eprintln!("Warning: {w}"); - } - // Display, export, or run TUI if cli.interactive { + // Warn only in interactive mode; non-interactive output never uses theme colors + if let Some(w) = tui::truecolor_warning(&themes, std::env::var("COLORTERM").ok().as_deref()) + { + eprintln!("Warning: {w}"); + } // Interactive TUI mode - pass the workbook so it can switch sheets tui::run_tui(wb, &sheet_name, &config, themes, &tui_options)?; } else { diff --git a/tests/integration.rs b/tests/integration.rs index 4b0ad45..ad4c449 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -682,3 +682,41 @@ fn test_theme_valid_with_export_succeeds() { String::from_utf8_lossy(&output.stderr) ); } + +#[test] +fn test_export_mode_suppresses_truecolor_warning() { + let tmpdir = std::env::temp_dir(); + let csv_path = tmpdir.join("xleak_truecolor_gate.csv"); + std::fs::write(&csv_path, "Name,Age\nAlice,30\n").unwrap(); + let config_path = tmpdir.join("xleak_truecolor_gate_config.toml"); + std::fs::write( + &config_path, + "[theme]\ndefault = \"rgbtest\"\n\n[[theme.custom]]\nname = \"rgbtest\"\ninherits = \"Nord\"\nstring_fg = \"#010203\"\n", + ) + .unwrap(); + + let mut cmd = Command::new(env!("CARGO_BIN_EXE_xleak")); + cmd.args([ + csv_path.to_str().unwrap(), + "--export", + "csv", + "--config", + config_path.to_str().unwrap(), + ]); + cmd.env_remove("COLORTERM"); + let output = cmd.output().expect("Failed to execute xleak"); + + assert!( + output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !stderr.contains("uses RGB colors"), + "truecolor warning should not fire in non-interactive export mode, got:\n{stderr}" + ); + + let _ = std::fs::remove_file(&csv_path); + let _ = std::fs::remove_file(&config_path); +}