From 16e27626e654b29cc2ea696a545b2cd5f29db136 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Thu, 12 Nov 2026 22:15:22 +0000 Subject: [PATCH] Unify TUI status content and layout caching in `StatusLine` (#61200) ## What changed - Replace `FooterContent` and the separate layout cache with a `StatusLine` that owns native or extension content and caches its layout by bounds. - Move footer padding into the caller's layout and retain cached wrapping for unchanged agent-labeled status lines. - Remove extension-to-single-line conversion and render prepared UI trees directly within their allotted rectangles. ## Testing Extend composer coverage to verify that replacing measured extension content updates the displayed rows. Adapt status-line and UI-tree tests to exercise direct rendering, clipping, hyperlinks, and viewport bounds. GitOrigin-RevId: 72273c87d689b8b59748ab243793a9e2a45f4884 --- codex-rs/tui/src/app/side_pane.rs | 21 +- codex-rs/tui/src/app/tests.rs | 10 +- codex-rs/tui/src/app_event.rs | 2 +- codex-rs/tui/src/bottom_pane/chat_composer.rs | 22 +- .../chat_composer/composer_layout.rs | 13 +- .../bottom_pane/chat_composer/footer_state.rs | 6 +- .../chat_composer/status_surface.rs | 82 +++--- .../chat_composer/status_surface_tests.rs | 26 +- codex-rs/tui/src/bottom_pane/footer.rs | 27 +- .../src/bottom_pane/footer/footer_content.rs | 135 --------- codex-rs/tui/src/bottom_pane/mod.rs | 6 +- .../src/bottom_pane/multi_select_picker.rs | 13 +- ...tree_paints_only_the_footer_viewport.snap} | 2 +- codex-rs/tui/src/bottom_pane/status_line.rs | 271 ++++++++++++------ .../tui/src/bottom_pane/status_line_setup.rs | 22 +- ..._content_tests.rs => status_line_tests.rs} | 16 +- .../src/bottom_pane/status_surface_preview.rs | 5 +- codex-rs/tui/src/chatwidget.rs | 14 +- .../tui/src/chatwidget/status_controls.rs | 4 +- .../tests/status_surface_previews.rs | 13 +- codex-rs/tui/src/ui_tree/render.rs | 46 --- codex-rs/tui/src/ui_tree/render/prepare.rs | 7 - codex-rs/tui/src/ui_tree_tests.rs | 243 ++++++---------- 23 files changed, 431 insertions(+), 575 deletions(-) delete mode 100644 codex-rs/tui/src/bottom_pane/footer/footer_content.rs rename codex-rs/tui/src/bottom_pane/{footer/snapshots/codex_tui__bottom_pane__footer__footer_content__tests__prepared_tree_uses_its_natural_width_and_paints_only_the_footer_viewport.snap => snapshots/codex_tui__bottom_pane__status_line__tests__prepared_tree_paints_only_the_footer_viewport.snap} (64%) rename codex-rs/tui/src/bottom_pane/{footer/footer_content_tests.rs => status_line_tests.rs} (87%) diff --git a/codex-rs/tui/src/app/side_pane.rs b/codex-rs/tui/src/app/side_pane.rs index fa8dbaa6fb..44238b5d9b 100644 --- a/codex-rs/tui/src/app/side_pane.rs +++ b/codex-rs/tui/src/app/side_pane.rs @@ -8,8 +8,7 @@ use super::sidebar::PaneFocus; use super::sidebar::SidebarContent; use super::sidebar::SidebarLayout; use crate::bottom_pane::FooterLayout; -use crate::bottom_pane::StatusLine; -use crate::bottom_pane::footer_content::StatusLineFocus; +use crate::bottom_pane::StatusLineFocus; use crate::custom_terminal::Frame; use crate::legacy_core::config::edit::ConfigEdit; use crate::legacy_core::config::edit::ConfigEditsBuilder; @@ -458,8 +457,9 @@ impl SidePaneLayout { /*priority_hint_rows*/ hint_rows, ); let status = companion.display.status_line.as_ref(); - let status_line = StatusLine::default(); - let wanted_status = status_line.height(status, area.width, footer.status_budget(available)); + let wanted_status = status.map_or(0, |status| { + status.height(footer.status_bounds(area.width, available)) + }); let footer = footer.allocate(available, wanted_status); let above = available - footer.height(); let mut transcript = Rect { @@ -512,12 +512,13 @@ impl SidePaneLayout { &mut Default::default(), ); } - status_line.render( - status, - footer.status_area(area), - frame.buffer, - StatusLineFocus::Unfocused, - ); + if let Some(status) = status { + status.render( + footer.status_area(area), + frame.buffer, + StatusLineFocus::Unfocused, + ); + } } } diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index bab07814fa..02a50518ca 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -2218,7 +2218,6 @@ async fn replayed_interrupted_turn_restores_queued_input_to_composer() { #[tokio::test] async fn token_usage_update_refreshes_status_line_with_runtime_context_window() { use crate::bottom_pane::StatusLineItem; - use crate::bottom_pane::footer_content::FooterContent; let mut app = make_test_app().await; app.chat_widget.setup_status_line( @@ -2249,10 +2248,11 @@ async fn token_usage_update_refreshes_status_line_with_runtime_context_window() app.handle_thread_event_now(ThreadBufferedEvent::Notification(Box::new( token_usage_notification(thread_id, "turn-1", Some(window)), ))); - assert!(matches!( - app.chat_widget.status_line_content(), - Some(FooterContent::HyperlinkLine(_)) - )); + assert!( + app.chat_widget + .status_line_content() + .is_some_and(|status| status.as_native().is_some()) + ); let rendered = render_bottom_popup(&app.chat_widget, /*width*/ 60); assert!( rendered.lines().any(|row| row.trim() == expected), diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index a8de774000..ab8735faad 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -317,7 +317,7 @@ pub(crate) struct CapturedSideDisplay { pub(crate) active: Option>, pub(crate) auxiliary: Vec, pub(crate) draft: Box, - pub(crate) status_line: Option, + pub(crate) status_line: Option, pub(crate) work: Option, pub(crate) last_completed_agent_message: Option<(String, String)>, pub(crate) cwd: AbsolutePathBuf, diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 61d93baa7e..e489134a1b 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -379,8 +379,8 @@ use super::skill_popup::SkillPopup; use super::slash_commands::ServiceTierCommand; use super::slash_commands::SlashCommandItem; use super::voice_strip::VoiceStrip; -use crate::bottom_pane::footer_content::FooterContent; -use crate::bottom_pane::footer_content::StatusLineFocus; +use crate::bottom_pane::StatusLine; +use crate::bottom_pane::StatusLineFocus; use crate::history_cell::sanitize_user_text; use crate::key_hint::KeyBindingListExt; use crate::keymap::EditorKeymap; @@ -795,7 +795,7 @@ impl ChatComposer { goal_status_indicator: None, ide_context_active: false, status_line_value: None, - status_layout: Default::default(), + labeled_status_line: Default::default(), status_line_enabled: false, side_conversation_context_label: None, active_agent_label: None, @@ -908,13 +908,12 @@ impl ChatComposer { let style = IgnitionStyle::random(self.effort_animation_style); self.effort_ignition = Some(EffortIgnition::new(tier, style)); self.effort_animation_style = Some(style); - if let Some(previous) = self.status_surface_content(StatusLineFocus::Focused) - && !matches!(previous.as_ref(), FooterContent::Extension { .. }) - { - self.effort_status_line_transition = Some(EffortStatusLineTransition::new( - tier, - previous.into_owned().into_hyperlink_line(), - )); + let previous = self.with_status_surface_content(StatusLineFocus::Focused, |status| { + status.and_then(StatusLine::as_native).cloned() + }); + if let Some(previous) = previous { + self.effort_status_line_transition = + Some(EffortStatusLineTransition::new(tier, previous)); } if let Some(frame_requester) = &self.frame_requester { frame_requester.schedule_frame(); @@ -1891,7 +1890,7 @@ impl ChatComposer { self.attachments.local_image_paths() } - pub(crate) fn status_line_content(&self) -> Option<&FooterContent> { + pub(crate) fn status_line_content(&self) -> Option<&StatusLine> { self.footer.status_line_value.as_ref() } @@ -4496,6 +4495,7 @@ impl ChatComposer { return false; } self.footer.active_agent_label = active_agent_label; + *self.footer.labeled_status_line.get_mut() = None; true } } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/composer_layout.rs b/codex-rs/tui/src/bottom_pane/chat_composer/composer_layout.rs index 1fe742e0cd..b1e92fb7f0 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/composer_layout.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/composer_layout.rs @@ -7,6 +7,7 @@ use crate::render::renderable::Renderable; use crate::tui::FrameRequester; use ratatui::buffer::Buffer; use ratatui::layout::Rect; +use ratatui::layout::Size; struct ComposerRows { input: u16, @@ -40,7 +41,11 @@ impl FooterLayout { } } - pub(crate) fn status_budget(&self, available: u16) -> u16 { + pub(crate) fn status_bounds(&self, width: u16, available: u16) -> Size { + inset_footer_hint_area(Rect::new(0, 0, width, self.status_budget(available))).as_size() + } + + fn status_budget(&self, available: u16) -> u16 { self.cap.min(available).saturating_sub(self.reserved_hints) } @@ -61,11 +66,11 @@ impl FooterRows { } pub(crate) fn status_area(&self, area: Rect) -> Rect { - Rect { + inset_footer_hint_area(Rect { y: area.bottom().saturating_sub(self.height()).max(area.y), height: self.status, ..area - } + }) } } @@ -297,7 +302,7 @@ impl ChatComposer { /*priority_hint_rows*/ if find_active { 2 } else { 1 }, ); let wanted_status = - self.status_surface_height(width, footer.status_budget(available), options); + self.status_surface_height(footer.status_bounds(width, available), options); let basic = (footer.reserved_hints + wanted_status.min(1)).min(2); let above_input = above_input.min(available.saturating_sub(basic)); available -= above_input; diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/footer_state.rs b/codex-rs/tui/src/bottom_pane/chat_composer/footer_state.rs index c7a70df106..6d6efe7d02 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/footer_state.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/footer_state.rs @@ -18,9 +18,9 @@ use super::super::footer::footer_height; use super::super::footer::reset_mode_after_activity; use super::ActivePopup; use super::warning_notice; +use crate::bottom_pane::StatusLine; use crate::bottom_pane::footer::FooterKeyHints; use crate::bottom_pane::footer::FooterProps; -use crate::bottom_pane::footer_content::FooterContent; use crate::key_hint; use crossterm::event::KeyCode; use ratatui::buffer::Buffer; @@ -383,8 +383,8 @@ pub(super) struct FooterState { pub(super) collaboration_mode_indicator: Option, pub(super) goal_status_indicator: Option, pub(super) ide_context_active: bool, - pub(super) status_line_value: Option, - pub(super) status_layout: crate::bottom_pane::status_line::StatusLine, + pub(super) status_line_value: Option, + pub(super) labeled_status_line: std::cell::RefCell>, pub(super) status_line_enabled: bool, pub(super) side_conversation_context_label: Option, pub(super) active_agent_label: Option, diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/status_surface.rs b/codex-rs/tui/src/bottom_pane/chat_composer/status_surface.rs index 4b85ae30b4..58cce3aa02 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/status_surface.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/status_surface.rs @@ -3,19 +3,20 @@ //! Shortcut help grows above the input, preserving status and close rows below it even when clipped. use super::*; -use crate::bottom_pane::footer_content::StatusLineFocus; - -use std::borrow::Cow; +use ratatui::layout::Size; impl ChatComposer { - pub(crate) fn set_status_line(&mut self, status_line: Option) -> bool { + pub(crate) fn set_status_line(&mut self, status_line: Option) -> bool { if self.footer.status_line_value == status_line { return false; } - if matches!(status_line, Some(FooterContent::Extension { .. })) { + if status_line + .as_ref() + .is_some_and(|status| status.as_native().is_none()) + { self.effort_status_line_transition = None; } - self.footer.status_layout = Default::default(); + *self.footer.labeled_status_line.get_mut() = None; self.footer.status_line_value = status_line; true } @@ -38,22 +39,16 @@ impl ChatComposer { pub(super) fn status_surface_height( &self, - width: u16, - available: u16, + mut bounds: Size, options: ComposerRenderOptions<'_>, ) -> u16 { let focus = options.status_line_focus(); - let available = - if focus == StatusLineFocus::Focused && self.active_status_transition().is_some() { - available.min(1) - } else { - available - }; - self.footer.status_layout.height( - self.status_surface_content(focus).as_deref(), - width, - available, - ) + if focus == StatusLineFocus::Focused && self.active_status_transition().is_some() { + bounds.height = bounds.height.min(1); + } + self.with_status_surface_content(focus, |status| { + status.map_or(0, |status| status.height(bounds)) + }) } pub(super) fn hint_footer_props(&self) -> FooterProps { @@ -66,25 +61,32 @@ impl ChatComposer { props } - pub(super) fn status_surface_content( + pub(super) fn with_status_surface_content( &self, focus: StatusLineFocus, - ) -> Option> { + use_status: impl FnOnce(Option<&StatusLine>) -> T, + ) -> T { if !self.footer.status_line_enabled { - return None; + return use_status(None); } let status = self.footer.status_line_value.as_ref(); - if matches!(status, Some(FooterContent::Extension { .. })) + if status.is_some_and(|status| status.as_native().is_none()) || self.footer.active_agent_label.is_none() || focus == StatusLineFocus::Unfocused { - return status.map(Cow::Borrowed); + return use_status(status); } let mut props = self.footer_props(); // The viewed agent belongs to native status even during a transient composer hint. props.mode = FooterMode::ComposerHasDraft; props.is_task_running = false; - passive_footer_status_line(&props).map(Cow::Owned) + let next = passive_footer_status_line(&props); + let mut labeled = self.footer.labeled_status_line.borrow_mut(); + // Preserve wrapped rows while the effective line, including the label style, stays equal. + if *labeled != next { + *labeled = next; + } + use_status(labeled.as_ref()) } fn active_status_transition(&self) -> Option<&EffortStatusLineTransition> { @@ -103,23 +105,21 @@ impl ChatComposer { return; } let focus = options.status_line_focus(); - let mut content = self.status_surface_content(focus); - if focus == StatusLineFocus::Focused - && let Some(transition) = self.active_status_transition() - { - content = transition - .render_line( - content.map(|line| line.into_owned().into_hyperlink_line()), - inset_footer_hint_area(area).width, - ) - .map(|frame| Cow::Owned(frame.into())); - if let Some(requester) = &self.frame_requester { - requester.schedule_frame_in(EFFORT_STATUS_LINE_FRAME_TICK); + self.with_status_surface_content(focus, |status| { + if focus == StatusLineFocus::Focused + && let Some(transition) = self.active_status_transition() + { + let target = status.and_then(StatusLine::as_native).cloned(); + if let Some(frame) = transition.render_line(target, area.width) { + StatusLine::from(frame).render(area, buf, focus); + } + if let Some(requester) = &self.frame_requester { + requester.schedule_frame_in(EFFORT_STATUS_LINE_FRAME_TICK); + } + } else if let Some(status) = status { + status.render(area, buf, focus); } - } - self.footer - .status_layout - .render(content.as_deref(), area, buf, focus); + }); } } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/status_surface_tests.rs b/codex-rs/tui/src/bottom_pane/chat_composer/status_surface_tests.rs index 06d2d358bb..3ad9a7aeff 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/status_surface_tests.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/status_surface_tests.rs @@ -39,7 +39,7 @@ fn extension_status_line_clips_links_to_footer_layout() { }"# .parse() .unwrap(); - composer.set_status_line(Some(FooterContent::from(tree))); + composer.set_status_line(Some(StatusLine::from(tree))); for separate_status_line in [false, true] { for width in [12, 40] { @@ -137,7 +137,7 @@ fn extension_uses_the_full_width_after_resize() { {"type":"link","props":{"href":"https://example.com/pr/12345"},"children":["12345"]}]}]}"# .parse() .unwrap(); - composer.set_status_line(Some(FooterContent::from(tree))); + composer.set_status_line(Some(StatusLine::from(tree))); composer.set_collaboration_modes_enabled(/*enabled*/ true); let linked_number: String = "12345" .chars() @@ -205,7 +205,7 @@ fn provider_status_and_hints_use_separate_rows() { fn status_rows_respect_host_budget_and_yield_to_the_draft() { let mut composer = composer(); let tree: crate::ui_tree::UiTree = r#""one\ntwo\nthree\nfour\nfive""#.parse().unwrap(); - composer.set_status_line(Some(FooterContent::from(tree))); + composer.set_status_line(Some(StatusLine::from(tree))); composer.set_text_content("a\nb\nc".into(), Vec::new(), Vec::new()); composer.move_cursor_to_end(); for separate_status_line in [false, true] { @@ -252,8 +252,19 @@ fn status_rows_respect_host_budget_and_yield_to_the_draft() { ["one…"] ); composer.set_collaboration_mode_indicator(/*indicator*/ None); + let options = ComposerRenderOptions { + separate_status_line: true, + max_height: Some(14), + ..Default::default() + }; + let area = Rect::new(0, 0, 7, 14); + assert_eq!( + render_status(&composer, area, options).1, + ["one", "two", "three", "four…"] + ); let tree: crate::ui_tree::UiTree = r#""x\ny""#.parse().unwrap(); - composer.set_status_line(Some(FooterContent::from(tree))); + composer.set_status_line(Some(StatusLine::from(tree))); + assert_eq!(render_status(&composer, area, options).1, ["x", "y"]); assert_eq!( composer.footer_hint_height(/*width*/ 7, ComposerRenderOptions::default()), 0 @@ -283,8 +294,7 @@ fn sidebar_focus_keeps_dim_extension_status_rows() { if unfocused { assert_eq!(composer.footer.warning_notice_area.get(), None); assert!(!buffer.content.iter().any(|cell| cell.symbol() == "⚠")); - let status = - inset_footer_hint_area(composer.layout_with_options(buffer.area, options).status); + let status = composer.layout_with_options(buffer.area, options).status; for y in status.y..status.bottom() { assert!( buffer[(status.x, y)] @@ -424,7 +434,7 @@ fn extension_status_paints_with_the_current_theme_at_the_same_width() { r#"{"type":"text","props":{"color":"primary"},"children":["λ"]}"# .parse() .unwrap(); - composer.set_status_line(Some(FooterContent::from(content))); + composer.set_status_line(Some(StatusLine::from(content))); let previous = current_syntax_theme(); let colors = [1, 2].map(|index| { let mut theme = previous.clone(); @@ -460,7 +470,7 @@ fn extension_status_paints_with_the_current_theme_at_the_same_width() { fn extension_status_keeps_its_rows_when_effort_changes() { let mut composer = composer(); let tree: crate::ui_tree::UiTree = r#""one\ntwo\nthree""#.parse().unwrap(); - composer.set_status_line(Some(FooterContent::from(tree))); + composer.set_status_line(Some(StatusLine::from(tree))); composer.set_active_reasoning_effort_baseline(/*effort*/ None); composer.set_active_reasoning_effort( Some(&ReasoningEffort::Max), diff --git a/codex-rs/tui/src/bottom_pane/footer.rs b/codex-rs/tui/src/bottom_pane/footer.rs index 78b99f3a15..3877cc6e98 100644 --- a/codex-rs/tui/src/bottom_pane/footer.rs +++ b/codex-rs/tui/src/bottom_pane/footer.rs @@ -22,9 +22,8 @@ //! agent label, side-conversation state, or some combination of those. //! //! The composer fits ordinary navigation, modes, actions, and warnings in `chat_composer::passive_hints`. -pub(crate) mod footer_content; -use self::footer_content::FooterContent; +use super::StatusLine; use super::shortcut_overlay; use super::status_line_style::STATUS_LINE_SEPARATOR; use crate::key_hint; @@ -63,7 +62,7 @@ pub(crate) struct FooterProps { /// /// This is rendered when `mode` is `FooterMode::QuitShortcutReminder`. pub(crate) quit_shortcut_key: KeyBinding, - pub(crate) status_line_value: Option, + pub(crate) status_line_value: Option, pub(crate) status_line_enabled: bool, pub(crate) key_hints: FooterKeyHints, /// Active thread label shown when the footer is rendering contextual information instead of an @@ -795,7 +794,7 @@ fn footer_from_props_lines( /// The returned line may contain the configured status line, the currently viewed agent label, or /// both combined. Active instructional states such as quit reminders, shortcut overlays, and queue /// prompts deliberately return `None` so those call-to-action hints stay visible. -pub(crate) fn passive_footer_status_line(props: &FooterProps) -> Option { +pub(crate) fn passive_footer_status_line(props: &FooterProps) -> Option { if !props.mode.shows_passive_line(props.is_task_running) { return None; } @@ -804,10 +803,13 @@ pub(crate) fn passive_footer_status_line(props: &FooterProps) -> Option line.as_ref().clone(), - Some(content @ FooterContent::Extension { .. }) => return Some(content.clone()), - None => HyperlinkLine::default(), + let mut line = if let Some(status) = status { + let Some(line) = status.as_native() else { + return Some(status.clone()); + }; + line.clone() + } else { + HyperlinkLine::default() }; let suffix = passive_footer_suffix(props, status.is_some()); if status.is_none() && suffix.spans.is_empty() { @@ -1040,8 +1042,13 @@ mod tests { let status_line_active = props.status_line_enabled && props.mode.shows_passive_line(props.is_task_running); let passive_status_line = if status_line_active { - passive_footer_status_line(props) - .map(|content| content.into_hyperlink_line().line) + passive_footer_status_line(props).map(|status| { + status + .as_native() + .expect("native footer fixture") + .line + .clone() + }) } else { None }; diff --git a/codex-rs/tui/src/bottom_pane/footer/footer_content.rs b/codex-rs/tui/src/bottom_pane/footer/footer_content.rs deleted file mode 100644 index c4cdf30534..0000000000 --- a/codex-rs/tui/src/bottom_pane/footer/footer_content.rs +++ /dev/null @@ -1,135 +0,0 @@ -//! Native and extension status content measured and painted inside the status rectangle. - -use crate::line_truncation::truncate_line_with_ellipsis; -use ratatui::buffer::Buffer; -use ratatui::layout::Rect; -use ratatui::style::Style; -use ratatui::text::Line; -use std::borrow::Cow; -use std::sync::Arc; - -use crate::render::highlight::with_syntax_theme; -use crate::terminal_hyperlinks::HyperlinkLine; -use crate::ui_tree::PreparedUiTree; -use crate::ui_tree::UiTree; - -#[derive(Clone, Copy, PartialEq, Eq)] -pub(crate) enum StatusLineFocus { - Focused, - Unfocused, -} - -/// Owned status content displayed below the composer. -#[derive(Clone, Debug, PartialEq)] -pub(crate) enum FooterContent { - HyperlinkLine(Box), - Extension { content: Arc }, -} - -impl From for FooterContent { - fn from(tree: UiTree) -> Self { - Self::Extension { - content: Arc::new(tree.prepare()), - } - } -} - -impl FooterContent { - /// Natural display width in terminal cells. - pub(crate) fn width(&self) -> u16 { - let width = match self { - Self::HyperlinkLine(line) => line.width(), - Self::Extension { content } => return content.measure_natural().width, - }; - u16::try_from(width).unwrap_or(u16::MAX) - } - - /// Paint a focused preview in its allotted rectangle. - pub(crate) fn render(&self, area: Rect, buf: &mut Buffer) { - self.render_with_focus(area, buf, StatusLineFocus::Focused); - } - - pub(in crate::bottom_pane) fn render_with_focus( - &self, - area: Rect, - buf: &mut Buffer, - focus: StatusLineFocus, - ) { - match self { - Self::Extension { content } => { - let plain_style = - (focus == StatusLineFocus::Unfocused).then(|| Style::reset().dim()); - with_syntax_theme(|theme| content.render(area, theme, buf, plain_style)); - } - Self::HyperlinkLine(line) => render_status_row( - line, - Rect { - height: area.height.min(1), - ..area - }, - buf, - focus, - /*more_rows*/ false, - ), - } - } - - pub(crate) fn into_hyperlink_line(self) -> HyperlinkLine { - match self { - Self::HyperlinkLine(line) => *line, - Self::Extension { content } => { - with_syntax_theme(|theme| content.render_line(u16::MAX, theme)) - } - } - } -} - -pub(in crate::bottom_pane) fn render_status_row( - line: &HyperlinkLine, - area: Rect, - buf: &mut Buffer, - focus: StatusLineFocus, - more_rows: bool, -) { - if area.is_empty() { - return; - } - let mut line = Cow::Borrowed(line); - let width = usize::from(area.width); - if more_rows || line.width() > width { - let line = line.to_mut(); - line.line = truncate_line_with_ellipsis(std::mem::take(&mut line.line), width); - line.clip_hyperlinks_to_width(line.width().saturating_sub(1)); - } - if focus == StatusLineFocus::Unfocused { - let line = line.to_mut(); - line.hyperlinks.clear(); - line.line.style = Style::reset().dim(); - for span in &mut line.line.spans { - span.style = Style::default(); - } - } - line.render_fitted(area, buf); -} - -impl From for FooterContent { - fn from(line: HyperlinkLine) -> Self { - Self::HyperlinkLine(Box::new(line)) - } -} - -impl From> for FooterContent { - fn from(line: Line<'static>) -> Self { - Self::from(HyperlinkLine::from(line)) - } -} - -impl From<&'static str> for FooterContent { - fn from(text: &'static str) -> Self { - Self::from(Line::from(text)) - } -} - -#[cfg(test)] -#[path = "footer_content_tests.rs"] -mod tests; diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 226dbee602..005dc50be8 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -28,6 +28,7 @@ pub(crate) use composer_gap::ComposerGap; pub(crate) use footer::footer_hint_items_line; pub(crate) use footer::inset_footer_hint_area; pub(crate) use status_line::StatusLine; +pub(crate) use status_line::StatusLineFocus; use std::collections::VecDeque; use std::path::PathBuf; @@ -121,7 +122,6 @@ pub(crate) use approval_overlay::format_requested_permissions_rule; pub(crate) use async_questions::AsyncQuestions; pub(crate) use async_questions::QuestionState; pub(crate) use async_questions::QuestionSubmission; -pub(crate) use footer::footer_content; pub(crate) use mcp_server_elicitation::McpServerElicitationFormRequest; pub(crate) use mcp_server_elicitation::McpServerElicitationOverlay; pub(crate) use request_user_input::RequestUserInputOverlay; @@ -1398,7 +1398,7 @@ impl BottomPane { self.status_widget().is_some() } - pub(crate) fn status_line_content(&self) -> Option<&footer_content::FooterContent> { + pub(crate) fn status_line_content(&self) -> Option<&StatusLine> { self.composer.status_line_content() } @@ -2626,7 +2626,7 @@ impl BottomPane { self.request_redraw(); } - pub(crate) fn set_status_line(&mut self, status_line: Option) { + pub(crate) fn set_status_line(&mut self, status_line: Option) { if self.composer.set_status_line(status_line) { self.request_redraw(); } diff --git a/codex-rs/tui/src/bottom_pane/multi_select_picker.rs b/codex-rs/tui/src/bottom_pane/multi_select_picker.rs index 3608d060ae..1dd7f213de 100644 --- a/codex-rs/tui/src/bottom_pane/multi_select_picker.rs +++ b/codex-rs/tui/src/bottom_pane/multi_select_picker.rs @@ -58,7 +58,8 @@ use ratatui::widgets::Paragraph; use ratatui::widgets::Widget; use ratatui::widgets::Wrap; -use super::footer_content::FooterContent; +use super::StatusLine; +use super::StatusLineFocus; use super::picker_rows; use super::picker_style; use super::selection_popup_common::ColumnWidthConfig; @@ -110,7 +111,7 @@ pub type CancelCallback = Box; /// Callback to generate an optional preview line based on current item states. /// Returns `None` to hide the preview area. -type PreviewCallback = Box Option + Send + Sync>; +type PreviewCallback = Box Option + Send + Sync>; /// A single selectable item in the multi-select picker. /// @@ -201,7 +202,7 @@ pub(crate) struct MultiSelectPicker { preview_builder: Option, /// Cached preview line (updated on item changes). - preview_line: Option, + preview_line: Option, /// Callback invoked when items change (toggle or reorder). on_change: Option, @@ -621,7 +622,7 @@ impl Renderable for MultiSelectPicker { Layout::vertical([Constraint::Length(1), Constraint::Length(hint_height)]) .areas(footer_area); let preview_area = preview_area.inset(Insets::vh(/*v*/ 0, /*h*/ 2)); - preview_line.render(preview_area, buf); + preview_line.render(preview_area, buf, StatusLineFocus::Focused); hint_area } else { footer_area @@ -711,12 +712,12 @@ impl MultiSelectPickerBuilder { where F: Fn(&[MultiSelectItem]) -> Option> + Send + Sync + 'static, { - self.on_preview_content(move |items| callback(items).map(FooterContent::from)) + self.on_preview_content(move |items| callback(items).map(StatusLine::from)) } pub(crate) fn on_preview_content( mut self, - callback: impl Fn(&[MultiSelectItem]) -> Option + Send + Sync + 'static, + callback: impl Fn(&[MultiSelectItem]) -> Option + Send + Sync + 'static, ) -> Self { self.preview_builder = Some(Box::new(callback)); self diff --git a/codex-rs/tui/src/bottom_pane/footer/snapshots/codex_tui__bottom_pane__footer__footer_content__tests__prepared_tree_uses_its_natural_width_and_paints_only_the_footer_viewport.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line__tests__prepared_tree_paints_only_the_footer_viewport.snap similarity index 64% rename from codex-rs/tui/src/bottom_pane/footer/snapshots/codex_tui__bottom_pane__footer__footer_content__tests__prepared_tree_uses_its_natural_width_and_paints_only_the_footer_viewport.snap rename to codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line__tests__prepared_tree_paints_only_the_footer_viewport.snap index e51326bbfe..20b4023849 100644 --- a/codex-rs/tui/src/bottom_pane/footer/snapshots/codex_tui__bottom_pane__footer__footer_content__tests__prepared_tree_uses_its_natural_width_and_paints_only_the_footer_viewport.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line__tests__prepared_tree_paints_only_the_footer_viewport.snap @@ -1,5 +1,5 @@ --- -source: tui/src/bottom_pane/footer/footer_content_tests.rs +source: tui/src/bottom_pane/status_line_tests.rs expression: "rows.join(\"\\n\")" --- ........ diff --git a/codex-rs/tui/src/bottom_pane/status_line.rs b/codex-rs/tui/src/bottom_pane/status_line.rs index 07b931173c..76109b748a 100644 --- a/codex-rs/tui/src/bottom_pane/status_line.rs +++ b/codex-rs/tui/src/bottom_pane/status_line.rs @@ -1,132 +1,209 @@ -//! Measure and paint status lines for the main conversation and side chats. +//! Measure and paint native and extension status lines within the caller's content rectangle. -use super::footer::inset_footer_hint_area; -use super::footer_content::FooterContent; -use super::footer_content::StatusLineFocus; -use super::footer_content::render_status_row; +use crate::line_truncation::truncate_line_with_ellipsis; +use crate::render::highlight::with_syntax_theme; use crate::render::line_utils::line_to_static; use crate::terminal_hyperlinks::HyperlinkLine; use crate::terminal_hyperlinks::project_hyperlink_columns; +use crate::ui_tree::PreparedUiTree; +use crate::ui_tree::UiTree; use crate::width::display_width; use crate::wrapping::adaptive_wrap_line_to_width; use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::layout::Size; +use ratatui::style::Style; use ratatui::text::Line; use ratatui::text::Span; -use std::cell::Cell; +use std::borrow::Cow; use std::cell::Ref; use std::cell::RefCell; +use std::sync::Arc; use unicode_segmentation::UnicodeSegmentation; -/// The owning composer resets this when its configured status changes. -#[derive(Default)] -pub(crate) struct StatusLine { - measured_extension: Cell>, - wrapped_native: RefCell>, +#[derive(Clone, Copy, PartialEq, Eq)] +pub(crate) enum StatusLineFocus { + Focused, + Unfocused, } -struct WrappedNativeStatus { - bounds: Size, - content: HyperlinkLine, - rows: Vec, +/// A status line and the cached layout for its content. +#[derive(Clone, Debug, PartialEq)] +pub(crate) struct StatusLine(StatusContent); + +#[derive(Clone, Debug, PartialEq)] +enum StatusContent { + Native(CachedLayout, Option>>), + Extension(CachedLayout, u16>), +} + +#[derive(Debug)] +struct CachedLayout { + content: C, + layout: RefCell>, +} + +impl CachedLayout { + fn new(content: C) -> Self { + Self { + content, + layout: RefCell::default(), + } + } + + fn get(&self, bounds: Size, compute: impl FnOnce(&C) -> L) -> Ref<'_, L> { + if self + .layout + .borrow() + .as_ref() + .is_none_or(|(previous, _)| *previous != bounds) + { + self.layout.replace(Some((bounds, compute(&self.content)))); + } + Ref::map(self.layout.borrow(), |entry| { + let Some((_, layout)) = entry else { + unreachable!("layout cache was populated"); + }; + layout + }) + } +} + +// Cached geometry belongs to the current surface. Clones and comparisons use only the content. +impl Clone for CachedLayout { + fn clone(&self) -> Self { + Self::new(self.content.clone()) + } +} + +impl PartialEq for CachedLayout { + fn eq(&self, other: &Self) -> bool { + self.content == other.content + } +} + +impl From for StatusLine { + fn from(tree: UiTree) -> Self { + Self(StatusContent::Extension(CachedLayout::new(Arc::new( + tree.prepare(), + )))) + } +} + +impl From for StatusLine { + fn from(line: HyperlinkLine) -> Self { + Self(StatusContent::Native(CachedLayout::new(Box::new(line)))) + } +} + +impl From> for StatusLine { + fn from(line: Line<'static>) -> Self { + Self::from(HyperlinkLine::from(line)) + } +} + +impl From<&'static str> for StatusLine { + fn from(text: &'static str) -> Self { + Self::from(Line::from(text)) + } } impl StatusLine { - pub(crate) fn height( - &self, - content: Option<&FooterContent>, - width: u16, - available: u16, - ) -> u16 { - let bounds = inset_footer_hint_area(Rect::new(0, 0, width, available)).as_size(); + pub(crate) fn as_native(&self) -> Option<&HyperlinkLine> { + match &self.0 { + StatusContent::Native(native) => Some(&native.content), + StatusContent::Extension(_) => None, + } + } + + /// Measure within the space available for content after the caller applies padding. + pub(crate) fn height(&self, bounds: Size) -> u16 { if bounds.width == 0 || bounds.height == 0 { return 0; } - match content { - Some(FooterContent::Extension { content }) => { - if let Some((previous, height)) = self.measured_extension.get() - && previous == bounds - { - return height; - } - let height = content.measure(bounds).height; - self.measured_extension.set(Some((bounds, height))); - height - } - Some(native) => self.wrapped_rows(native, bounds).map_or_else( - || u16::from(native.width() > 0), + match &self.0 { + StatusContent::Native(native) => wrapped_rows(native, bounds).map_or_else( + || u16::from(native.content.width() > 0), |rows| rows.len().max(1).min(usize::from(bounds.height)) as u16, ), - None => 0, + StatusContent::Extension(extension) => { + *extension.get(bounds, |tree| tree.measure(bounds).height) + } } } - pub(crate) fn render( - &self, - content: Option<&FooterContent>, - area: Rect, - buf: &mut Buffer, - focus: StatusLineFocus, - ) { - let area = inset_footer_hint_area(area); - let Some(content) = content.filter(|_| !area.is_empty()) else { + /// Paint within the final content rectangle after the caller applies padding. + pub(crate) fn render(&self, area: Rect, buf: &mut Buffer, focus: StatusLineFocus) { + if area.is_empty() { return; - }; - if let Some(rows) = self.wrapped_rows(content, area.as_size()) { - let height = usize::from(area.height); - for (index, line) in rows.iter().take(height).enumerate() { - let row = Rect::new(area.x, area.y + index as u16, area.width, 1); - render_status_row( - line, - row, - buf, - focus, - index + 1 == height && rows.len() > height, - ); + } + match &self.0 { + StatusContent::Native(native) => { + let wrapped = wrapped_rows(native, area.as_size()); + let rows = wrapped + .as_deref() + .unwrap_or_else(|| std::slice::from_ref(native.content.as_ref())); + let more_rows = rows.len() > usize::from(area.height); + for (row, line) in area.rows().zip(rows) { + render_status_row( + line, + row, + buf, + focus, + more_rows && row.bottom() == area.bottom(), + ); + } + } + StatusContent::Extension(extension) => { + let plain_style = + (focus == StatusLineFocus::Unfocused).then(|| Style::reset().dim()); + with_syntax_theme(|theme| extension.content.render(area, theme, buf, plain_style)); } - } else { - content.render_with_focus(area, buf, focus); } } +} - fn wrapped_rows( - &self, - content: &FooterContent, - bounds: Size, - ) -> Option> { - let FooterContent::HyperlinkLine(line) = content else { - return None; - }; - if bounds.height <= 1 { - return None; - } - // Reuse the same wrapped prefix across small height changes. - let bounds = Size { - height: bounds.height.max(4), - ..bounds - }; - let line = line.as_ref(); - let cache = &self.wrapped_native; - if cache - .borrow() - .as_ref() - .is_none_or(|entry| entry.bounds != bounds || &entry.content != line) - { - if line.width() <= usize::from(bounds.width) { - return None; - } - cache.replace(Some(WrappedNativeStatus { - bounds, - content: line.clone(), - rows: wrap_rows(line, bounds), - })); - } - Ref::filter_map(cache.borrow(), |entry| { - entry.as_ref().map(|entry| entry.rows.as_slice()) - }) - .ok() +fn wrapped_rows( + native: &CachedLayout, Option>>, + bounds: Size, +) -> Option> { + if bounds.height <= 1 { + return None; } + // Reuse the same wrapped prefix across small height changes. + let bounds = Size { + height: bounds.height.max(4), + ..bounds + }; + let cached = native.get(bounds, |line| { + (line.width() > usize::from(bounds.width)).then(|| wrap_rows(line, bounds)) + }); + Ref::filter_map(cached, Option::as_deref).ok() +} + +fn render_status_row( + line: &HyperlinkLine, + area: Rect, + buf: &mut Buffer, + focus: StatusLineFocus, + more_rows: bool, +) { + let mut line = Cow::Borrowed(line); + let width = usize::from(area.width); + if more_rows || line.width() > width { + let line = line.to_mut(); + line.line = truncate_line_with_ellipsis(std::mem::take(&mut line.line), width); + line.clip_hyperlinks_to_width(line.width().saturating_sub(1)); + } + if focus == StatusLineFocus::Unfocused { + let line = line.to_mut(); + line.hyperlinks.clear(); + line.line.style = Style::reset().dim(); + for span in &mut line.line.spans { + span.style = Style::default(); + } + } + line.render_fitted(area, buf); } fn wrap_rows(status: &HyperlinkLine, bounds: Size) -> Vec { @@ -185,3 +262,7 @@ fn wrap_rows(status: &HyperlinkLine, bounds: Size) -> Vec { }) .collect() } + +#[cfg(test)] +#[path = "status_line_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/bottom_pane/status_line_setup.rs b/codex-rs/tui/src/bottom_pane/status_line_setup.rs index 0d7985f6ea..a7c9ae417e 100644 --- a/codex-rs/tui/src/bottom_pane/status_line_setup.rs +++ b/codex-rs/tui/src/bottom_pane/status_line_setup.rs @@ -357,13 +357,15 @@ impl StatusLineSetupView { let use_theme_colors = items .iter() .any(|item| item.id == STATUS_LINE_USE_THEME_COLORS_ITEM_ID && item.enabled); - preview_data.status_line_for_items( - items - .iter() - .filter(|item| item.enabled) - .filter_map(|item| item.id.parse::().ok()), - use_theme_colors, - ) + preview_data + .status_line_for_items( + items + .iter() + .filter(|item| item.enabled) + .filter_map(|item| item.id.parse::().ok()), + use_theme_colors, + ) + .map(Into::into) }) .on_confirm(|ids, app_event| { let use_theme_colors = ids @@ -449,7 +451,7 @@ impl Renderable for StatusLineSetupView { mod tests { use super::*; use crate::app_event_sender::AppEventSender; - use crate::bottom_pane::footer_content::FooterContent; + use crate::terminal_hyperlinks::HyperlinkLine; use insta::assert_snapshot; use pretty_assertions::assert_eq; use ratatui::buffer::Buffer; @@ -840,7 +842,7 @@ mod tests { .join("\n") } - fn line_text(content: Option) -> Option { - content.map(|content| content.into_hyperlink_line().line.to_string()) + fn line_text(content: Option) -> Option { + content.map(|line| line.line.to_string()) } } diff --git a/codex-rs/tui/src/bottom_pane/footer/footer_content_tests.rs b/codex-rs/tui/src/bottom_pane/status_line_tests.rs similarity index 87% rename from codex-rs/tui/src/bottom_pane/footer/footer_content_tests.rs rename to codex-rs/tui/src/bottom_pane/status_line_tests.rs index 246465dd5a..5c0b76fce3 100644 --- a/codex-rs/tui/src/bottom_pane/footer/footer_content_tests.rs +++ b/codex-rs/tui/src/bottom_pane/status_line_tests.rs @@ -1,6 +1,7 @@ #![cfg(test)] -use super::FooterContent; +use super::StatusLine; +use super::StatusLineFocus; use crate::terminal_hyperlinks::HyperlinkLine; use crate::terminal_hyperlinks::osc8_hyperlink; use pretty_assertions::assert_eq; @@ -20,7 +21,7 @@ fn clipped_links_keep_their_cells_after_a_halfwidth_katakana_grapheme() { /*x*/ 0, /*y*/ 0, /*width*/ 10, /*height*/ 1, ); let mut buf = Buffer::empty(area); - FooterContent::from(line).render(area, &mut buf); + StatusLine::from(line).render(area, &mut buf, StatusLineFocus::Focused); assert_eq!(buf[(0, 0)].symbol(), osc8_hyperlink(project_url, "ｶﾞ")); for (x, ch) in (2..5).zip(" · ".chars()) { assert_eq!(buf[(x, 0)].symbol(), ch.to_string()); @@ -40,12 +41,12 @@ fn arabic_prefix_keeps_link_on_the_painted_cells_at_exact_fit_and_overflow() { let mut line = HyperlinkLine::from("لا · "); line.push_span("PR #42".into(), Some(url)); assert_eq!(line.width(), 11); - let content = FooterContent::from(line); + let content = StatusLine::from(line); for (width, visible_link) in [(11, "PR #42"), (10, "PR #")] { let area = Rect::new(/*x*/ 0, /*y*/ 0, width, /*height*/ 1); let mut buf = Buffer::empty(area); - content.render(area, &mut buf); + content.render(area, &mut buf, StatusLineFocus::Focused); assert_eq!(buf[(4, 0)].symbol(), " "); for (x, ch) in (5..width).zip(visible_link.chars()) { assert_eq!(buf[(x, 0)].symbol(), osc8_hyperlink(url, &ch.to_string())); @@ -57,7 +58,7 @@ fn arabic_prefix_keeps_link_on_the_painted_cells_at_exact_fit_and_overflow() { } #[test] -fn prepared_tree_uses_its_natural_width_and_paints_only_the_footer_viewport() { +fn prepared_tree_paints_only_the_footer_viewport() { use crate::terminal_hyperlinks::strip_osc8; use crate::ui_tree::UiTree; use ratatui::buffer::Cell; @@ -69,8 +70,7 @@ fn prepared_tree_uses_its_natural_width_and_paints_only_the_footer_viewport() { .to_string() .parse() .unwrap(); - let prepared = FooterContent::from(tree); - assert_eq!(prepared.width(), 11); + let prepared = StatusLine::from(tree); let area = Rect::new( /*x*/ 1, /*y*/ 1, /*width*/ 6, /*height*/ 4, @@ -81,7 +81,7 @@ fn prepared_tree_uses_its_natural_width_and_paints_only_the_footer_viewport() { ), Cell::new("."), ); - prepared.render(area, &mut buffer); + prepared.render(area, &mut buffer, StatusLineFocus::Focused); let rows: Vec = buffer .content .chunks(8) diff --git a/codex-rs/tui/src/bottom_pane/status_surface_preview.rs b/codex-rs/tui/src/bottom_pane/status_surface_preview.rs index f559bbadeb..efd29b8130 100644 --- a/codex-rs/tui/src/bottom_pane/status_surface_preview.rs +++ b/codex-rs/tui/src/bottom_pane/status_surface_preview.rs @@ -1,6 +1,6 @@ use std::collections::BTreeMap; -use super::footer_content::FooterContent; +use crate::terminal_hyperlinks::HyperlinkLine; use codex_protocol::ThreadId; use super::status_line_from_segments; @@ -248,7 +248,7 @@ impl StatusSurfacePreviewData { &self, items: I, use_theme_colors: bool, - ) -> Option + ) -> Option where I: IntoIterator, { @@ -263,7 +263,6 @@ impl StatusSurfacePreviewData { self.live_value_for(StatusSurfacePreviewItem::PullRequestNumber) .and(self.pull_request_url.as_deref()), ) - .map(FooterContent::from) } } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index eace65fcd9..fe00942844 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2122,14 +2122,16 @@ impl ChatWidget { #[cfg(test)] pub(crate) fn status_line_text(&self) -> Option { - self.status_line_content() - .cloned() - .map(|content| content.into_hyperlink_line().line.to_string()) + self.status_line_content().map(|status| { + status + .as_native() + .expect("native status line") + .line + .to_string() + }) } - pub(crate) fn status_line_content( - &self, - ) -> Option<&crate::bottom_pane::footer_content::FooterContent> { + pub(crate) fn status_line_content(&self) -> Option<&crate::bottom_pane::StatusLine> { self.bottom_pane.status_line_content() } diff --git a/codex-rs/tui/src/chatwidget/status_controls.rs b/codex-rs/tui/src/chatwidget/status_controls.rs index 4d947a952b..7fb8adb577 100644 --- a/codex-rs/tui/src/chatwidget/status_controls.rs +++ b/codex-rs/tui/src/chatwidget/status_controls.rs @@ -5,7 +5,7 @@ //! history-facing `/status` surface. use super::*; -use crate::bottom_pane::footer_content::FooterContent; +use crate::bottom_pane::StatusLine; use crate::status::thread_cost_micros; impl ChatWidget { @@ -84,7 +84,7 @@ impl ChatWidget { } /// Sets the currently rendered footer status-line value. - pub(crate) fn set_status_line(&mut self, status_line: Option) { + pub(crate) fn set_status_line(&mut self, status_line: Option) { self.bottom_pane.set_status_line(status_line); } diff --git a/codex-rs/tui/src/chatwidget/tests/status_surface_previews.rs b/codex-rs/tui/src/chatwidget/tests/status_surface_previews.rs index 70d000f94b..4936642c7b 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_surface_previews.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_surface_previews.rs @@ -3,14 +3,6 @@ use crate::bottom_pane::preview_line_for_title_items; use crate::chatwidget::ThreadUsageOutcome; use codex_app_server_protocol::ThreadUsage; use pretty_assertions::assert_eq; -use ratatui::text::Line; - -fn line_text(line: Line<'static>) -> String { - line.spans - .into_iter() - .map(|span| span.content.into_owned()) - .collect() -} #[tokio::test] async fn thread_color_preview_matches_footer() { @@ -31,7 +23,6 @@ async fn thread_color_preview_matches_footer() { let line = preview .status_line_for_items(items, use_colors) .unwrap() - .into_hyperlink_line() .line; let footer = crate::bottom_pane::status_line_from_segments( items @@ -54,7 +45,7 @@ fn status_preview_line_option(chat: &mut ChatWidget, items: &[StatusLineItem]) - let preview_data = chat.status_surface_preview_data(); preview_data .status_line_for_items(items.iter().copied(), /*use_theme_colors*/ true) - .map(|content| line_text(content.into_hyperlink_line().line)) + .map(|line| line.line.to_string()) } fn status_preview_line(chat: &mut ChatWidget, items: &[StatusLineItem]) -> String { @@ -65,7 +56,7 @@ fn title_preview_line(chat: &mut ChatWidget, items: &[TerminalTitleItem]) -> Str let preview_data = chat.terminal_title_preview_data(); let preview = preview_line_for_title_items(items, &preview_data).expect("terminal title preview line"); - line_text(preview) + preview.to_string() } fn combined_preview_snapshot( diff --git a/codex-rs/tui/src/ui_tree/render.rs b/codex-rs/tui/src/ui_tree/render.rs index 7502882293..3a347a5ed6 100644 --- a/codex-rs/tui/src/ui_tree/render.rs +++ b/codex-rs/tui/src/ui_tree/render.rs @@ -14,16 +14,12 @@ use super::TextProps; use super::TextWeight; use super::UiTree; use super::theme::RenderTheme; -use crate::terminal_hyperlinks::HyperlinkLine; -use crate::terminal_hyperlinks::TerminalHyperlink; use crate::terminal_hyperlinks::mark_hyperlink_cells; use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::layout::Size; use ratatui::style::Modifier; use ratatui::style::Style; -use ratatui::text::Span; -use std::ops::Range; use syntect::highlighting::Theme; use vtparse::CollectingVTActor; use vtparse::VTAction; @@ -42,13 +38,6 @@ impl UiTree { } impl PreparedUiTree { - pub(crate) fn measure_natural(&self) -> Size { - self.0.as_ref().map_or_else(Size::default, |node| { - let width = u16::try_from(node.widths.preferred).unwrap_or(u16::MAX); - Size::new(width, node.natural_height) - }) - } - pub(crate) fn measure(&self, bounds: Size) -> Size { layout(self.0.as_ref(), bounds, /*draw*/ None) } @@ -83,29 +72,6 @@ impl PreparedUiTree { }), ); } - - pub(crate) fn render_line(&self, width: u16, theme: &Theme) -> HyperlinkLine { - let natural = self.measure_natural(); - if width == 1 && (natural.width > 1 || natural.height > 1) { - return ratatui::text::Line::from("…").into(); - } - let mut line = HyperlinkLine::default(); - let mut theme = RenderTheme::new(theme); - layout( - self.0.as_ref(), - Size::new(width, /*height*/ 1), - Some(&mut |_, col, text, cells, paint| { - line.line - .push_span(Span::styled(text.to_owned(), paint.style(&mut theme))); - if cells > 0 - && let Some(href) = &paint.href - { - push_link(&mut line, col..col + cells, href); - } - }), - ); - line - } } #[derive(Clone, Copy)] @@ -332,18 +298,6 @@ fn sanitize_leaf(mut raw: String) -> String { .collect() } -fn push_link(line: &mut HyperlinkLine, columns: Range, href: &str) { - if let Some(last) = line.hyperlinks.last_mut() - && last.columns.end == columns.start - && last.destination == href - { - last.columns.end = columns.end; - } else { - line.hyperlinks - .push(TerminalHyperlink::web(columns, href.to_owned())); - } -} - impl TextProps { fn apply_to(&self, paint: &mut Paint) { if let Some(color) = self.color { diff --git a/codex-rs/tui/src/ui_tree/render/prepare.rs b/codex-rs/tui/src/ui_tree/render/prepare.rs index 095ddea11f..8a870e9743 100644 --- a/codex-rs/tui/src/ui_tree/render/prepare.rs +++ b/codex-rs/tui/src/ui_tree/render/prepare.rs @@ -10,7 +10,6 @@ use crate::ui_tree::UiNode; pub(super) struct PreparedNode { pub(super) kind: NodeKind, pub(super) widths: Widths, - pub(super) natural_height: u16, } #[derive(Debug, PartialEq)] @@ -62,7 +61,6 @@ impl PreparedNode { widths.preferred = widths.preferred.max(width); } Some(Self { - natural_height: u16::try_from(text.lines.len()).unwrap_or(u16::MAX), kind: NodeKind::Text(text), widths, }) @@ -97,11 +95,6 @@ impl PreparedNode { }; Some(Self { widths, - natural_height: children - .iter() - .map(|child| child.natural_height) - .max() - .unwrap_or_default(), kind: NodeKind::Row { children, gap, diff --git a/codex-rs/tui/src/ui_tree_tests.rs b/codex-rs/tui/src/ui_tree_tests.rs index 9f6ff6c65f..9a781822c2 100644 --- a/codex-rs/tui/src/ui_tree_tests.rs +++ b/codex-rs/tui/src/ui_tree_tests.rs @@ -34,12 +34,7 @@ fn elements_round_trip_and_require_explicit_row_spacing() { } for (gap, expected) in [(0, "AB"), (1, "A B"), (2, "A B"), (MAX_GAP, "A …")] { let row = tree(json!({"type":"row","props":{"gap":gap},"children":["A","B"]})); - assert_eq!( - row.render_line(/*width*/ 4, &Theme::default()) - .line - .to_string(), - expected - ); + assert_eq!(painted_rows(&row, Size::new(4, 1)), [expected]); } } @@ -104,8 +99,12 @@ fn links_only_annotate_their_own_cells() { /*x*/ 0, /*y*/ 0, /*width*/ 16, /*height*/ 1, ); let mut buffer = Buffer::empty(area); - tree.render_line(area.width, &Theme::default()) - .render_fitted(area, &mut buffer); + tree.render( + area, + &Theme::default(), + &mut buffer, + /*plain_style*/ None, + ); assert_eq!(buffer[(0, 0)].symbol(), "界"); assert!( buffer[(3, 0)] @@ -113,86 +112,43 @@ fn links_only_annotate_their_own_cells() { .contains("https://example.com/pr/42") ); insta::assert_snapshot!(format!("{buffer:?}")); - - assert_eq!( - tree.render_line(/*width*/ 1, &Theme::default()) - .line - .to_string(), - "…" - ); - let narrow = Rect::new( - /*x*/ 0, /*y*/ 0, /*width*/ 6, /*height*/ 2, - ); - let mut buffer = Buffer::empty(narrow); - tree.render_line(narrow.width, &Theme::default()) - .render_fitted(narrow, &mut buffer); - assert!( - buffer[(4, 0)] - .symbol() - .contains("https://example.com/pr/42") - ); - assert_eq!(buffer[(5, 0)].symbol(), "…"); - assert!((0..narrow.width).all(|x| buffer[(x, 1)].symbol() == " ")); } #[test] -fn render_line_keeps_exactly_fitting_halfwidth_voiced_katakana() { - let text = tree(json!("ｶﾞ")); - let area = Rect::new( - /*x*/ 0, /*y*/ 0, /*width*/ 2, /*height*/ 1, - ); - let mut buffer = Buffer::empty(area); - text.render_line(area.width, &Theme::default()) - .render_fitted(area, &mut buffer); - assert_eq!(buffer[(0, 0)].symbol(), "ｶﾞ"); +fn exactly_fitting_halfwidth_voiced_katakana_is_visible() { + assert_eq!(painted_rows(&tree(json!("ｶﾞ")), Size::new(2, 1)), ["ｶﾞ"]); } #[test] fn ellipsis_counts_toward_width_only_when_there_is_overflow() { let row = tree(json!({"type":"row","props":{"gap":1},"children":["abcde", null]})); - for (width, expected) in [(0, ""), (1, "…"), (4, "abc…"), (5, "abcde")] { - let line = row.render_line(width, &Theme::default()); - assert_eq!(line.line.to_string(), expected); - assert!(line.width() <= usize::from(width)); + for (width, expected) in [(1, "…"), (4, "abc…"), (5, "abcde")] { + assert_eq!(painted_rows(&row, Size::new(width, 1)), [expected]); } let wide = tree(json!("a界界")); - assert_eq!( - wide.render_line(/*width*/ 4, &Theme::default()) - .line - .to_string(), - "a界…" - ); - let long = "x".repeat(513); - assert_eq!( - tree(json!(&long)) - .render_line(/*width*/ 513, &Theme::default()) - .line - .to_string(), - long - ); + assert_eq!(painted_rows(&wide, Size::new(4, 1)), ["a界…"]); } #[test] fn zero_width_fragments_do_not_style_the_ellipsis_or_add_row_gaps() { let row = |text| json!({"type":"row","props":{"gap":0},"children":[text]}); - let mark = json!({"type":"bold","children":["\u{301}"]}); - for (mut root, width) in [ - (json!({"type":"text"}), 2), - (json!({"type":"row","props":{"gap":1}}), 4), + let children = json!([row("x"), {"type":"bold","children":["\u{301}"]}, row("yz")]); + for (root, expected) in [ + (json!({"type":"text","children":children}), "x…"), + ( + json!({"type":"row","props":{"gap":1},"children":children}), + "x yz", + ), + ( + json!({"type":"text","children":["A", {"type":"bold","children":["\u{ad}BB"]}]}), + "A…", + ), + ( + json!({"type":"text","children":[{"type":"bold","children":["\u{ad}"]}, "AB"]}), + "…", + ), ] { - root["children"] = json!([row("x"), mark, row("yz")]); - let node = tree(root.clone()); - root["children"] = json!([row("x"), row("yz")]); - assert_eq!( - node.render_line(width, &Theme::default()), - tree(root).render_line(width, &Theme::default()) - ); - } - for (children, expected) in [ - (json!(["A", {"type":"bold","children":["\u{ad}BB"]}]), "A…"), - (json!([{"type":"bold","children":["\u{ad}"]}, "AB"]), "…"), - ] { - let node = tree(json!({"type":"text","children":children})); + let node = tree(root); let expected = Buffer::with_lines([expected]); let mut buf = Buffer::empty(expected.area); node.render( @@ -208,18 +164,12 @@ fn zero_width_fragments_do_not_style_the_ellipsis_or_add_row_gaps() { #[test] fn single_line_view_marks_vertical_overflow() { let invisible = tree(json!({"type":"bold","children":["\u{ad}\na"]})); - let clipped = invisible.render_line(/*width*/ 1, &Theme::default()); - assert_eq!(clipped.line.to_string(), "…"); + assert_eq!(painted_rows(&invisible, Size::new(1, 1)), ["…"]); let multiline = format!("\u{1b}[2Jok\n{}", "x".repeat(1024)); let tree = tree(json!({"type":"row","props":{"gap":0},"children":[ &multiline, "!" ]})); - assert_eq!( - tree.render_line(/*width*/ 4, &Theme::default()) - .line - .to_string(), - "ok …" - ); + assert_eq!(painted_rows(&tree, Size::new(4, 1)), ["ok …"]); } #[test] @@ -227,10 +177,8 @@ 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")))).prepare(); assert_eq!( - text.render_line(u16::MAX, &Theme::default()) - .line - .to_string(), - prefix + painted_rows(&text, Size::new(MAX_TEXT_BYTES as u16, 1)), + [prefix] ); } @@ -288,21 +236,28 @@ fn inline_modifiers_and_row_surfaces_follow_the_tree() { {"type":"link","props":{"href":"https://example.com"},"children":["f"]} ] })); - let clipped = node.render_line(/*width*/ 6, &Theme::default()); - assert_eq!( - clipped.line.spans.last().unwrap(), - &ratatui::text::Span::styled( - "…", - ratatui::style::Style::default().bg(ratatui::style::Color::Blue), - ) - ); - let line = node.render_line(/*width*/ 12, &Theme::default()); let area = Rect::new( /*x*/ 0, /*y*/ 0, /*width*/ 12, /*height*/ 1, ); let mut buffer = Buffer::empty(area); - line.render_fitted(area, &mut buffer); + node.render( + area, + &Theme::default(), + &mut buffer, + /*plain_style*/ None, + ); insta::assert_snapshot!(format!("{buffer:?}")); + + let clipped = Rect { width: 6, ..area }; + let mut buffer = Buffer::empty(clipped); + node.render( + clipped, + &Theme::default(), + &mut buffer, + /*plain_style*/ None, + ); + assert_eq!(buffer[(5, 0)].symbol(), "…"); + assert_eq!(buffer[(5, 0)].bg, ratatui::style::Color::Blue); } #[test] @@ -318,18 +273,27 @@ fn hex_colors_use_the_terminal_palette_for_both_channels() { bg: (0, 0, 0), }, || { - let line = tree.render_line(/*width*/ 3, &Theme::default()); - let colors: Vec<_> = line - .line - .spans + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 3, /*height*/ 1, + ); + let mut buffer = Buffer::empty(area); + tree.render( + area, + &Theme::default(), + &mut buffer, + /*plain_style*/ None, + ); + let colors: Vec<_> = buffer + .content .iter() - .map(|s| (s.style.fg, s.style.bg)) + .map(|cell| (cell.fg, cell.bg)) .collect(); - let surface = Some(crate::terminal_palette::rgb_color((16, 32, 240))); - let foreground = Some(crate::terminal_palette::rgb_color((171, 18, 240))); + let surface = crate::terminal_palette::rgb_color((16, 32, 240)); + let foreground = crate::terminal_palette::rgb_color((171, 18, 240)); + let reset = ratatui::style::Color::Reset; assert_eq!( colors, - [(foreground, surface), (None, surface), (None, surface)] + [(foreground, surface), (reset, surface), (reset, surface)] ); }, ); @@ -439,22 +403,16 @@ fn terminal_commands_are_sanitized_within_each_string() { "\u{1b}[31mre\rd\u{1b}[0m \u{1b}]8;;https://example.com\u{7}label\u{1b}]8;;\u{1b}\\\u{7}", " \u{1b}[31", {"type":"link","props":{"href":url},"children":["Ready"]} ]})); - let line = node.render_line(/*width*/ 40, &Theme::default()); assert_eq!( - line.hyperlinks, - [crate::terminal_hyperlinks::TerminalHyperlink::web( - 10..15, - url.to_owned() - )] + painted_rows(&node, Size::new(40, 1)), + [linked_columns("red label Ready", 10..15, url)] ); - insta::assert_snapshot!(line.line.to_string(), @"red label Ready"); } #[test] fn split_graphemes_use_the_first_style_and_require_unanimous_links() { - use crate::terminal_hyperlinks::TerminalHyperlink; - use ratatui::style::Stylize; - use ratatui::text::Span; + use crate::terminal_hyperlinks::osc8_hyperlink; + use ratatui::style::Modifier; let url = "https://example.com/a"; let link = |href, text| json!({"type":"link","props":{"href":href},"children":[text]}); @@ -465,44 +423,42 @@ fn split_graphemes_use_the_first_style_and_require_unanimous_links() { {"type":"bold","children":["\u{600}"]}, " x", link(url, "तक्क"), "\u{301}" ]})); - let line = node.render_line(/*width*/ 20, &Theme::default()); - assert_eq!( - line.line.spans, - [ - Span::raw("R").bold(), - Span::raw("e\u{301}").bold(), - Span::raw("👩‍💻").underlined(), - Span::raw("👩‍💻").underlined(), - Span::raw("\u{600} ").bold(), - Span::raw("x"), - Span::raw("त").underlined(), - Span::raw("क्क\u{301}").underlined() - ] + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 20, /*height*/ 1, ); - assert_eq!( - line.hyperlinks, - [ - TerminalHyperlink::web(2..4, url.to_owned()), - TerminalHyperlink::web(9..10, url.to_owned()) - ] + let mut buffer = Buffer::empty(area); + node.render( + area, + &Theme::default(), + &mut buffer, + /*plain_style*/ None, ); + for (x, symbol, modifier) in [ + (0, "R".to_owned(), Modifier::BOLD), + (1, "e\u{301}".to_owned(), Modifier::BOLD), + (2, osc8_hyperlink(url, "👩‍💻"), Modifier::UNDERLINED), + (4, "👩‍💻".to_owned(), Modifier::UNDERLINED), + (6, "\u{600} ".to_owned(), Modifier::BOLD), + (8, "x".to_owned(), Modifier::empty()), + (9, osc8_hyperlink(url, "त"), Modifier::UNDERLINED), + (10, "क्क\u{301}".to_owned(), Modifier::UNDERLINED), + ] { + let cell = &buffer[(x, 0)]; + assert_eq!((cell.symbol(), cell.modifier), (symbol.as_str(), modifier)); + } } #[test] fn collapsed_space_keeps_the_first_whitespace_owners_link() { - use crate::terminal_hyperlinks::TerminalHyperlink; - let href = "https://example.com/build"; let node = tree(json!({"type":"text","children":[ "a", {"type":"link","props":{"href":href},"children":["\t"]}, "\u{1b}[31m", " b", {"type":"link","props":{"href":href},"children":[" "]}, {"type":"row","props":{"gap":0},"children":[" c "]}, " d" ]})); - let line = node.render_line(/*width*/ 10, &Theme::default()); - assert_eq!(line.line.to_string(), "a bcd"); assert_eq!( - line.hyperlinks, - [TerminalHyperlink::web(1..2, href.to_owned())] + painted_rows(&node, Size::new(10, 1)), + [linked_columns("a bcd", 1..2, href)] ); } @@ -539,18 +495,13 @@ fn linked_columns(text: &str, columns: std::ops::Range, href: &str) -> St } #[test] -fn measures_natural_and_viewport_sizes_in_cells() { +fn measures_viewport_sizes_in_cells() { let node = tree(json!({"type":"row","props":{"gap":1,"wrap":"wrap"},"children":["ab\ncd","界界"]})); - assert_eq!(node.measure_natural(), Size::new(7, 2)); assert_eq!(node.measure(Size::new(4, 8)), Size::new(4, 3)); let wide = tree(json!("a界\nnext")); assert_eq!(wide.measure(Size::new(3, 1)), Size::new(2, 1)); assert_eq!(painted_rows(&wide, Size::new(3, 1)), ["a…"]); - let huge = - tree(json!({"type":"row","props":{"gap":MAX_GAP,"wrap":"wrap"},"children":["x","y"]})); - assert_eq!(huge.measure_natural(), Size::new(u16::MAX, 1)); - assert_eq!(tree(Value::Null).measure_natural(), Size::default()); } #[test] @@ -684,12 +635,6 @@ fn authored_line_breaks_and_collapsed_whitespace_preserve_styles_and_links() { "{buffer:?}" ))); assert_eq!(buffer[(6, 3)].symbol(), "…"); - assert_eq!( - node.render_line(/*width*/ 20, &Theme::default()) - .line - .to_string(), - "ready now…" - ); } #[test]