From ba86538a347866cecd3fee94a35d6ca242017e5e Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 21 Sep 2026 13:34:36 -0700 Subject: [PATCH 1/4] feat(theme): adapt the painted backgrounds to a light terminal The three indexed backgrounds - the focused block and tree row, the help bar, the keyboard cursor - and the reversed selection were absolute, so a light terminal drew its own dark text on them and lost it. Collect every background we paint ourselves into one palette with a light variant, ask the terminal for its background colour once before the screen is taken, and let [ui] theme or PLANNOTATOR_TUI_THEME settle it outright. The dark palette is the shipped one, value for value. refs #58 --- Cargo.lock | 33 +++ crates/plannotator-tui/Cargo.toml | 3 + crates/plannotator-tui/src/app/draw.rs | 23 +- crates/plannotator-tui/src/app/header.rs | 8 +- crates/plannotator-tui/src/app/menu.rs | 3 +- crates/plannotator-tui/src/app/pick.rs | 3 +- crates/plannotator-tui/src/cli.rs | 17 +- crates/plannotator-tui/src/config.rs | 31 +++ crates/plannotator-tui/src/main.rs | 1 + crates/plannotator-tui/src/theme.rs | 303 +++++++++++++++++++++++ 10 files changed, 402 insertions(+), 23 deletions(-) create mode 100644 crates/plannotator-tui/src/theme.rs diff --git a/Cargo.lock b/Cargo.lock index 82f0179..97a1303 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1304,6 +1304,7 @@ dependencies = [ "serde", "serde_json", "similar", + "terminal-colorsaurus", "time", "toml", "tui-markdown", @@ -1923,6 +1924,32 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "terminal-colorsaurus" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a46bb5364467da040298c573c8a95dbf9a512efc039630409a03126e3703e90" +dependencies = [ + "cfg-if", + "libc", + "memchr", + "mio", + "terminal-trx", + "windows-sys", + "xterm-color", +] + +[[package]] +name = "terminal-trx" +version = "0.2.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b3f27d9a8a177e57545481faec87acb45c6e854ed1e5a3658ad186c106f38ed" +dependencies = [ + "cfg-if", + "libc", + "windows-sys", +] + [[package]] name = "terminfo" version = "0.9.0" @@ -2446,6 +2473,12 @@ version = "0.57.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1ebf944e87a7c253233ad6766e082e3cd714b5d03812acc24c318f549614536e" +[[package]] +name = "xterm-color" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7008a9d8ba97a7e47d9b2df63fcdb8dade303010c5a7cd5bf2469d4da6eba673" + [[package]] name = "yansi" version = "1.0.1" diff --git a/crates/plannotator-tui/Cargo.toml b/crates/plannotator-tui/Cargo.toml index 0d9d661..00f7268 100644 --- a/crates/plannotator-tui/Cargo.toml +++ b/crates/plannotator-tui/Cargo.toml @@ -27,6 +27,9 @@ pulldown-cmark = { version = "0.13.4", default-features = false } ratatui = "0.30" # highlight-code pulls syntect + a C oniguruma build; plain code blocks keep the build pure Rust. tui-markdown = { version = "0.3.9", default-features = false } +# OSC 11 by hand is forty lines and a dozen terminal quirks; this reads the reply off its own +# tty handle, so it cannot race the event loop's reader. Three small crates, one author. +terminal-colorsaurus = "1.0.3" [dev-dependencies] rusqlite = { version = "0.31.0", features = ["bundled"] } diff --git a/crates/plannotator-tui/src/app/draw.rs b/crates/plannotator-tui/src/app/draw.rs index 9a44667..96e3528 100644 --- a/crates/plannotator-tui/src/app/draw.rs +++ b/crates/plannotator-tui/src/app/draw.rs @@ -11,6 +11,7 @@ use ratatui::widgets::{Block, BorderType, Borders, Clear, Paragraph}; use unicode_width::UnicodeWidthStr; use super::{App, Focus, GUTTER, Geometry, Mode, TOOLBAR, glyph, label}; +use crate::theme::palette; use crate::wrap::wrap_line; const RAIL_WIDTH: u16 = 36; @@ -22,12 +23,6 @@ const TREE_WIDTH: u16 = 28; pub(super) const TREE_MIN_TOTAL_WIDTH: u16 = 120; const COMPOSE_WIDTH: u16 = 48; -pub(crate) const COMMENT_BG: Color = Color::Indexed(58); -pub(crate) const APPROVE_BG: Color = Color::Indexed(22); -const BLOCK_BG: Color = Color::Indexed(236); -const TOOLBAR_BG: Color = Color::Indexed(238); -const CURSOR_BG: Color = Color::Indexed(240); - fn accent(kind: Kind) -> Color { match kind { Kind::Comment => Color::Yellow, @@ -129,7 +124,7 @@ impl App { if open_path == Some(row.path.as_path()) { style = style.bold().fg(Color::Cyan); } - let row_bg = (focused && i == self.tree_cursor).then_some(BLOCK_BG); + let row_bg = (focused && i == self.tree_cursor).then(|| palette().block_bg); if let Some(bg) = row_bg { style = style.bg(bg); } @@ -165,7 +160,7 @@ impl App { if block == self.selected && !text_selection_active && self.pending.is_none() && doc_focused { buf.set_style( Rect { x: doc.x, y: screen_y, width: doc.width, height: 1 }, - Style::new().bg(BLOCK_BG), + Style::new().bg(palette().block_bg), ); } @@ -180,8 +175,8 @@ impl App { let Some(kind) = kind else { continue }; row_has_annotation = true; let style = match kind { - Kind::Comment => Style::new().bg(COMMENT_BG), - Kind::LooksGood => Style::new().bg(APPROVE_BG), + Kind::Comment => Style::new().bg(palette().comment_bg), + Kind::LooksGood => Style::new().bg(palette().approve_bg), Kind::Delete => { Style::new().fg(Color::Red).add_modifier(Modifier::CROSSED_OUT | Modifier::DIM) } @@ -194,7 +189,7 @@ impl App { let end = cols.end.min(usize::from(doc.width)) as u16; if end > start { let rect = Rect { x: doc.x + start, y: screen_y, width: end - start, height: 1 }; - buf.set_style(rect, Style::new().add_modifier(Modifier::REVERSED)); + buf.set_style(rect, palette().selection); } } @@ -204,7 +199,7 @@ impl App { && row_index == self.cursor.0 { let x = doc.x + (self.cursor.1.min(usize::from(doc.width).saturating_sub(1))) as u16; - buf.set_style(Rect { x, y: screen_y, width: 1, height: 1 }, Style::new().bg(CURSOR_BG)); + buf.set_style(Rect { x, y: screen_y, width: 1, height: 1 }, palette().cursor); } let marker = match (block == self.selected, row_has_annotation) { @@ -244,12 +239,12 @@ impl App { let Some(rect) = self.float_origin(1, width) else { return }; frame.render_widget(Clear, rect); let buf = frame.buffer_mut(); - buf.set_style(rect, Style::new().bg(TOOLBAR_BG)); + buf.set_style(rect, Style::new().bg(palette().toolbar_bg)); let mut x = rect.x + 1; let mut spans = [0..0, 0..0, 0..0]; for ((label, item), span) in labels.iter().zip(TOOLBAR.iter()).zip(spans.iter_mut()) { let w = label.width() as u16; - let style = Style::new().fg(accent(item.3)).bg(TOOLBAR_BG).bold(); + let style = Style::new().fg(accent(item.3)).bg(palette().toolbar_bg).bold(); buf.set_span(x, rect.y, &Span::styled(label.as_str(), style), w); *span = x..x + w; x += w; diff --git a/crates/plannotator-tui/src/app/header.rs b/crates/plannotator-tui/src/app/header.rs index b791029..e8322e2 100644 --- a/crates/plannotator-tui/src/app/header.rs +++ b/crates/plannotator-tui/src/app/header.rs @@ -9,9 +9,11 @@ use unicode_width::UnicodeWidthStr; use super::App; use super::send::SendState; +use crate::theme::palette; +// These three set their own foreground as well as their background, so they read the same +// on a light terminal as on a dark one; only the idle button borrows the theme's grey. const SEND_BG: Color = Color::Indexed(30); -const IDLE_BG: Color = Color::Indexed(238); const SENT_BG: Color = Color::Indexed(22); const BLOCKED_BG: Color = Color::Indexed(58); @@ -70,7 +72,7 @@ impl App { } Button::Review => { self.geometry.review_button = Some(rect); - Style::new().fg(Color::Cyan).bg(IDLE_BG) + Style::new().fg(Color::Cyan).bg(palette().toolbar_bg) } }; frame.buffer_mut().set_span(rect.x, rect.y, &Span::styled(label, style), rect.width); @@ -81,7 +83,7 @@ impl App { fn button_style(&self) -> Style { match &self.send_state { SendState::Ready if self.send_count() == 0 => { - Style::new().fg(Color::Gray).bg(IDLE_BG).add_modifier(Modifier::DIM) + Style::new().fg(palette().idle_fg).bg(palette().toolbar_bg).add_modifier(Modifier::DIM) } SendState::Ready => Style::new().fg(Color::Black).bg(SEND_BG).bold(), SendState::Sent => Style::new().fg(Color::Black).bg(SENT_BG).bold(), diff --git a/crates/plannotator-tui/src/app/menu.rs b/crates/plannotator-tui/src/app/menu.rs index 3ad0fec..0fba3b6 100644 --- a/crates/plannotator-tui/src/app/menu.rs +++ b/crates/plannotator-tui/src/app/menu.rs @@ -11,6 +11,7 @@ use ratatui::widgets::{Block, BorderType, Borders, Clear, Paragraph}; use unicode_width::UnicodeWidthStr as _; use super::{App, Mode}; +use crate::theme::palette; #[cfg(test)] mod tests; @@ -190,7 +191,7 @@ impl App { let style = if !self.action_applies(action) { Style::new().dim() } else if index == self.menu_cursor { - Style::new().reversed() + palette().selection } else { Style::new() }; diff --git a/crates/plannotator-tui/src/app/pick.rs b/crates/plannotator-tui/src/app/pick.rs index 41586d8..8380f10 100644 --- a/crates/plannotator-tui/src/app/pick.rs +++ b/crates/plannotator-tui/src/app/pick.rs @@ -16,6 +16,7 @@ use unicode_width::UnicodeWidthStr as _; use super::{App, Mode, Open}; use crate::last::message_source; +use crate::theme::palette; const PICK_MAX_WIDTH: u16 = 90; @@ -160,7 +161,7 @@ impl App { pick_rows.push((row, index)); let text = fit(&pick_label(message, self.clock_offset), usize::from(inner.width).saturating_sub(1)); - let style = if index == self.pick_cursor { Style::new().reversed() } else { Style::new() }; + let style = if index == self.pick_cursor { palette().selection } else { Style::new() }; Line::from(Span::styled(format!(" {text}"), style)) }) .collect(); diff --git a/crates/plannotator-tui/src/cli.rs b/crates/plannotator-tui/src/cli.rs index 8f2fa47..beff4f0 100644 --- a/crates/plannotator-tui/src/cli.rs +++ b/crates/plannotator-tui/src/cli.rs @@ -139,7 +139,9 @@ pub(crate) fn run(args: &[String]) -> Result<()> { fn show_config() -> Result<()> { let home = std::env::home_dir().unwrap_or_else(|| PathBuf::from("/")); let path = crate::config::config_path(|k| std::env::var(k).ok(), &home); - let config = Config::load_from(&path)?; + let mut config = Config::load_from(&path)?; + // `PLANNOTATOR_TUI_THEME` overrides the file, so the file's value is not the effective one. + config.ui.theme = crate::theme::effective_setting(|key| std::env::var(key).ok(), config.ui.theme)?; let state = if path.is_file() { "" } else { " (not present; defaults)" }; println!("# {}{state}", path.display()); print!("{}", config.to_toml()?); @@ -257,6 +259,13 @@ fn interactive(path: &PathBuf) -> Result<()> { /// Own the terminal for one app: `build` gets the document width the screen allows. pub(crate) fn run_ui(build: impl FnOnce(usize) -> Result) -> Result<()> { + // Settle the palette before the screen is ours: the background-colour query talks to + // the terminal directly, and it must not race the alternate screen or the event loop. + crate::theme::install(crate::theme::resolve( + |key| std::env::var(key).ok(), + Config::load()?.ui.theme, + crate::theme::detect, + )?); let mut terminal = ratatui::init(); execute!(stdout(), EnableMouseCapture)?; let _ = execute!(stdout(), EnableBracketedPaste); @@ -355,7 +364,7 @@ fn snapshot( menu: bool, ) -> Result<()> { use ratatui::backend::TestBackend; - use ratatui::style::{Color, Modifier}; + use ratatui::style::Modifier; let mut terminal = ratatui::Terminal::new(TestBackend::new(cols, rows))?; let mut app = open_app(path, doc_width(cols), false)?; terminal.draw(|frame| app.draw(frame))?; @@ -380,9 +389,9 @@ fn snapshot( '%' } else if style.add_modifier.contains(Modifier::CROSSED_OUT) { '-' - } else if style.bg == Some(Color::Indexed(22)) { + } else if style.bg == Some(crate::theme::palette().approve_bg) { '+' - } else if style.bg == Some(Color::Indexed(58)) { + } else if style.bg == Some(crate::theme::palette().comment_bg) { '#' } else { ' ' diff --git a/crates/plannotator-tui/src/config.rs b/crates/plannotator-tui/src/config.rs index 030b3c4..999d7d4 100644 --- a/crates/plannotator-tui/src/config.rs +++ b/crates/plannotator-tui/src/config.rs @@ -11,10 +11,22 @@ use std::str::FromStr; use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; +use crate::theme::ThemeSetting; + #[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize)] #[serde(deny_unknown_fields, default)] pub(crate) struct Config { pub(crate) herdr: HerdrConfig, + pub(crate) ui: UiConfig, +} + +/// How the app looks. One key so far; the colours themselves are not configurable yet. +#[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize)] +#[serde(deny_unknown_fields, default)] +pub(crate) struct UiConfig { + /// `auto` asks the terminal for its background colour; `light` and `dark` skip the + /// question. `PLANNOTATOR_TUI_THEME` overrides whatever is written here. + pub(crate) theme: ThemeSetting, } /// How plannotator-tui opens inside Herdr. @@ -213,6 +225,25 @@ mod tests { ); } + #[test] + fn the_theme_defaults_to_asking_the_terminal() { + assert_eq!(Config::default().ui.theme, ThemeSetting::Auto); + assert_eq!(Config::parse("").expect("parses").ui.theme, ThemeSetting::Auto); + } + + #[test] + fn a_configured_theme_is_read_and_leaves_the_rest_alone() { + let config = Config::parse("[ui]\ntheme = \"light\"\n").expect("parses"); + assert_eq!(config.ui.theme, ThemeSetting::Light); + assert_eq!(config.herdr.placement, Placement::Overlay); + } + + #[test] + fn an_unknown_theme_error_names_the_value() { + let err = Config::parse("[ui]\ntheme = \"solarized\"\n").expect_err("rejected"); + assert!(err.to_string().contains("solarized"), "{err}"); + } + #[test] fn roundtrips_through_toml() { let text = Config::default().to_toml().expect("serializes"); diff --git a/crates/plannotator-tui/src/main.rs b/crates/plannotator-tui/src/main.rs index 4a50811..b08126f 100644 --- a/crates/plannotator-tui/src/main.rs +++ b/crates/plannotator-tui/src/main.rs @@ -13,6 +13,7 @@ mod last; mod layout; mod srcmap; mod store; +mod theme; mod tree; mod workspace_paths; mod wrap; diff --git a/crates/plannotator-tui/src/theme.rs b/crates/plannotator-tui/src/theme.rs new file mode 100644 index 0000000..e5a50fd --- /dev/null +++ b/crates/plannotator-tui/src/theme.rs @@ -0,0 +1,303 @@ +//! Light and dark terminals: the palette of roles that carry an absolute colour. +//! +//! Almost everything on screen uses the terminal's own colours, so only the backgrounds we +//! paint ourselves need a light variant. Which palette is in force is decided once, before +//! the screen is taken over (`install`), and read from everywhere by [`palette`]. Nothing +//! installs a palette in tests or in headless runs, so those keep the dark one. +//! +//! Precedence: `PLANNOTATOR_TUI_THEME` → `[ui] theme` in the config file → asking the +//! terminal → dark. + +use std::fmt; +use std::io::IsTerminal as _; +use std::str::FromStr; +use std::sync::OnceLock; +use std::time::Duration; + +use anyhow::Result; +use ratatui::style::{Color, Modifier, Style}; +use serde::{Deserialize, Serialize}; + +/// How long the terminal gets to answer the background-colour query. +/// +/// A terminal that does not implement the query still answers the `DA1` sent behind it, and +/// is recognised as unsupported at round-trip speed; this only bounds the wait on something +/// that answers nothing at all, which is a pty with no emulator behind it rather than a +/// terminal anybody is looking at. +const DETECT_TIMEOUT: Duration = Duration::from_millis(100); + +/// What the user asked for, as written in `[ui] theme` or `PLANNOTATOR_TUI_THEME`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub(crate) enum ThemeSetting { + /// Ask the terminal, and use the dark palette when it does not answer. + #[default] + Auto, + Light, + Dark, +} + +impl ThemeSetting { + pub(crate) fn as_str(self) -> &'static str { + match self { + Self::Auto => "auto", + Self::Light => "light", + Self::Dark => "dark", + } + } +} + +impl fmt::Display for ThemeSetting { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.as_str()) + } +} + +impl FromStr for ThemeSetting { + type Err = anyhow::Error; + + fn from_str(s: &str) -> Result { + match s { + "auto" => Ok(Self::Auto), + "light" => Ok(Self::Light), + "dark" => Ok(Self::Dark), + other => anyhow::bail!("unknown theme {other:?}; expected auto, light or dark"), + } + } +} + +/// The palette actually in force, once `auto` has been settled. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum Theme { + Dark, + Light, +} + +impl Theme { + fn palette(self) -> &'static Palette { + match self { + Self::Dark => &DARK, + Self::Light => &LIGHT, + } + } +} + +/// Every colour we paint that is not the terminal's own. +/// +/// A role is here because it is painted *under* text whose colour belongs to the terminal: +/// a fixed dark background hides dark text on a light terminal, and the other way round. +/// Styles that set both a foreground and a background read the same either way and stay +/// where they are used. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct Palette { + /// The focused block's bar, and the focused row of the file tree. + pub(crate) block_bg: Color, + /// The floating toolbar and the idle header buttons. + pub(crate) toolbar_bg: Color, + /// The foreground of a header button with nothing to do. + pub(crate) idle_fg: Color, + /// Text carrying a comment. + pub(crate) comment_bg: Color, + /// Text marked as looking good. + pub(crate) approve_bg: Color, + /// The keyboard cursor cell. + pub(crate) cursor: Style, + /// A character selection, a picker row, a review-menu row. + pub(crate) selection: Style, +} + +/// The palette this app has always had. Dark terminals must see no change at all. +pub(crate) static DARK: Palette = Palette { + block_bg: Color::Indexed(236), + toolbar_bg: Color::Indexed(238), + idle_fg: Color::Gray, + comment_bg: Color::Indexed(58), + approve_bg: Color::Indexed(22), + cursor: Style::new().bg(Color::Indexed(240)), + selection: Style::new().add_modifier(Modifier::REVERSED), +}; + +/// The same roles, light. The greys mirror the dark ones about the middle of the ramp, so +/// the focused block still sits just off the page and the cursor is a shade firmer. The +/// annotation tints are the pale ends of the same hues. `REVERSED` becomes a real +/// background: reversing dark-on-light gives a black bar that swallows the tints under it. +pub(crate) static LIGHT: Palette = Palette { + block_bg: Color::Indexed(254), + toolbar_bg: Color::Indexed(252), + idle_fg: Color::DarkGray, + comment_bg: Color::Indexed(229), + approve_bg: Color::Indexed(194), + // A dark cell with its own light foreground, so the glyph under the cursor stays legible. + cursor: Style::new().bg(Color::Indexed(238)).fg(Color::White), + selection: Style::new().bg(Color::Indexed(153)), +}; + +static PALETTE: OnceLock<&'static Palette> = OnceLock::new(); + +/// The palette in force. Dark until [`install`] says otherwise, which is what tests, +/// `--snapshot`, `--export` and every other headless run get. +pub(crate) fn palette() -> &'static Palette { + PALETTE.get().copied().unwrap_or(&DARK) +} + +/// Fix the palette for this process. The first call wins; later ones are ignored. +pub(crate) fn install(theme: Theme) { + let _ = PALETTE.set(theme.palette()); +} + +/// Ask the terminal whether its background is light or dark. +/// +/// The query opens the tty itself rather than reading our stdin, so it cannot race the +/// event loop's reader or leave a half-parsed escape sequence behind it. It does hold the +/// tty in raw mode for one round trip, and anything typed into that window is read and +/// discarded with the reply; `theme = "light"`, `theme = "dark"` or `PLANNOTATOR_TUI_THEME` +/// skip the query outright for anyone that window bothers. +/// +/// `None` means the terminal did not answer, answered something unreadable, or there is no +/// terminal to ask - all of which keep the dark palette. +pub(crate) fn detect() -> Option { + if !std::io::stdout().is_terminal() { + return None; + } + #[allow( + clippy::field_reassign_with_default, + reason = "QueryOptions is #[non_exhaustive]; struct-update syntax is not available to us" + )] + let options = { + let mut options = terminal_colorsaurus::QueryOptions::default(); + options.timeout = DETECT_TIMEOUT; + options + }; + match terminal_colorsaurus::theme_mode(options) { + Ok(terminal_colorsaurus::ThemeMode::Light) => Some(Theme::Light), + Ok(terminal_colorsaurus::ThemeMode::Dark) => Some(Theme::Dark), + Err(_) => None, + } +} + +/// `PLANNOTATOR_TUI_THEME`, when it is set to something. An unreadable value is an error +/// that names it, like a bad value in the config file. +fn setting_from_env(env: &impl Fn(&str) -> Option) -> Result> { + match env("PLANNOTATOR_TUI_THEME").filter(|value| !value.is_empty()) { + Some(value) => value.parse().map(Some), + None => Ok(None), + } +} + +/// What `plannotator-tui config` should print: the environment's answer if it has one, +/// else the file's. +pub(crate) fn effective_setting( + env: impl Fn(&str) -> Option, + configured: ThemeSetting, +) -> Result { + Ok(setting_from_env(&env)?.unwrap_or(configured)) +} + +/// Settle on a palette: environment, then config file, then the terminal, then dark. +pub(crate) fn resolve( + env: impl Fn(&str) -> Option, + configured: ThemeSetting, + detect: impl FnOnce() -> Option, +) -> Result { + Ok(match effective_setting(env, configured)? { + ThemeSetting::Light => Theme::Light, + ThemeSetting::Dark => Theme::Dark, + ThemeSetting::Auto => detect().unwrap_or(Theme::Dark), + }) +} + +#[cfg(test)] +#[allow(clippy::expect_used, reason = "tests assert by panicking")] +mod tests { + use super::*; + + fn env(vars: &'static [(&'static str, &'static str)]) -> impl Fn(&str) -> Option { + move |key: &str| vars.iter().find(|(name, _)| *name == key).map(|(_, v)| (*v).to_owned()) + } + + #[test] + fn the_environment_beats_the_config_file() { + let vars = env(&[("PLANNOTATOR_TUI_THEME", "light")]); + assert_eq!(resolve(vars, ThemeSetting::Dark, || None).expect("resolves"), Theme::Light); + } + + #[test] + fn the_config_file_beats_detection() { + let detect = || Some(Theme::Light); + assert_eq!(resolve(env(&[]), ThemeSetting::Dark, detect).expect("resolves"), Theme::Dark); + } + + #[test] + fn auto_in_the_environment_reopens_the_question_a_configured_theme_closed() { + let vars = env(&[("PLANNOTATOR_TUI_THEME", "auto")]); + let resolved = resolve(vars, ThemeSetting::Dark, || Some(Theme::Light)).expect("resolves"); + assert_eq!(resolved, Theme::Light); + } + + #[test] + fn auto_uses_the_terminals_answer_and_falls_back_to_dark_without_one() { + let auto = ThemeSetting::Auto; + assert_eq!(resolve(env(&[]), auto, || Some(Theme::Light)).expect("resolves"), Theme::Light); + assert_eq!(resolve(env(&[]), auto, || None).expect("resolves"), Theme::Dark); + } + + #[test] + fn an_empty_environment_variable_is_not_a_choice() { + let vars = env(&[("PLANNOTATOR_TUI_THEME", "")]); + assert_eq!(resolve(vars, ThemeSetting::Light, || None).expect("resolves"), Theme::Light); + } + + #[test] + fn an_unreadable_environment_variable_is_an_error_naming_the_value() { + let vars = env(&[("PLANNOTATOR_TUI_THEME", "solarized")]); + let err = resolve(vars, ThemeSetting::Auto, || None).expect_err("rejected"); + assert!(err.to_string().contains("solarized"), "{err}"); + } + + /// The dark palette is the one shipped before light terminals were supported; a change + /// here is a visible change for every existing user. + #[test] + fn the_dark_palette_is_unchanged() { + assert_eq!(DARK.block_bg, Color::Indexed(236)); + assert_eq!(DARK.toolbar_bg, Color::Indexed(238)); + assert_eq!(DARK.idle_fg, Color::Gray); + assert_eq!(DARK.comment_bg, Color::Indexed(58)); + assert_eq!(DARK.approve_bg, Color::Indexed(22)); + assert_eq!(DARK.cursor, Style::new().bg(Color::Indexed(240))); + assert_eq!(DARK.selection, Style::new().add_modifier(Modifier::REVERSED)); + } + + /// Nothing installs a palette in a test binary, so every existing snapshot and style + /// assertion keeps reading the dark one. + #[test] + fn the_palette_is_dark_until_something_installs_one() { + assert_eq!(*palette(), DARK); + } + + /// Both palettes must answer for every role; a light background painted on a light + /// terminal is the bug this exists to fix. + #[test] + fn no_light_role_reuses_its_dark_value() { + assert_ne!(LIGHT.block_bg, DARK.block_bg); + assert_ne!(LIGHT.toolbar_bg, DARK.toolbar_bg); + assert_ne!(LIGHT.comment_bg, DARK.comment_bg); + assert_ne!(LIGHT.approve_bg, DARK.approve_bg); + assert_ne!(LIGHT.cursor, DARK.cursor); + assert_ne!(LIGHT.selection, DARK.selection); + } + + /// `REVERSED` is what made a selection unreadable on a light terminal; the light + /// palette must not reach for it again. + #[test] + fn the_light_selection_is_a_background_not_a_reversal() { + assert!(!LIGHT.selection.add_modifier.contains(Modifier::REVERSED)); + assert!(LIGHT.selection.bg.is_some()); + } + + #[test] + fn a_theme_setting_roundtrips_through_its_own_name() { + for setting in [ThemeSetting::Auto, ThemeSetting::Light, ThemeSetting::Dark] { + assert_eq!(setting.as_str().parse::().expect("parses"), setting); + } + } +} From 4b0c940ee73c777322722ab0bd3b86426cdef139 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 21 Sep 2026 13:34:41 -0700 Subject: [PATCH 2/4] test(theme): check the effective theme through the binary The environment overriding the config file is what `plannotator-tui config` has to print, and only the real binary reads both. refs #58 --- crates/plannotator-tui/tests/theme.rs | 42 +++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 crates/plannotator-tui/tests/theme.rs diff --git a/crates/plannotator-tui/tests/theme.rs b/crates/plannotator-tui/tests/theme.rs new file mode 100644 index 0000000..fc566f9 --- /dev/null +++ b/crates/plannotator-tui/tests/theme.rs @@ -0,0 +1,42 @@ +//! `plannotator-tui config` reports the theme that would actually be used, through the +//! real binary: the environment overriding the file is the part a unit test cannot see. + +#![allow(clippy::expect_used, reason = "tests assert by panicking")] + +use std::process::Command; + +fn config_output(label: &str, theme_in_file: &str, theme_in_env: Option<&str>) -> String { + let dir = std::env::temp_dir().join(format!("plannotator-tui-theme-{label}-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("dir"); + let path = dir.join("config.toml"); + std::fs::write(&path, format!("[ui]\ntheme = \"{theme_in_file}\"\n")).expect("config"); + let mut command = Command::new(env!("CARGO_BIN_EXE_plannotator-tui")); + command.env("PLANNOTATOR_TUI_CONFIG", &path).arg("config"); + match theme_in_env { + Some(theme) => command.env("PLANNOTATOR_TUI_THEME", theme), + None => command.env_remove("PLANNOTATOR_TUI_THEME"), + }; + let out = command.output().expect("config runs"); + let text = String::from_utf8_lossy(&out.stdout).into_owned(); + std::fs::remove_dir_all(&dir).expect("cleanup"); + assert!(out.status.success(), "{text}{}", String::from_utf8_lossy(&out.stderr)); + text +} + +#[test] +fn the_printed_theme_is_the_files_when_the_environment_is_silent() { + assert!(config_output("file", "dark", None).contains("theme = \"dark\"")); +} + +#[test] +fn the_printed_theme_is_the_environments_when_it_speaks() { + assert!(config_output("env", "dark", Some("light")).contains("theme = \"light\"")); +} + +#[test] +fn an_unreadable_theme_in_the_environment_is_reported_rather_than_ignored() { + let mut command = Command::new(env!("CARGO_BIN_EXE_plannotator-tui")); + let out = command.env("PLANNOTATOR_TUI_THEME", "solarized").arg("config").output().expect("config runs"); + assert!(!out.status.success()); + assert!(String::from_utf8_lossy(&out.stderr).contains("solarized")); +} From a048a6a8844fb0904e778be418ef908e0c9e7274 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 21 Sep 2026 13:34:41 -0700 Subject: [PATCH 3/4] docs: document theme = auto|light|dark refs #58 --- README.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/README.md b/README.md index f36521a..c8cdf3f 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,23 @@ one; without the flag the picker comes first, as it always has. `plannotator-tui config` prints the file's path and the values in effect. The `herdr/` directory in this repo is the development manifest; users should install Herdr Annotate. +The same file chooses the theme: + +```toml +[ui] +theme = "auto" # auto (ask the terminal, default) | light | dark +``` + +On `auto` the viewer asks the terminal for its background colour once at startup and uses a +light palette when it finds one; a terminal that does not answer keeps the dark palette it +has always used. The question costs one round trip before the screen is drawn, and a key +pressed into that window is read along with the reply and lost, so set the theme outright if +you habitually type ahead. `light` and `dark` skip the +question, and `PLANNOTATOR_TUI_THEME=light|dark` does the same for one run — the variable +wins over the file, and `plannotator-tui config` prints whichever is in effect. Only the +backgrounds plannotator-tui paints itself change; the document keeps your terminal's own +colours either way. + Actions forwarded by Herdr Mirror default to a split beside the invoking remote pane. Mirror does not preserve overlay presentation, and Herdr 0.8.2 opens an overlay in its server's active tab, which can differ from the tab you are viewing. From 6629879dfbcf2ee41d2da3786f167dae0be01327 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 21 Sep 2026 13:41:01 -0700 Subject: [PATCH 4/4] fix(theme): short detection deadline, readable light tints, tolerant config Herdr answers the colour query for its panes but never the DA1 probe, so the query always ran to the deadline there; 30 ms is plenty for the answer and shrinks the window in which a keystroke can be lost. The light comment and approve tints were near-invisible against a light page. A config that fails to parse no longer stops the plain TUI from starting, as it never did before. refs #58 --- crates/plannotator-tui/src/cli.rs | 4 +++- crates/plannotator-tui/src/theme.rs | 15 ++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/crates/plannotator-tui/src/cli.rs b/crates/plannotator-tui/src/cli.rs index beff4f0..80a59fc 100644 --- a/crates/plannotator-tui/src/cli.rs +++ b/crates/plannotator-tui/src/cli.rs @@ -263,7 +263,9 @@ pub(crate) fn run_ui(build: impl FnOnce(usize) -> Result) -> Result<()> { // the terminal directly, and it must not race the alternate screen or the event loop. crate::theme::install(crate::theme::resolve( |key| std::env::var(key).ok(), - Config::load()?.ui.theme, + // A config that fails to parse never kept the plain TUI from starting; it still does + // not. `plannotator-tui config` is where the error is reported. + Config::load().map(|config| config.ui.theme).unwrap_or_default(), crate::theme::detect, )?); let mut terminal = ratatui::init(); diff --git a/crates/plannotator-tui/src/theme.rs b/crates/plannotator-tui/src/theme.rs index e5a50fd..377c7d0 100644 --- a/crates/plannotator-tui/src/theme.rs +++ b/crates/plannotator-tui/src/theme.rs @@ -20,11 +20,12 @@ use serde::{Deserialize, Serialize}; /// How long the terminal gets to answer the background-colour query. /// -/// A terminal that does not implement the query still answers the `DA1` sent behind it, and -/// is recognised as unsupported at round-trip speed; this only bounds the wait on something -/// that answers nothing at all, which is a pty with no emulator behind it rather than a -/// terminal anybody is looking at. -const DETECT_TIMEOUT: Duration = Duration::from_millis(100); +/// Most terminals answer the query itself, or the `DA1` sent behind it, within a few +/// milliseconds, so the answer arrives long before this. Herdr answers the colour query for +/// its panes but not `DA1`, so inside a pane the wait always runs to this deadline, and a key +/// pressed during it is lost: keep it short. A terminal that answers nothing at all is +/// treated as dark. +const DETECT_TIMEOUT: Duration = Duration::from_millis(30); /// What the user asked for, as written in `[ui] theme` or `PLANNOTATOR_TUI_THEME`. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] @@ -125,8 +126,8 @@ pub(crate) static LIGHT: Palette = Palette { block_bg: Color::Indexed(254), toolbar_bg: Color::Indexed(252), idle_fg: Color::DarkGray, - comment_bg: Color::Indexed(229), - approve_bg: Color::Indexed(194), + comment_bg: Color::Indexed(222), + approve_bg: Color::Indexed(157), // A dark cell with its own light foreground, so the glyph under the cursor stays legible. cursor: Style::new().bg(Color::Indexed(238)).fg(Color::White), selection: Style::new().bg(Color::Indexed(153)),