Skip to content

tui: show diff hunk headers to separate sections #2488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 21, 2025
Merged

tui: show diff hunk headers to separate sections #2488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
16b0005
tui: show diff hunk headers to separate sections
nornagon-openai Aug 19, 2025
a56172d
fix style, fix wrapping
nornagon-openai Aug 20, 2025
c73ff63
fix missing context on empty lines
nornagon-openai Aug 20, 2025
72aa5fa
⋮
nornagon-openai Aug 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 88 additions & 9 deletions codex-rs/tui/src/diff_render.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,18 @@ fn render_patch_details(changes: &HashMap<PathBuf, FileChange>) -> Vec<RtLine<'s
move_path: _,
} => {
if let Ok(patch) = diffy::Patch::from_str(unified_diff) {
let mut is_first_hunk = true;
for h in patch.hunks() {
// Render a simple separator between non-contiguous hunks
// instead of diff-style @@ headers.
if !is_first_hunk {
out.push(RtLine::from(vec![
RtSpan::raw(" "),
RtSpan::styled("⋮", style_dim()),
]));
}
is_first_hunk = false;

let mut old_ln = h.old_range().start();
let mut new_ln = h.new_range().start();
for l in h.lines() {
Expand Down Expand Up @@ -276,8 +287,7 @@ fn push_wrapped_diff_line(
// ("+"/"-" 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 first_prefix_cols = indent.len() + ln_str.len() + gap_after_ln;
let cont_prefix_cols = indent.len() + ln_str.len() + gap_after_ln;
let prefix_cols = indent.len() + ln_str.len() + gap_after_ln;

let mut first = true;
let (sign_opt, line_style) = match kind {
Expand All @@ -286,16 +296,14 @@ fn push_wrapped_diff_line(
DiffLineType::Context => (None, None),
};
let mut lines: Vec<RtLine<'static>> = Vec::new();
while !remaining_text.is_empty() {
let prefix_cols = if first {
first_prefix_cols
} else {
cont_prefix_cols
};

loop {
// 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 available_content_cols = term_cols
.saturating_sub(if first { prefix_cols + 1 } else { prefix_cols })
.max(1);
let split_at_byte_index = remaining_text
.char_indices()
.nth(available_content_cols)
Expand Down Expand Up @@ -341,6 +349,9 @@ fn push_wrapped_diff_line(
}
lines.push(line);
}
if remaining_text.is_empty() {
break;
}
}
lines
}
Expand Down Expand Up @@ -430,4 +441,72 @@ mod tests {
// Render into a small terminal to capture the visual layout
snapshot_lines("wrap_behavior_insert", lines, DEFAULT_WRAP_COLS + 10, 8);
}

#[test]
fn ui_snapshot_single_line_replacement_counts() {
// Reproduce: one deleted line replaced by one inserted line, no extra context
let original = "# Codex CLI (Rust Implementation)\n";
let modified = "# Codex CLI (Rust Implementation) banana\n";
let patch = diffy::create_patch(original, modified).to_string();

let mut changes: HashMap<PathBuf, FileChange> = HashMap::new();
changes.insert(
PathBuf::from("README.md"),
FileChange::Update {
unified_diff: patch,
move_path: None,
},
);

let lines =
create_diff_summary("proposed patch", &changes, PatchEventType::ApprovalRequest);

snapshot_lines("single_line_replacement_counts", lines, 80, 8);
}

#[test]
fn ui_snapshot_blank_context_line() {
// Ensure a hunk that includes a blank context line at the beginning is rendered visibly
let original = "\nY\n";
let modified = "\nY changed\n";
let patch = diffy::create_patch(original, modified).to_string();

let mut changes: HashMap<PathBuf, FileChange> = HashMap::new();
changes.insert(
PathBuf::from("example.txt"),
FileChange::Update {
unified_diff: patch,
move_path: None,
},
);

let lines =
create_diff_summary("proposed patch", &changes, PatchEventType::ApprovalRequest);

snapshot_lines("blank_context_line", lines, 80, 10);
}

#[test]
fn ui_snapshot_vertical_ellipsis_between_hunks() {
// Create a patch with two separate hunks to ensure we render the vertical ellipsis (⋮)
let original =
"line 1\nline 2\nline 3\nline 4\nline 5\nline 6\nline 7\nline 8\nline 9\nline 10\n";
let modified = "line 1\nline two changed\nline 3\nline 4\nline 5\nline 6\nline 7\nline 8\nline nine changed\nline 10\n";
let patch = diffy::create_patch(original, modified).to_string();

let mut changes: HashMap<PathBuf, FileChange> = HashMap::new();
changes.insert(
PathBuf::from("example.txt"),
FileChange::Update {
unified_diff: patch,
move_path: None,
},
);

let lines =
create_diff_summary("proposed patch", &changes, PatchEventType::ApprovalRequest);

// Height is large enough to show both hunks and the separator
snapshot_lines("vertical_ellipsis_between_hunks", lines, 80, 16);
}
}
14 changes: 14 additions & 0 deletions codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__blank_context_line.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
source: tui/src/diff_render.rs
expression: terminal.backend()
---
"proposed patch to 1 file (+1 -1) "
" └ example.txt "
" 1 "
" 2 -Y "
" 2 +Y changed "
" "
" "
" "
" "
" "
12 changes: 12 additions & 0 deletions ...x-rs/tui/src/snapshots/codex_tui__diff_render__tests__single_line_replacement_counts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
source: tui/src/diff_render.rs
expression: terminal.backend()
---
"proposed patch to 1 file (+1 -1) "
" └ README.md "
" 1 -# Codex CLI (Rust Implementation) "
" 1 +# Codex CLI (Rust Implementation) banana "
" "
" "
" "
" "
1 change: 0 additions & 1 deletion codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__update_details_with_rename.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: tui/src/diff_render.rs
assertion_line: 380
expression: terminal.backend()
---
"proposed patch to 1 file (+1 -1) "
Expand Down
20 changes: 20 additions & 0 deletions ...-rs/tui/src/snapshots/codex_tui__diff_render__tests__vertical_ellipsis_between_hunks.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
source: tui/src/diff_render.rs
expression: terminal.backend()
---
"proposed patch to 1 file (+2 -2) "
" └ example.txt "
" 1 line 1 "
" 2 -line 2 "
" 2 +line two changed "
" 3 line 3 "
" 4 line 4 "
" 5 line 5 "
" ⋮ "
" 6 line 6 "
" 7 line 7 "
" 8 line 8 "
" 9 -line 9 "
" 9 +line nine changed "
" 10 line 10 "
" "
5 changes: 2 additions & 3 deletions codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__wrap_behavior_insert.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
---
source: tui/src/diff_render.rs
assertion_line: 380
expression: terminal.backend()
---
" 1 +this is a very long line that should wrap across multiple terminal col "
" umns and continue "
" 1 +this is a very long line that should wrap across multiple terminal co "
" lumns and continue "
" "
" "
" "
Expand Down
Loading