mirror of
https://github.com/openai/codex.git
synced 2026-09-06 15:29:32 +00:00
Avoid cloning file changes in TUI diff rendering (#34224)
## What changed - Consume and sort `DiffSummary` entries directly when building renderables. - Borrow paths and `FileChange` values in the shared row representation used by line-based summaries. - Share line-count calculation between both rendering paths. GitOrigin-RevId: b02668074def7529ff39445e9970a1fec209f02b
This commit is contained in:
@@ -323,18 +323,21 @@ impl Renderable for FileChange {
|
||||
impl From<DiffSummary> for Box<dyn Renderable> {
|
||||
fn from(val: DiffSummary) -> Self {
|
||||
let mut rows: Vec<Box<dyn Renderable>> = vec![];
|
||||
let mut changes: Vec<_> = val.changes.into_iter().collect();
|
||||
changes.sort_by(|left, right| left.0.cmp(&right.0));
|
||||
|
||||
for (i, row) in collect_rows(&val.changes).into_iter().enumerate() {
|
||||
for (i, (path, change)) in changes.into_iter().enumerate() {
|
||||
if i > 0 {
|
||||
rows.push(Box::new(RtLine::from("")));
|
||||
}
|
||||
let mut path = RtLine::from(display_path_for(&row.path, val.cwd.as_path()));
|
||||
let (added, removed) = line_counts(&change);
|
||||
let mut path = RtLine::from(display_path_for(&path, val.cwd.as_path()));
|
||||
path.push_span(" ");
|
||||
path.extend(render_line_count_summary(row.added, row.removed));
|
||||
path.extend(render_line_count_summary(added, removed));
|
||||
rows.push(Box::new(path));
|
||||
rows.push(Box::new(RtLine::from("")));
|
||||
rows.push(Box::new(InsetRenderable::new(
|
||||
Box::new(row.change) as Box<dyn Renderable>,
|
||||
Box::new(change) as Box<dyn Renderable>,
|
||||
Insets::tlbr(
|
||||
/*top*/ 0, /*left*/ 2, /*bottom*/ 0, /*right*/ 0,
|
||||
),
|
||||
@@ -355,43 +358,45 @@ pub(crate) fn create_diff_summary(
|
||||
}
|
||||
|
||||
// Shared row for per-file presentation
|
||||
#[derive(Clone)]
|
||||
struct Row {
|
||||
#[allow(dead_code)]
|
||||
path: PathBuf,
|
||||
move_path: Option<PathBuf>,
|
||||
struct Row<'a> {
|
||||
path: &'a Path,
|
||||
move_path: Option<&'a Path>,
|
||||
added: usize,
|
||||
removed: usize,
|
||||
change: FileChange,
|
||||
change: &'a FileChange,
|
||||
}
|
||||
|
||||
fn collect_rows(changes: &HashMap<PathBuf, FileChange>) -> Vec<Row> {
|
||||
let mut rows: Vec<Row> = Vec::new();
|
||||
fn collect_rows(changes: &HashMap<PathBuf, FileChange>) -> Vec<Row<'_>> {
|
||||
let mut rows = Vec::with_capacity(changes.len());
|
||||
for (path, change) in changes.iter() {
|
||||
let (added, removed) = match change {
|
||||
FileChange::Add { content } => (content.lines().count(), 0),
|
||||
FileChange::Delete { content } => (0, content.lines().count()),
|
||||
FileChange::Update { unified_diff, .. } => calculate_add_remove_from_diff(unified_diff),
|
||||
};
|
||||
let (added, removed) = line_counts(change);
|
||||
let move_path = match change {
|
||||
FileChange::Update {
|
||||
move_path: Some(new),
|
||||
..
|
||||
} => Some(new.clone()),
|
||||
} => Some(new.as_path()),
|
||||
_ => None,
|
||||
};
|
||||
rows.push(Row {
|
||||
path: path.clone(),
|
||||
path: path.as_path(),
|
||||
move_path,
|
||||
added,
|
||||
removed,
|
||||
change: change.clone(),
|
||||
change,
|
||||
});
|
||||
}
|
||||
rows.sort_by_key(|r| r.path.clone());
|
||||
rows.sort_by(|left, right| left.path.cmp(right.path));
|
||||
rows
|
||||
}
|
||||
|
||||
fn line_counts(change: &FileChange) -> (usize, usize) {
|
||||
match change {
|
||||
FileChange::Add { content } => (content.lines().count(), 0),
|
||||
FileChange::Delete { content } => (0, content.lines().count()),
|
||||
FileChange::Update { unified_diff, .. } => calculate_add_remove_from_diff(unified_diff),
|
||||
}
|
||||
}
|
||||
|
||||
fn render_line_count_summary(added: usize, removed: usize) -> Vec<RtSpan<'static>> {
|
||||
let mut spans = Vec::new();
|
||||
spans.push("(".into());
|
||||
@@ -402,13 +407,13 @@ fn render_line_count_summary(added: usize, removed: usize) -> Vec<RtSpan<'static
|
||||
spans
|
||||
}
|
||||
|
||||
fn render_changes_block(rows: Vec<Row>, wrap_cols: usize, cwd: &Path) -> Vec<RtLine<'static>> {
|
||||
fn render_changes_block(rows: Vec<Row<'_>>, wrap_cols: usize, cwd: &Path) -> Vec<RtLine<'static>> {
|
||||
let mut out: Vec<RtLine<'static>> = Vec::new();
|
||||
|
||||
let render_path = |row: &Row| -> Vec<RtSpan<'static>> {
|
||||
let render_path = |row: &Row<'_>| -> Vec<RtSpan<'static>> {
|
||||
let mut spans = Vec::new();
|
||||
spans.push(display_path_for(&row.path, cwd).into());
|
||||
if let Some(move_path) = &row.move_path {
|
||||
spans.push(display_path_for(row.path, cwd).into());
|
||||
if let Some(move_path) = row.move_path {
|
||||
spans.push(format!(" → {}", display_path_for(move_path, cwd)).into());
|
||||
}
|
||||
spans
|
||||
@@ -421,7 +426,7 @@ fn render_changes_block(rows: Vec<Row>, wrap_cols: usize, cwd: &Path) -> Vec<RtL
|
||||
let noun = if file_count == 1 { "file" } else { "files" };
|
||||
let mut header_spans: Vec<RtSpan<'static>> = vec!["• ".dim()];
|
||||
if let [row] = &rows[..] {
|
||||
let verb = match &row.change {
|
||||
let verb = match row.change {
|
||||
FileChange::Add { .. } => "Added",
|
||||
FileChange::Delete { .. } => "Deleted",
|
||||
_ => "Edited",
|
||||
@@ -456,10 +461,10 @@ fn render_changes_block(rows: Vec<Row>, wrap_cols: usize, cwd: &Path) -> Vec<RtL
|
||||
|
||||
// For renames, use the destination extension for highlighting — the
|
||||
// diff content reflects the new file, not the old one.
|
||||
let lang_path = r.move_path.as_deref().unwrap_or(&r.path);
|
||||
let lang_path = r.move_path.unwrap_or(r.path);
|
||||
let lang = detect_lang_for_path(lang_path);
|
||||
let mut lines = vec![];
|
||||
render_change(&r.change, &mut lines, wrap_cols - 4, lang.as_deref());
|
||||
render_change(r.change, &mut lines, wrap_cols - 4, lang.as_deref());
|
||||
out.extend(prefix_lines(lines, " ".into(), " ".into()));
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user