From 28aacbb9d9e4fd1d8f566ab221834e74fd83875a Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 20 Jul 2026 15:30:06 +0000 Subject: [PATCH] Avoid cloning hyperlink text during TUI rendering (#34366) ## What changed Add a borrowed `Line` conversion and use it when rendering and measuring hyperlink-aware history and pager content. This preserves line styling and alignment while avoiding clones of the underlying span strings. GitOrigin-RevId: c9cd6ff5ec63cd29719ea2ae51acede7364f2bd8 --- codex-rs/tui/src/history_cell/mod.rs | 3 ++- codex-rs/tui/src/pager_overlay.rs | 8 ++++---- codex-rs/tui/src/render/line_utils.rs | 16 ++++++++++++++++ codex-rs/tui/src/terminal_hyperlinks.rs | 11 ++++++++++- 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/codex-rs/tui/src/history_cell/mod.rs b/codex-rs/tui/src/history_cell/mod.rs index 5708f27a21..d0ce17edc5 100644 --- a/codex-rs/tui/src/history_cell/mod.rs +++ b/codex-rs/tui/src/history_cell/mod.rs @@ -37,6 +37,7 @@ use crate::terminal_hyperlinks::mark_buffer_hyperlinks; use crate::terminal_hyperlinks::plain_hyperlink_lines; use crate::terminal_hyperlinks::prefix_hyperlink_lines; use crate::terminal_hyperlinks::visible_lines; +use crate::terminal_hyperlinks::visible_lines_ref; #[cfg(test)] use crate::test_support::PathBufExt; #[cfg(test)] @@ -309,7 +310,7 @@ pub(crate) trait HistoryCell: std::fmt::Debug + Send + Sync + Any { impl Renderable for Box { fn render(&self, area: Rect, buf: &mut Buffer) { let hyperlink_lines = self.display_hyperlink_lines(area.width); - let lines = visible_lines(hyperlink_lines.clone()); + let lines = visible_lines_ref(&hyperlink_lines); let paragraph = Paragraph::new(Text::from(lines)).wrap(Wrap { trim: false }); let y = if area.height == 0 { 0 diff --git a/codex-rs/tui/src/pager_overlay.rs b/codex-rs/tui/src/pager_overlay.rs index 487ad32c01..23e470cb00 100644 --- a/codex-rs/tui/src/pager_overlay.rs +++ b/codex-rs/tui/src/pager_overlay.rs @@ -31,7 +31,7 @@ use crate::render::renderable::Renderable; use crate::style::user_message_style; use crate::terminal_hyperlinks::HyperlinkLine; use crate::terminal_hyperlinks::mark_buffer_hyperlinks; -use crate::terminal_hyperlinks::visible_lines; +use crate::terminal_hyperlinks::visible_lines_ref; use crate::tui; use crate::tui::TuiEvent; use crossterm::event::KeyCode; @@ -408,7 +408,7 @@ impl Renderable for CellRenderable { } else { Style::default() }; - let p = Paragraph::new(Text::from(visible_lines(hyperlink_lines.clone()))) + let p = Paragraph::new(Text::from(visible_lines_ref(&hyperlink_lines))) .style(style) .wrap(Wrap { trim: false }); p.render(area, buf); @@ -426,14 +426,14 @@ struct HyperlinkLinesRenderable { impl Renderable for HyperlinkLinesRenderable { fn render(&self, area: Rect, buf: &mut Buffer) { - Paragraph::new(Text::from(visible_lines(self.lines.clone()))) + Paragraph::new(Text::from(visible_lines_ref(&self.lines))) .wrap(Wrap { trim: false }) .render(area, buf); mark_buffer_hyperlinks(buf, area, &self.lines, /*scroll_rows*/ 0); } fn desired_height(&self, width: u16) -> u16 { - Paragraph::new(Text::from(visible_lines(self.lines.clone()))) + Paragraph::new(Text::from(visible_lines_ref(&self.lines))) .wrap(Wrap { trim: false }) .line_count(width) .try_into() diff --git a/codex-rs/tui/src/render/line_utils.rs b/codex-rs/tui/src/render/line_utils.rs index 54970f4486..284fea4d16 100644 --- a/codex-rs/tui/src/render/line_utils.rs +++ b/codex-rs/tui/src/render/line_utils.rs @@ -1,6 +1,22 @@ use ratatui::text::Line; use ratatui::text::Span; +/// Create a ratatui `Line` that borrows the contents of another line. +pub fn line_to_borrowed<'a>(line: &'a Line<'_>) -> Line<'a> { + Line { + style: line.style, + alignment: line.alignment, + spans: line + .spans + .iter() + .map(|span| Span { + style: span.style, + content: std::borrow::Cow::Borrowed(span.content.as_ref()), + }) + .collect(), + } +} + /// Clone a borrowed ratatui `Line` into an owned `'static` line. pub fn line_to_static(line: &Line<'_>) -> Line<'static> { Line { diff --git a/codex-rs/tui/src/terminal_hyperlinks.rs b/codex-rs/tui/src/terminal_hyperlinks.rs index 6fe458d9c6..d2ccfede04 100644 --- a/codex-rs/tui/src/terminal_hyperlinks.rs +++ b/codex-rs/tui/src/terminal_hyperlinks.rs @@ -19,6 +19,7 @@ use unicode_width::UnicodeWidthChar; use unicode_width::UnicodeWidthStr; use url::Url; +use crate::render::line_utils::line_to_borrowed; use crate::render::line_utils::line_to_static; use crate::wrapping::RtOptions; use crate::wrapping::adaptive_wrap_line; @@ -127,6 +128,13 @@ pub(crate) fn visible_lines(lines: Vec) -> Vec> { lines.into_iter().map(|line| line.line).collect() } +pub(crate) fn visible_lines_ref(lines: &[HyperlinkLine]) -> Vec> { + lines + .iter() + .map(|line| line_to_borrowed(&line.line)) + .collect() +} + pub(crate) fn plain_hyperlink_lines(lines: Vec>) -> Vec { lines.into_iter().map(HyperlinkLine::new).collect() } @@ -483,7 +491,8 @@ pub(crate) fn mark_buffer_hyperlinks( } let mut logical_row = 0usize; for line in lines { - let paragraph = Paragraph::new(Text::from(line.line.clone())).wrap(Wrap { trim: false }); + let paragraph = + Paragraph::new(Text::from(line_to_borrowed(&line.line))).wrap(Wrap { trim: false }); let rendered_height = paragraph.line_count(area.width).max(/*other*/ 1); if line.hyperlinks.is_empty() { logical_row += rendered_height;