From c4f1af7a86a08d52a5bbbd27dcb5314c779827db Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sun, 22 Feb 2026 01:26:58 -0300 Subject: [PATCH] feat(tui): syntax highlighting via syntect with theme picker (#11447) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Adds syntax highlighting to the TUI for fenced code blocks in markdown responses and file diffs, plus a `/theme` command with live preview and persistent theme selection. Uses syntect (~250 grammars, 32 bundled themes, ~1 MB binary cost) — the same engine behind `bat`, `delta`, and `xi-editor`. Includes guardrails for large inputs, graceful fallback to plain text, and SSH-aware clipboard integration for the `/copy` command. image image ## Problem Code blocks in the TUI (markdown responses and file diffs) render without syntax highlighting, making it hard to scan code at a glance. Users also have no way to pick a color theme that matches their terminal aesthetic. ## Mental model The highlighting system has three layers: 1. **Syntax engine** (`render::highlight`) -- a thin wrapper around syntect + two-face. It owns a process-global `SyntaxSet` (~250 grammars) and a `RwLock` that can be swapped at runtime. All public entry points accept `(code, lang)` and return ratatui `Span`/`Line` vectors or `None` when the language is unrecognized or the input exceeds safety guardrails. 2. **Rendering consumers** -- `markdown_render` feeds fenced code blocks through the engine; `diff_render` highlights Add/Delete content as a whole file and Update hunks per-hunk (preserving parser state across hunk lines). Both callers fall back to plain unstyled text when the engine returns `None`. 3. **Theme lifecycle** -- at startup the config's `tui.theme` is resolved to a syntect `Theme` via `set_theme_override`. At runtime the `/theme` picker calls `set_syntax_theme` to swap themes live; on cancel it restores the snapshot taken at open. On confirm it persists `[tui] theme = "..."` to config.toml. ## Non-goals - Inline diff highlighting (word-level change detection within a line). - Semantic / LSP-backed highlighting. - Theme authoring tooling; users supply standard `.tmTheme` files. ## Tradeoffs | Decision | Upside | Downside | | ------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | | syntect over tree-sitter / arborium | ~1 MB binary increase for ~250 grammars + 32 themes; battle-tested crate powering widely-used tools (`bat`, `delta`, `xi-editor`). tree-sitter would add ~12 MB for 20-30 languages or ~35 MB for full coverage. | Regex-based; less structurally accurate than tree-sitter for some languages (e.g. language injections like JS-in-HTML). | | Global `RwLock` | Enables live `/theme` preview without threading Theme through every call site | Lock contention risk (mitigated: reads vastly outnumber writes, single UI thread) | | Skip background / italic / underline from themes | Terminal BG preserved, avoids ugly rendering on some themes | Themes that rely on these properties lose fidelity | | Guardrails: 512 KB / 10k lines | Prevents pathological stalls on huge diffs or pastes | Very large files render without color | ## Architecture ``` config.toml ─[tui.theme]─> set_theme_override() ─> THEME (RwLock) │ ┌───────────────────────────────────────────┘ │ markdown_render ─── highlight_code_to_lines(code, lang) ─> Vec diff_render ─── highlight_code_to_styled_spans(code, lang) ─> Option>> │ │ (None ⇒ plain text fallback) │ /theme picker ─── set_syntax_theme(theme) // live preview swap ─── current_syntax_theme() // snapshot for cancel ─── resolve_theme_by_name(name) // lookup by kebab-case ``` Key files: - `tui/src/render/highlight.rs` -- engine, theme management, guardrails - `tui/src/diff_render.rs` -- syntax-aware diff line wrapping - `tui/src/theme_picker.rs` -- `/theme` command builder - `tui/src/bottom_pane/list_selection_view.rs` -- side content panel, callbacks - `core/src/config/types.rs` -- `Tui::theme` field - `core/src/config/edit.rs` -- `syntax_theme_edit()` helper ## Observability - `tracing::warn` when a configured theme name cannot be resolved. - `Config::startup_warnings` surfaces the same message as a TUI banner. - `tracing::error` when persisting theme selection fails. ## Tests - Unit tests in `highlight.rs`: language coverage, fallback behavior, CRLF stripping, style conversion, guardrail enforcement, theme name mapping exhaustiveness. - Unit tests in `diff_render.rs`: snapshot gallery at multiple terminal sizes (80x24, 94x35, 120x40), syntax-highlighted wrapping, large-diff guardrail, rename-to-different-extension highlighting, parser state preservation across hunk lines. - Unit tests in `theme_picker.rs`: preview rendering (wide + narrow), dim overlay on deletions, subtitle truncation, cancel-restore, fallback for unavailable configured theme. - Unit tests in `list_selection_view.rs`: side layout geometry, stacked fallback, buffer clearing, cancel/selection-changed callbacks. - Integration test in `lib.rs`: theme warning uses the final (post-resume) config. ## Cargo Deny: Unmaintained Dependency Exceptions This PR adds two `cargo deny` advisory exceptions for transitive dependencies pulled in by `syntect v5.3.0`: | Advisory | Crate | Status | |----------|-------|--------| | RUSTSEC-2024-0320 | `yaml-rust` | Unmaintained (maintainer unreachable) | | RUSTSEC-2025-0141 | `bincode` | Unmaintained (development ceased; v1.3.3 considered complete) | **Why this is safe in our usage:** - Neither advisory describes a known security vulnerability. Both are "unmaintained" notices only. - `bincode` is used by syntect to deserialize pre-compiled syntax sets. Again, these are **static vendored artifacts** baked into the binary at build time. No user-supplied bincode data is ever deserialized. - Attack surface is zero for both crates; exploitation would require a supply-chain compromise of our own build artifacts. - These exceptions can be removed when syntect migrates to `yaml-rust2` and drops `bincode`, or when alternative crates are available upstream. --- codex-rs/Cargo.lock | 107 +- codex-rs/Cargo.toml | 2 +- codex-rs/core/config.schema.json | 5 + codex-rs/core/src/config/edit.rs | 8 + codex-rs/core/src/config/mod.rs | 33 + codex-rs/core/src/config/types.rs | 7 + codex-rs/deny.toml | 3 + codex-rs/tui/Cargo.toml | 4 +- codex-rs/tui/src/app.rs | 64 + codex-rs/tui/src/app_event.rs | 5 + .../src/bottom_pane/list_selection_view.rs | 625 ++++++++- codex-rs/tui/src/bottom_pane/mod.rs | 3 + codex-rs/tui/src/chatwidget.rs | 22 + ...dget__tests__exec_approval_modal_exec.snap | 3 + codex-rs/tui/src/diff_render.rs | 889 ++++++++++-- codex-rs/tui/src/lib.rs | 59 +- codex-rs/tui/src/markdown_render.rs | 79 +- codex-rs/tui/src/markdown_render_tests.rs | 137 +- codex-rs/tui/src/render/highlight.rs | 1233 ++++++++++++++--- codex-rs/tui/src/slash_command.rs | 3 + ...ff_render__tests__diff_gallery_120x40.snap | 44 + ...iff_render__tests__diff_gallery_80x24.snap | 28 + ...iff_render__tests__diff_gallery_94x35.snap | 39 + ...ests__syntax_highlighted_insert_wraps.snap | 14 + ..._syntax_highlighted_insert_wraps_text.snap | 7 + codex-rs/tui/src/theme_picker.rs | 620 +++++++++ 26 files changed, 3726 insertions(+), 317 deletions(-) create mode 100644 codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__diff_gallery_120x40.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__diff_gallery_80x24.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__diff_gallery_94x35.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__syntax_highlighted_insert_wraps.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__syntax_highlighted_insert_wraps_text.snap create mode 100644 codex-rs/tui/src/theme_picker.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 31bc8d65a..c910f9544 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -852,6 +852,15 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a8241f3ebb85c056b509d4327ad0358fbbba6ffb340bf388f26350aeda225b1" +[[package]] +name = "bincode" +version = "1.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1f45e9417d87227c7a56d22e471c6206462cba514c7590c09aff4cf6d1ddcad" +dependencies = [ + "serde", +] + [[package]] name = "bit-set" version = "0.5.3" @@ -2297,6 +2306,7 @@ dependencies = [ "strum 0.27.2", "strum_macros 0.27.2", "supports-color 3.0.2", + "syntect", "tempfile", "textwrap 0.16.2", "thiserror 2.0.18", @@ -2307,8 +2317,7 @@ dependencies = [ "tracing", "tracing-appender", "tracing-subscriber", - "tree-sitter-bash", - "tree-sitter-highlight", + "two-face", "unicode-segmentation", "unicode-width 0.2.1", "url", @@ -5157,6 +5166,12 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "linked-hash-map" +version = "0.5.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0717cef1bc8b636c6e1c1bbdefc09e6322da8a9321966e8928ef80d20f7f770f" + [[package]] name = "linux-keyutils" version = "0.2.4" @@ -5964,6 +5979,28 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" +[[package]] +name = "onig" +version = "6.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "336b9c63443aceef14bea841b899035ae3abe89b7c486aaf4c5bd8aafedac3f0" +dependencies = [ + "bitflags 2.10.0", + "libc", + "once_cell", + "onig_sys", +] + +[[package]] +name = "onig_sys" +version = "69.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c7f86c6eef3d6df15f23bcfb6af487cbd2fed4e5581d58d5bf1f5f8b7f6727dc" +dependencies = [ + "cc", + "pkg-config", +] + [[package]] name = "opaque-debug" version = "0.3.1" @@ -6381,6 +6418,19 @@ version = "0.3.32" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c" +[[package]] +name = "plist" +version = "1.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "740ebea15c5d1428f910cd1a5f52cebf8d25006245ed8ade92702f4943d91e07" +dependencies = [ + "base64 0.22.1", + "indexmap 2.13.0", + "quick-xml", + "serde", + "time", +] + [[package]] name = "png" version = "0.18.0" @@ -8886,6 +8936,27 @@ dependencies = [ "syn 2.0.114", ] +[[package]] +name = "syntect" +version = "5.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "656b45c05d95a5704399aeef6bd0ddec7b2b3531b7c9e900abbf7c4d2190c925" +dependencies = [ + "bincode", + "flate2", + "fnv", + "once_cell", + "onig", + "plist", + "regex-syntax 0.8.8", + "serde", + "serde_derive", + "serde_json", + "thiserror 2.0.18", + "walkdir", + "yaml-rust", +] + [[package]] name = "sys-locale" version = "0.3.2" @@ -9622,18 +9693,6 @@ dependencies = [ "tree-sitter-language", ] -[[package]] -name = "tree-sitter-highlight" -version = "0.25.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "adc5f880ad8d8f94e88cb81c3557024cf1a8b75e3b504c50481ed4f5a6006ff3" -dependencies = [ - "regex", - "streaming-iterator", - "thiserror 2.0.18", - "tree-sitter", -] - [[package]] name = "tree-sitter-language" version = "0.1.7" @@ -9701,6 +9760,17 @@ dependencies = [ "utf-8", ] +[[package]] +name = "two-face" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b285c51f8a6ade109ed4566d33ac4fb289fb5d6cf87ed70908a5eaf65e948e34" +dependencies = [ + "serde", + "serde_derive", + "syntect", +] + [[package]] name = "type-map" version = "0.5.1" @@ -10965,6 +11035,15 @@ dependencies = [ "lzma-sys", ] +[[package]] +name = "yaml-rust" +version = "0.4.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56c1936c4cc7a1c9ab21a1ebb602eb942ba868cbd44a99cb7cdc5892335e1c85" +dependencies = [ + "linked-hash-map", +] + [[package]] name = "yansi" version = "1.0.1" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index d23ed3c27..e22dba12c 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -276,7 +276,7 @@ tracing-subscriber = "0.3.22" tracing-test = "0.2.5" tree-sitter = "0.25.10" tree-sitter-bash = "0.25" -tree-sitter-highlight = "0.25.10" +syntect = "5" ts-rs = "11" tungstenite = { version = "0.27.0", features = ["deflate", "proxy"] } uds_windows = "1.1.0" diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index addbcbe31..ed1ae8ddb 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -1400,6 +1400,11 @@ "type": "string" }, "type": "array" + }, + "theme": { + "default": null, + "description": "Syntax highlighting theme name (kebab-case).\n\nWhen set, overrides automatic light/dark theme detection. Use `/theme` in the TUI or see `$CODEX_HOME/themes` for custom themes.", + "type": "string" } }, "type": "object" diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index 0dfb4efd8..fceb96599 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -55,6 +55,14 @@ pub enum ConfigEdit { ClearPath { segments: Vec }, } +/// Produces a config edit that sets `[tui] theme = ""`. +pub fn syntax_theme_edit(name: &str) -> ConfigEdit { + ConfigEdit::SetPath { + segments: vec!["tui".to_string(), "theme".to_string()], + value: value(name.to_string()), + } +} + pub fn status_line_items_edit(items: &[String]) -> ConfigEdit { let mut array = toml_edit::Array::new(); for item in items { diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index ddc5aa9cd..6474ca4bc 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -278,6 +278,9 @@ pub struct Config { /// `current-dir`. pub tui_status_line: Option>, + /// Syntax highlighting theme override (kebab-case name). + pub tui_theme: Option, + /// The directory that should be treated as the current working directory /// for the session. All relative paths inside the business-logic layer are /// resolved against this path. @@ -2120,6 +2123,7 @@ impl Config { .map(|t| t.alternate_screen) .unwrap_or_default(), tui_status_line: cfg.tui.as_ref().and_then(|t| t.status_line.clone()), + tui_theme: cfg.tui.as_ref().and_then(|t| t.theme.clone()), otel: { let t: OtelConfigToml = cfg.otel.unwrap_or_default(); let log_user_prompt = t.log_user_prompt.unwrap_or(false); @@ -2518,6 +2522,30 @@ allowed_domains = ["openai.com"] Ok(()) } + #[test] + fn tui_theme_deserializes_from_toml() { + let cfg = r#" +[tui] +theme = "dracula" +"#; + let parsed = + toml::from_str::(cfg).expect("TOML deserialization should succeed"); + assert_eq!( + parsed.tui.as_ref().and_then(|t| t.theme.as_deref()), + Some("dracula"), + ); + } + + #[test] + fn tui_theme_defaults_to_none() { + let cfg = r#" +[tui] +"#; + let parsed = + toml::from_str::(cfg).expect("TOML deserialization should succeed"); + assert_eq!(parsed.tui.as_ref().and_then(|t| t.theme.as_deref()), None); + } + #[test] fn tui_config_missing_notifications_field_defaults_to_enabled() { let cfg = r#" @@ -2537,6 +2565,7 @@ allowed_domains = ["openai.com"] show_tooltips: true, alternate_screen: AltScreenMode::Auto, status_line: None, + theme: None, } ); } @@ -4646,6 +4675,7 @@ model_verbosity = "high" feedback_enabled: true, tui_alternate_screen: AltScreenMode::Auto, tui_status_line: None, + tui_theme: None, otel: OtelConfig::default(), }, o3_profile_config @@ -4768,6 +4798,7 @@ model_verbosity = "high" feedback_enabled: true, tui_alternate_screen: AltScreenMode::Auto, tui_status_line: None, + tui_theme: None, otel: OtelConfig::default(), }; @@ -4888,6 +4919,7 @@ model_verbosity = "high" feedback_enabled: true, tui_alternate_screen: AltScreenMode::Auto, tui_status_line: None, + tui_theme: None, otel: OtelConfig::default(), }; @@ -4994,6 +5026,7 @@ model_verbosity = "high" feedback_enabled: true, tui_alternate_screen: AltScreenMode::Auto, tui_status_line: None, + tui_theme: None, otel: OtelConfig::default(), }; diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index 2e3573f77..678766e80 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -681,6 +681,13 @@ pub struct Tui { /// `current-dir`. #[serde(default)] pub status_line: Option>, + + /// Syntax highlighting theme name (kebab-case). + /// + /// When set, overrides automatic light/dark theme detection. + /// Use `/theme` in the TUI or see `$CODEX_HOME/themes` for custom themes. + #[serde(default)] + pub theme: Option, } const fn default_true() -> bool { diff --git a/codex-rs/deny.toml b/codex-rs/deny.toml index e96e6fbd3..29bf6c588 100644 --- a/codex-rs/deny.toml +++ b/codex-rs/deny.toml @@ -75,6 +75,9 @@ ignore = [ { id = "RUSTSEC-2024-0436", reason = "paste is unmaintained; pulled in via ratatui/rmcp/starlark used by tui/execpolicy; no fixed release yet" }, # TODO(joshka, nornagon): remove this exception when once we update the ratatui fork to a version that uses lru 0.13+. { id = "RUSTSEC-2026-0002", reason = "lru 0.12.5 is pulled in via ratatui fork; cannot upgrade until the fork is updated" }, + # TODO(fcoury): remove this exception when syntect drops yaml-rust and bincode, or updates to versions that have fixed the vulnerabilities. + { id = "RUSTSEC-2024-0320", reason = "yaml-rust is unmaintained; pulled in via syntect v5.3.0 used by codex-tui for syntax highlighting; no fixed release yet" }, + { id = "RUSTSEC-2025-0141", reason = "bincode is unmaintained; pulled in via syntect v5.3.0 used by codex-tui for syntax highlighting; no fixed release yet" }, ] # If this is true, then cargo deny will use the git executable to fetch advisory database. # If this is false, then it uses a built-in git library. diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 683b0828d..1b0ed9771 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -93,8 +93,8 @@ toml = { workspace = true } tracing = { workspace = true, features = ["log"] } tracing-appender = { workspace = true } tracing-subscriber = { workspace = true, features = ["env-filter"] } -tree-sitter-bash = { workspace = true } -tree-sitter-highlight = { workspace = true } +syntect = "5" +two-face = { version = "0.5", default-features = false, features = ["syntect-default-onig"] } unicode-segmentation = { workspace = true } unicode-width = { workspace = true } url = { workspace = true } diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index ebbf75752..22ceed1ff 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -2605,6 +2605,34 @@ impl App { AppEvent::StatusLineSetupCancelled => { self.chat_widget.cancel_status_line_setup(); } + AppEvent::SyntaxThemeSelected { name } => { + let edit = codex_core::config::edit::syntax_theme_edit(&name); + let apply_result = ConfigEditsBuilder::new(&self.config.codex_home) + .with_edits([edit]) + .apply() + .await; + match apply_result { + Ok(()) => { + // Ensure the selected theme is active in the current + // session. The preview callback covers arrow-key + // navigation, but if the user presses Enter without + // navigating, the runtime theme must still be applied. + if let Some(theme) = crate::render::highlight::resolve_theme_by_name( + &name, + Some(&self.config.codex_home), + ) { + crate::render::highlight::set_syntax_theme(theme); + } + self.sync_tui_theme_selection(name); + } + Err(err) => { + self.restore_runtime_theme_from_config(); + tracing::error!(error = %err, "failed to persist theme selection"); + self.chat_widget + .add_error_message(format!("Failed to save theme: {err}")); + } + } + } } Ok(AppRunControl::Continue) } @@ -2783,6 +2811,29 @@ impl App { self.chat_widget.set_personality(personality); } + fn sync_tui_theme_selection(&mut self, name: String) { + self.config.tui_theme = Some(name.clone()); + self.chat_widget.set_tui_theme(Some(name)); + } + + fn restore_runtime_theme_from_config(&self) { + if let Some(name) = self.config.tui_theme.as_deref() + && let Some(theme) = + crate::render::highlight::resolve_theme_by_name(name, Some(&self.config.codex_home)) + { + crate::render::highlight::set_syntax_theme(theme); + return; + } + + let auto_theme_name = crate::render::highlight::adaptive_default_theme_name(); + if let Some(theme) = crate::render::highlight::resolve_theme_by_name( + auto_theme_name, + Some(&self.config.codex_home), + ) { + crate::render::highlight::set_syntax_theme(theme); + } + } + fn personality_label(personality: Personality) -> &'static str { match personality { Personality::None => "None", @@ -3655,6 +3706,19 @@ mod tests { Ok(()) } + #[tokio::test] + async fn sync_tui_theme_selection_updates_chat_widget_config_copy() { + let mut app = make_test_app().await; + + app.sync_tui_theme_selection("dracula".to_string()); + + assert_eq!(app.config.tui_theme.as_deref(), Some("dracula")); + assert_eq!( + app.chat_widget.config_ref().tui_theme.as_deref(), + Some("dracula") + ); + } + #[tokio::test] async fn backtrack_selection_with_duplicate_history_targets_unique_turn() { let (mut app, _app_event_rx, mut op_rx) = make_test_app_with_channels().await; diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 159cda88f..60c25fcc4 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -360,6 +360,11 @@ pub(crate) enum AppEvent { }, /// Dismiss the status-line setup UI without changing config. StatusLineSetupCancelled, + + /// Apply a user-confirmed syntax theme selection. + SyntaxThemeSelected { + name: String, + }, } /// The exit strategy requested by the UI layer. diff --git a/codex-rs/tui/src/bottom_pane/list_selection_view.rs b/codex-rs/tui/src/bottom_pane/list_selection_view.rs index 14f94ca6f..ad40c1660 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -33,9 +33,76 @@ use super::selection_popup_common::render_rows_stable_col_widths; use super::selection_popup_common::render_rows_with_col_width_mode; use unicode_width::UnicodeWidthStr; +/// Minimum list width (in content columns) required before the side-by-side +/// layout is activated. Keeps the list usable even when sharing horizontal +/// space with the side content panel. +const MIN_LIST_WIDTH_FOR_SIDE: u16 = 40; + +/// Horizontal gap (in columns) between the list area and the side content +/// panel when side-by-side layout is active. +const SIDE_CONTENT_GAP: u16 = 2; + +/// Shared menu-surface horizontal inset (2 cells per side) used by selection popups. +const MENU_SURFACE_HORIZONTAL_INSET: u16 = 4; + +/// Controls how the side content panel is sized relative to the popup width. +/// +/// When the computed side width falls below `side_content_min_width` or the +/// remaining list area would be narrower than [`MIN_LIST_WIDTH_FOR_SIDE`], the +/// side-by-side layout is abandoned and the stacked fallback is used instead. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum SideContentWidth { + /// Fixed number of columns. `Fixed(0)` disables side content entirely. + Fixed(u16), + /// Exact 50/50 split of the content area (minus the inter-column gap). + Half, +} + +impl Default for SideContentWidth { + fn default() -> Self { + Self::Fixed(0) + } +} + +/// Returns the popup content width after subtracting the shared menu-surface +/// horizontal inset (2 columns on each side). +pub(crate) fn popup_content_width(total_width: u16) -> u16 { + total_width.saturating_sub(MENU_SURFACE_HORIZONTAL_INSET) +} + +/// Returns side-by-side layout widths as `(list_width, side_width)` when the +/// layout can fit. Returns `None` when the side panel is disabled/too narrow or +/// when the remaining list width would become unusably small. +pub(crate) fn side_by_side_layout_widths( + content_width: u16, + side_content_width: SideContentWidth, + side_content_min_width: u16, +) -> Option<(u16, u16)> { + let side_width = match side_content_width { + SideContentWidth::Fixed(0) => return None, + SideContentWidth::Fixed(width) => width, + SideContentWidth::Half => content_width.saturating_sub(SIDE_CONTENT_GAP) / 2, + }; + if side_width < side_content_min_width { + return None; + } + let list_width = content_width.saturating_sub(SIDE_CONTENT_GAP + side_width); + (list_width >= MIN_LIST_WIDTH_FOR_SIDE).then_some((list_width, side_width)) +} + /// One selectable item in the generic selection list. pub(crate) type SelectionAction = Box; +/// Callback invoked whenever the highlighted item changes (arrow keys, search +/// filter, number-key jump). Receives the *actual* index into the unfiltered +/// `items` list and the event sender. Used by the theme picker for live preview. +pub(crate) type OnSelectionChangedCallback = + Option>; + +/// Callback invoked when the picker is dismissed without accepting (Esc or +/// Ctrl+C). Used by the theme picker to restore the pre-open theme. +pub(crate) type OnCancelCallback = Option>; + /// One row in a [`ListSelectionView`] selection list. /// /// This is the source-of-truth model for row state before filtering and @@ -79,6 +146,28 @@ pub(crate) struct SelectionViewParams { pub col_width_mode: ColumnWidthMode, pub header: Box, pub initial_selected_idx: Option, + + /// Rich content rendered beside (wide terminals) or below (narrow terminals) + /// the list items, inside the bordered menu surface. Used by the theme picker + /// to show a syntax-highlighted preview. + pub side_content: Box, + + /// Width mode for side content when side-by-side layout is active. + pub side_content_width: SideContentWidth, + + /// Minimum side panel width required before side-by-side layout activates. + pub side_content_min_width: u16, + + /// Optional fallback content rendered when side-by-side does not fit. + /// When absent, `side_content` is reused. + pub stacked_side_content: Option>, + + /// Called when the highlighted item changes (navigation, filter, number-key). + /// Receives the *actual* item index, not the filtered/visible index. + pub on_selection_changed: OnSelectionChangedCallback, + + /// Called when the picker is dismissed via Esc/Ctrl+C without selecting. + pub on_cancel: OnCancelCallback, } impl Default for SelectionViewParams { @@ -95,6 +184,12 @@ impl Default for SelectionViewParams { col_width_mode: ColumnWidthMode::AutoVisible, header: Box::new(()), initial_selected_idx: None, + side_content: Box::new(()), + side_content_width: SideContentWidth::default(), + side_content_min_width: 0, + stacked_side_content: None, + on_selection_changed: None, + on_cancel: None, } } } @@ -120,6 +215,16 @@ pub(crate) struct ListSelectionView { last_selected_actual_idx: Option, header: Box, initial_selected_idx: Option, + side_content: Box, + side_content_width: SideContentWidth, + side_content_min_width: u16, + stacked_side_content: Option>, + + /// Called when the highlighted item changes (navigation, filter, number-key). + on_selection_changed: OnSelectionChangedCallback, + + /// Called when the picker is dismissed via Esc/Ctrl+C without selecting. + on_cancel: OnCancelCallback, } impl ListSelectionView { @@ -161,6 +266,12 @@ impl ListSelectionView { last_selected_actual_idx: None, header, initial_selected_idx: params.initial_selected_idx, + side_content: params.side_content, + side_content_width: params.side_content_width, + side_content_min_width: params.side_content_min_width, + stacked_side_content: params.stacked_side_content, + on_selection_changed: params.on_selection_changed, + on_cancel: params.on_cancel, }; s.apply_filter(); s @@ -174,11 +285,15 @@ impl ListSelectionView { MAX_POPUP_ROWS.min(len.max(1)) } - fn apply_filter(&mut self) { - let previously_selected = self - .state + fn selected_actual_idx(&self) -> Option { + self.state .selected_idx .and_then(|visible_idx| self.filtered_indices.get(visible_idx).copied()) + } + + fn apply_filter(&mut self) { + let previously_selected = self + .selected_actual_idx() .or_else(|| { (!self.is_searchable) .then(|| self.items.iter().position(|item| item.is_current)) @@ -222,6 +337,13 @@ impl ListSelectionView { let visible = Self::max_visible_rows(len); self.state.clamp_selection(len); self.state.ensure_visible(len, visible); + + // Notify the callback when filtering changes the selected actual item + // so live preview stays in sync (e.g. typing in the theme picker). + let new_actual = self.selected_actual_idx(); + if new_actual != previously_selected { + self.fire_selection_changed(); + } } fn build_rows(&self) -> Vec { @@ -273,19 +395,35 @@ impl ListSelectionView { } fn move_up(&mut self) { + let before = self.selected_actual_idx(); let len = self.visible_len(); self.state.move_up_wrap(len); let visible = Self::max_visible_rows(len); self.state.ensure_visible(len, visible); self.skip_disabled_up(); + if self.selected_actual_idx() != before { + self.fire_selection_changed(); + } } fn move_down(&mut self) { + let before = self.selected_actual_idx(); let len = self.visible_len(); self.state.move_down_wrap(len); let visible = Self::max_visible_rows(len); self.state.ensure_visible(len, visible); self.skip_disabled_down(); + if self.selected_actual_idx() != before { + self.fire_selection_changed(); + } + } + + fn fire_selection_changed(&self) { + if let Some(cb) = &self.on_selection_changed + && let Some(actual) = self.selected_actual_idx() + { + cb(actual, &self.app_event_tx); + } } fn accept(&mut self) { @@ -310,6 +448,9 @@ impl ListSelectionView { self.complete = true; } } else if selected_item.is_none() { + if let Some(cb) = &self.on_cancel { + cb(&self.app_event_tx); + } self.complete = true; } } @@ -328,6 +469,63 @@ impl ListSelectionView { total_width.saturating_sub(2) } + fn clear_to_terminal_bg(buf: &mut Buffer, area: Rect) { + let buf_area = buf.area(); + let min_x = area.x.max(buf_area.x); + let min_y = area.y.max(buf_area.y); + let max_x = area + .x + .saturating_add(area.width) + .min(buf_area.x.saturating_add(buf_area.width)); + let max_y = area + .y + .saturating_add(area.height) + .min(buf_area.y.saturating_add(buf_area.height)); + for y in min_y..max_y { + for x in min_x..max_x { + buf[(x, y)] + .set_symbol(" ") + .set_style(ratatui::style::Style::reset()); + } + } + } + + fn force_bg_to_terminal_bg(buf: &mut Buffer, area: Rect) { + let buf_area = buf.area(); + let min_x = area.x.max(buf_area.x); + let min_y = area.y.max(buf_area.y); + let max_x = area + .x + .saturating_add(area.width) + .min(buf_area.x.saturating_add(buf_area.width)); + let max_y = area + .y + .saturating_add(area.height) + .min(buf_area.y.saturating_add(buf_area.height)); + for y in min_y..max_y { + for x in min_x..max_x { + buf[(x, y)].set_bg(ratatui::style::Color::Reset); + } + } + } + + fn stacked_side_content(&self) -> &dyn Renderable { + self.stacked_side_content + .as_deref() + .unwrap_or_else(|| self.side_content.as_ref()) + } + + /// Returns `Some(side_width)` when the content area is wide enough for a + /// side-by-side layout (list + gap + side panel), `None` otherwise. + fn side_layout_width(&self, content_width: u16) -> Option { + side_by_side_layout_widths( + content_width, + self.side_content_width, + self.side_content_min_width, + ) + .map(|(_, side_width)| side_width) + } + fn skip_disabled_down(&mut self) { let len = self.visible_len(); for _ in 0..len { @@ -469,6 +667,9 @@ impl BottomPaneView for ListSelectionView { } fn on_ctrl_c(&mut self) -> CancellationEvent { + if let Some(cb) = &self.on_cancel { + cb(&self.app_event_tx); + } self.complete = true; CancellationEvent::Handled } @@ -476,38 +677,59 @@ impl BottomPaneView for ListSelectionView { impl Renderable for ListSelectionView { fn desired_height(&self, width: u16) -> u16 { - // Measure wrapped height for up to MAX_POPUP_ROWS items at the given width. - // Build the same display rows used by the renderer so wrapping math matches. + // Inner content width after menu surface horizontal insets (2 per side). + let inner_width = popup_content_width(width); + + // When side-by-side is active, measure the list at the reduced width + // that accounts for the gap and side panel. + let effective_rows_width = if let Some(side_w) = self.side_layout_width(inner_width) { + Self::rows_width(width).saturating_sub(SIDE_CONTENT_GAP + side_w) + } else { + Self::rows_width(width) + }; + + // Measure wrapped height for up to MAX_POPUP_ROWS items. let rows = self.build_rows(); - let rows_width = Self::rows_width(width); let rows_height = match self.col_width_mode { ColumnWidthMode::AutoVisible => measure_rows_height( &rows, &self.state, MAX_POPUP_ROWS, - rows_width.saturating_add(1), + effective_rows_width.saturating_add(1), ), ColumnWidthMode::AutoAllRows => measure_rows_height_stable_col_widths( &rows, &self.state, MAX_POPUP_ROWS, - rows_width.saturating_add(1), + effective_rows_width.saturating_add(1), ), ColumnWidthMode::Fixed => measure_rows_height_with_col_width_mode( &rows, &self.state, MAX_POPUP_ROWS, - rows_width.saturating_add(1), + effective_rows_width.saturating_add(1), ColumnWidthMode::Fixed, ), }; - // Subtract 4 for the padding on the left and right of the header. - let mut height = self.header.desired_height(width.saturating_sub(4)); + let mut height = self.header.desired_height(inner_width); height = height.saturating_add(rows_height + 3); if self.is_searchable { height = height.saturating_add(1); } + + // Side content: when the terminal is wide enough the panel sits beside + // the list and shares vertical space; otherwise it stacks below. + if self.side_layout_width(inner_width).is_some() { + // Side-by-side — side content shares list rows vertically so it + // doesn't add to total height. + } else { + let side_h = self.stacked_side_content().desired_height(inner_width); + if side_h > 0 { + height = height.saturating_add(1 + side_h); + } + } + if let Some(note) = &self.footer_note { let note_width = width.saturating_sub(2); let note_lines = wrap_styled_line(note, note_width); @@ -538,41 +760,60 @@ impl Renderable for ListSelectionView { // Paint the shared menu surface and then layout inside the returned inset. let content_area = render_menu_surface(outer_content_area, buf); - let header_height = self - .header - // Subtract 4 for the padding on the left and right of the header. - .desired_height(outer_content_area.width.saturating_sub(4)); + let inner_width = popup_content_width(outer_content_area.width); + let side_w = self.side_layout_width(inner_width); + + // When side-by-side is active, shrink the list to make room. + let full_rows_width = Self::rows_width(outer_content_area.width); + let effective_rows_width = if let Some(sw) = side_w { + full_rows_width.saturating_sub(SIDE_CONTENT_GAP + sw) + } else { + full_rows_width + }; + + let header_height = self.header.desired_height(inner_width); let rows = self.build_rows(); - let rows_width = Self::rows_width(outer_content_area.width); let rows_height = match self.col_width_mode { ColumnWidthMode::AutoVisible => measure_rows_height( &rows, &self.state, MAX_POPUP_ROWS, - rows_width.saturating_add(1), + effective_rows_width.saturating_add(1), ), ColumnWidthMode::AutoAllRows => measure_rows_height_stable_col_widths( &rows, &self.state, MAX_POPUP_ROWS, - rows_width.saturating_add(1), + effective_rows_width.saturating_add(1), ), ColumnWidthMode::Fixed => measure_rows_height_with_col_width_mode( &rows, &self.state, MAX_POPUP_ROWS, - rows_width.saturating_add(1), + effective_rows_width.saturating_add(1), ColumnWidthMode::Fixed, ), }; - let [header_area, _, search_area, list_area] = Layout::vertical([ + + // Stacked (fallback) side content height — only used when not side-by-side. + let stacked_side_h = if side_w.is_none() { + self.stacked_side_content().desired_height(inner_width) + } else { + 0 + }; + let stacked_gap = if stacked_side_h > 0 { 1 } else { 0 }; + + let [header_area, _, search_area, list_area, _, stacked_side_area] = Layout::vertical([ Constraint::Max(header_height), Constraint::Max(1), Constraint::Length(if self.is_searchable { 1 } else { 0 }), Constraint::Length(rows_height), + Constraint::Length(stacked_gap), + Constraint::Length(stacked_side_h), ]) .areas(content_area); + // -- Header -- if header_area.height < header_height { let [header_area, elision_area] = Layout::vertical([Constraint::Fill(1), Constraint::Length(1)]).areas(header_area); @@ -585,6 +826,7 @@ impl Renderable for ListSelectionView { self.header.render(header_area, buf); } + // -- Search bar -- if self.is_searchable { Line::from(self.search_query.clone()).render(search_area, buf); let query_span: Span<'static> = if self.search_query.is_empty() { @@ -598,11 +840,12 @@ impl Renderable for ListSelectionView { Line::from(query_span).render(search_area, buf); } + // -- List rows -- if list_area.height > 0 { let render_area = Rect { x: list_area.x.saturating_sub(2), y: list_area.y, - width: rows_width.max(1), + width: effective_rows_width.max(1), height: list_area.height, }; match self.col_width_mode { @@ -634,6 +877,53 @@ impl Renderable for ListSelectionView { }; } + // -- Side content (preview panel) -- + if let Some(sw) = side_w { + // Side-by-side: render to the right half of the popup content + // area so preview content can center vertically in that panel. + let side_x = content_area.x + content_area.width - sw; + let side_area = Rect::new(side_x, content_area.y, sw, content_area.height); + + // Clear the menu-surface background behind the side panel so the + // preview appears on the terminal's own background. + let clear_x = side_x.saturating_sub(SIDE_CONTENT_GAP); + let clear_w = outer_content_area + .x + .saturating_add(outer_content_area.width) + .saturating_sub(clear_x); + Self::clear_to_terminal_bg( + buf, + Rect::new( + clear_x, + outer_content_area.y, + clear_w, + outer_content_area.height, + ), + ); + self.side_content.render(side_area, buf); + Self::force_bg_to_terminal_bg( + buf, + Rect::new( + clear_x, + outer_content_area.y, + clear_w, + outer_content_area.height, + ), + ); + } else if stacked_side_area.height > 0 { + // Stacked fallback: render below the list (same as old footer_content). + let clear_height = (outer_content_area.y + outer_content_area.height) + .saturating_sub(stacked_side_area.y); + let clear_area = Rect::new( + outer_content_area.x, + stacked_side_area.y, + outer_content_area.width, + clear_height, + ); + Self::clear_to_terminal_bg(buf, clear_area); + self.stacked_side_content().render(stacked_side_area, buf); + } + if footer_area.height > 0 { let [note_area, hint_area] = Layout::vertical([ Constraint::Length(note_height), @@ -683,9 +973,33 @@ mod tests { use crossterm::event::KeyCode; use insta::assert_snapshot; use pretty_assertions::assert_eq; + use ratatui::buffer::Buffer; use ratatui::layout::Rect; + use ratatui::style::Color; + use ratatui::style::Style; use tokio::sync::mpsc::unbounded_channel; + struct MarkerRenderable { + marker: &'static str, + height: u16, + } + + impl Renderable for MarkerRenderable { + fn render(&self, area: Rect, buf: &mut Buffer) { + for y in area.y..area.y.saturating_add(area.height) { + for x in area.x..area.x.saturating_add(area.width) { + if x < buf.area().width && y < buf.area().height { + buf[(x, y)].set_symbol(self.marker); + } + } + } + } + + fn desired_height(&self, _width: u16) -> u16 { + self.height + } + } + fn make_selection_view(subtitle: Option<&str>) -> ListSelectionView { let (tx_raw, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); @@ -722,7 +1036,10 @@ mod tests { } fn render_lines_with_width(view: &ListSelectionView, width: u16) -> String { - let height = view.desired_height(width); + render_lines_in_area(view, width, view.desired_height(width)) + } + + fn render_lines_in_area(view: &ListSelectionView, width: u16, height: u16) -> String { let area = Rect::new(0, 0, width, height); let mut buf = Buffer::empty(area); view.render(area, &mut buf); @@ -808,6 +1125,20 @@ mod tests { assert_snapshot!("list_selection_spacing_with_subtitle", render_lines(&view)); } + #[test] + fn theme_picker_subtitle_uses_fallback_text_in_94x35_terminal() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let home = dirs::home_dir().expect("home directory should be available"); + let codex_home = home.join(".codex"); + let params = + crate::theme_picker::build_theme_picker_params(None, Some(&codex_home), Some(94)); + let view = ListSelectionView::new(params, tx); + + let rendered = render_lines_in_area(&view, 94, 35); + assert!(rendered.contains("Move up/down to live preview themes")); + } + #[test] fn snapshot_footer_note_wraps() { let (tx_raw, _rx) = unbounded_channel::(); @@ -871,6 +1202,66 @@ mod tests { ); } + #[test] + fn enter_with_no_matches_triggers_cancel_callback() { + let (tx_raw, mut rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut view = ListSelectionView::new( + SelectionViewParams { + items: vec![SelectionItem { + name: "Read Only".to_string(), + dismiss_on_select: true, + ..Default::default() + }], + is_searchable: true, + on_cancel: Some(Box::new(|tx: &_| { + tx.send(AppEvent::OpenApprovalsPopup); + })), + ..Default::default() + }, + tx, + ); + view.set_search_query("no-matches".to_string()); + + view.handle_key_event(KeyEvent::from(KeyCode::Enter)); + + assert!(view.is_complete()); + match rx.try_recv() { + Ok(AppEvent::OpenApprovalsPopup) => {} + Ok(other) => panic!("expected OpenApprovalsPopup cancel event, got {other:?}"), + Err(err) => panic!("expected cancel callback event, got {err}"), + } + } + + #[test] + fn move_down_without_selection_change_does_not_fire_callback() { + let (tx_raw, mut rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut view = ListSelectionView::new( + SelectionViewParams { + items: vec![SelectionItem { + name: "Only choice".to_string(), + dismiss_on_select: true, + ..Default::default() + }], + on_selection_changed: Some(Box::new(|_idx, tx: &_| { + tx.send(AppEvent::OpenApprovalsPopup); + })), + ..Default::default() + }, + tx, + ); + + while rx.try_recv().is_ok() {} + + view.handle_key_event(KeyEvent::from(KeyCode::Down)); + + assert!( + rx.try_recv().is_err(), + "moving down in a single-item list should not fire on_selection_changed", + ); + } + #[test] fn wraps_long_option_without_overflowing_columns() { let (tx_raw, _rx) = unbounded_channel::(); @@ -1159,4 +1550,194 @@ mod tests { "fixed description column changed across scroll:\nbefore:\n{before_scroll}\nafter:\n{after_scroll}" ); } + + #[test] + fn side_layout_width_half_uses_exact_split() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let view = ListSelectionView::new( + SelectionViewParams { + items: vec![SelectionItem { + name: "Item 1".to_string(), + dismiss_on_select: true, + ..Default::default() + }], + side_content: Box::new(MarkerRenderable { + marker: "W", + height: 1, + }), + side_content_width: SideContentWidth::Half, + side_content_min_width: 10, + ..Default::default() + }, + tx, + ); + + let content_width: u16 = 120; + let expected = content_width.saturating_sub(SIDE_CONTENT_GAP) / 2; + assert_eq!(view.side_layout_width(content_width), Some(expected)); + } + + #[test] + fn side_layout_width_half_falls_back_when_list_would_be_too_narrow() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let view = ListSelectionView::new( + SelectionViewParams { + items: vec![SelectionItem { + name: "Item 1".to_string(), + dismiss_on_select: true, + ..Default::default() + }], + side_content: Box::new(MarkerRenderable { + marker: "W", + height: 1, + }), + side_content_width: SideContentWidth::Half, + side_content_min_width: 50, + ..Default::default() + }, + tx, + ); + + assert_eq!(view.side_layout_width(80), None); + } + + #[test] + fn stacked_side_content_is_used_when_side_by_side_does_not_fit() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let view = ListSelectionView::new( + SelectionViewParams { + title: Some("Debug".to_string()), + items: vec![SelectionItem { + name: "Item 1".to_string(), + dismiss_on_select: true, + ..Default::default() + }], + side_content: Box::new(MarkerRenderable { + marker: "W", + height: 1, + }), + stacked_side_content: Some(Box::new(MarkerRenderable { + marker: "N", + height: 1, + })), + side_content_width: SideContentWidth::Half, + side_content_min_width: 60, + ..Default::default() + }, + tx, + ); + + let rendered = render_lines_with_width(&view, 70); + assert!( + rendered.contains('N'), + "expected stacked marker to be rendered:\n{rendered}" + ); + assert!( + !rendered.contains('W'), + "wide marker should not render in stacked mode:\n{rendered}" + ); + } + + #[test] + fn side_content_clearing_resets_symbols_and_style() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let view = ListSelectionView::new( + SelectionViewParams { + title: Some("Debug".to_string()), + items: vec![SelectionItem { + name: "Item 1".to_string(), + dismiss_on_select: true, + ..Default::default() + }], + side_content: Box::new(MarkerRenderable { + marker: "W", + height: 1, + }), + side_content_width: SideContentWidth::Half, + side_content_min_width: 10, + ..Default::default() + }, + tx, + ); + + let width = 120; + let height = view.desired_height(width); + let area = Rect::new(0, 0, width, height); + let mut buf = Buffer::empty(area); + for y in 0..height { + for x in 0..width { + buf[(x, y)] + .set_symbol("X") + .set_style(Style::default().bg(Color::Red)); + } + } + view.render(area, &mut buf); + + let cell = &buf[(width - 1, 0)]; + assert_eq!(cell.symbol(), " "); + let style = cell.style(); + assert_eq!(style.fg, Some(Color::Reset)); + assert_eq!(style.bg, Some(Color::Reset)); + assert_eq!(style.underline_color, Some(Color::Reset)); + + let mut saw_marker = false; + for y in 0..height { + for x in 0..width { + let cell = &buf[(x, y)]; + if cell.symbol() == "W" { + saw_marker = true; + assert_eq!(cell.style().bg, Some(Color::Reset)); + } + } + } + assert!( + saw_marker, + "expected side marker renderable to draw into buffer" + ); + } + + #[test] + fn side_content_clearing_handles_non_zero_buffer_origin() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let view = ListSelectionView::new( + SelectionViewParams { + title: Some("Debug".to_string()), + items: vec![SelectionItem { + name: "Item 1".to_string(), + dismiss_on_select: true, + ..Default::default() + }], + side_content: Box::new(MarkerRenderable { + marker: "W", + height: 1, + }), + side_content_width: SideContentWidth::Half, + side_content_min_width: 10, + ..Default::default() + }, + tx, + ); + + let width = 120; + let height = view.desired_height(width); + let area = Rect::new(0, 20, width, height); + let mut buf = Buffer::empty(area); + for y in area.y..area.y + height { + for x in area.x..area.x + width { + buf[(x, y)] + .set_symbol("X") + .set_style(Style::default().bg(Color::Red)); + } + } + view.render(area, &mut buf); + + let cell = &buf[(area.x + width - 1, area.y)]; + assert_eq!(cell.symbol(), " "); + assert_eq!(cell.style().bg, Some(Color::Reset)); + } } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 234bb8b72..9c7b940f0 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -78,6 +78,9 @@ mod slash_commands; pub(crate) use footer::CollaborationModeIndicator; pub(crate) use list_selection_view::ColumnWidthMode; pub(crate) use list_selection_view::SelectionViewParams; +pub(crate) use list_selection_view::SideContentWidth; +pub(crate) use list_selection_view::popup_content_width; +pub(crate) use list_selection_view::side_by_side_layout_widths; mod feedback_view; pub(crate) use feedback_view::FeedbackAudience; pub(crate) use feedback_view::feedback_disabled_params; diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 9bfddd18e..ba7013c3c 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -3482,6 +3482,9 @@ impl ChatWidget { SlashCommand::Statusline => { self.open_status_line_setup(); } + SlashCommand::Theme => { + self.open_theme_picker(); + } SlashCommand::Ps => { self.add_ps_output(); } @@ -4431,6 +4434,20 @@ impl ChatWidget { self.bottom_pane.show_view(Box::new(view)); } + fn open_theme_picker(&mut self) { + let codex_home = codex_core::config::find_codex_home().ok(); + let terminal_width = self + .last_rendered_width + .get() + .and_then(|width| u16::try_from(width).ok()); + let params = crate::theme_picker::build_theme_picker_params( + self.config.tui_theme.as_deref(), + codex_home.as_deref(), + terminal_width, + ); + self.bottom_pane.show_selection_view(params); + } + /// Parses configured status-line ids into known items and collects unknown ids. /// /// Unknown ids are deduplicated in insertion order for warning messages. @@ -6338,6 +6355,11 @@ impl ChatWidget { self.config.personality = Some(personality); } + /// Set the syntax theme override in the widget's config copy. + pub(crate) fn set_tui_theme(&mut self, theme: Option) { + self.config.tui_theme = theme; + } + /// Set the model in the widget's config copy and stored collaboration mode. pub(crate) fn set_model(&mut self, model: &str) { self.current_collaboration_mode = diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_modal_exec.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_modal_exec.snap index 1c6a3ef13..055a6292f 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_modal_exec.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_modal_exec.snap @@ -27,6 +27,9 @@ Buffer { x: 73, y: 4, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, x: 2, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: ITALIC, x: 7, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 4, y: 7, fg: Rgb(137, 180, 250), bg: Reset, underline: Reset, modifier: NONE, + x: 8, y: 7, fg: Rgb(205, 214, 244), bg: Reset, underline: Reset, modifier: NONE, + x: 20, y: 7, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, x: 0, y: 9, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD, x: 21, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, x: 48, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, diff --git a/codex-rs/tui/src/diff_render.rs b/codex-rs/tui/src/diff_render.rs index 6fdf3a43f..aa0481d04 100644 --- a/codex-rs/tui/src/diff_render.rs +++ b/codex-rs/tui/src/diff_render.rs @@ -1,3 +1,23 @@ +//! Renders unified diffs with line numbers, gutter signs, and optional syntax +//! highlighting. +//! +//! Each `FileChange` variant (Add / Delete / Update) is rendered as a block of +//! diff lines, each prefixed by a right-aligned line number, a gutter sign +//! (`+` / `-` / ` `), and the content text. When a recognized file extension +//! is present, the content text is syntax-highlighted using +//! [`crate::render::highlight`]. +//! +//! **Highlighting strategy for `Update` diffs:** the renderer highlights each +//! hunk as a single concatenated block rather than line-by-line. This +//! preserves syntect's parser state across consecutive lines within a hunk +//! (important for multi-line strings, block comments, etc.). Cross-hunk state +//! is intentionally *not* preserved because hunks are visually separated and +//! re-synchronize at context boundaries anyway. +//! +//! **Wrapping:** long lines are hard-wrapped at the available column width. +//! Syntax-highlighted spans are split at character boundaries with styles +//! preserved across the split so that no color information is lost. + use diffy::Hunk; use ratatui::buffer::Buffer; use ratatui::layout::Rect; @@ -12,8 +32,15 @@ use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; +use unicode_width::UnicodeWidthChar; + +/// Display width of a tab character in columns. +const TAB_WIDTH: usize = 4; + use crate::exec_command::relativize_to_home; use crate::render::Insets; +use crate::render::highlight::exceeds_highlight_limits; +use crate::render::highlight::highlight_code_to_styled_spans; use crate::render::line_utils::prefix_lines; use crate::render::renderable::ColumnRenderable; use crate::render::renderable::InsetRenderable; @@ -21,8 +48,13 @@ use crate::render::renderable::Renderable; use codex_core::git_info::get_git_repo_root; use codex_protocol::protocol::FileChange; -// Internal representation for diff line rendering -enum DiffLineType { +/// Classifies a diff line for gutter sign rendering and style selection. +/// +/// `Insert` renders with a `+` sign and green text, `Delete` with `-` and red +/// text (plus dim overlay when syntax-highlighted), and `Context` with a space +/// and default styling. +#[derive(Clone, Copy)] +pub(crate) enum DiffLineType { Insert, Delete, Context, @@ -42,13 +74,13 @@ impl DiffSummary { impl Renderable for FileChange { fn render(&self, area: Rect, buf: &mut Buffer) { let mut lines = vec![]; - render_change(self, &mut lines, area.width as usize); + render_change(self, &mut lines, area.width as usize, None); Paragraph::new(lines).render(area, buf); } fn desired_height(&self, width: u16) -> u16 { let mut lines = vec![]; - render_change(self, &mut lines, width as usize); + render_change(self, &mut lines, width as usize, None); lines.len() as u16 } } @@ -185,47 +217,100 @@ fn render_changes_block(rows: Vec, wrap_cols: usize, cwd: &Path) -> Vec>, width: usize) { +/// Detect the programming language for a file path by its extension. +/// Returns the raw extension string for `normalize_lang` / `find_syntax` +/// to resolve downstream. +fn detect_lang_for_path(path: &Path) -> Option { + let ext = path.extension()?.to_str()?; + Some(ext.to_string()) +} + +fn render_change( + change: &FileChange, + out: &mut Vec>, + width: usize, + lang: Option<&str>, +) { match change { FileChange::Add { content } => { + // Pre-highlight the entire file content as a whole. + let syntax_lines = lang.and_then(|l| highlight_code_to_styled_spans(content, l)); let line_number_width = line_number_width(content.lines().count()); for (i, raw) in content.lines().enumerate() { - out.extend(push_wrapped_diff_line( - i + 1, - DiffLineType::Insert, - raw, - width, - line_number_width, - )); + let syn = syntax_lines.as_ref().and_then(|sl| sl.get(i)); + if let Some(spans) = syn { + out.extend(push_wrapped_diff_line_with_syntax( + i + 1, + DiffLineType::Insert, + raw, + width, + line_number_width, + spans, + )); + } else { + out.extend(push_wrapped_diff_line( + i + 1, + DiffLineType::Insert, + raw, + width, + line_number_width, + )); + } } } FileChange::Delete { content } => { + let syntax_lines = lang.and_then(|l| highlight_code_to_styled_spans(content, l)); let line_number_width = line_number_width(content.lines().count()); for (i, raw) in content.lines().enumerate() { - out.extend(push_wrapped_diff_line( - i + 1, - DiffLineType::Delete, - raw, - width, - line_number_width, - )); + let syn = syntax_lines.as_ref().and_then(|sl| sl.get(i)); + if let Some(spans) = syn { + out.extend(push_wrapped_diff_line_with_syntax( + i + 1, + DiffLineType::Delete, + raw, + width, + line_number_width, + spans, + )); + } else { + out.extend(push_wrapped_diff_line( + i + 1, + DiffLineType::Delete, + raw, + width, + line_number_width, + )); + } } } FileChange::Update { unified_diff, .. } => { if let Ok(patch) = diffy::Patch::from_str(unified_diff) { let mut max_line_number = 0; + let mut total_diff_bytes: usize = 0; + let mut total_diff_lines: usize = 0; for h in patch.hunks() { let mut old_ln = h.old_range().start(); let mut new_ln = h.new_range().start(); for l in h.lines() { + let text = match l { + diffy::Line::Insert(t) + | diffy::Line::Delete(t) + | diffy::Line::Context(t) => t, + }; + total_diff_bytes += text.len(); + total_diff_lines += 1; match l { diffy::Line::Insert(_) => { max_line_number = max_line_number.max(new_ln); @@ -243,6 +328,16 @@ fn render_change(change: &FileChange, out: &mut Vec>, width: usi } } } + + // Skip per-line syntax highlighting when the patch is too + // large — avoids thousands of parser initializations that + // would stall rendering on big diffs. + let diff_lang = if exceeds_highlight_limits(total_diff_bytes, total_diff_lines) { + None + } else { + lang + }; + let line_number_width = line_number_width(max_line_number); let mut is_first_hunk = true; for h in patch.hunks() { @@ -253,41 +348,93 @@ fn render_change(change: &FileChange, out: &mut Vec>, width: usi } is_first_hunk = false; + // Highlight each hunk as a single block so syntect parser + // state is preserved across consecutive lines. + let hunk_syntax_lines = diff_lang.and_then(|language| { + let hunk_text: String = h + .lines() + .iter() + .map(|line| match line { + diffy::Line::Insert(text) + | diffy::Line::Delete(text) + | diffy::Line::Context(text) => *text, + }) + .collect(); + let syntax_lines = highlight_code_to_styled_spans(&hunk_text, language)?; + (syntax_lines.len() == h.lines().len()).then_some(syntax_lines) + }); + let mut old_ln = h.old_range().start(); let mut new_ln = h.new_range().start(); - for l in h.lines() { + for (line_idx, l) in h.lines().iter().enumerate() { + let syntax_spans = hunk_syntax_lines + .as_ref() + .and_then(|syntax_lines| syntax_lines.get(line_idx)); match l { diffy::Line::Insert(text) => { let s = text.trim_end_matches('\n'); - out.extend(push_wrapped_diff_line( - new_ln, - DiffLineType::Insert, - s, - width, - line_number_width, - )); + if let Some(syn) = syntax_spans { + out.extend(push_wrapped_diff_line_with_syntax( + new_ln, + DiffLineType::Insert, + s, + width, + line_number_width, + syn, + )); + } else { + out.extend(push_wrapped_diff_line( + new_ln, + DiffLineType::Insert, + s, + width, + line_number_width, + )); + } new_ln += 1; } diffy::Line::Delete(text) => { let s = text.trim_end_matches('\n'); - out.extend(push_wrapped_diff_line( - old_ln, - DiffLineType::Delete, - s, - width, - line_number_width, - )); + if let Some(syn) = syntax_spans { + out.extend(push_wrapped_diff_line_with_syntax( + old_ln, + DiffLineType::Delete, + s, + width, + line_number_width, + syn, + )); + } else { + out.extend(push_wrapped_diff_line( + old_ln, + DiffLineType::Delete, + s, + width, + line_number_width, + )); + } old_ln += 1; } diffy::Line::Context(text) => { let s = text.trim_end_matches('\n'); - out.extend(push_wrapped_diff_line( - new_ln, - DiffLineType::Context, - s, - width, - line_number_width, - )); + if let Some(syn) = syntax_spans { + out.extend(push_wrapped_diff_line_with_syntax( + new_ln, + DiffLineType::Context, + s, + width, + line_number_width, + syn, + )); + } else { + out.extend(push_wrapped_diff_line( + new_ln, + DiffLineType::Context, + s, + width, + line_number_width, + )); + } old_ln += 1; new_ln += 1; } @@ -342,68 +489,213 @@ pub(crate) fn calculate_add_remove_from_diff(diff: &str) -> (usize, usize) { } } -fn push_wrapped_diff_line( +/// Render a single diff line (no syntax highlighting) as one or more wrapped +/// ratatui `Line`s. The first output line carries the gutter sign; continuation +/// lines are indented under the line number column. +pub(crate) fn push_wrapped_diff_line( line_number: usize, kind: DiffLineType, text: &str, width: usize, line_number_width: usize, +) -> Vec> { + push_wrapped_diff_line_inner(line_number, kind, text, width, line_number_width, None) +} + +/// Render a single diff line with pre-computed syntax spans. The sign character +/// uses the diff color; content gets syntax colors with a dim overlay for delete +/// lines. +pub(crate) fn push_wrapped_diff_line_with_syntax( + line_number: usize, + kind: DiffLineType, + text: &str, + width: usize, + line_number_width: usize, + syntax_spans: &[RtSpan<'static>], +) -> Vec> { + push_wrapped_diff_line_inner( + line_number, + kind, + text, + width, + line_number_width, + Some(syntax_spans), + ) +} + +fn push_wrapped_diff_line_inner( + line_number: usize, + kind: DiffLineType, + text: &str, + width: usize, + line_number_width: usize, + syntax_spans: Option<&[RtSpan<'static>]>, ) -> Vec> { let ln_str = line_number.to_string(); - let mut remaining_text: &str = text; // Reserve a fixed number of spaces (equal to the widest line number plus a // trailing spacer) so the sign column stays aligned across the diff block. let gutter_width = line_number_width.max(1); let prefix_cols = gutter_width + 1; - let mut first = true; let (sign_char, line_style) = match kind { DiffLineType::Insert => ('+', style_add()), DiffLineType::Delete => ('-', style_del()), DiffLineType::Context => (' ', style_context()), }; - let mut lines: Vec> = Vec::new(); - loop { - // Fit the content for the current terminal row: - // compute how many columns are available after the prefix, then split - // at a UTF-8 character boundary so this row's chunk fits exactly. + // When we have syntax spans, compose them with the diff style for a richer + // view. The sign character keeps the diff color; content gets syntax colors + // with an overlay modifier for delete lines (dim). + if let Some(syn_spans) = syntax_spans { + let gutter = format!("{ln_str:>gutter_width$} "); + let sign = format!("{sign_char}"); + let dim_overlay = matches!(kind, DiffLineType::Delete); + + // Apply dim overlay to syntax spans if this is a delete line. + let styled: Vec> = syn_spans + .iter() + .map(|sp| { + let mut style = sp.style; + if dim_overlay { + style.add_modifier |= Modifier::DIM; + } + RtSpan::styled(sp.content.clone().into_owned(), style) + }) + .collect(); + + // Determine how many display columns remain for content after the + // gutter and sign character. let available_content_cols = width.saturating_sub(prefix_cols + 1).max(1); - let split_at_byte_index = remaining_text - .char_indices() - .nth(available_content_cols) - .map(|(i, _)| i) - .unwrap_or_else(|| remaining_text.len()); - let (chunk, rest) = remaining_text.split_at(split_at_byte_index); - remaining_text = rest; - if first { - // Build gutter (right-aligned line number plus spacer) as a dimmed span - let gutter = format!("{ln_str:>gutter_width$} "); - // Content with a sign ('+'/'-'/' ') styled per diff kind - let content = format!("{sign_char}{chunk}"); - lines.push(RtLine::from(vec![ - RtSpan::styled(gutter, style_gutter()), - RtSpan::styled(content, line_style), - ])); - first = false; - } else { - // Continuation lines keep a space for the sign column so content aligns - let gutter = format!("{:gutter_width$} ", ""); - lines.push(RtLine::from(vec![ - RtSpan::styled(gutter, style_gutter()), - RtSpan::styled(chunk.to_string(), line_style), - ])); - } - if remaining_text.is_empty() { - break; + // Wrap the styled content spans to fit within the available columns. + let wrapped_chunks = wrap_styled_spans(&styled, available_content_cols); + + let mut lines: Vec> = Vec::new(); + for (i, chunk) in wrapped_chunks.into_iter().enumerate() { + let mut row_spans: Vec> = Vec::new(); + if i == 0 { + // First line: gutter + sign + content + row_spans.push(RtSpan::styled(gutter.clone(), style_gutter())); + row_spans.push(RtSpan::styled(sign.clone(), line_style)); + } else { + // Continuation: empty gutter + two-space indent (matches + // the plain-text wrapping continuation style). + let cont_gutter = format!("{:gutter_width$} ", ""); + row_spans.push(RtSpan::styled(cont_gutter, style_gutter())); + } + row_spans.extend(chunk); + lines.push(RtLine::from(row_spans)); } + return lines; } + + let available_content_cols = width.saturating_sub(prefix_cols + 1).max(1); + let styled = vec![RtSpan::styled(text.to_string(), line_style)]; + let wrapped_chunks = wrap_styled_spans(&styled, available_content_cols); + + let mut lines: Vec> = Vec::new(); + for (i, chunk) in wrapped_chunks.into_iter().enumerate() { + let mut row_spans: Vec> = Vec::new(); + if i == 0 { + let gutter = format!("{ln_str:>gutter_width$} "); + let sign = format!("{sign_char}"); + row_spans.push(RtSpan::styled(gutter, style_gutter())); + row_spans.push(RtSpan::styled(sign, line_style)); + } else { + let cont_gutter = format!("{:gutter_width$} ", ""); + row_spans.push(RtSpan::styled(cont_gutter, style_gutter())); + } + row_spans.extend(chunk); + lines.push(RtLine::from(row_spans)); + } + lines } -fn line_number_width(max_line_number: usize) -> usize { +/// Split styled spans into chunks that fit within `max_cols` display columns. +/// +/// Returns one `Vec` per output line. Styles are preserved across +/// split boundaries so that wrapping never loses syntax coloring. +/// +/// The algorithm walks characters using their Unicode display width (with tabs +/// expanded to [`TAB_WIDTH`] columns). When a character would overflow the +/// current line, the accumulated text is flushed and a new line begins. A +/// single character wider than the remaining space forces a line break *before* +/// the character so that progress is always made (avoiding infinite loops on +/// CJK characters or tabs at the end of a line). +fn wrap_styled_spans(spans: &[RtSpan<'static>], max_cols: usize) -> Vec>> { + let mut result: Vec>> = Vec::new(); + let mut current_line: Vec> = Vec::new(); + let mut col: usize = 0; + + for span in spans { + let style = span.style; + let text = span.content.as_ref(); + let mut remaining = text; + + while !remaining.is_empty() { + // Accumulate characters until we fill the line. + let mut byte_end = 0; + let mut chars_col = 0; + + for ch in remaining.chars() { + // Tabs have no Unicode width; treat them as TAB_WIDTH columns. + let w = ch.width().unwrap_or(if ch == '\t' { TAB_WIDTH } else { 0 }); + if col + chars_col + w > max_cols { + // Adding this character would exceed the line width. + // Break here; if this is the first character in `remaining` + // we will flush/start a new line in the `byte_end == 0` + // branch below before consuming it. + break; + } + byte_end += ch.len_utf8(); + chars_col += w; + } + + if byte_end == 0 { + // Single character wider than remaining space — force onto a + // new line so we make progress. + if !current_line.is_empty() { + result.push(std::mem::take(&mut current_line)); + } + // Take at least one character to avoid an infinite loop. + let Some(ch) = remaining.chars().next() else { + break; + }; + let ch_len = ch.len_utf8(); + current_line.push(RtSpan::styled(remaining[..ch_len].to_string(), style)); + // Use fallback width 1 (not 0) so this branch always advances + // even if `ch` has unknown/zero display width. + col = ch.width().unwrap_or(if ch == '\t' { TAB_WIDTH } else { 1 }); + remaining = &remaining[ch_len..]; + continue; + } + + let (chunk, rest) = remaining.split_at(byte_end); + current_line.push(RtSpan::styled(chunk.to_string(), style)); + col += chars_col; + remaining = rest; + + // If we exactly filled or exceeded the line, start a new one. + // Do not gate on !remaining.is_empty() — the next span in the + // outer loop may still have content that must start on a fresh line. + if col >= max_cols { + result.push(std::mem::take(&mut current_line)); + col = 0; + } + } + } + + // Push the last line (always at least one, even if empty). + if !current_line.is_empty() || result.is_empty() { + result.push(current_line); + } + + result +} + +pub(crate) fn line_number_width(max_line_number: usize) -> usize { if max_line_number == 0 { 1 } else { @@ -454,6 +746,19 @@ mod tests { assert_snapshot!(name, terminal.backend()); } + fn display_width(text: &str) -> usize { + text.chars() + .map(|ch| ch.width().unwrap_or(if ch == '\t' { TAB_WIDTH } else { 0 })) + .sum() + } + + fn line_display_width(line: &RtLine<'static>) -> usize { + line.spans + .iter() + .map(|span| display_width(span.content.as_ref())) + .sum() + } + fn snapshot_lines_text(name: &str, lines: &[RtLine<'static>]) { // Convert Lines to plain text rows and trim trailing spaces so it's // easier to validate indentation visually in snapshots. @@ -471,6 +776,71 @@ mod tests { assert_snapshot!(name, text); } + fn diff_gallery_changes() -> HashMap { + let mut changes: HashMap = HashMap::new(); + + let rust_original = + "fn greet(name: &str) {\n println!(\"hello\");\n println!(\"bye\");\n}\n"; + let rust_modified = "fn greet(name: &str) {\n println!(\"hello {name}\");\n println!(\"emoji: 🚀✨ and CJK: 你好世界\");\n}\n"; + let rust_patch = diffy::create_patch(rust_original, rust_modified).to_string(); + changes.insert( + PathBuf::from("src/lib.rs"), + FileChange::Update { + unified_diff: rust_patch, + move_path: None, + }, + ); + + let py_original = "def add(a, b):\n\treturn a + b\n\nprint(add(1, 2))\n"; + let py_modified = "def add(a, b):\n\treturn a + b + 42\n\nprint(add(1, 2))\n"; + let py_patch = diffy::create_patch(py_original, py_modified).to_string(); + changes.insert( + PathBuf::from("scripts/calc.txt"), + FileChange::Update { + unified_diff: py_patch, + move_path: Some(PathBuf::from("scripts/calc.py")), + }, + ); + + changes.insert( + PathBuf::from("assets/banner.txt"), + FileChange::Add { + content: "HEADER\tVALUE\nrocket\t🚀\ncity\t東京\n".to_string(), + }, + ); + changes.insert( + PathBuf::from("examples/new_sample.rs"), + FileChange::Add { + content: "pub fn greet(name: &str) {\n println!(\"Hello, {name}!\");\n}\n" + .to_string(), + }, + ); + + changes.insert( + PathBuf::from("tmp/obsolete.log"), + FileChange::Delete { + content: "old line 1\nold line 2\nold line 3\n".to_string(), + }, + ); + changes.insert( + PathBuf::from("legacy/old_script.py"), + FileChange::Delete { + content: "def legacy(x):\n return x + 1\nprint(legacy(3))\n".to_string(), + }, + ); + + changes + } + + fn snapshot_diff_gallery(name: &str, width: u16, height: u16) { + let lines = create_diff_summary( + &diff_gallery_changes(), + &PathBuf::from("/"), + usize::from(width), + ); + snapshot_lines(name, lines, width, height); + } + #[test] fn display_path_prefers_cwd_without_git_repo() { let cwd = if cfg!(windows) { @@ -694,4 +1064,367 @@ mod tests { snapshot_lines("apply_update_block_relativizes_path", lines, 80, 10); } + + #[test] + fn ui_snapshot_syntax_highlighted_insert_wraps() { + // A long Rust line that exceeds 80 cols with syntax highlighting should + // wrap to multiple output lines rather than being clipped. + let long_rust = "fn very_long_function_name(arg_one: String, arg_two: String, arg_three: String, arg_four: String) -> Result> { Ok(arg_one) }"; + + let syntax_spans = + highlight_code_to_styled_spans(long_rust, "rust").expect("rust highlighting"); + let spans = &syntax_spans[0]; + + let lines = push_wrapped_diff_line_with_syntax( + 1, + DiffLineType::Insert, + long_rust, + 80, + line_number_width(1), + spans, + ); + + assert!( + lines.len() > 1, + "syntax-highlighted long line should wrap to multiple lines, got {}", + lines.len() + ); + + snapshot_lines("syntax_highlighted_insert_wraps", lines, 90, 10); + } + + #[test] + fn ui_snapshot_syntax_highlighted_insert_wraps_text() { + let long_rust = "fn very_long_function_name(arg_one: String, arg_two: String, arg_three: String, arg_four: String) -> Result> { Ok(arg_one) }"; + + let syntax_spans = + highlight_code_to_styled_spans(long_rust, "rust").expect("rust highlighting"); + let spans = &syntax_spans[0]; + + let lines = push_wrapped_diff_line_with_syntax( + 1, + DiffLineType::Insert, + long_rust, + 80, + line_number_width(1), + spans, + ); + + snapshot_lines_text("syntax_highlighted_insert_wraps_text", &lines); + } + + #[test] + fn ui_snapshot_diff_gallery_80x24() { + snapshot_diff_gallery("diff_gallery_80x24", 80, 24); + } + + #[test] + fn ui_snapshot_diff_gallery_94x35() { + snapshot_diff_gallery("diff_gallery_94x35", 94, 35); + } + + #[test] + fn ui_snapshot_diff_gallery_120x40() { + snapshot_diff_gallery("diff_gallery_120x40", 120, 40); + } + + #[test] + fn add_diff_uses_path_extension_for_highlighting() { + let mut changes: HashMap = HashMap::new(); + changes.insert( + PathBuf::from("highlight_add.rs"), + FileChange::Add { + content: "pub fn sum(a: i32, b: i32) -> i32 { a + b }\n".to_string(), + }, + ); + + let lines = create_diff_summary(&changes, &PathBuf::from("/"), 80); + let has_rgb = lines.iter().any(|line| { + line.spans + .iter() + .any(|s| matches!(s.style.fg, Some(ratatui::style::Color::Rgb(..)))) + }); + assert!( + has_rgb, + "add diff for .rs file should produce syntax-highlighted (RGB) spans" + ); + } + + #[test] + fn delete_diff_uses_path_extension_for_highlighting() { + let mut changes: HashMap = HashMap::new(); + changes.insert( + PathBuf::from("highlight_delete.py"), + FileChange::Delete { + content: "def scale(x):\n return x * 2\n".to_string(), + }, + ); + + let lines = create_diff_summary(&changes, &PathBuf::from("/"), 80); + let has_rgb = lines.iter().any(|line| { + line.spans + .iter() + .any(|s| matches!(s.style.fg, Some(ratatui::style::Color::Rgb(..)))) + }); + assert!( + has_rgb, + "delete diff for .py file should produce syntax-highlighted (RGB) spans" + ); + } + + #[test] + fn detect_lang_for_common_paths() { + // Standard extensions are detected. + assert!(detect_lang_for_path(Path::new("foo.rs")).is_some()); + assert!(detect_lang_for_path(Path::new("bar.py")).is_some()); + assert!(detect_lang_for_path(Path::new("app.tsx")).is_some()); + + // Extensionless files return None. + assert!(detect_lang_for_path(Path::new("Makefile")).is_none()); + assert!(detect_lang_for_path(Path::new("randomfile")).is_none()); + } + + #[test] + fn wrap_styled_spans_single_line() { + // Content that fits in one line should produce exactly one chunk. + let spans = vec![RtSpan::raw("short")]; + let result = wrap_styled_spans(&spans, 80); + assert_eq!(result.len(), 1); + } + + #[test] + fn wrap_styled_spans_splits_long_content() { + // Content wider than max_cols should produce multiple chunks. + let long_text = "a".repeat(100); + let spans = vec![RtSpan::raw(long_text)]; + let result = wrap_styled_spans(&spans, 40); + assert!( + result.len() >= 3, + "100 chars at 40 cols should produce at least 3 lines, got {}", + result.len() + ); + } + + #[test] + fn wrap_styled_spans_flushes_at_span_boundary() { + // When span A fills exactly to max_cols and span B follows, the line + // must be flushed before B starts. Otherwise B's first character lands + // on an already-full line, producing over-width output. + let style_a = Style::default().fg(Color::Red); + let style_b = Style::default().fg(Color::Blue); + let spans = vec![ + RtSpan::styled("aaaa", style_a), // 4 cols, fills line exactly at max_cols=4 + RtSpan::styled("bb", style_b), // should start on a new line + ]; + let result = wrap_styled_spans(&spans, 4); + assert_eq!( + result.len(), + 2, + "span ending exactly at max_cols should flush before next span: {result:?}" + ); + // First line should only contain the 'a' span. + let first_width: usize = result[0].iter().map(|s| s.content.chars().count()).sum(); + assert!( + first_width <= 4, + "first line should be at most 4 cols wide, got {first_width}" + ); + } + + #[test] + fn wrap_styled_spans_preserves_styles() { + // Verify that styles survive split boundaries. + let style = Style::default().fg(Color::Green); + let text = "x".repeat(50); + let spans = vec![RtSpan::styled(text, style)]; + let result = wrap_styled_spans(&spans, 20); + for chunk in &result { + for span in chunk { + assert_eq!(span.style, style, "style should be preserved across wraps"); + } + } + } + + #[test] + fn wrap_styled_spans_tabs_have_visible_width() { + // A tab should count as TAB_WIDTH columns, not zero. + // With max_cols=8, a tab (4 cols) + "abcde" (5 cols) = 9 cols → must wrap. + let spans = vec![RtSpan::raw("\tabcde")]; + let result = wrap_styled_spans(&spans, 8); + assert!( + result.len() >= 2, + "tab + 5 chars should exceed 8 cols and wrap, got {} line(s): {result:?}", + result.len() + ); + } + + #[test] + fn wrap_styled_spans_wraps_before_first_overflowing_char() { + let spans = vec![RtSpan::raw("abcd\t界")]; + let result = wrap_styled_spans(&spans, 5); + + let line_text: Vec = result + .iter() + .map(|line| { + line.iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect(); + assert_eq!(line_text, vec!["abcd", "\t", "界"]); + + let line_width = |line: &[RtSpan<'static>]| -> usize { + line.iter() + .flat_map(|span| span.content.chars()) + .map(|ch| ch.width().unwrap_or(if ch == '\t' { TAB_WIDTH } else { 0 })) + .sum() + }; + for line in &result { + assert!( + line_width(line) <= 5, + "wrapped line exceeded width 5: {line:?}" + ); + } + } + + #[test] + fn fallback_wrapping_uses_display_width_for_tabs_and_wide_chars() { + let width = 8; + let lines = push_wrapped_diff_line( + 1, + DiffLineType::Insert, + "abcd\t界🙂", + width, + line_number_width(1), + ); + + assert!(lines.len() >= 2, "expected wrapped output, got {lines:?}"); + for line in &lines { + assert!( + line_display_width(line) <= width, + "fallback wrapped line exceeded width {width}: {line:?}" + ); + } + } + + #[test] + fn large_update_diff_skips_highlighting() { + // Build a patch large enough to exceed MAX_HIGHLIGHT_LINES (10_000). + // Without the pre-check this would attempt 10k+ parser initializations. + let line_count = 10_500; + let original: String = (0..line_count).map(|i| format!("line {i}\n")).collect(); + let modified: String = (0..line_count) + .map(|i| { + if i % 2 == 0 { + format!("line {i} changed\n") + } else { + format!("line {i}\n") + } + }) + .collect(); + let patch = diffy::create_patch(&original, &modified).to_string(); + + let mut changes: HashMap = HashMap::new(); + changes.insert( + PathBuf::from("huge.rs"), + FileChange::Update { + unified_diff: patch, + move_path: None, + }, + ); + + // Should complete quickly (no per-line parser init). If guardrails + // are bypassed this would be extremely slow. + let lines = create_diff_summary(&changes, &PathBuf::from("/"), 80); + + // The diff rendered without timing out — the guardrails prevented + // thousands of per-line parser initializations. Verify we actually + // got output (the patch is non-empty). + assert!( + lines.len() > 100, + "expected many output lines from large diff, got {}", + lines.len(), + ); + + // No span should contain an RGB foreground color (syntax themes + // produce RGB; plain diff styles only use named Color variants). + for line in &lines { + for span in &line.spans { + if let Some(ratatui::style::Color::Rgb(..)) = span.style.fg { + panic!( + "large diff should not have syntax-highlighted spans, \ + got RGB color in style {:?} for {:?}", + span.style, span.content, + ); + } + } + } + } + + #[test] + fn rename_diff_uses_destination_extension_for_highlighting() { + // A rename from an unknown extension to .rs should highlight as Rust. + // Without the fix, detect_lang_for_path uses the source path (.xyzzy), + // which has no syntax definition, so highlighting is skipped. + let original = "fn main() {}\n"; + let modified = "fn main() { println!(\"hi\"); }\n"; + let patch = diffy::create_patch(original, modified).to_string(); + + let mut changes: HashMap = HashMap::new(); + changes.insert( + PathBuf::from("foo.xyzzy"), + FileChange::Update { + unified_diff: patch, + move_path: Some(PathBuf::from("foo.rs")), + }, + ); + + let lines = create_diff_summary(&changes, &PathBuf::from("/"), 80); + let has_rgb = lines.iter().any(|line| { + line.spans + .iter() + .any(|s| matches!(s.style.fg, Some(ratatui::style::Color::Rgb(..)))) + }); + assert!( + has_rgb, + "rename from .xyzzy to .rs should produce syntax-highlighted (RGB) spans" + ); + } + + #[test] + fn update_diff_preserves_multiline_highlight_state_within_hunk() { + let original = "fn demo() {\n let s = \"hello\";\n}\n"; + let modified = "fn demo() {\n let s = \"hello\nworld\";\n}\n"; + let patch = diffy::create_patch(original, modified).to_string(); + + let mut changes: HashMap = HashMap::new(); + changes.insert( + PathBuf::from("demo.rs"), + FileChange::Update { + unified_diff: patch, + move_path: None, + }, + ); + + let expected_multiline = + highlight_code_to_styled_spans(" let s = \"hello\nworld\";\n", "rust") + .expect("rust highlighting"); + let expected_style = expected_multiline + .get(1) + .and_then(|line| { + line.iter() + .find(|span| span.content.as_ref().contains("world")) + }) + .map(|span| span.style) + .expect("expected highlighted span for second multiline string line"); + + let lines = create_diff_summary(&changes, &PathBuf::from("/"), 120); + let actual_style = lines + .iter() + .flat_map(|line| line.spans.iter()) + .find(|span| span.content.as_ref().contains("world")) + .map(|span| span.style) + .expect("expected rendered diff span containing 'world'"); + + assert_eq!(actual_style, expected_style); + } } diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index c44b5eb75..87d708959 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -105,6 +105,7 @@ mod streaming; mod style; mod terminal_palette; mod text_formatting; +mod theme_picker; mod tooltips; mod tui; mod ui_consts; @@ -545,6 +546,7 @@ async fn run_ratatui_app( } else { initial_config }; + let mut missing_session_exit = |id_str: &str, action: &str| { error!("Error finding conversation path: {id_str}"); restore(); @@ -693,7 +695,7 @@ async fn run_ratatui_app( None => None, }; - let config = match &session_selection { + let mut config = match &session_selection { resume_picker::SessionSelection::Resume(_) | resume_picker::SessionSelection::Fork(_) => { load_config_or_exit_with_fallback_cwd( cli_kv_overrides.clone(), @@ -705,6 +707,17 @@ async fn run_ratatui_app( } _ => config, }; + + // Configure syntax highlighting theme from the final config — onboarding + // and resume/fork can both reload config with a different tui_theme, so + // this must happen after the last possible reload. + if let Some(w) = crate::render::highlight::set_theme_override( + config.tui_theme.clone(), + find_codex_home().ok(), + ) { + config.startup_warnings.push(w); + } + set_default_client_residency_requirement(config.enforce_residency.value()); let active_profile = config.active_profile.clone(); let should_show_trust_screen = should_show_trust_screen(&config); @@ -1186,6 +1199,50 @@ trust_level = "untrusted" Ok(()) } + /// Regression: theme must be configured from the *final* config. + /// + /// `run_ratatui_app` can reload config during onboarding and again + /// during session resume/fork. The syntax theme override (stored in + /// a `OnceLock`) must use the final config's `tui_theme`, not the + /// initial one — otherwise users resuming a thread in a project with + /// a different theme get the wrong highlighting. + /// + /// We verify the invariant indirectly: `validate_theme_name` (the + /// pure validation core of `set_theme_override`) must be called with + /// the *final* config's theme, and its warning must land in the + /// final config's `startup_warnings`. + #[tokio::test] + async fn theme_warning_uses_final_config() -> std::io::Result<()> { + use crate::render::highlight::validate_theme_name; + + let temp_dir = TempDir::new()?; + + // initial_config has a valid theme — no warning. + let initial_config = build_config(&temp_dir).await?; + assert!(initial_config.tui_theme.is_none()); + + // Simulate resume/fork reload: the final config has an invalid theme. + let mut config = build_config(&temp_dir).await?; + config.tui_theme = Some("bogus-theme".into()); + + // Theme override must use the final config (not initial_config). + // This mirrors the real call site in run_ratatui_app. + if let Some(w) = validate_theme_name(config.tui_theme.as_deref(), Some(temp_dir.path())) { + config.startup_warnings.push(w); + } + + assert_eq!( + config.startup_warnings.len(), + 1, + "warning from final config's invalid theme should be present" + ); + assert!( + config.startup_warnings[0].contains("bogus-theme"), + "warning should reference the final config's theme name" + ); + Ok(()) + } + #[tokio::test] async fn read_session_cwd_falls_back_to_session_meta() -> std::io::Result<()> { let temp_dir = TempDir::new()?; diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index 35b0e8f5f..ba8b47d5e 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -1,3 +1,4 @@ +use crate::render::highlight::highlight_code_to_lines; use crate::render::line_utils::line_to_static; use crate::wrapping::RtOptions; use crate::wrapping::adaptive_wrap_line; @@ -99,6 +100,8 @@ where pending_marker_line: bool, in_paragraph: bool, in_code_block: bool, + code_block_lang: Option, + code_block_buffer: String, wrap_width: Option, current_line_content: Option>, current_initial_indent: Vec>, @@ -124,6 +127,8 @@ where pending_marker_line: false, in_paragraph: false, in_code_block: false, + code_block_lang: None, + code_block_buffer: String::new(), wrap_width, current_line_content: None, current_initial_indent: Vec::new(), @@ -278,6 +283,16 @@ where self.push_line(Line::default()); } self.pending_marker_line = false; + + // When inside a fenced code block with a known language, accumulate + // text into the buffer for batch highlighting in end_codeblock(). + // Append verbatim — pulldown-cmark text events already contain the + // original line breaks, so inserting separators would double them. + if self.in_code_block && self.code_block_lang.is_some() { + self.code_block_buffer.push_str(&text); + return; + } + if self.in_code_block && !self.needs_newline { let has_content = self .current_line_content @@ -394,12 +409,25 @@ where self.needs_newline = false; } - fn start_codeblock(&mut self, _lang: Option, indent: Option>) { + fn start_codeblock(&mut self, lang: Option, indent: Option>) { self.flush_current_line(); if !self.text.lines.is_empty() { self.push_blank_line(); } self.in_code_block = true; + + // Extract the language token from the info string. CommonMark info + // strings can contain metadata after the language, separated by commas, + // spaces, or other delimiters (e.g. "rust,no_run", "rust title=demo"). + // Take only the first token so the syntax lookup succeeds. + let lang = lang + .as_deref() + .and_then(|s| s.split([',', ' ', '\t']).next()) + .filter(|s| !s.is_empty()) + .map(std::string::ToString::to_string); + self.code_block_lang = lang; + self.code_block_buffer.clear(); + self.indent_stack.push(IndentContext::new( vec![indent.unwrap_or_default()], None, @@ -409,6 +437,20 @@ where } fn end_codeblock(&mut self) { + // If we buffered code for a known language, syntax-highlight it now. + if let Some(lang) = self.code_block_lang.take() { + let code = std::mem::take(&mut self.code_block_buffer); + if !code.is_empty() { + let highlighted = highlight_code_to_lines(&code, &lang); + for hl_line in highlighted { + self.push_line(Line::default()); + for span in hl_line.spans { + self.push_span(span); + } + } + } + } + self.needs_newline = true; self.in_code_block = false; self.indent_stack.pop(); @@ -689,4 +731,39 @@ mod tests { "expected full URL-like token in one rendered line, got: {lines:?}" ); } + + #[test] + fn fenced_code_info_string_with_metadata_highlights() { + // CommonMark info strings like "rust,no_run" or "rust title=demo" + // contain metadata after the language token. The language must be + // extracted (first word / comma-separated token) so highlighting works. + for info in &["rust,no_run", "rust no_run", "rust title=\"demo\""] { + let markdown = format!("```{info}\nfn main() {{}}\n```\n"); + let rendered = render_markdown_text(&markdown); + let has_rgb = rendered.lines.iter().any(|line| { + line.spans + .iter() + .any(|s| matches!(s.style.fg, Some(ratatui::style::Color::Rgb(..)))) + }); + assert!( + has_rgb, + "info string \"{info}\" should still produce syntax highlighting" + ); + } + } + + #[test] + fn crlf_code_block_no_extra_blank_lines() { + // pulldown-cmark can split CRLF code blocks into multiple Text events. + // The buffer must concatenate them verbatim — no inserted separators. + let markdown = "```rust\r\nfn main() {}\r\n line2\r\n```\r\n"; + let rendered = render_markdown_text(markdown); + let lines = lines_to_strings(&rendered); + // Should be exactly two code lines; no spurious blank line between them. + assert_eq!( + lines, + vec!["fn main() {}".to_string(), " line2".to_string()], + "CRLF code block should not produce extra blank lines: {lines:?}" + ); + } } diff --git a/codex-rs/tui/src/markdown_render_tests.rs b/codex-rs/tui/src/markdown_render_tests.rs index c54969969..68daaaab7 100644 --- a/codex-rs/tui/src/markdown_render_tests.rs +++ b/codex-rs/tui/src/markdown_render_tests.rs @@ -652,10 +652,83 @@ fn link() { } #[test] -fn code_block_unhighlighted() { +fn code_block_known_lang_has_syntax_colors() { let text = render_markdown_text("```rust\nfn main() {}\n```\n"); - let expected = Text::from_iter([Line::from_iter(["", "fn main() {}"])]); - assert_eq!(text, expected); + let content: Vec = text + .lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.clone()) + .collect::() + }) + .collect(); + // Content should be preserved; ignore trailing empty line from highlighting. + let content: Vec<&str> = content + .iter() + .map(std::string::String::as_str) + .filter(|s| !s.is_empty()) + .collect(); + assert_eq!(content, vec!["fn main() {}"]); + + // At least one span should have non-default style (syntax highlighting). + let has_colored_span = text + .lines + .iter() + .flat_map(|l| l.spans.iter()) + .any(|sp| sp.style.fg.is_some()); + assert!(has_colored_span, "expected syntax-highlighted spans with color"); +} + +#[test] +fn code_block_unknown_lang_plain() { + let text = render_markdown_text("```xyzlang\nhello world\n```\n"); + let content: Vec = text + .lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.clone()) + .collect::() + }) + .collect(); + let content: Vec<&str> = content + .iter() + .map(std::string::String::as_str) + .filter(|s| !s.is_empty()) + .collect(); + assert_eq!(content, vec!["hello world"]); + + // No syntax coloring for unknown language — all spans have default style. + let has_colored_span = text + .lines + .iter() + .flat_map(|l| l.spans.iter()) + .any(|sp| sp.style.fg.is_some()); + assert!(!has_colored_span, "expected no syntax coloring for unknown lang"); +} + +#[test] +fn code_block_no_lang_plain() { + let text = render_markdown_text("```\nno lang specified\n```\n"); + let content: Vec = text + .lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.clone()) + .collect::() + }) + .collect(); + let content: Vec<&str> = content + .iter() + .map(std::string::String::as_str) + .filter(|s| !s.is_empty()) + .collect(); + assert_eq!(content, vec!["no lang specified"]); } #[test] @@ -721,16 +794,25 @@ Here is a code block that shows another fenced block: .collect::() }) .collect(); + // Filter empty trailing lines for stability; the code block may or may + // not emit a trailing blank depending on the highlighting path. + let trimmed: Vec<&str> = { + let mut v: Vec<&str> = lines.iter().map(std::string::String::as_str).collect(); + while v.last() == Some(&"") { + v.pop(); + } + v + }; assert_eq!( - lines, + trimmed, vec![ - "Here is a code block that shows another fenced block:".to_string(), - String::new(), - "```md".to_string(), - "# Inside fence".to_string(), - "- bullet".to_string(), - "- `inline code`".to_string(), - "```".to_string(), + "Here is a code block that shows another fenced block:", + "", + "```md", + "# Inside fence", + "- bullet", + "- `inline code`", + "```", ] ); } @@ -993,3 +1075,36 @@ fn nested_item_continuation_paragraph_is_indented() { ]); assert_eq!(text, expected); } + +#[test] +fn code_block_preserves_trailing_blank_lines() { + // A fenced code block with an intentional trailing blank line must keep it. + let md = "```rust\nfn main() {}\n\n```\n"; + let text = render_markdown_text(md); + let content: Vec = text + .lines + .iter() + .map(|l| { + l.spans + .iter() + .map(|s| s.content.clone()) + .collect::() + }) + .collect(); + // Should have: "fn main() {}" then "" (the blank line). + // Filter only to content lines (skip leading/trailing empty from rendering). + assert!( + content.iter().any(|c| c == "fn main() {}"), + "expected code line, got {content:?}" + ); + // The trailing blank line inside the fence should be preserved. + let code_start = content.iter().position(|c| c == "fn main() {}").unwrap(); + assert!( + content.len() > code_start + 1, + "expected a line after 'fn main() {{}}' but content ends: {content:?}" + ); + assert_eq!( + content[code_start + 1], "", + "trailing blank line inside code fence was lost: {content:?}" + ); +} diff --git a/codex-rs/tui/src/render/highlight.rs b/codex-rs/tui/src/render/highlight.rs index e6d200cc3..b0898ffe0 100644 --- a/codex-rs/tui/src/render/highlight.rs +++ b/codex-rs/tui/src/render/highlight.rs @@ -1,154 +1,574 @@ +//! Syntax highlighting engine for the TUI. +//! +//! Wraps [syntect] with the [two_face] grammar and theme bundles to provide +//! ~250-language syntax highlighting and 32 bundled color themes. The module +//! owns four process-global singletons: +//! +//! | Singleton | Type | Purpose | +//! |---|---|---| +//! | `SYNTAX_SET` | `OnceLock` | Grammar database, immutable after init | +//! | `THEME` | `OnceLock>` | Active color theme, swappable at runtime | +//! | `THEME_OVERRIDE` | `OnceLock>` | Persisted user preference (write-once) | +//! | `CODEX_HOME` | `OnceLock>` | Root for custom `.tmTheme` discovery | +//! +//! **Lifecycle:** call [`set_theme_override`] once at startup (after the final +//! config is resolved) to persist the user preference and seed the `THEME` +//! lock. After that, [`set_syntax_theme`] and [`current_syntax_theme`] can +//! swap/snapshot the theme for live preview. All highlighting functions read +//! the theme via `theme_lock()`. +//! +//! **Guardrails:** inputs exceeding 512 KB or 10 000 lines are rejected early +//! (returns `None`) to prevent pathological CPU/memory usage. Callers must +//! fall back to plain unstyled text. + +use ratatui::style::Color as RtColor; +use ratatui::style::Modifier; use ratatui::style::Style; -use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::text::Span; +use std::path::Path; +use std::path::PathBuf; use std::sync::OnceLock; -use tree_sitter_highlight::Highlight; -use tree_sitter_highlight::HighlightConfiguration; -use tree_sitter_highlight::HighlightEvent; -use tree_sitter_highlight::Highlighter; +use std::sync::RwLock; +use syntect::easy::HighlightLines; +use syntect::highlighting::FontStyle; +use syntect::highlighting::Style as SyntectStyle; +use syntect::highlighting::Theme; +use syntect::highlighting::ThemeSet; +use syntect::parsing::SyntaxReference; +use syntect::parsing::SyntaxSet; +use syntect::util::LinesWithEndings; +use two_face::theme::EmbeddedThemeName; -// Ref: https://github.com/tree-sitter/tree-sitter-bash/blob/master/queries/highlights.scm -#[derive(Copy, Clone)] -enum BashHighlight { - Comment, - Constant, - Embedded, - Function, - Keyword, - Number, - Operator, - Property, - String, +// -- Global singletons ------------------------------------------------------- + +static SYNTAX_SET: OnceLock = OnceLock::new(); +static THEME: OnceLock> = OnceLock::new(); +static THEME_OVERRIDE: OnceLock> = OnceLock::new(); +static CODEX_HOME: OnceLock> = OnceLock::new(); + +fn syntax_set() -> &'static SyntaxSet { + SYNTAX_SET.get_or_init(two_face::syntax::extra_newlines) } -impl BashHighlight { - const ALL: [Self; 9] = [ - Self::Comment, - Self::Constant, - Self::Embedded, - Self::Function, - Self::Keyword, - Self::Number, - Self::Operator, - Self::Property, - Self::String, - ]; - - const fn as_str(self) -> &'static str { - match self { - Self::Comment => "comment", - Self::Constant => "constant", - Self::Embedded => "embedded", - Self::Function => "function", - Self::Keyword => "keyword", - Self::Number => "number", - Self::Operator => "operator", - Self::Property => "property", - Self::String => "string", +/// Set the user-configured syntax theme override and codex home path. +/// +/// Call this with the **final resolved config** (after onboarding, resume, and +/// fork reloads complete). The first call persists `name` and `codex_home` in +/// `OnceLock`s used by startup/default theme resolution. +/// +/// 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. +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 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() { + set_syntax_theme(resolve_theme_with_override( + name.as_deref(), + codex_home.as_deref(), + )); + } + 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()); } } + warning +} - fn style(self) -> Style { - match self { - Self::Comment | Self::Operator | Self::String => Style::default().dim(), - _ => Style::default(), - } +/// Check whether a theme name resolves to a bundled theme or a custom +/// `.tmTheme` file. Returns a user-facing warning when it does not. +pub(crate) fn validate_theme_name(name: Option<&str>, codex_home: Option<&Path>) -> Option { + let name = name?; + let custom_theme_path_display = codex_home + .map(|home| custom_theme_path(name, home).display().to_string()) + .unwrap_or_else(|| format!("$CODEX_HOME/themes/{name}.tmTheme")); + // Bundled themes always resolve. + if parse_theme_name(name).is_some() { + return None; } -} - -static HIGHLIGHT_CONFIG: OnceLock = OnceLock::new(); - -fn highlight_names() -> &'static [&'static str] { - static NAMES: OnceLock<[&'static str; BashHighlight::ALL.len()]> = OnceLock::new(); - NAMES - .get_or_init(|| BashHighlight::ALL.map(BashHighlight::as_str)) - .as_slice() -} - -fn highlight_config() -> &'static HighlightConfiguration { - HIGHLIGHT_CONFIG.get_or_init(|| { - let language = tree_sitter_bash::LANGUAGE.into(); - #[expect(clippy::expect_used)] - let mut config = HighlightConfiguration::new( - language, - "bash", - tree_sitter_bash::HIGHLIGHT_QUERY, - "", - "", - ) - .expect("load bash highlight query"); - config.configure(highlight_names()); - config - }) -} - -fn highlight_for(highlight: Highlight) -> BashHighlight { - BashHighlight::ALL[highlight.0] -} - -fn push_segment(lines: &mut Vec>, segment: &str, style: Option