diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 173ab64a..6719cfb7 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -46,6 +46,7 @@ use crate::bottom_pane::BottomPane; use crate::bottom_pane::BottomPaneParams; use crate::bottom_pane::CancellationEvent; use crate::bottom_pane::InputResult; +use crate::common::DEFAULT_WRAP_COLS; use crate::history_cell::CommandOutput; use crate::history_cell::ExecCell; use crate::history_cell::HistoryCell; @@ -223,7 +224,7 @@ impl ChatWidget<'_> { content_buffer: String::new(), answer_buffer: String::new(), running_commands: HashMap::new(), - live_builder: RowBuilder::new(80), + live_builder: RowBuilder::new(DEFAULT_WRAP_COLS.into()), current_stream: None, stream_header_emitted: false, live_max_rows: 3, diff --git a/codex-rs/tui/src/common.rs b/codex-rs/tui/src/common.rs new file mode 100644 index 00000000..2c19b587 --- /dev/null +++ b/codex-rs/tui/src/common.rs @@ -0,0 +1 @@ +pub(crate) const DEFAULT_WRAP_COLS: u16 = 80; diff --git a/codex-rs/tui/src/diff_render.rs b/codex-rs/tui/src/diff_render.rs index f5366817..ada84b89 100644 --- a/codex-rs/tui/src/diff_render.rs +++ b/codex-rs/tui/src/diff_render.rs @@ -1,3 +1,4 @@ +use crossterm::terminal; use ratatui::style::Color; use ratatui::style::Modifier; use ratatui::style::Style; @@ -6,36 +7,44 @@ use ratatui::text::Span as RtSpan; use std::collections::HashMap; use std::path::PathBuf; +use crate::common::DEFAULT_WRAP_COLS; use codex_core::protocol::FileChange; -struct FileSummary { - display_path: String, - added: usize, - removed: usize, +use crate::history_cell::PatchEventType; + +const SPACES_AFTER_LINE_NUMBER: usize = 6; + +// Internal representation for diff line rendering +enum DiffLineType { + Insert, + Delete, + Context, } pub(crate) fn create_diff_summary( title: &str, - changes: HashMap, + changes: &HashMap, + event_type: PatchEventType, ) -> Vec> { - let mut files: Vec = Vec::new(); + struct FileSummary { + display_path: String, + added: usize, + removed: usize, + } - // Count additions/deletions from a unified diff body let count_from_unified = |diff: &str| -> (usize, usize) { if let Ok(patch) = diffy::Patch::from_str(diff) { - let mut adds = 0usize; - let mut dels = 0usize; - for hunk in patch.hunks() { - for line in hunk.lines() { - match line { - diffy::Line::Insert(_) => adds += 1, - diffy::Line::Delete(_) => dels += 1, - _ => {} - } - } - } - (adds, dels) + patch + .hunks() + .iter() + .flat_map(|h| h.lines()) + .fold((0, 0), |(a, d), l| match l { + diffy::Line::Insert(_) => (a + 1, d), + diffy::Line::Delete(_) => (a, d + 1), + _ => (a, d), + }) } else { + // Fallback: manual scan to preserve counts even for unparsable diffs let mut adds = 0usize; let mut dels = 0usize; for l in diff.lines() { @@ -52,29 +61,23 @@ pub(crate) fn create_diff_summary( } }; - for (path, change) in &changes { - use codex_core::protocol::FileChange::*; + let mut files: Vec = Vec::new(); + for (path, change) in changes.iter() { match change { - Add { content } => { - let added = content.lines().count(); - files.push(FileSummary { - display_path: path.display().to_string(), - added, - removed: 0, - }); - } - Delete => { - let removed = std::fs::read_to_string(path) + FileChange::Add { content } => files.push(FileSummary { + display_path: path.display().to_string(), + added: content.lines().count(), + removed: 0, + }), + FileChange::Delete => files.push(FileSummary { + display_path: path.display().to_string(), + added: 0, + removed: std::fs::read_to_string(path) .ok() .map(|s| s.lines().count()) - .unwrap_or(0); - files.push(FileSummary { - display_path: path.display().to_string(), - added: 0, - removed, - }); - } - Update { + .unwrap_or(0), + }), + FileChange::Update { unified_diff, move_path, } => { @@ -142,11 +145,278 @@ pub(crate) fn create_diff_summary( let mut line = RtLine::from(spans); let prefix = if idx == 0 { " ⎿ " } else { " " }; line.spans.insert(0, prefix.into()); - line.spans.iter_mut().for_each(|span| { - span.style = span.style.add_modifier(Modifier::DIM); - }); + line.spans + .iter_mut() + .for_each(|span| span.style = span.style.add_modifier(Modifier::DIM)); out.push(line); } + let show_details = matches!( + event_type, + PatchEventType::ApplyBegin { + auto_approved: true + } | PatchEventType::ApprovalRequest + ); + + if show_details { + out.extend(render_patch_details(changes)); + } + out } + +fn render_patch_details(changes: &HashMap) -> Vec> { + let mut out: Vec> = Vec::new(); + let term_cols: usize = terminal::size() + .map(|(w, _)| w as usize) + .unwrap_or(DEFAULT_WRAP_COLS.into()); + + for (index, (path, change)) in changes.iter().enumerate() { + let is_first_file = index == 0; + // Add separator only between files (not at the very start) + if !is_first_file { + out.push(RtLine::from(vec![ + RtSpan::raw(" "), + RtSpan::styled("...", style_dim()), + ])); + } + match change { + FileChange::Add { content } => { + for (i, raw) in content.lines().enumerate() { + let ln = i + 1; + out.extend(push_wrapped_diff_line( + ln, + DiffLineType::Insert, + raw, + term_cols, + )); + } + } + FileChange::Delete => { + let original = std::fs::read_to_string(path).unwrap_or_default(); + for (i, raw) in original.lines().enumerate() { + let ln = i + 1; + out.extend(push_wrapped_diff_line( + ln, + DiffLineType::Delete, + raw, + term_cols, + )); + } + } + FileChange::Update { + unified_diff, + move_path: _, + } => { + if let Ok(patch) = diffy::Patch::from_str(unified_diff) { + 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(text) => { + let s = text.trim_end_matches('\n'); + out.extend(push_wrapped_diff_line( + new_ln, + DiffLineType::Insert, + s, + term_cols, + )); + new_ln += 1; + } + diffy::Line::Delete(text) => { + let s = text.trim_end_matches('\n'); + out.extend(push_wrapped_diff_line( + old_ln, + DiffLineType::Delete, + s, + term_cols, + )); + old_ln += 1; + } + diffy::Line::Context(text) => { + let s = text.trim_end_matches('\n'); + out.extend(push_wrapped_diff_line( + new_ln, + DiffLineType::Context, + s, + term_cols, + )); + old_ln += 1; + new_ln += 1; + } + } + } + } + } + } + } + + out.push(RtLine::from(RtSpan::raw(""))); + } + + out +} + +fn push_wrapped_diff_line( + line_number: usize, + kind: DiffLineType, + text: &str, + term_cols: 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. The sign ("+"/"-") is rendered as part of the content + // with the same background as the edit, not as a separate dimmed column. + let gap_after_ln = SPACES_AFTER_LINE_NUMBER.saturating_sub(ln_str.len()); + let first_prefix_cols = indent.len() + ln_str.len() + gap_after_ln; + let cont_prefix_cols = indent.len() + ln_str.len() + gap_after_ln; + + let mut first = true; + let (sign_opt, bg_style) = match kind { + DiffLineType::Insert => (Some('+'), Some(style_add())), + DiffLineType::Delete => (Some('-'), Some(style_del())), + DiffLineType::Context => (None, None), + }; + let mut lines: Vec> = Vec::new(); + while !remaining_text.is_empty() { + let prefix_cols = if first { + first_prefix_cols + } else { + cont_prefix_cols + }; + // Fit the content for the current terminal row: + // compute how many columns are available after the prefix, then split + // at a UTF-8 character boundary so this row's chunk fits exactly. + let available_content_cols = term_cols.saturating_sub(prefix_cols).max(1); + let split_at_byte_index = remaining_text + .char_indices() + .nth(available_content_cols) + .map(|(i, _)| i) + .unwrap_or_else(|| remaining_text.len()); + let (chunk, rest) = remaining_text.split_at(split_at_byte_index); + remaining_text = rest; + + if first { + let mut spans: Vec> = Vec::new(); + spans.push(RtSpan::raw(indent)); + spans.push(RtSpan::styled(ln_str.clone(), style_dim())); + spans.push(RtSpan::raw(" ".repeat(gap_after_ln))); + + // Prefix the content with the sign if it is an insertion or deletion, and color + // the sign with the same background as the edited text. + let display_chunk = match sign_opt { + Some(sign_char) => format!("{sign_char}{chunk}"), + None => chunk.to_string(), + }; + + let content_span = match bg_style { + Some(style) => RtSpan::styled(display_chunk, style), + None => RtSpan::raw(display_chunk), + }; + spans.push(content_span); + lines.push(RtLine::from(spans)); + first = false; + } else { + let hang_prefix = format!( + "{indent}{}{}", + " ".repeat(ln_str.len()), + " ".repeat(gap_after_ln) + ); + let content_span = match bg_style { + Some(style) => RtSpan::styled(chunk.to_string(), style), + None => RtSpan::raw(chunk.to_string()), + }; + lines.push(RtLine::from(vec![RtSpan::raw(hang_prefix), content_span])); + } + } + lines +} + +fn style_dim() -> Style { + Style::default().add_modifier(Modifier::DIM) +} + +fn style_add() -> Style { + Style::default().bg(Color::Green) +} + +fn style_del() -> Style { + Style::default().bg(Color::Red) +} + +#[allow(clippy::expect_used)] +#[cfg(test)] +mod tests { + use super::*; + use crate::history_cell::HistoryCell; + use crate::text_block::TextBlock; + use insta::assert_snapshot; + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + fn snapshot_lines(name: &str, lines: Vec>, width: u16, height: u16) { + let mut terminal = Terminal::new(TestBackend::new(width, height)).expect("terminal"); + let cell = HistoryCell::PendingPatch { + view: TextBlock::new(lines), + }; + terminal + .draw(|f| f.render_widget_ref(&cell, f.area())) + .expect("draw"); + assert_snapshot!(name, terminal.backend()); + } + + #[test] + fn ui_snapshot_add_details() { + let mut changes: HashMap = HashMap::new(); + changes.insert( + PathBuf::from("README.md"), + FileChange::Add { + content: "first line\nsecond line\n".to_string(), + }, + ); + + let lines = + create_diff_summary("proposed patch", &changes, PatchEventType::ApprovalRequest); + + snapshot_lines("add_details", lines, 80, 10); + } + + #[test] + fn ui_snapshot_update_details_with_rename() { + let mut changes: HashMap = HashMap::new(); + + let original = "line one\nline two\nline three\n"; + let modified = "line one\nline two changed\nline three\n"; + let patch = diffy::create_patch(original, modified).to_string(); + + changes.insert( + PathBuf::from("src/lib.rs"), + FileChange::Update { + unified_diff: patch, + move_path: Some(PathBuf::from("src/lib_new.rs")), + }, + ); + + let lines = + create_diff_summary("proposed patch", &changes, PatchEventType::ApprovalRequest); + + snapshot_lines("update_details_with_rename", lines, 80, 12); + } + + #[test] + fn ui_snapshot_wrap_behavior_insert() { + // Narrow width to force wrapping within our diff line rendering + 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, DEFAULT_WRAP_COLS.into()); + + // Render into a small terminal to capture the visual layout + snapshot_lines("wrap_behavior_insert", lines, DEFAULT_WRAP_COLS + 10, 8); + } +} diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 95ed8efa..b49e5997 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -449,7 +449,7 @@ impl HistoryCell { } pub(crate) fn new_completed_mcp_tool_call( - num_cols: u16, + num_cols: usize, invocation: McpInvocation, duration: Duration, success: bool, @@ -487,7 +487,7 @@ impl HistoryCell { format_and_truncate_tool_result( &text.text, TOOL_CALL_MAX_LINES, - num_cols as usize, + num_cols, ) } mcp_types::ContentBlock::ImageContent(_) => { @@ -848,7 +848,9 @@ impl HistoryCell { } }; - let lines: Vec> = create_diff_summary(title, changes); + let mut lines: Vec> = create_diff_summary(title, &changes, event_type); + + lines.push(Line::from("")); HistoryCell::PendingPatch { view: TextBlock::new(lines), diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 27c850ca..8f64f324 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -30,6 +30,7 @@ mod chatwidget; mod citation_regex; mod cli; mod colors; +mod common; pub mod custom_terminal; mod diff_render; mod exec_command; diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__add_details.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__add_details.snap new file mode 100644 index 00000000..06fc8a68 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__add_details.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/diff_render.rs +expression: terminal.backend() +--- +"proposed patch to 1 file (+2 -0) " +" ⎿ README.md (+2 -0) " +" 1 +first line " +" 2 +second line " +" " +" " +" " +" " +" " +" " diff --git a/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__update_details_with_rename.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__update_details_with_rename.snap new file mode 100644 index 00000000..0eebe09d --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__update_details_with_rename.snap @@ -0,0 +1,16 @@ +--- +source: tui/src/diff_render.rs +expression: terminal.backend() +--- +"proposed patch to 1 file (+1 -1) " +" ⎿ src/lib.rs → src/lib_new.rs (+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__wrap_behavior_insert.snap b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__wrap_behavior_insert.snap new file mode 100644 index 00000000..641552d5 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__wrap_behavior_insert.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/diff_render.rs +expression: terminal.backend() +--- +" 1 +this is a very long line that should wrap across multiple terminal col " +" umns and continue " +" " +" " +" " +" " +" " +" "