diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 7f52b11b..6585124c 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -315,15 +315,16 @@ impl From for ApprovalRequestState { changes, } => { let mut header: Vec> = Vec::new(); - header.push(DiffSummary::new(changes, cwd).into()); if let Some(reason) = reason && !reason.is_empty() { - header.push(Box::new(Line::from(""))); header.push(Box::new( - Paragraph::new(reason.italic()).wrap(Wrap { trim: false }), + Paragraph::new(Line::from_iter(["Reason: ".into(), reason.italic()])) + .wrap(Wrap { trim: false }), )); + header.push(Box::new(Line::from(""))); } + header.push(DiffSummary::new(changes, cwd).into()); Self { variant: ApprovalVariant::ApplyPatch { id }, header: Box::new(ColumnRenderable::new(header)), diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_manual_flow_history_approved.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_manual_flow_history_approved.snap index 6e22bceb..e139b510 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_manual_flow_history_approved.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_manual_flow_history_approved.snap @@ -3,4 +3,4 @@ source: tui/src/chatwidget/tests.rs expression: lines_to_single_string(&approved_lines) --- • Added foo.txt (+1 -0) - 1 +hello + 1 +hello diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap index ab88ffaf..96dde8fb 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap @@ -4,12 +4,12 @@ expression: terminal.backend().vt100().screen().contents() --- Would you like to make the following edits? + Reason: The model wants to apply changes + README.md (+2 -0) - 1 +hello - 2 +world - - The model wants to apply changes + 1 +hello + 2 +world › 1. Yes, proceed 2. No, and tell Codex what to do differently esc diff --git a/codex-rs/tui/src/diff_render.rs b/codex-rs/tui/src/diff_render.rs index 6f5f7e7d..676b0559 100644 --- a/codex-rs/tui/src/diff_render.rs +++ b/codex-rs/tui/src/diff_render.rs @@ -13,13 +13,14 @@ use std::path::Path; use std::path::PathBuf; use crate::exec_command::relativize_to_home; +use crate::render::Insets; +use crate::render::line_utils::prefix_lines; use crate::render::renderable::ColumnRenderable; +use crate::render::renderable::InsetRenderable; use crate::render::renderable::Renderable; use codex_core::git_info::get_git_repo_root; use codex_core::protocol::FileChange; -const SPACES_AFTER_LINE_NUMBER: usize = 6; - // Internal representation for diff line rendering enum DiffLineType { Insert, @@ -65,7 +66,10 @@ impl From for Box { path.extend(render_line_count_summary(row.added, row.removed)); rows.push(Box::new(path)); rows.push(Box::new(RtLine::from(""))); - rows.push(Box::new(row.change)); + rows.push(Box::new(InsetRenderable::new( + Box::new(row.change), + Insets::tlbr(0, 2, 0, 0), + ))); } Box::new(ColumnRenderable::new(rows)) @@ -181,7 +185,9 @@ fn render_changes_block(rows: Vec, wrap_cols: usize, cwd: &Path) -> Vec, wrap_cols: usize, cwd: &Path) -> Vec>, width: usize) { match change { FileChange::Add { content } => { + let line_number_width = line_number_width(content.lines().count()); for (i, raw) in content.lines().enumerate() { out.extend(push_wrapped_diff_line( i + 1, DiffLineType::Insert, raw, width, + line_number_width, )); } } FileChange::Delete { content } => { + let line_number_width = line_number_width(content.lines().count()); for (i, raw) in content.lines().enumerate() { out.extend(push_wrapped_diff_line( i + 1, DiffLineType::Delete, raw, width, + line_number_width, )); } } FileChange::Update { unified_diff, .. } => { if let Ok(patch) = diffy::Patch::from_str(unified_diff) { + let mut max_line_number = 0; + for h in patch.hunks() { + let mut old_ln = h.old_range().start(); + let mut new_ln = h.new_range().start(); + for l in h.lines() { + match l { + diffy::Line::Insert(_) => { + max_line_number = max_line_number.max(new_ln); + new_ln += 1; + } + diffy::Line::Delete(_) => { + max_line_number = max_line_number.max(old_ln); + old_ln += 1; + } + diffy::Line::Context(_) => { + max_line_number = max_line_number.max(new_ln); + old_ln += 1; + new_ln += 1; + } + } + } + } + let line_number_width = line_number_width(max_line_number); let mut is_first_hunk = true; for h in patch.hunks() { if !is_first_hunk { - out.push(RtLine::from(vec![" ".into(), "⋮".dim()])); + let spacer = format!("{:width$} ", "", width = line_number_width.max(1)); + let spacer_span = RtSpan::styled(spacer, style_gutter()); + out.push(RtLine::from(vec![spacer_span, "⋮".dim()])); } is_first_hunk = false; @@ -229,6 +264,7 @@ fn render_change(change: &FileChange, out: &mut Vec>, width: usi DiffLineType::Insert, s, width, + line_number_width, )); new_ln += 1; } @@ -239,6 +275,7 @@ fn render_change(change: &FileChange, out: &mut Vec>, width: usi DiffLineType::Delete, s, width, + line_number_width, )); old_ln += 1; } @@ -249,6 +286,7 @@ fn render_change(change: &FileChange, out: &mut Vec>, width: usi DiffLineType::Context, s, width, + line_number_width, )); old_ln += 1; new_ln += 1; @@ -298,17 +336,15 @@ fn push_wrapped_diff_line( kind: DiffLineType, text: &str, width: usize, + line_number_width: usize, ) -> Vec> { - let indent = " "; let ln_str = line_number.to_string(); let mut remaining_text: &str = text; - // Reserve a fixed number of spaces after the line number so that content starts - // at a consistent column. Content includes a 1-character diff sign prefix - // ("+"/"-" for inserts/deletes, or a space for context lines) so alignment - // stays consistent across all diff lines. - let gap_after_ln = SPACES_AFTER_LINE_NUMBER.saturating_sub(ln_str.len()); - let prefix_cols = indent.len() + ln_str.len() + gap_after_ln; + // Reserve a fixed number of spaces (equal to the widest line number plus a + // trailing spacer) so the sign column stays aligned across the diff block. + let gutter_width = line_number_width.max(1); + let prefix_cols = gutter_width + 1; let mut first = true; let (sign_char, line_style) = match kind { @@ -332,8 +368,8 @@ fn push_wrapped_diff_line( remaining_text = rest; if first { - // Build gutter (indent + line number + spacing) as a dimmed span - let gutter = format!("{indent}{ln_str}{}", " ".repeat(gap_after_ln)); + // Build gutter (right-aligned line number plus spacer) as a dimmed span + let gutter = format!("{ln_str:>gutter_width$} "); // Content with a sign ('+'/'-'/' ') styled per diff kind let content = format!("{sign_char}{chunk}"); lines.push(RtLine::from(vec![ @@ -343,7 +379,7 @@ fn push_wrapped_diff_line( first = false; } else { // Continuation lines keep a space for the sign column so content aligns - let gutter = format!("{indent}{} ", " ".repeat(ln_str.len() + gap_after_ln)); + let gutter = format!("{:gutter_width$} ", ""); lines.push(RtLine::from(vec![ RtSpan::styled(gutter, style_gutter()), RtSpan::styled(chunk.to_string(), line_style), @@ -356,6 +392,14 @@ fn push_wrapped_diff_line( lines } +fn line_number_width(max_line_number: usize) -> usize { + if max_line_number == 0 { + 1 + } else { + max_line_number.to_string().len() + } +} + fn style_gutter() -> Style { Style::default().add_modifier(Modifier::DIM) } @@ -421,7 +465,8 @@ mod tests { let long_line = "this is a very long line that should wrap across multiple terminal columns and continue"; // Call the wrapping function directly so we can precisely control the width - let lines = push_wrapped_diff_line(1, DiffLineType::Insert, long_line, 80); + let lines = + push_wrapped_diff_line(1, DiffLineType::Insert, long_line, 80, line_number_width(1)); // Render into a small terminal to capture the visual layout snapshot_lines("wrap_behavior_insert", lines, 90, 8); @@ -442,11 +487,9 @@ mod tests { }, ); - for name in ["apply_update_block", "apply_update_block_manual"] { - let lines = diff_summary_for_tests(&changes); + let lines = diff_summary_for_tests(&changes); - snapshot_lines(name, lines, 80, 12); - } + snapshot_lines("apply_update_block", lines, 80, 12); } #[test] @@ -573,14 +616,37 @@ mod tests { }, ); - let mut lines = create_diff_summary(&changes, &PathBuf::from("/"), 28); - // Drop the combined header for this text-only snapshot - if !lines.is_empty() { - lines.remove(0); - } + let lines = create_diff_summary(&changes, &PathBuf::from("/"), 28); snapshot_lines_text("apply_update_block_wraps_long_lines_text", &lines); } + #[test] + fn ui_snapshot_apply_update_block_line_numbers_three_digits_text() { + let original = (1..=110).map(|i| format!("line {i}\n")).collect::(); + let modified = (1..=110) + .map(|i| { + if i == 100 { + format!("line {i} changed\n") + } else { + format!("line {i}\n") + } + }) + .collect::(); + let patch = diffy::create_patch(&original, &modified).to_string(); + + let mut changes: HashMap = HashMap::new(); + changes.insert( + PathBuf::from("hundreds.txt"), + FileChange::Update { + unified_diff: patch, + move_path: None, + }, + ); + + let lines = create_diff_summary(&changes, &PathBuf::from("/"), 80); + snapshot_lines_text("apply_update_block_line_numbers_three_digits_text", &lines); + } + #[test] fn ui_snapshot_apply_update_block_relativizes_path() { let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("/")); diff --git a/codex-rs/tui/src/render/mod.rs b/codex-rs/tui/src/render/mod.rs index 441c1d6b..fe92eea9 100644 --- a/codex-rs/tui/src/render/mod.rs +++ b/codex-rs/tui/src/render/mod.rs @@ -4,6 +4,7 @@ pub mod highlight; pub mod line_utils; pub mod renderable; +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct Insets { pub left: u16, pub top: u16, diff --git a/codex-rs/tui/src/render/renderable.rs b/codex-rs/tui/src/render/renderable.rs index 868d0726..c5542633 100644 --- a/codex-rs/tui/src/render/renderable.rs +++ b/codex-rs/tui/src/render/renderable.rs @@ -4,6 +4,9 @@ use ratatui::text::Line; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; +use crate::render::Insets; +use crate::render::RectExt as _; + pub trait Renderable { fn render(&self, area: Rect, buf: &mut Buffer); fn desired_height(&self, width: u16) -> u16; @@ -100,3 +103,26 @@ impl ColumnRenderable { } } } + +pub struct InsetRenderable { + child: Box, + insets: Insets, +} + +impl Renderable for InsetRenderable { + fn render(&self, area: Rect, buf: &mut Buffer) { + self.child.render(area.inset(self.insets), buf); + } + fn desired_height(&self, width: u16) -> u16 { + self.child + .desired_height(width - self.insets.left - self.insets.right) + + self.insets.top + + self.insets.bottom + } +} + +impl InsetRenderable { + pub fn new(child: Box, insets: Insets) -> Self { + Self { child, insets } + } +} diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_add_block.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_add_block.snap index 254f51b6..cd5aaf5f 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_add_block.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_add_block.snap @@ -3,8 +3,8 @@ source: tui/src/diff_render.rs expression: terminal.backend() --- "• Added new_file.txt (+2 -0) " -" 1 +alpha " -" 2 +beta " +" 1 +alpha " +" 2 +beta " " " " " " " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_delete_block.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_delete_block.snap index 989200b2..edfdb2c0 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_delete_block.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_delete_block.snap @@ -3,9 +3,9 @@ source: tui/src/diff_render.rs expression: terminal.backend() --- "• Deleted tmp_delete_example.txt (+0 -3) " -" 1 -first " -" 2 -second " -" 3 -third " +" 1 -first " +" 2 -second " +" 3 -third " " " " " " " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_multiple_files_block.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_multiple_files_block.snap index 62e1f58e..62fc671d 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_multiple_files_block.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_multiple_files_block.snap @@ -4,11 +4,11 @@ expression: terminal.backend() --- "• Edited 2 files (+2 -1) " " └ a.txt (+1 -1) " -" 1 -one " -" 1 +one changed " +" 1 -one " +" 1 +one changed " " " " └ b.txt (+1 -0) " -" 1 +new " +" 1 +new " " " " " " " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block.snap index 305120d3..8cc31efd 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block.snap @@ -1,13 +1,12 @@ --- source: tui/src/diff_render.rs -assertion_line: 748 expression: terminal.backend() --- "• Edited example.txt (+1 -1) " -" 1 line one " -" 2 -line two " -" 2 +line two changed " -" 3 line three " +" 1 line one " +" 2 -line two " +" 2 +line two changed " +" 3 line three " " " " " " " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_line_numbers_three_digits_text.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_line_numbers_three_digits_text.snap new file mode 100644 index 00000000..56058ee7 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_line_numbers_three_digits_text.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/diff_render.rs +expression: text +--- +• Edited hundreds.txt (+1 -1) + 97 line 97 + 98 line 98 + 99 line 99 + 100 -line 100 + 100 +line 100 changed + 101 line 101 + 102 line 102 + 103 line 103 diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_manual.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_manual.snap deleted file mode 100644 index d188b2fd..00000000 --- a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_manual.snap +++ /dev/null @@ -1,16 +0,0 @@ ---- -source: tui/src/diff_render.rs -expression: terminal.backend() ---- -"• Edited example.txt (+1 -1) " -" 1 line one " -" 2 -line two " -" 2 +line two changed " -" 3 line three " -" " -" " -" " -" " -" " -" " -" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_relativizes_path.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_relativizes_path.snap index 825e50eb..a50f7700 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_relativizes_path.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_relativizes_path.snap @@ -1,12 +1,11 @@ --- source: tui/src/diff_render.rs -assertion_line: 748 expression: terminal.backend() --- "• Edited abs_old.rs → abs_new.rs (+1 -1) " -" 1 -X " -" 1 +X changed " -" 2 Y " +" 1 -X " +" 1 +X changed " +" 2 Y " " " " " " " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines.snap index d571b006..72b9528e 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines.snap @@ -1,15 +1,14 @@ --- source: tui/src/diff_render.rs -assertion_line: 748 expression: terminal.backend() --- "• Edited long_example.txt (+1 -1) " -" 1 line 1 " -" 2 -short " -" 2 +short this_is_a_very_long_modified_line_that_should_wrap_acro " -" ss_multiple_terminal_columns_and_continue_even_further_beyond " -" _eighty_columns_to_force_multiple_wraps " -" 3 line 3 " +" 1 line 1 " +" 2 -short " +" 2 +short this_is_a_very_long_modified_line_that_should_wrap_across_m " +" ultiple_terminal_columns_and_continue_even_further_beyond_eighty_ " +" columns_to_force_multiple_wraps " +" 3 line 3 " " " " " " " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines_text.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines_text.snap index ea495c10..17c92c1b 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines_text.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines_text.snap @@ -2,15 +2,14 @@ source: tui/src/diff_render.rs expression: text --- - 1 1 - 2 -2 - 2 +added long line w - hich wraps and_if - _there_is_a_long_ - token_it_will_be_ - broken - 3 3 - 4 -4 - 4 +4 context line wh - ich also wraps ac - ross +• Edited wrap_demo.txt (+2 -2) + 1 1 + 2 -2 + 2 +added long line which + wraps and_if_there_i + s_a_long_token_it_wil + l_be_broken + 3 3 + 4 -4 + 4 +4 context line which + also wraps across diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_with_rename_block.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_with_rename_block.snap index 89c2ddf1..29b32101 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_with_rename_block.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_with_rename_block.snap @@ -1,13 +1,12 @@ --- source: tui/src/diff_render.rs -assertion_line: 748 expression: terminal.backend() --- "• Edited old_name.rs → new_name.rs (+1 -1) " -" 1 A " -" 2 -B " -" 2 +B changed " -" 3 C " +" 1 A " +" 2 -B " +" 2 +B changed " +" 3 C " " " " " " " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__wrap_behavior_insert.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__wrap_behavior_insert.snap index b14dafaa..7532977c 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__wrap_behavior_insert.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__wrap_behavior_insert.snap @@ -2,8 +2,8 @@ source: tui/src/diff_render.rs expression: terminal.backend() --- -" 1 +this is a very long line that should wrap across multiple terminal co " -" lumns and continue " +"1 +this is a very long line that should wrap across multiple terminal columns an " +" d continue " " " " " " " diff --git a/codex-rs/tui/src/snapshots/codex_tui__pager_overlay__tests__transcript_overlay_apply_patch_scroll_vt100.snap b/codex-rs/tui/src/snapshots/codex_tui__pager_overlay__tests__transcript_overlay_apply_patch_scroll_vt100.snap index 7beca4a4..1f086e81 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__pager_overlay__tests__transcript_overlay_apply_patch_scroll_vt100.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__pager_overlay__tests__transcript_overlay_apply_patch_scroll_vt100.snap @@ -4,12 +4,12 @@ expression: snapshot --- / T R A N S C R I P T / / / / / / / / / / / / / / / / / / / / / / / / / / / / / • Added foo.txt (+2 -0) - 1 +hello - 2 +world + 1 +hello + 2 +world • Added foo.txt (+2 -0) - 1 +hello - 2 +world + 1 +hello + 2 +world ─────────────────────────────────────────────────────────────────────────── 0% ─ ↑/↓ to scroll pgup/pgdn to page home/end to jump q to quit esc to edit prev