From 85e4f564a33a3a877fc4f279bf870c7ef9ed2645 Mon Sep 17 00:00:00 2001 From: aibrahim-oai Date: Mon, 11 Aug 2025 12:31:34 -0700 Subject: [PATCH] Chores: Refactor approval Patch UI. Stack: [1/2] (#2049) - Moved the logic for the apply patch in its own file Stack: #2050 -> #2049 --- codex-rs/tui/src/diff_render.rs | 152 ++++++++++++++++++++++++++++++ codex-rs/tui/src/history_cell.rs | 155 +------------------------------ codex-rs/tui/src/lib.rs | 1 + 3 files changed, 156 insertions(+), 152 deletions(-) create mode 100644 codex-rs/tui/src/diff_render.rs diff --git a/codex-rs/tui/src/diff_render.rs b/codex-rs/tui/src/diff_render.rs new file mode 100644 index 00000000..f5366817 --- /dev/null +++ b/codex-rs/tui/src/diff_render.rs @@ -0,0 +1,152 @@ +use ratatui::style::Color; +use ratatui::style::Modifier; +use ratatui::style::Style; +use ratatui::text::Line as RtLine; +use ratatui::text::Span as RtSpan; +use std::collections::HashMap; +use std::path::PathBuf; + +use codex_core::protocol::FileChange; + +struct FileSummary { + display_path: String, + added: usize, + removed: usize, +} + +pub(crate) fn create_diff_summary( + title: &str, + changes: HashMap, +) -> Vec> { + let mut files: Vec = Vec::new(); + + // 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) + } else { + let mut adds = 0usize; + let mut dels = 0usize; + for l in diff.lines() { + if l.starts_with("+++") || l.starts_with("---") || l.starts_with("@@") { + continue; + } + match l.as_bytes().first() { + Some(b'+') => adds += 1, + Some(b'-') => dels += 1, + _ => {} + } + } + (adds, dels) + } + }; + + for (path, change) in &changes { + use codex_core::protocol::FileChange::*; + 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) + .ok() + .map(|s| s.lines().count()) + .unwrap_or(0); + files.push(FileSummary { + display_path: path.display().to_string(), + added: 0, + removed, + }); + } + Update { + unified_diff, + move_path, + } => { + let (added, removed) = count_from_unified(unified_diff); + let display_path = if let Some(new_path) = move_path { + format!("{} → {}", path.display(), new_path.display()) + } else { + path.display().to_string() + }; + files.push(FileSummary { + display_path, + added, + removed, + }); + } + } + } + + let file_count = files.len(); + let total_added: usize = files.iter().map(|f| f.added).sum(); + let total_removed: usize = files.iter().map(|f| f.removed).sum(); + let noun = if file_count == 1 { "file" } else { "files" }; + + let mut out: Vec> = Vec::new(); + + // Header + let mut header_spans: Vec> = Vec::new(); + header_spans.push(RtSpan::styled( + title.to_owned(), + Style::default() + .fg(Color::Magenta) + .add_modifier(Modifier::BOLD), + )); + header_spans.push(RtSpan::raw(" to ")); + header_spans.push(RtSpan::raw(format!("{file_count} {noun} "))); + header_spans.push(RtSpan::raw("(")); + header_spans.push(RtSpan::styled( + format!("+{total_added}"), + Style::default().fg(Color::Green), + )); + header_spans.push(RtSpan::raw(" ")); + header_spans.push(RtSpan::styled( + format!("-{total_removed}"), + Style::default().fg(Color::Red), + )); + header_spans.push(RtSpan::raw(")")); + out.push(RtLine::from(header_spans)); + + // Dimmed per-file lines with prefix + for (idx, f) in files.iter().enumerate() { + let mut spans: Vec> = Vec::new(); + spans.push(RtSpan::raw(f.display_path.clone())); + spans.push(RtSpan::raw(" (")); + spans.push(RtSpan::styled( + format!("+{}", f.added), + Style::default().fg(Color::Green), + )); + spans.push(RtSpan::raw(" ")); + spans.push(RtSpan::styled( + format!("-{}", f.removed), + Style::default().fg(Color::Red), + )); + spans.push(RtSpan::raw(")")); + + 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); + }); + out.push(line); + } + + out +} diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 6b9c5df7..29b62ecb 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -1,4 +1,5 @@ use crate::colors::LIGHT_BLUE; +use crate::diff_render::create_diff_summary; use crate::exec_command::relativize_to_home; use crate::exec_command::strip_bash_lc_and_escape; use crate::slash_command::SlashCommand; @@ -28,8 +29,6 @@ use ratatui::prelude::*; use ratatui::style::Color; use ratatui::style::Modifier; use ratatui::style::Style; -use ratatui::text::Line as RtLine; -use ratatui::text::Span as RtSpan; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; use ratatui::widgets::Wrap; @@ -45,12 +44,6 @@ pub(crate) struct CommandOutput { pub(crate) stderr: String, } -struct FileSummary { - display_path: String, - added: usize, - removed: usize, -} - pub(crate) enum PatchEventType { ApprovalRequest, ApplyBegin { auto_approved: bool }, @@ -803,7 +796,7 @@ impl HistoryCell { event_type: PatchEventType, changes: HashMap, ) -> Self { - let title = match event_type { + let title = match &event_type { PatchEventType::ApprovalRequest => "proposed patch", PatchEventType::ApplyBegin { auto_approved: true, @@ -821,15 +814,7 @@ impl HistoryCell { } }; - let summary_lines = create_diff_summary(title, changes); - - let mut lines: Vec> = Vec::new(); - - for line in summary_lines { - lines.push(line); - } - - lines.push(Line::from("")); + let lines: Vec> = create_diff_summary(title, changes); HistoryCell::PendingPatch { view: TextBlock::new(lines), @@ -931,140 +916,6 @@ fn output_lines( out } -fn create_diff_summary(title: &str, changes: HashMap) -> Vec> { - let mut files: Vec = Vec::new(); - - // 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) - } else { - let mut adds = 0usize; - let mut dels = 0usize; - for l in diff.lines() { - if l.starts_with("+++") || l.starts_with("---") || l.starts_with("@@") { - continue; - } - match l.as_bytes().first() { - Some(b'+') => adds += 1, - Some(b'-') => dels += 1, - _ => {} - } - } - (adds, dels) - } - }; - - for (path, change) in &changes { - use codex_core::protocol::FileChange::*; - 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) - .ok() - .map(|s| s.lines().count()) - .unwrap_or(0); - files.push(FileSummary { - display_path: path.display().to_string(), - added: 0, - removed, - }); - } - Update { - unified_diff, - move_path, - } => { - let (added, removed) = count_from_unified(unified_diff); - let display_path = if let Some(new_path) = move_path { - format!("{} → {}", path.display(), new_path.display()) - } else { - path.display().to_string() - }; - files.push(FileSummary { - display_path, - added, - removed, - }); - } - } - } - - let file_count = files.len(); - let total_added: usize = files.iter().map(|f| f.added).sum(); - let total_removed: usize = files.iter().map(|f| f.removed).sum(); - let noun = if file_count == 1 { "file" } else { "files" }; - - let mut out: Vec> = Vec::new(); - - // Header - let mut header_spans: Vec> = Vec::new(); - header_spans.push(RtSpan::styled( - title.to_owned(), - Style::default() - .fg(Color::Magenta) - .add_modifier(Modifier::BOLD), - )); - header_spans.push(RtSpan::raw(" to ")); - header_spans.push(RtSpan::raw(format!("{file_count} {noun} "))); - header_spans.push(RtSpan::raw("(")); - header_spans.push(RtSpan::styled( - format!("+{total_added}"), - Style::default().fg(Color::Green), - )); - header_spans.push(RtSpan::raw(" ")); - header_spans.push(RtSpan::styled( - format!("-{total_removed}"), - Style::default().fg(Color::Red), - )); - header_spans.push(RtSpan::raw(")")); - out.push(RtLine::from(header_spans)); - - // Dimmed per-file lines with prefix - for (idx, f) in files.iter().enumerate() { - let mut spans: Vec> = Vec::new(); - spans.push(RtSpan::raw(f.display_path.clone())); - spans.push(RtSpan::raw(" (")); - spans.push(RtSpan::styled( - format!("+{}", f.added), - Style::default().fg(Color::Green), - )); - spans.push(RtSpan::raw(" ")); - spans.push(RtSpan::styled( - format!("-{}", f.removed), - Style::default().fg(Color::Red), - )); - spans.push(RtSpan::raw(")")); - - 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); - }); - out.push(line); - } - - out -} - fn format_mcp_invocation<'a>(invocation: McpInvocation) -> Line<'a> { let args_str = invocation .arguments diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index e15a235a..27c850ca 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -31,6 +31,7 @@ mod citation_regex; mod cli; mod colors; pub mod custom_terminal; +mod diff_render; mod exec_command; mod file_search; mod get_git_diff;