diff --git a/codex-rs/tui/src/render/highlight.rs b/codex-rs/tui/src/render/highlight.rs index 2fd0abbb9..b44fc0a74 100644 --- a/codex-rs/tui/src/render/highlight.rs +++ b/codex-rs/tui/src/render/highlight.rs @@ -31,6 +31,7 @@ use std::path::PathBuf; use std::sync::OnceLock; use std::sync::RwLock; use syntect::easy::HighlightLines; +use syntect::highlighting::Color as SyntectColor; use syntect::highlighting::FontStyle; use syntect::highlighting::Highlighter; use syntect::highlighting::Style as SyntectStyle; @@ -49,10 +50,23 @@ static THEME: OnceLock> = OnceLock::new(); static THEME_OVERRIDE: OnceLock> = OnceLock::new(); static CODEX_HOME: OnceLock> = OnceLock::new(); +// Syntect/bat encode ANSI palette semantics in alpha: +// `a=0` => indexed ANSI palette via RGB payload, `a=1` => terminal default. +const ANSI_ALPHA_INDEX: u8 = 0x00; +const ANSI_ALPHA_DEFAULT: u8 = 0x01; +const OPAQUE_ALPHA: u8 = 0xFF; + fn syntax_set() -> &'static SyntaxSet { SYNTAX_SET.get_or_init(two_face::syntax::extra_newlines) } +// NOTE: We intentionally do NOT emit a runtime diagnostic when an ANSI-family +// theme (ansi, base16, base16-256) lacks the expected alpha-channel marker +// encoding. If the upstream two_face/syntect theme format changes, the +// `ansi_themes_use_only_ansi_palette_colors` test will catch it at build +// time — long before it reaches users. A runtime warning would be +// unactionable noise since users can't fix upstream themes. + /// Set the user-configured syntax theme override and codex home path. /// /// Call this with the **final resolved config** (after onboarding, resume, and @@ -62,15 +76,13 @@ fn syntax_set() -> &'static SyntaxSet { /// Subsequent calls cannot change the persisted `OnceLock` values, but they /// still update the runtime theme immediately for live preview flows. /// -/// Returns a warning message when the configured theme name cannot be -/// resolved to a bundled theme or a custom `.tmTheme` file on disk. -/// The caller should surface this via `Config::startup_warnings` so it -/// appears as a `⚠` banner in the TUI. +/// Returns user-facing warnings for actionable configuration issues, such as +/// unknown/invalid theme names or duplicate override persistence. pub(crate) fn set_theme_override( name: Option, codex_home: Option, ) -> Option { - let mut warning = validate_theme_name(name.as_deref(), codex_home.as_deref()); + let warning = validate_theme_name(name.as_deref(), codex_home.as_deref()); let override_set_ok = THEME_OVERRIDE.set(name.clone()).is_ok(); let codex_home_set_ok = CODEX_HOME.set(codex_home.clone()).is_ok(); if THEME.get().is_some() { @@ -80,11 +92,10 @@ pub(crate) fn set_theme_override( )); } if !override_set_ok || !codex_home_set_ok { - let duplicate_msg = "Ignoring duplicate or late syntax theme override persistence; runtime theme was updated from the latest override, but persisted override config can only be initialized once."; - tracing::warn!("{duplicate_msg}"); - if warning.is_none() { - warning = Some(duplicate_msg.to_string()); - } + // This should never happen in practice — set_theme_override is only + // called once at startup. Keep as a debug breadcrumb in case a second + // call site is added in the future. + tracing::debug!("set_theme_override called more than once; OnceLock values unchanged"); } warning } @@ -109,15 +120,15 @@ pub(crate) fn validate_theme_name(name: Option<&str>, codex_home: Option<&Path>) return None; } return Some(format!( - "Syntax theme \"{name}\" was found at {custom_theme_path_display} \ - but could not be parsed. Falling back to auto-detection." + "Custom theme \"{name}\" at {custom_theme_path_display} could not \ + be loaded (invalid .tmTheme format). Falling back to the default theme." )); } } Some(format!( - "Unknown syntax theme \"{name}\", falling back to auto-detection. \ - Use a bundled name or place a .tmTheme file at \ - {custom_theme_path_display}" + "Theme \"{name}\" not found. Using the default theme. \ + To use a custom theme, place a .tmTheme file at \ + {custom_theme_path_display}." )) } @@ -189,7 +200,7 @@ pub(crate) fn adaptive_default_theme_name() -> &'static str { adaptive_default_theme_selection().1 } -/// Build the theme from current override/auto-detection settings. +/// Build the theme from current override/default-theme settings. /// Extracted from the old `theme()` init closure so it can be reused. fn resolve_theme_with_override(name: Option<&str>, codex_home: Option<&Path>) -> Theme { let ts = two_face::theme::extra(); @@ -206,13 +217,13 @@ fn resolve_theme_with_override(name: Option<&str>, codex_home: Option<&Path>) -> { return theme; } - tracing::warn!("unknown syntax theme \"{name}\", falling back to auto-detection"); + tracing::debug!("Theme \"{name}\" not recognized; using default theme"); } ts.get(adaptive_default_embedded_theme_name()).clone() } -/// Build the theme from current override/auto-detection settings. +/// Build the theme from current override/default-theme settings. /// Extracted from the old `theme()` init closure so it can be reused. fn build_default_theme() -> Theme { let name = THEME_OVERRIDE.get().and_then(|name| name.as_deref()); @@ -412,20 +423,71 @@ const BUILTIN_THEME_NAMES: &[&str] = &[ // -- Style conversion (syntect -> ratatui) ------------------------------------ +/// Map a low ANSI palette index (0–7) to ratatui's named color variants, +/// falling back to `Indexed(n)` for indices 8–255. +/// +/// Named variants are preferred over `Indexed(0)`…`Indexed(7)` because many +/// terminals apply bold/bright treatment differently for named vs indexed +/// colors, and ANSI themes expect the named behavior. +/// +/// `clippy::disallowed_methods` is explicitly allowed here because this helper +/// intentionally constructs `ratatui::style::Color::Indexed`. +#[allow(clippy::disallowed_methods)] +fn ansi_palette_color(index: u8) -> RtColor { + match index { + 0x00 => RtColor::Black, + 0x01 => RtColor::Red, + 0x02 => RtColor::Green, + 0x03 => RtColor::Yellow, + 0x04 => RtColor::Blue, + 0x05 => RtColor::Magenta, + 0x06 => RtColor::Cyan, + // ANSI code 37 is "white", represented as `Gray` in ratatui. + 0x07 => RtColor::Gray, + n => RtColor::Indexed(n), + } +} + +/// Decode a syntect foreground `Color` into a ratatui color, respecting the +/// alpha-channel encoding that bat's `ansi`, `base16`, and `base16-256` themes +/// use to signal ANSI palette semantics instead of true RGB. +/// +/// Returns `None` when the color signals "use the terminal's default +/// foreground", allowing the caller to omit the foreground attribute entirely. +/// +/// Passing a color from a standard RGB theme (alpha 0xFF) returns +/// `Some(Rgb(..))`, so this function is backward-compatible with non-ANSI +/// themes. Unexpected intermediate alpha values are treated as RGB. +/// +/// `clippy::disallowed_methods` is explicitly allowed here because this helper +/// intentionally constructs `ratatui::style::Color::Rgb`. +#[allow(clippy::disallowed_methods)] +fn convert_syntect_color(color: SyntectColor) -> Option { + match color.a { + // Bat-compatible encoding used by `ansi`, `base16`, and `base16-256`: + // alpha 0x00 means `r` stores an ANSI palette index, not RGB red. + ANSI_ALPHA_INDEX => Some(ansi_palette_color(color.r)), + // alpha 0x01 means "use terminal default foreground/background". + ANSI_ALPHA_DEFAULT => None, + OPAQUE_ALPHA => Some(RtColor::Rgb(color.r, color.g, color.b)), + // Non-ANSI alpha values appear in some bundled themes; treat as plain RGB. + _ => Some(RtColor::Rgb(color.r, color.g, color.b)), + } +} + /// Convert a syntect `Style` to a ratatui `Style`. /// -/// Syntax highlighting themes inherently produce RGB colors, so we allow -/// `Color::Rgb` here despite the project-wide preference for ANSI colors. -#[allow(clippy::disallowed_methods)] +/// Most themes produce RGB colors. The built-in `ansi`/`base16`/`base16-256` +/// themes encode ANSI palette semantics in the alpha channel, matching bat. fn convert_style(syn_style: SyntectStyle) -> Style { let mut rt_style = Style::default(); - // Map foreground color when visible. - let fg = syn_style.foreground; - if fg.a > 0 { - rt_style = rt_style.fg(RtColor::Rgb(fg.r, fg.g, fg.b)); + if let Some(fg) = convert_syntect_color(syn_style.foreground) { + rt_style = rt_style.fg(fg); } // Intentionally skip background to avoid overwriting terminal bg. + // If background support is added later, decode with `convert_syntect_color` + // to reuse the same alpha-marker semantics as foreground. if syn_style.font_style.contains(FontStyle::BOLD) { rt_style.add_modifier |= Modifier::BOLD; @@ -500,10 +562,16 @@ pub(crate) fn exceeds_highlight_limits(total_bytes: usize, total_lines: usize) - // -- Core highlighting -------------------------------------------------------- -/// Parse `code` using syntect for `lang` and return per-line styled spans. -/// Each inner Vec represents one source line. Returns None when the language -/// is not recognized or the input exceeds safety limits. -fn highlight_to_line_spans(code: &str, lang: &str) -> Option>>> { +/// Core highlighter that accepts an explicit theme reference. +/// +/// This keeps production behavior and test behavior on the same code path: +/// production callers pass the global theme lock, while tests can pass a +/// concrete theme without mutating process-global state. +fn highlight_to_line_spans_with_theme( + code: &str, + lang: &str, + theme: &Theme, +) -> Option>>> { // Empty input has nothing to highlight; fall back to the plain text path // which correctly produces a single empty Line. if code.is_empty() { @@ -518,11 +586,7 @@ fn highlight_to_line_spans(code: &str, lang: &str) -> Option theme_guard, - Err(poisoned) => poisoned.into_inner(), - }; - let mut h = HighlightLines::new(syntax, &theme_guard); + let mut h = HighlightLines::new(syntax, theme); let mut lines: Vec>> = Vec::new(); for line in LinesWithEndings::from(code) { @@ -546,6 +610,17 @@ fn highlight_to_line_spans(code: &str, lang: &str) -> Option Option>>> { + let theme_guard = match theme_lock().read() { + Ok(theme_guard) => theme_guard, + Err(poisoned) => poisoned.into_inner(), + }; + highlight_to_line_spans_with_theme(code, lang, &theme_guard) +} + // -- Public API --------------------------------------------------------------- /// Highlight code in any supported language, returning styled ratatui `Line`s. @@ -596,6 +671,7 @@ pub(crate) fn highlight_code_to_styled_spans( #[cfg(test)] mod tests { use super::*; + use insta::assert_snapshot; use pretty_assertions::assert_eq; use std::str::FromStr; use syntect::highlighting::Color as SyntectColor; @@ -673,6 +749,25 @@ mod tests { .join("\n") } + fn unique_foreground_colors_for_theme(theme_name: &str) -> Vec { + let theme = resolve_theme_by_name(theme_name, None) + .unwrap_or_else(|| panic!("expected built-in theme {theme_name} to resolve")); + let lines = highlight_to_line_spans_with_theme( + "fn main() { let answer = 42; println!(\"hello\"); }\n", + "rust", + &theme, + ) + .expect("expected highlighted spans"); + let mut colors: Vec = lines + .iter() + .flat_map(|line| line.iter().filter_map(|span| span.style.fg)) + .map(|fg| format!("{fg:?}")) + .collect(); + colors.sort(); + colors.dedup(); + colors + } + fn theme_item(scope: &str, background: Option<(u8, u8, u8)>) -> ThemeItem { ThemeItem { scope: ScopeSelectors::from_str(scope).expect("scope selector should parse"), @@ -791,7 +886,6 @@ mod tests { } #[test] - #[allow(clippy::disallowed_methods)] fn convert_style_suppresses_underline() { // Dracula (and other themes) set FontStyle::UNDERLINE on type scopes, // producing distracting underlines on type names in terminal output. @@ -807,7 +901,7 @@ mod tests { r: 0, g: 0, b: 0, - a: 0, + a: 0xFF, }, font_style: FontStyle::UNDERLINE, }; @@ -820,6 +914,138 @@ mod tests { ); } + #[test] + fn style_conversion_uses_ansi_named_color_when_alpha_is_zero_low_index() { + let syn = SyntectStyle { + foreground: syntect::highlighting::Color { + r: 0x02, + g: 0, + b: 0, + a: 0, + }, + background: syntect::highlighting::Color { + r: 0, + g: 0, + b: 0, + a: 0xFF, + }, + font_style: FontStyle::empty(), + }; + let rt = convert_style(syn); + assert_eq!(rt.fg, Some(RtColor::Green)); + } + + #[test] + fn style_conversion_uses_indexed_color_when_alpha_is_zero_high_index() { + let syn = SyntectStyle { + foreground: syntect::highlighting::Color { + r: 0x9a, + g: 0, + b: 0, + a: 0, + }, + background: syntect::highlighting::Color { + r: 0, + g: 0, + b: 0, + a: 0xFF, + }, + font_style: FontStyle::empty(), + }; + let rt = convert_style(syn); + assert!(matches!(rt.fg, Some(RtColor::Indexed(0x9a)))); + } + + #[test] + fn style_conversion_uses_terminal_default_when_alpha_is_one() { + let syn = SyntectStyle { + foreground: syntect::highlighting::Color { + r: 0, + g: 0, + b: 0, + a: 1, + }, + background: syntect::highlighting::Color { + r: 0, + g: 0, + b: 0, + a: 0xFF, + }, + font_style: FontStyle::empty(), + }; + let rt = convert_style(syn); + assert_eq!(rt.fg, None); + } + + #[test] + fn style_conversion_unexpected_alpha_falls_back_to_rgb() { + let syn = SyntectStyle { + foreground: syntect::highlighting::Color { + r: 10, + g: 20, + b: 30, + a: 0x80, + }, + background: syntect::highlighting::Color { + r: 0, + g: 0, + b: 0, + a: 0xFF, + }, + font_style: FontStyle::empty(), + }; + let rt = convert_style(syn); + assert!(matches!(rt.fg, Some(RtColor::Rgb(10, 20, 30)))); + } + + #[test] + fn ansi_palette_color_maps_ansi_white_to_gray() { + assert_eq!(ansi_palette_color(0x07), RtColor::Gray); + } + + #[test] + fn ansi_family_themes_use_terminal_palette_colors_not_rgb() { + for theme_name in ["ansi", "base16", "base16-256"] { + let theme = resolve_theme_by_name(theme_name, None) + .unwrap_or_else(|| panic!("expected built-in theme {theme_name} to resolve")); + let lines = highlight_to_line_spans_with_theme( + "fn main() { let answer = 42; println!(\"hello\"); }\n", + "rust", + &theme, + ) + .expect("expected highlighted spans"); + let mut has_non_default_fg = false; + for line in &lines { + for span in line { + match span.style.fg { + Some(RtColor::Rgb(..)) => { + panic!("theme {theme_name} produced RGB foreground: {span:?}") + } + Some(_) => has_non_default_fg = true, + None => {} + } + } + } + assert!( + has_non_default_fg, + "theme {theme_name} should produce at least one non-default foreground color" + ); + } + } + + #[test] + fn ansi_family_foreground_palette_snapshot() { + let mut out = String::new(); + for theme_name in ["ansi", "base16", "base16-256"] { + let colors = unique_foreground_colors_for_theme(theme_name); + out.push_str(&format!("{theme_name}:\n")); + for color in colors { + out.push_str(&format!(" {color}\n")); + } + } + assert_snapshot!("ansi_family_foreground_palette", out); + } + #[test] fn highlight_multiline_python() { let code = "def hello():\n print(\"hi\")\n return 42"; @@ -1152,7 +1378,7 @@ mod tests { assert!( warning .as_deref() - .is_some_and(|msg| msg.contains("could not be parsed")), + .is_some_and(|msg| msg.contains("could not be loaded")), "warning should explain that the theme file is invalid" ); } diff --git a/codex-rs/tui/src/render/snapshots/codex_tui__render__highlight__tests__ansi_family_foreground_palette.snap b/codex-rs/tui/src/render/snapshots/codex_tui__render__highlight__tests__ansi_family_foreground_palette.snap new file mode 100644 index 000000000..79e130916 --- /dev/null +++ b/codex-rs/tui/src/render/snapshots/codex_tui__render__highlight__tests__ansi_family_foreground_palette.snap @@ -0,0 +1,21 @@ +--- +source: tui/src/render/highlight.rs +expression: out +--- +ansi: + Blue + Green + Magenta + Yellow +base16: + Blue + Gray + Green + Indexed(9) + Magenta +base16-256: + Blue + Gray + Green + Indexed(16) + Magenta