From 95b41dd7f18b85a696758e3efe242c8880e9755a Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Thu, 9 Oct 2025 10:39:45 -0700 Subject: [PATCH] tui: fix wrapping in trust_directory (#5007) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor trust_directory to use ColumnRenderable & friends, thus correcting wrapping behavior at small widths. Also introduce RowRenderable with fixed-width rows. - fixed wrapping in trust_directory - changed selector cursor to match other list item selections - allow y/n to work as well as 1/2 - fixed key_hint to be standard before: Screenshot 2025-10-09 at 9 50 36 AM after: Screenshot 2025-10-09 at 9 51 31 AM --- .../tui/src/bottom_pane/approval_overlay.rs | 8 +- .../src/bottom_pane/list_selection_view.rs | 2 +- ...exec_and_status_layout_vt100_snapshot.snap | 1 + codex-rs/tui/src/diff_render.rs | 2 +- ..._tests__renders_snapshot_for_git_repo.snap | 14 ++ .../tui/src/onboarding/trust_directory.rs | 171 ++++++++++++------ codex-rs/tui/src/render/mod.rs | 8 +- codex-rs/tui/src/render/renderable.rs | 78 +++++++- codex-rs/tui/src/test_backend.rs | 3 +- codex-rs/tui/src/wrapping.rs | 1 + 10 files changed, 220 insertions(+), 68 deletions(-) create mode 100644 codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_snapshot_for_git_repo.snap diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index d6e96cf7..ce0e3c9c 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -104,9 +104,9 @@ impl ApprovalOverlay { ), }; - let header = Box::new(ColumnRenderable::new([ - Box::new(Line::from(title.bold())), - Box::new(Line::from("")), + let header = Box::new(ColumnRenderable::with([ + Line::from(title.bold()).into(), + Line::from("").into(), header, ])); @@ -323,7 +323,7 @@ impl From for ApprovalRequestState { header.push(DiffSummary::new(changes, cwd).into()); Self { variant: ApprovalVariant::ApplyPatch { id }, - header: Box::new(ColumnRenderable::new(header)), + header: Box::new(ColumnRenderable::with(header)), } } } diff --git a/codex-rs/tui/src/bottom_pane/list_selection_view.rs b/codex-rs/tui/src/bottom_pane/list_selection_view.rs index 238f74d6..1e8d1983 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -87,7 +87,7 @@ impl ListSelectionView { if params.title.is_some() || params.subtitle.is_some() { let title = params.title.map(|title| Line::from(title.bold())); let subtitle = params.subtitle.map(|subtitle| Line::from(subtitle.dim())); - header = Box::new(ColumnRenderable::new([ + header = Box::new(ColumnRenderable::with([ header, Box::new(title), Box::new(subtitle), diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap index f913ce08..c3bdf60b 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap @@ -13,4 +13,5 @@ expression: term.backend().vt100().screen().contents() › Summarize recent commits + 100% context left diff --git a/codex-rs/tui/src/diff_render.rs b/codex-rs/tui/src/diff_render.rs index a4b7c87f..665fad15 100644 --- a/codex-rs/tui/src/diff_render.rs +++ b/codex-rs/tui/src/diff_render.rs @@ -72,7 +72,7 @@ impl From for Box { ))); } - Box::new(ColumnRenderable::new(rows)) + Box::new(ColumnRenderable::with(rows)) } } diff --git a/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_snapshot_for_git_repo.snap b/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_snapshot_for_git_repo.snap new file mode 100644 index 00000000..71cbc3d6 --- /dev/null +++ b/codex-rs/tui/src/onboarding/snapshots/codex_tui__onboarding__trust_directory__tests__renders_snapshot_for_git_repo.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/onboarding/trust_directory.rs +expression: terminal.backend() +--- +> You are running Codex in /workspace/project + + Since this folder is version controlled, you may wish to allow Codex + to work in this folder without asking for approval. + +› 1. Yes, allow Codex to work in this folder without asking for + approval + 2. No, ask me to approve edits and commands + + Press enter to continue diff --git a/codex-rs/tui/src/onboarding/trust_directory.rs b/codex-rs/tui/src/onboarding/trust_directory.rs index 1058208b..3324c3d8 100644 --- a/codex-rs/tui/src/onboarding/trust_directory.rs +++ b/codex-rs/tui/src/onboarding/trust_directory.rs @@ -7,19 +7,25 @@ use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; use ratatui::buffer::Buffer; use ratatui::layout::Rect; -use ratatui::prelude::Widget; -use ratatui::style::Color; -use ratatui::style::Modifier; +use ratatui::style::Style; +use ratatui::style::Styled as _; use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; use ratatui::widgets::Wrap; +use crate::key_hint; use crate::onboarding::onboarding_screen::KeyboardHandler; use crate::onboarding::onboarding_screen::StepStateProvider; +use crate::render::Insets; +use crate::render::renderable::ColumnRenderable; +use crate::render::renderable::Renderable; +use crate::render::renderable::RenderableExt as _; +use crate::render::renderable::RowRenderable; use super::onboarding_screen::StepState; +use unicode_width::UnicodeWidthStr; pub(crate) struct TrustDirectoryWidget { pub codex_home: PathBuf, @@ -38,76 +44,106 @@ pub enum TrustDirectorySelection { impl WidgetRef for &TrustDirectoryWidget { fn render_ref(&self, area: Rect, buf: &mut Buffer) { - let mut lines: Vec = vec![ - Line::from(vec![ - "> ".into(), - "You are running Codex in ".bold(), - self.cwd.to_string_lossy().to_string().into(), - ]), - "".into(), - ]; + let mut column = ColumnRenderable::new(); - if self.is_git_repo { - lines.push( - " Since this folder is version controlled, you may wish to allow Codex".into(), - ); - lines.push(" to work in this folder without asking for approval.".into()); + column.push(Line::from(vec![ + "> ".into(), + "You are running Codex in ".bold(), + self.cwd.to_string_lossy().to_string().into(), + ])); + column.push(""); + + let guidance = if self.is_git_repo { + "Since this folder is version controlled, you may wish to allow Codex to work in this folder without asking for approval." } else { - lines.push( - " Since this folder is not version controlled, we recommend requiring".into(), - ); - lines.push(" approval of all edits and commands.".into()); - } - lines.push("".into()); + "Since this folder is not version controlled, we recommend requiring approval of all edits and commands." + }; - let create_option = - |idx: usize, option: TrustDirectorySelection, text: &str| -> Line<'static> { - let is_selected = self.highlighted == option; - if is_selected { - Line::from(format!("> {}. {text}", idx + 1)).cyan() - } else { - Line::from(format!(" {}. {}", idx + 1, text)) - } - }; + column.push( + Paragraph::new(guidance.to_string()) + .wrap(Wrap { trim: true }) + .inset(Insets::tlbr(0, 2, 0, 0)), + ); + column.push(""); + let mut options: Vec<(&str, TrustDirectorySelection)> = Vec::new(); if self.is_git_repo { - lines.push(create_option( - 0, - TrustDirectorySelection::Trust, + options.push(( "Yes, allow Codex to work in this folder without asking for approval", + TrustDirectorySelection::Trust, )); - lines.push(create_option( - 1, - TrustDirectorySelection::DontTrust, + options.push(( "No, ask me to approve edits and commands", + TrustDirectorySelection::DontTrust, )); } else { - lines.push(create_option( - 0, - TrustDirectorySelection::Trust, + options.push(( "Allow Codex to work in this folder without asking for approval", + TrustDirectorySelection::Trust, )); - lines.push(create_option( - 1, - TrustDirectorySelection::DontTrust, + options.push(( "Require approval of edits and commands", + TrustDirectorySelection::DontTrust, )); } - lines.push("".into()); - if let Some(error) = &self.error { - lines.push(Line::from(format!(" {error}")).fg(Color::Red)); - lines.push("".into()); - } - // AE: Following styles.md, this should probably be Cyan because it's a user input tip. - // But leaving this for a future cleanup. - lines.push(Line::from(" Press Enter to continue").add_modifier(Modifier::DIM)); - Paragraph::new(lines) - .wrap(Wrap { trim: false }) - .render(area, buf); + for (idx, (text, selection)) in options.iter().enumerate() { + column.push(new_option_row( + idx, + text.to_string(), + self.highlighted == *selection, + )); + } + + column.push(""); + + if let Some(error) = &self.error { + column.push( + Paragraph::new(error.to_string()) + .red() + .wrap(Wrap { trim: true }) + .inset(Insets::tlbr(0, 2, 0, 0)), + ); + column.push(""); + } + + column.push( + Line::from(vec![ + "Press ".dim(), + key_hint::plain(KeyCode::Enter).into(), + " to continue".dim(), + ]) + .inset(Insets::tlbr(0, 2, 0, 0)), + ); + + column.render(area, buf); } } +fn new_option_row(index: usize, label: String, is_selected: bool) -> Box { + let prefix = if is_selected { + format!("› {}. ", index + 1) + } else { + format!(" {}. ", index + 1) + }; + + let mut style = Style::default(); + if is_selected { + style = style.cyan(); + } + + let mut row = RowRenderable::new(); + row.push(prefix.width() as u16, prefix.set_style(style)); + row.push( + u16::MAX, + Paragraph::new(label) + .style(style) + .wrap(Wrap { trim: false }), + ); + + row.into() +} + impl KeyboardHandler for TrustDirectoryWidget { fn handle_key_event(&mut self, key_event: KeyEvent) { if key_event.kind == KeyEventKind::Release { @@ -121,8 +157,8 @@ impl KeyboardHandler for TrustDirectoryWidget { KeyCode::Down | KeyCode::Char('j') => { self.highlighted = TrustDirectorySelection::DontTrust; } - KeyCode::Char('1') => self.handle_trust(), - KeyCode::Char('2') => self.handle_dont_trust(), + KeyCode::Char('1') | KeyCode::Char('y') => self.handle_trust(), + KeyCode::Char('2') | KeyCode::Char('n') => self.handle_dont_trust(), KeyCode::Enter => match self.highlighted { TrustDirectorySelection::Trust => self.handle_trust(), TrustDirectorySelection::DontTrust => self.handle_dont_trust(), @@ -161,12 +197,16 @@ impl TrustDirectoryWidget { #[cfg(test)] mod tests { + use crate::test_backend::VT100Backend; + use super::*; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; use crossterm::event::KeyModifiers; use pretty_assertions::assert_eq; + use ratatui::Terminal; + use std::path::PathBuf; #[test] @@ -191,4 +231,23 @@ mod tests { widget.handle_key_event(press); assert_eq!(widget.selection, Some(TrustDirectorySelection::DontTrust)); } + + #[test] + fn renders_snapshot_for_git_repo() { + let widget = TrustDirectoryWidget { + codex_home: PathBuf::from("."), + cwd: PathBuf::from("/workspace/project"), + is_git_repo: true, + selection: None, + highlighted: TrustDirectorySelection::Trust, + error: None, + }; + + let mut terminal = Terminal::new(VT100Backend::new(70, 14)).expect("terminal"); + terminal + .draw(|f| (&widget).render_ref(f.area(), f.buffer_mut())) + .expect("draw"); + + insta::assert_snapshot!(terminal.backend()); + } } diff --git a/codex-rs/tui/src/render/mod.rs b/codex-rs/tui/src/render/mod.rs index bf4fb553..a2e49205 100644 --- a/codex-rs/tui/src/render/mod.rs +++ b/codex-rs/tui/src/render/mod.rs @@ -6,10 +6,10 @@ pub mod renderable; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct Insets { - pub left: u16, - pub top: u16, - pub right: u16, - pub bottom: u16, + left: u16, + top: u16, + right: u16, + bottom: u16, } impl Insets { diff --git a/codex-rs/tui/src/render/renderable.rs b/codex-rs/tui/src/render/renderable.rs index 62c27206..df064495 100644 --- a/codex-rs/tui/src/render/renderable.rs +++ b/codex-rs/tui/src/render/renderable.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::text::Line; +use ratatui::text::Span; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; @@ -45,6 +46,15 @@ impl Renderable for String { } } +impl<'a> Renderable for Span<'a> { + fn render(&self, area: Rect, buf: &mut Buffer) { + self.render_ref(area, buf); + } + fn desired_height(&self, _width: u16) -> u16 { + 1 + } +} + impl<'a> Renderable for Line<'a> { fn render(&self, area: Rect, buf: &mut Buffer) { WidgetRef::render_ref(self, area, buf); @@ -114,11 +124,64 @@ impl Renderable for ColumnRenderable { } impl ColumnRenderable { - pub fn new(children: impl IntoIterator>) -> Self { + pub fn new() -> Self { + Self::with(vec![]) + } + + pub fn with(children: impl IntoIterator>) -> Self { Self { children: children.into_iter().collect(), } } + + pub fn push(&mut self, child: impl Into>) { + self.children.push(child.into()); + } +} + +pub struct RowRenderable { + children: Vec<(u16, Box)>, +} + +impl Renderable for RowRenderable { + fn render(&self, area: Rect, buf: &mut Buffer) { + let mut x = area.x; + for (width, child) in &self.children { + let available_width = area.width.saturating_sub(x - area.x); + let child_area = Rect::new(x, area.y, (*width).min(available_width), area.height); + if child_area.is_empty() { + break; + } + child.render(child_area, buf); + x = x.saturating_add(*width); + } + } + fn desired_height(&self, width: u16) -> u16 { + let mut max_height = 0; + let mut width_remaining = width; + for (child_width, child) in &self.children { + let w = (*child_width).min(width_remaining); + if w == 0 { + break; + } + let height = child.desired_height(w); + if height > max_height { + max_height = height; + } + width_remaining = width_remaining.saturating_sub(w); + } + max_height + } +} + +impl RowRenderable { + pub fn new() -> Self { + Self { children: vec![] } + } + + pub fn push(&mut self, width: u16, child: impl Into>) { + self.children.push((width, child.into())); + } } pub struct InsetRenderable { @@ -146,3 +209,16 @@ impl InsetRenderable { } } } + +pub trait RenderableExt { + fn inset(self, insets: Insets) -> Box; +} + +impl>> RenderableExt for R { + fn inset(self, insets: Insets) -> Box { + Box::new(InsetRenderable { + child: self.into(), + insets, + }) + } +} diff --git a/codex-rs/tui/src/test_backend.rs b/codex-rs/tui/src/test_backend.rs index 52e72547..a5460af2 100644 --- a/codex-rs/tui/src/test_backend.rs +++ b/codex-rs/tui/src/test_backend.rs @@ -91,7 +91,8 @@ impl Backend for VT100Backend { } fn size(&self) -> io::Result { - Ok(self.vt100().screen().size().into()) + let (rows, cols) = self.vt100().screen().size(); + Ok(Size::new(cols, rows)) } fn window_size(&mut self) -> io::Result { diff --git a/codex-rs/tui/src/wrapping.rs b/codex-rs/tui/src/wrapping.rs index 70ca2e46..3c3b775e 100644 --- a/codex-rs/tui/src/wrapping.rs +++ b/codex-rs/tui/src/wrapping.rs @@ -154,6 +154,7 @@ impl<'a> RtOptions<'a> { } } +#[must_use] pub(crate) fn word_wrap_line<'a, O>(line: &'a Line<'a>, width_or_options: O) -> Vec> where O: Into>,