From f8df49a196c4da4b6b482c727aa087fa7e07f5e0 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Sat, 7 Nov 2026 01:18:47 +0000 Subject: [PATCH] Reuse prepared UI trees for footer rendering (#60028) ## Why Footer width measurement and rendering each prepared the same UI tree again. ## What changed - Prepare footer extension content once and share the owned `PreparedUiTree` through `Arc`. - Resolve colors against the active theme at render time, caching each color resolution within a render pass. - Share leaf text buffers across prepared chunks instead of copying their slices. ## Testing Add a test verifying that chunks from a clean text leaf retain slices of the original buffer. Update existing UI tree rendering tests to use the owned prepared representation. GitOrigin-RevId: daba035c291338f3e0036e9ad4f2fd26f56a3910 --- .../src/bottom_pane/footer/footer_content.rs | 10 +- .../tui/src/bottom_pane/status_line_style.rs | 2 +- codex-rs/tui/src/ui_tree.rs | 1 + codex-rs/tui/src/ui_tree/render.rs | 75 ++++---- codex-rs/tui/src/ui_tree/render/prepare.rs | 80 ++++----- .../tui/src/ui_tree/render/prepare_tests.rs | 13 +- codex-rs/tui/src/ui_tree/render/text.rs | 168 ++++++++++++------ codex-rs/tui/src/ui_tree/render/text_tests.rs | 18 ++ codex-rs/tui/src/ui_tree/theme.rs | 19 +- codex-rs/tui/src/ui_tree_tests.rs | 52 +++--- 10 files changed, 252 insertions(+), 186 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/footer/footer_content.rs b/codex-rs/tui/src/bottom_pane/footer/footer_content.rs index 2ba57376fe..46078d1d1f 100644 --- a/codex-rs/tui/src/bottom_pane/footer/footer_content.rs +++ b/codex-rs/tui/src/bottom_pane/footer/footer_content.rs @@ -5,10 +5,12 @@ use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::text::Line; use ratatui::widgets::Widget; +use std::sync::Arc; use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; use crate::render::highlight::with_syntax_theme; use crate::terminal_hyperlinks::HyperlinkLine; +use crate::ui_tree::PreparedUiTree; use crate::ui_tree::UiTree; /// Owned content for the footer's negotiated left-hand area. @@ -16,12 +18,14 @@ use crate::ui_tree::UiTree; pub(crate) enum FooterContent { Line(Line<'static>), HyperlinkLine(HyperlinkLine), - Extension { content: UiTree }, + Extension { content: Arc }, } impl From for FooterContent { - fn from(content: UiTree) -> Self { - Self::Extension { content } + fn from(tree: UiTree) -> Self { + Self::Extension { + content: Arc::new(tree.prepare()), + } } } diff --git a/codex-rs/tui/src/bottom_pane/status_line_style.rs b/codex-rs/tui/src/bottom_pane/status_line_style.rs index 8fa1b120c2..2efa1a07db 100644 --- a/codex-rs/tui/src/bottom_pane/status_line_style.rs +++ b/codex-rs/tui/src/bottom_pane/status_line_style.rs @@ -19,7 +19,7 @@ pub(crate) const STATUS_LINE_SEPARATOR: &str = " · "; const STATUS_LINE_COLOR_SATURATION_PERCENT: u16 = 85; const STATUS_LINE_COLOR_BRIGHTNESS_PERCENT: u16 = 100; -#[derive(Clone, Copy, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)] +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, serde::Deserialize, serde::Serialize)] pub(crate) enum StatusLineAccent { #[serde(rename = "status-model")] Model, diff --git a/codex-rs/tui/src/ui_tree.rs b/codex-rs/tui/src/ui_tree.rs index 9c3c855946..e7aa362de4 100644 --- a/codex-rs/tui/src/ui_tree.rs +++ b/codex-rs/tui/src/ui_tree.rs @@ -4,6 +4,7 @@ mod decode; mod render; mod theme; +pub(crate) use render::PreparedUiTree; use theme::Color; use theme::TextWeight; diff --git a/codex-rs/tui/src/ui_tree/render.rs b/codex-rs/tui/src/ui_tree/render.rs index d4b92538f2..c587bcde9e 100644 --- a/codex-rs/tui/src/ui_tree/render.rs +++ b/codex-rs/tui/src/ui_tree/render.rs @@ -18,39 +18,34 @@ use crate::terminal_hyperlinks::HyperlinkLine; use crate::terminal_hyperlinks::TerminalHyperlink; use ratatui::layout::Size; use ratatui::style::Modifier; -use ratatui::style::Style; use ratatui::text::Span; -use std::borrow::Cow; use std::ops::Range; use syntect::highlighting::Theme; use vtparse::CollectingVTActor; use vtparse::VTAction; use vtparse::VTParser; -pub(crate) struct PreparedUiTree<'a>(Option>); +#[derive(Debug, PartialEq)] +pub(crate) struct PreparedUiTree(Option); impl UiTree { - pub(crate) fn prepare(&self, theme: &Theme) -> PreparedUiTree<'_> { - let theme = RenderTheme::new(theme); - PreparedUiTree(self.0.as_ref().and_then(|node| { - PreparedNode::from_node(node, Style::default(), /*href*/ None, &theme) - })) - } - - pub(crate) fn render_line(&self, width: u16, theme: &Theme) -> HyperlinkLine { - self.prepare(theme).render_line(width) + pub(crate) fn prepare(self) -> PreparedUiTree { + PreparedUiTree( + self.0 + .and_then(|node| PreparedNode::from_node(node, Paint::default())), + ) } } -impl PreparedUiTree<'_> { - pub(crate) fn render_line(&self, width: u16) -> HyperlinkLine { - self.render_lines(Size::new(width, /*height*/ 1)) +impl PreparedUiTree { + pub(crate) fn render_line(&self, width: u16, theme: &Theme) -> HyperlinkLine { + self.render_lines(Size::new(width, /*height*/ 1), theme) .pop() .unwrap_or_default() } /// Fit the tree to the viewport, marking clipped rows and vertical overflow with an unlinked ellipsis. - pub(crate) fn render_lines(&self, size: Size) -> Vec { + pub(crate) fn render_lines(&self, size: Size, theme: &Theme) -> Vec { let Some(item) = &self.0 else { return Vec::new(); }; @@ -60,7 +55,7 @@ impl PreparedUiTree<'_> { let width = usize::from(size.width); let height = usize::from(size.height); // The extra row distinguishes an exact fit from vertical overflow. - let mut lines = layout_node(item, width, height + 1); + let mut lines = layout_node(item, width, height + 1, &mut RenderTheme::new(theme)); let more = lines.len() > height; lines.truncate(height); for (index, line) in lines.iter_mut().enumerate() { @@ -73,9 +68,14 @@ impl PreparedUiTree<'_> { } } -fn layout_node(node: &PreparedNode<'_>, width: usize, limit: usize) -> Vec { +fn layout_node( + node: &PreparedNode, + width: usize, + limit: usize, + theme: &mut RenderTheme<'_>, +) -> Vec { match &node.kind { - NodeKind::Text(text) => text.layout(width, limit), + NodeKind::Text(text) => text.layout(width, limit, theme), NodeKind::Row { children, gap, @@ -94,7 +94,8 @@ fn layout_node(node: &PreparedNode<'_>, width: usize, limit: usize) -> Vec, width: usize, limit: usize) -> Vec, width: usize, limit: usize) -> Vec], + children: &[PreparedNode], width: usize, limit: usize, gap: usize, - paint: Paint<'_>, + paint: &Paint, + theme: &mut RenderTheme<'_>, ) -> Vec { let bounded = |cells: u64| cells.min((width + 1) as u64) as usize; let mut reserved = children @@ -140,7 +143,7 @@ fn layout_band( let available = bounded(child.widths.preferred) .min(width.saturating_sub(offset + reserved)) .max(minimum); - let fragments = layout_node(child, available.clamp(1, width), limit); + let fragments = layout_node(child, available.clamp(1, width), limit, theme); if rows.len() < fragments.len() { rows.resize_with(fragments.len(), Default::default); } @@ -149,8 +152,8 @@ fn layout_band( let start = offset.min(width + 1); if start > *end { line.line - .push_span(Span::styled(" ".repeat(start - *end), paint.style)); - if let Some(href) = paint.href { + .push_span(Span::styled(" ".repeat(start - *end), paint.style(theme))); + if let Some(href) = &paint.href { push_link(line, *end..start, href); } *end = start; @@ -175,14 +178,17 @@ fn layout_band( rows.into_iter().map(|(line, _)| line).collect() } -fn sanitize_leaf(raw: &str) -> Cow<'_, str> { +fn sanitize_leaf(mut raw: String) -> String { // Built-in values can bypass the wire decoder's text limit. - let raw = &raw[..raw.floor_char_boundary(MAX_TEXT_BYTES)]; + if raw.len() > MAX_TEXT_BYTES { + raw.truncate(raw.floor_char_boundary(MAX_TEXT_BYTES)); + raw.shrink_to_fit(); + } if raw .chars() .all(|ch| !ch.is_control() || matches!(ch, '\n' | '\t')) { - return Cow::Borrowed(raw); + return raw; } let mut actions = CollectingVTActor::default(); VTParser::new().parse(raw.as_bytes(), &mut actions); @@ -209,9 +215,9 @@ fn push_link(line: &mut HyperlinkLine, columns: Range, href: &str) { } impl TextProps { - fn style(&self, mut style: Style, theme: &RenderTheme<'_>) -> Style { - if let Some(color) = &self.color { - style = style.fg(color.resolve(theme)); + fn apply_to(&self, paint: &mut Paint) { + if let Some(color) = self.color { + paint.foreground = Some(color); } for (enabled, modifier) in [ ( @@ -222,11 +228,10 @@ impl TextProps { (self.underline, Modifier::UNDERLINED), ] { match enabled { - Some(true) => style = style.add_modifier(modifier), - Some(false) => style = style.remove_modifier(modifier), + Some(true) => paint.modifiers = paint.modifiers.add_modifier(modifier), + Some(false) => paint.modifiers = paint.modifiers.remove_modifier(modifier), None => {} } } - style } } diff --git a/codex-rs/tui/src/ui_tree/render/prepare.rs b/codex-rs/tui/src/ui_tree/render/prepare.rs index a9df9056f5..8a870e9743 100644 --- a/codex-rs/tui/src/ui_tree/render/prepare.rs +++ b/codex-rs/tui/src/ui_tree/render/prepare.rs @@ -5,53 +5,44 @@ use super::text::Paint; use super::text::PreparedText; use crate::ui_tree::RowWrap; use crate::ui_tree::UiNode; -use crate::ui_tree::theme::RenderTheme; -use ratatui::style::Style; -pub(super) struct PreparedNode<'a> { - pub(super) kind: NodeKind<'a>, +#[derive(Debug, PartialEq)] +pub(super) struct PreparedNode { + pub(super) kind: NodeKind, pub(super) widths: Widths, } +#[derive(Debug, PartialEq)] pub(super) struct Widths { pub(super) minimum: u64, pub(super) preferred: u64, } -pub(super) enum NodeKind<'a> { - Text(PreparedText<'a>), +#[derive(Debug, PartialEq)] +pub(super) enum NodeKind { + Text(PreparedText), Row { - children: Vec>, + children: Vec, gap: u64, wrap: RowWrap, - paint: Paint<'a>, + paint: Paint, }, } -impl<'a> PreparedNode<'a> { - pub(super) fn from_node( - node: &'a UiNode, - style: Style, - href: Option<&'a str>, - theme: &RenderTheme<'_>, - ) -> Option { +impl PreparedNode { + pub(super) fn from_node(node: UiNode, paint: Paint) -> Option { let mut items = Vec::new(); let mut inline = InlineText::default(); - Self::collect_inline(&mut items, &mut inline, node, style, href, theme); + Self::collect_inline(&mut items, &mut inline, node, paint.clone()); items.extend(Self::text(inline)); if items.len() <= 1 { items.pop() } else { - Self::row( - items, - /*gap*/ 0, - RowWrap::Nowrap, - Paint { style, href }, - ) + Self::row(items, /*gap*/ 0, RowWrap::Nowrap, paint) } } - fn text(inline: InlineText<'a>) -> Option { + fn text(inline: InlineText) -> Option { let text = inline.finish(); if text.lines.is_empty() { return None; @@ -75,7 +66,7 @@ impl<'a> PreparedNode<'a> { }) } - fn row(mut children: Vec, gap: u64, wrap: RowWrap, paint: Paint<'a>) -> Option { + fn row(mut children: Vec, gap: u64, wrap: RowWrap, paint: Paint) -> Option { children.retain(|child| { child.widths.preferred > 0 || match &child.kind { @@ -115,59 +106,52 @@ impl<'a> PreparedNode<'a> { fn collect_inline( items: &mut Vec, - inline: &mut InlineText<'a>, - node: &'a UiNode, - mut style: Style, - mut href: Option<&'a str>, - theme: &RenderTheme<'_>, + inline: &mut InlineText, + node: UiNode, + mut paint: Paint, ) { let children = match node { UiNode::String(raw) => { - inline.push_leaf(raw, Paint { style, href }); + inline.push_leaf(raw, paint); return; } UiNode::Row { props, children } => { items.extend(Self::text(std::mem::take(inline))); - if let Some(color) = &props.background { - style = style.bg(color.resolve(theme)); + if let Some(color) = props.background { + paint.background = Some(color); } let children = children - .iter() + .into_iter() .flatten() - .filter_map(|node| Self::from_node(node, style, href, theme)) + .filter_map(|node| Self::from_node(node, paint.clone())) .collect(); - items.extend(Self::row( - children, - props.gap, - props.wrap, - Paint { style, href }, - )); + items.extend(Self::row(children, props.gap, props.wrap, paint)); return; } UiNode::Text { props, children } => { - style = props.style(style, theme); + props.apply_to(&mut paint); children } UiNode::Link { props, children } => { - href = Some(&props.href.0); - style = style.underlined(); + paint.href = Some(props.href.0.into()); + paint.modifiers = paint.modifiers.underlined(); children } UiNode::Bold { children } => { - style = style.bold(); + paint.modifiers = paint.modifiers.bold(); children } UiNode::Italic { children } => { - style = style.italic(); + paint.modifiers = paint.modifiers.italic(); children } UiNode::Underline { children } => { - style = style.underlined(); + paint.modifiers = paint.modifiers.underlined(); children } }; - for node in children.iter().flatten() { - Self::collect_inline(items, inline, node, style, href, theme); + for node in children.into_iter().flatten() { + Self::collect_inline(items, inline, node, paint.clone()); } } } diff --git a/codex-rs/tui/src/ui_tree/render/prepare_tests.rs b/codex-rs/tui/src/ui_tree/render/prepare_tests.rs index 440cc671c6..a3bf77f819 100644 --- a/codex-rs/tui/src/ui_tree/render/prepare_tests.rs +++ b/codex-rs/tui/src/ui_tree/render/prepare_tests.rs @@ -1,12 +1,10 @@ #![cfg(test)] +use super::Paint; use super::PreparedNode; use crate::ui_tree::UiNode; -use crate::ui_tree::theme::RenderTheme; use pretty_assertions::assert_eq; -use ratatui::style::Style; use serde_json::json; -use syntect::highlighting::Theme; #[test] fn a_row_measures_whole_children_and_intervening_gaps() { @@ -17,13 +15,6 @@ fn a_row_measures_whole_children_and_intervening_gaps() { ] })) .unwrap(); - let theme = Theme::default(); - let node = PreparedNode::from_node( - &node, - Style::default(), - /*href*/ None, - &RenderTheme::new(&theme), - ) - .unwrap(); + let node = PreparedNode::from_node(node, Paint::default()).unwrap(); assert_eq!((node.widths.minimum, node.widths.preferred), (16, 22)); } diff --git a/codex-rs/tui/src/ui_tree/render/text.rs b/codex-rs/tui/src/ui_tree/render/text.rs index 985f5278c3..fcc5539d96 100644 --- a/codex-rs/tui/src/ui_tree/render/text.rs +++ b/codex-rs/tui/src/ui_tree/render/text.rs @@ -1,13 +1,16 @@ //! Prepared inline text retains styled chunks and their legal line breaks across leaves. +use super::RenderTheme; use super::push_link; use super::sanitize_leaf; use crate::line_truncation::span_width; use crate::terminal_hyperlinks::HyperlinkLine; +use crate::ui_tree::theme::Color; use ratatui::style::Style; use ratatui::text::Span; -use std::borrow::Cow; +use std::ops::Deref; use std::ops::Range; +use std::sync::Arc; use textwrap::core::Fragment; use textwrap::wrap_algorithms::wrap_first_fit; use unicode_linebreak::linebreaks; @@ -16,48 +19,93 @@ use unicode_segmentation::GraphemeCursor; use unicode_segmentation::GraphemeIncomplete; use unicode_segmentation::UnicodeSegmentation; -#[derive(Clone, Copy, Debug)] -pub(super) struct Paint<'a> { - pub(super) style: Style, - pub(super) href: Option<&'a str>, +#[derive(Clone, Debug, Default, PartialEq)] +pub(super) struct Paint { + pub(super) modifiers: Style, + pub(super) foreground: Option, + pub(super) background: Option, + pub(super) href: Option>, } -#[derive(Debug)] -struct Run<'a> { - text: Cow<'a, str>, - paint: Paint<'a>, -} - -fn slice<'a>(text: &Cow<'a, str>, range: Range) -> Cow<'a, str> { - match text { - Cow::Borrowed(raw) => Cow::Borrowed(&raw[range]), - Cow::Owned(raw) => Cow::Owned(raw[range].to_owned()), +impl Paint { + pub(super) fn style(&self, theme: &mut RenderTheme<'_>) -> Style { + let mut style = self.modifiers; + style.fg = self.foreground.map(|c| theme.resolve(c)); + style.bg = self.background.map(|c| theme.resolve(c)); + style } } -#[derive(Debug, Default)] -pub(super) struct Chunk<'a> { - runs: Vec>, +#[derive(Debug, PartialEq)] +struct Run { + text: RunText, + paint: Paint, +} + +// Splitting a leaf at style, whitespace or wrap boundaries shares its original buffer. +#[derive(Debug)] +enum RunText { + Slice(Arc, Range), + Space, +} + +impl RunText { + fn new(text: String) -> Self { + let len = text.len(); + Self::Slice(Arc::new(text), 0..len) + } + + fn slice(&self, range: Range) -> Self { + match self { + Self::Slice(text, bytes) => Self::Slice( + Arc::clone(text), + bytes.start + range.start..bytes.start + range.end, + ), + Self::Space => Self::Space, + } + } +} + +impl Deref for RunText { + type Target = str; + + fn deref(&self) -> &str { + match self { + Self::Slice(text, bytes) => &text[bytes.clone()], + Self::Space => " ", + } + } +} + +impl PartialEq for RunText { + fn eq(&self, other: &Self) -> bool { + **self == **other + } +} + +#[derive(Debug, Default, PartialEq)] +pub(super) struct Chunk { + runs: Vec, pub(super) width_cells: usize, - pub(super) trailing_space: Option>, + pub(super) trailing_space: Option, } -#[derive(Default)] -pub(super) struct PreparedText<'a> { +#[derive(Debug, Default, PartialEq)] +pub(super) struct PreparedText { pub(super) has_authored_break: bool, - pub(super) lines: Vec>>, + pub(super) lines: Vec>, } #[derive(Default)] -pub(super) struct InlineText<'a> { - text: PreparedText<'a>, - line: Vec>, - space: Option>, +pub(super) struct InlineText { + text: PreparedText, + line: Vec, + space: Option, } -impl<'a> InlineText<'a> { - pub(super) fn push_leaf(&mut self, raw: &'a str, paint: Paint<'a>) { - let text = sanitize_leaf(raw); +impl InlineText { + pub(super) fn push_leaf(&mut self, raw: String, paint: Paint) { + let text = RunText::new(sanitize_leaf(raw)); let mut start = 0; for (end, ch) in text .match_indices([' ', '\t', '\n']) @@ -66,26 +114,26 @@ impl<'a> InlineText<'a> { if start < end { if let Some(paint) = self.space.take() { self.line.push(Run { - text: " ".into(), + text: RunText::Space, paint, }); } self.line.push(Run { - text: slice(&text, start..end), - paint, + text: text.slice(start..end), + paint: paint.clone(), }); } if ch == "\n" { self.text.lines.push(chunks(std::mem::take(&mut self.line))); self.space = None; } else if !ch.is_empty() && !self.line.is_empty() { - self.space.get_or_insert(paint); + self.space.get_or_insert_with(|| paint.clone()); } start = end + ch.len(); } } - pub(super) fn finish(mut self) -> PreparedText<'a> { + pub(super) fn finish(mut self) -> PreparedText { self.text.has_authored_break = !self.text.lines.is_empty(); if !self.line.is_empty() { self.text.lines.push(chunks(self.line)); @@ -96,7 +144,7 @@ impl<'a> InlineText<'a> { // Most seams already fall between graphemes. Only a cluster that actually crosses a // seam needs contiguous storage, including when it spans more than two leaves. -fn join_graphemes(runs: Vec>) -> Vec> { +fn join_graphemes(runs: Vec) -> Vec { let mut runs = runs.into_iter().peekable(); let mut joined = Vec::new(); while let Some(run) = runs.next() { @@ -130,8 +178,8 @@ fn join_graphemes(runs: Vec>) -> Vec> { let prefix = run.text.len() - split; if prefix > 0 { joined.push(Run { - text: slice(&run.text, 0..prefix), - paint, + text: run.text.slice(0..prefix), + paint: paint.clone(), }); } // Resume in the growing cluster so even a long chain of combining marks @@ -153,14 +201,14 @@ fn join_graphemes(runs: Vec>) -> Vec> { } if consumed < next.text.len() { if consumed > 0 { - next.text = slice(&next.text, consumed..next.text.len()); + next.text = next.text.slice(consumed..next.text.len()); } break; } runs.next(); } joined.push(Run { - text: Cow::Owned(cluster), + text: RunText::new(cluster), paint, }); } @@ -209,7 +257,7 @@ fn break_offsets<'a>(runs: impl Iterator) -> Vec { breaks } -fn chunks(runs: Vec>) -> Vec> { +fn chunks(runs: Vec) -> Vec { let runs = join_graphemes(runs); let mut breaks = break_offsets(runs.iter().map(|run| run.text.as_ref())) .into_iter() @@ -232,7 +280,7 @@ fn chunks(runs: Vec>) -> Vec> { }; if can_break { chunk.push(&run, start..i); - if chunk.runs.last().is_some_and(|r| r.text == " ") { + if chunk.runs.last().is_some_and(|r| &*r.text == " ") { chunk.trailing_space = chunk.runs.pop().map(|r| r.paint); chunk.width_cells -= 1; } @@ -255,17 +303,20 @@ fn chunks(runs: Vec>) -> Vec> { out } -impl<'a> Chunk<'a> { - fn push(&mut self, run: &Run<'a>, range: Range) { +impl Chunk { + fn push(&mut self, run: &Run, range: Range) { if !range.is_empty() { - let text = slice(&run.text, range); - self.width_cells += span_width(&Span::raw(text.as_ref())); - self.runs.push(Run { text, ..*run }); + let text = run.text.slice(range); + self.width_cells += span_width(&Span::raw(&*text)); + self.runs.push(Run { + text, + paint: run.paint.clone(), + }); } } } -impl Fragment for Chunk<'_> { +impl Fragment for Chunk { fn width(&self) -> f64 { self.width_cells as f64 } @@ -277,25 +328,30 @@ impl Fragment for Chunk<'_> { } } -impl PreparedText<'_> { - pub(super) fn layout(&self, width: usize, limit: usize) -> Vec { +impl PreparedText { + pub(super) fn layout( + &self, + width: usize, + limit: usize, + theme: &mut RenderTheme<'_>, + ) -> Vec { self.lines .iter() .flat_map(|line| wrap_first_fit(line, &[width as f64])) .take(limit) - .map(paint_chunks) + .map(|chunks| paint_chunks(chunks, theme)) .collect() } } -fn paint_chunks(chunks: &[Chunk<'_>]) -> HyperlinkLine { +fn paint_chunks(chunks: &[Chunk], theme: &mut RenderTheme<'_>) -> HyperlinkLine { let mut line = HyperlinkLine::default(); let mut col = 0; for (index, chunk) in chunks.iter().enumerate() { for run in &chunk.runs { - let span = Span::styled(run.text.to_string(), run.paint.style); + let span = Span::styled(run.text.to_string(), run.paint.style(theme)); let end = col + span_width(&span); - if let Some(url) = run.paint.href + if let Some(url) = &run.paint.href && end > col { push_link(&mut line, col..end, url); @@ -304,10 +360,10 @@ fn paint_chunks(chunks: &[Chunk<'_>]) -> HyperlinkLine { col = end; } if index + 1 < chunks.len() - && let Some(paint) = chunk.trailing_space + && let Some(paint) = &chunk.trailing_space { - line.line.push_span(Span::styled(" ", paint.style)); - if let Some(url) = paint.href { + line.line.push_span(Span::styled(" ", paint.style(theme))); + if let Some(url) = &paint.href { push_link(&mut line, col..col + 1, url); } col += 1; diff --git a/codex-rs/tui/src/ui_tree/render/text_tests.rs b/codex-rs/tui/src/ui_tree/render/text_tests.rs index c1ffff0924..2cf66ddf59 100644 --- a/codex-rs/tui/src/ui_tree/render/text_tests.rs +++ b/codex-rs/tui/src/ui_tree/render/text_tests.rs @@ -14,3 +14,21 @@ fn styled_runs_have_the_same_breaks_as_contiguous_text() { let expected: Vec<_> = linebreaks(&text).map(|(i, _)| i).collect(); assert_eq!(super::break_offsets(runs), expected[..expected.len() - 1]); } + +#[test] +fn chunks_of_a_clean_leaf_share_its_original_buffer() { + let leaf = String::from("build ready"); + let start = leaf.as_ptr(); + let mut inline = super::InlineText::default(); + inline.push_leaf(leaf, super::Paint::default()); + let prepared = inline.finish(); + let runs: Vec<_> = prepared.lines[0] + .iter() + .map(|chunk| &*chunk.runs[0].text) + .collect(); + assert_eq!(runs, ["build", "ready"]); + assert_eq!( + [runs[0].as_ptr(), runs[1].as_ptr()], + [start, start.wrapping_add(6)] + ); +} diff --git a/codex-rs/tui/src/ui_tree/theme.rs b/codex-rs/tui/src/ui_tree/theme.rs index 78c963ca8b..d98aab72ef 100644 --- a/codex-rs/tui/src/ui_tree/theme.rs +++ b/codex-rs/tui/src/ui_tree/theme.rs @@ -9,6 +9,7 @@ use crate::render::highlight; use crate::terminal_palette; use crate::terminal_palette::StdoutColorLevel; use std::cell::OnceCell; +use std::collections::HashMap; use syntect::highlighting::Highlighter; use syntect::highlighting::Theme; use syntect::parsing::Scope; @@ -16,6 +17,7 @@ use syntect::parsing::Scope; pub(super) struct RenderTheme<'a> { theme: &'a Theme, highlighter: OnceCell>, + resolved: HashMap, } impl<'a> RenderTheme<'a> { @@ -23,9 +25,20 @@ impl<'a> RenderTheme<'a> { Self { theme, highlighter: OnceCell::new(), + resolved: HashMap::new(), } } + // A color can occur on many chunks. Match its theme scopes once per paint. + pub(super) fn resolve(&mut self, color: Color) -> TerminalColor { + if let Some(&resolved) = self.resolved.get(&color) { + return resolved; + } + let resolved = color.resolve(self); + self.resolved.insert(color, resolved); + resolved + } + // Plain text and literal colors need no TextMate scope matching. fn highlighter(&self) -> &Highlighter<'a> { self.highlighter @@ -48,7 +61,7 @@ fn terminal_color(color: Option, fallback: TerminalColor) -> Term } } -#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub(super) enum Color { Primary, @@ -69,7 +82,7 @@ pub(super) enum Color { } impl Color { - pub(super) fn resolve(&self, palette: &RenderTheme<'_>) -> TerminalColor { + fn resolve(self, palette: &RenderTheme<'_>) -> TerminalColor { let setting = |color: Option| { terminal_color( color.and_then(highlight::convert_syntect_color), @@ -108,7 +121,7 @@ impl Color { } } -#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)] #[serde(try_from = "String", into = "String")] pub(super) struct HexRgb((u8, u8, u8)); diff --git a/codex-rs/tui/src/ui_tree_tests.rs b/codex-rs/tui/src/ui_tree_tests.rs index c3c78959e3..51857a1607 100644 --- a/codex-rs/tui/src/ui_tree_tests.rs +++ b/codex-rs/tui/src/ui_tree_tests.rs @@ -14,14 +14,17 @@ fn parse(value: Value) -> serde_json::Result { value.to_string().parse() } -fn tree(value: Value) -> UiTree { - parse(value).unwrap() +fn tree(value: Value) -> PreparedUiTree { + parse(value).unwrap().prepare() } #[test] fn elements_round_trip_and_require_explicit_row_spacing() { let value = json!({"type":"row","props":{"gap":1},"children":[{"type":"link","props":{"href":"https://example.com/pr/42","title":"PR details"},"children":["12%"]}, null, " "]}); - assert_eq!(serde_json::to_value(tree(value.clone())).unwrap(), value); + assert_eq!( + serde_json::to_value(parse(value.clone()).unwrap()).unwrap(), + value + ); for href in [ "relative/path", "file:///tmp/example", @@ -204,7 +207,7 @@ fn single_line_view_marks_vertical_overflow() { #[test] fn directly_constructed_text_keeps_a_bounded_utf8_prefix() { let prefix = "x".repeat(MAX_TEXT_BYTES - 1); - let text = UiTree(Some(UiNode::String(format!("{prefix}界tail")))); + let text = UiTree(Some(UiNode::String(format!("{prefix}界tail")))).prepare(); assert_eq!( text.render_line(u16::MAX, &Theme::default()) .line @@ -227,7 +230,10 @@ fn text_and_row_share_the_color_contract() { let value = json!({"type":"row","props":{"gap":0,"background":color},"children":[ {"type":"text","props":{"color":color,"weight":"bold","italic":true,"underline":false},"children":["ok"]} ]}); - assert_eq!(serde_json::to_value(tree(value.clone())).unwrap(), value); + assert_eq!( + serde_json::to_value(parse(value.clone()).unwrap()).unwrap(), + value + ); } for props in [ json!({"weight":"semibold"}), @@ -488,9 +494,7 @@ fn whitespace_collapses_across_styled_leaves() { let node = tree(json!({"type":"text","children":[ " build\t", {"type":"bold","children":[" ready "]}, "\nnext" ]})); - let lines = node - .prepare(&Theme::default()) - .render_lines(Size::new(/*width*/ 20, /*height*/ 2)); + let lines = node.render_lines(Size::new(/*width*/ 20, /*height*/ 2), &Theme::default()); let text: Vec<_> = lines .into_iter() .map(|line| line.line.to_string()) @@ -505,12 +509,12 @@ fn whitespace_collapses_across_styled_leaves() { fn reads_row_wrap_without_moving_existing_positional_props() { let row = |props| json!({"type":"row","props":props,"children":["ok"]}); assert_eq!( - tree(row(json!([1, "red"]))), - tree(row(json!({"gap":1,"background":"red"}))) + parse(row(json!([1, "red"]))).unwrap(), + parse(row(json!({"gap":1,"background":"red"}))).unwrap() ); assert_eq!( - tree(row(json!({"gap":1,"wrap":"nowrap"}))), - tree(row(json!({"gap":1}))) + parse(row(json!({"gap":1,"wrap":"nowrap"}))).unwrap(), + parse(row(json!({"gap":1}))).unwrap() ); for invalid in [json!("wrap-reverse"), json!(null), json!({"wrap":null})] { assert!(parse(row(json!({"gap":0,"wrap":invalid}))).is_err()); @@ -530,7 +534,7 @@ fn wrapped_link_offsets_follow_each_row_and_stop_before_clipping() { (Size::new(4, 2), vec![(">12…", 1..3), (" 67", 1..3)]), (Size::new(6, 1), vec![(">1234…", 1..5)]), ] { - let lines = node.prepare(&Theme::default()).render_lines(size); + let lines = node.render_lines(size, &Theme::default()); assert_eq!(lines.len(), expected.len()); for (line, (text, columns)) in lines.into_iter().zip(expected) { assert_eq!(line.line.to_string(), text); @@ -555,9 +559,7 @@ fn styled_text_wraps_in_the_space_its_nowrap_row_assigns() { {"type":"link","props":{"href":href},"children":["was green"]} ]}, "!" ]})); - let lines = node - .prepare(&Theme::default()) - .render_lines(Size::new(10, 4)); + let lines = node.render_lines(Size::new(10, 4), &Theme::default()); assert_eq!( lines .iter() @@ -579,9 +581,7 @@ fn styled_text_wraps_in_the_space_its_nowrap_row_assigns() { #[test] fn shrinking_a_hyphenated_word_reserves_the_siblings_column() { let node = tree(json!({"type":"row","props":{"gap":1},"children":["aa-bb", "!"]})); - let lines = node - .prepare(&Theme::default()) - .render_lines(Size::new(6, 2)); + let lines = node.render_lines(Size::new(6, 2), &Theme::default()); assert_eq!( lines .iter() @@ -606,9 +606,7 @@ fn wrapping_row_moves_a_hash_and_nonbreaking_pr_label_together() { (20, vec!["gpt-5", "a1b2c3d PR\u{a0}#123"], 8..15), (12, vec!["gpt-5", "a1b2c3d PR\u{a0}…"], 8..11), ] { - let lines = node - .prepare(&Theme::default()) - .render_lines(Size::new(width, 4)); + let lines = node.render_lines(Size::new(width, 4), &Theme::default()); let text: Vec<_> = lines.iter().map(|line| line.line.to_string()).collect(); assert_eq!(text, expected); assert_eq!( @@ -629,7 +627,7 @@ fn authored_line_breaks_and_collapsed_whitespace_preserve_styles_and_links() { let area = Rect::new( /*x*/ 0, /*y*/ 0, /*width*/ 8, /*height*/ 4, ); - let lines = node.prepare(&Theme::default()).render_lines(area.as_size()); + let lines = node.render_lines(area.as_size(), &Theme::default()); assert_eq!( lines[3].hyperlinks, [TerminalHyperlink::web(0..6, href.to_owned())] @@ -653,9 +651,7 @@ fn authored_line_breaks_and_collapsed_whitespace_preserve_styles_and_links() { #[test] fn earlier_sibling_grows_after_reserving_the_later_siblings_minimum() { let node = tree(json!({"type":"row","props":{"gap":0},"children":["aaa a bb", "aaa a bb"]})); - let lines = node - .prepare(&Theme::default()) - .render_lines(Size::new(10, 3)); + let lines = node.render_lines(Size::new(10, 3), &Theme::default()); assert_eq!( lines .iter() @@ -675,9 +671,7 @@ fn width_fitting_respects_word_hyphens_across_styles() { ] { let node = tree(json!({"type":"text","children":[{"type":"bold","children":[prefix]},suffix]})); - let lines = node - .prepare(&Theme::default()) - .render_lines(Size::new(2, 3)); + let lines = node.render_lines(Size::new(2, 3), &Theme::default()); assert_eq!( lines .iter()