Skip to content

Commit 75176da

Browse files
nornagon-openainornagon-openai
authored
dynamic width for line numbers in diffs (#4664)
instead of always reserving 6 spaces for the line number and gutter, we now dynamically adjust to the width of the longest number. <img width="871" height="616" alt="Screenshot 2025-10-03 at 8 21 00 AM" src="https://github.com/user-attachments/assets/5f18eae6-7c85-48fc-9a41-31978ae71a62" /> <img width="871" height="616" alt="Screenshot 2025-10-03 at 8 21 21 AM" src="https://github.com/user-attachments/assets/9009297d-7690-42b9-ae42-9566b3fea86c" /> <img width="871" height="616" alt="Screenshot 2025-10-03 at 8 21 57 AM" src="https://github.com/user-attachments/assets/669096fd-dddc-407e-bae8-d0c6626fa0bc" />
1 parent 12fd2b4 commit 75176da

18 files changed

+182
-96
lines changed

‎codex-rs/tui/src/bottom_pane/approval_overlay.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -315,15 +315,16 @@ impl From<ApprovalRequest> for ApprovalRequestState {
315315
changes,
316316
} => {
317317
let mut header: Vec<Box<dyn Renderable>> = Vec::new();
318-
header.push(DiffSummary::new(changes, cwd).into());
319318
if let Some(reason) = reason
320319
&& !reason.is_empty()
321320
{
322-
header.push(Box::new(Line::from("")));
323321
header.push(Box::new(
324-
Paragraph::new(reason.italic()).wrap(Wrap { trim: false }),
322+
Paragraph::new(Line::from_iter(["Reason: ".into(), reason.italic()]))
323+
.wrap(Wrap { trim: false }),
325324
));
325+
header.push(Box::new(Line::from("")));
326326
}
327+
header.push(DiffSummary::new(changes, cwd).into());
327328
Self {
328329
variant: ApprovalVariant::ApplyPatch { id },
329330
header: Box::new(ColumnRenderable::new(header)),

‎codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_manual_flow_history_approved.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ source: tui/src/chatwidget/tests.rs
33
expression: lines_to_single_string(&approved_lines)
44
---
55
• Added foo.txt (+1 -0)
6-
1 +hello
6+
1 +hello

‎codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@ expression: terminal.backend().vt100().screen().contents()
44
---
55
Would you like to make the following edits?
66

7-
README.md (+2 -0)
7+
Reason: The model wants to apply changes
88

9-
1 +hello
10-
2 +world
9+
README.md (+2 -0)
1110

12-
The model wants to apply changes
11+
1 +hello
12+
2 +world
1313

1414
› 1. Yes, proceed
1515
2. No, and tell Codex what to do differently esc

‎codex-rs/tui/src/diff_render.rs‎

Lines changed: 91 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,14 @@ use std::path::Path;
1313
use std::path::PathBuf;
1414

1515
use crate::exec_command::relativize_to_home;
16+
use crate::render::Insets;
17+
use crate::render::line_utils::prefix_lines;
1618
use crate::render::renderable::ColumnRenderable;
19+
use crate::render::renderable::InsetRenderable;
1720
use crate::render::renderable::Renderable;
1821
use codex_core::git_info::get_git_repo_root;
1922
use codex_core::protocol::FileChange;
2023

21-
const SPACES_AFTER_LINE_NUMBER: usize = 6;
22-
2324
// Internal representation for diff line rendering
2425
enum DiffLineType {
2526
Insert,
@@ -65,7 +66,10 @@ impl From<DiffSummary> for Box<dyn Renderable> {
6566
path.extend(render_line_count_summary(row.added, row.removed));
6667
rows.push(Box::new(path));
6768
rows.push(Box::new(RtLine::from("")));
68-
rows.push(Box::new(row.change));
69+
rows.push(Box::new(InsetRenderable::new(
70+
Box::new(row.change),
71+
Insets::tlbr(0, 2, 0, 0),
72+
)));
6973
}
7074

7175
Box::new(ColumnRenderable::new(rows))
@@ -181,7 +185,9 @@ fn render_changes_block(rows: Vec<Row>, wrap_cols: usize, cwd: &Path) -> Vec<RtL
181185
out.push(RtLine::from(header));
182186
}
183187

184-
render_change(&r.change, &mut out, wrap_cols);
188+
let mut lines = vec![];
189+
render_change(&r.change, &mut lines, wrap_cols - 4);
190+
out.extend(prefix_lines(lines, " ".into(), " ".into()));
185191
}
186192

187193
out
@@ -190,31 +196,60 @@ fn render_changes_block(rows: Vec<Row>, wrap_cols: usize, cwd: &Path) -> Vec<RtL
190196
fn render_change(change: &FileChange, out: &mut Vec<RtLine<'static>>, width: usize) {
191197
match change {
192198
FileChange::Add { content } => {
199+
let line_number_width = line_number_width(content.lines().count());
193200
for (i, raw) in content.lines().enumerate() {
194201
out.extend(push_wrapped_diff_line(
195202
i + 1,
196203
DiffLineType::Insert,
197204
raw,
198205
width,
206+
line_number_width,
199207
));
200208
}
201209
}
202210
FileChange::Delete { content } => {
211+
let line_number_width = line_number_width(content.lines().count());
203212
for (i, raw) in content.lines().enumerate() {
204213
out.extend(push_wrapped_diff_line(
205214
i + 1,
206215
DiffLineType::Delete,
207216
raw,
208217
width,
218+
line_number_width,
209219
));
210220
}
211221
}
212222
FileChange::Update { unified_diff, .. } => {
213223
if let Ok(patch) = diffy::Patch::from_str(unified_diff) {
224+
let mut max_line_number = 0;
225+
for h in patch.hunks() {
226+
let mut old_ln = h.old_range().start();
227+
let mut new_ln = h.new_range().start();
228+
for l in h.lines() {
229+
match l {
230+
diffy::Line::Insert(_) => {
231+
max_line_number = max_line_number.max(new_ln);
232+
new_ln += 1;
233+
}
234+
diffy::Line::Delete(_) => {
235+
max_line_number = max_line_number.max(old_ln);
236+
old_ln += 1;
237+
}
238+
diffy::Line::Context(_) => {
239+
max_line_number = max_line_number.max(new_ln);
240+
old_ln += 1;
241+
new_ln += 1;
242+
}
243+
}
244+
}
245+
}
246+
let line_number_width = line_number_width(max_line_number);
214247
let mut is_first_hunk = true;
215248
for h in patch.hunks() {
216249
if !is_first_hunk {
217-
out.push(RtLine::from(vec![" ".into(), "⋮".dim()]));
250+
let spacer = format!("{:width$} ", "", width = line_number_width.max(1));
251+
let spacer_span = RtSpan::styled(spacer, style_gutter());
252+
out.push(RtLine::from(vec![spacer_span, "⋮".dim()]));
218253
}
219254
is_first_hunk = false;
220255

@@ -229,6 +264,7 @@ fn render_change(change: &FileChange, out: &mut Vec<RtLine<'static>>, width: usi
229264
DiffLineType::Insert,
230265
s,
231266
width,
267+
line_number_width,
232268
));
233269
new_ln += 1;
234270
}
@@ -239,6 +275,7 @@ fn render_change(change: &FileChange, out: &mut Vec<RtLine<'static>>, width: usi
239275
DiffLineType::Delete,
240276
s,
241277
width,
278+
line_number_width,
242279
));
243280
old_ln += 1;
244281
}
@@ -249,6 +286,7 @@ fn render_change(change: &FileChange, out: &mut Vec<RtLine<'static>>, width: usi
249286
DiffLineType::Context,
250287
s,
251288
width,
289+
line_number_width,
252290
));
253291
old_ln += 1;
254292
new_ln += 1;
@@ -298,17 +336,15 @@ fn push_wrapped_diff_line(
298336
kind: DiffLineType,
299337
text: &str,
300338
width: usize,
339+
line_number_width: usize,
301340
) -> Vec<RtLine<'static>> {
302-
let indent = " ";
303341
let ln_str = line_number.to_string();
304342
let mut remaining_text: &str = text;
305343

306-
// Reserve a fixed number of spaces after the line number so that content starts
307-
// at a consistent column. Content includes a 1-character diff sign prefix
308-
// ("+"/"-" for inserts/deletes, or a space for context lines) so alignment
309-
// stays consistent across all diff lines.
310-
let gap_after_ln = SPACES_AFTER_LINE_NUMBER.saturating_sub(ln_str.len());
311-
let prefix_cols = indent.len() + ln_str.len() + gap_after_ln;
344+
// Reserve a fixed number of spaces (equal to the widest line number plus a
345+
// trailing spacer) so the sign column stays aligned across the diff block.
346+
let gutter_width = line_number_width.max(1);
347+
let prefix_cols = gutter_width + 1;
312348

313349
let mut first = true;
314350
let (sign_char, line_style) = match kind {
@@ -332,8 +368,8 @@ fn push_wrapped_diff_line(
332368
remaining_text = rest;
333369

334370
if first {
335-
// Build gutter (indent + line number + spacing) as a dimmed span
336-
let gutter = format!("{indent}{ln_str}{}", " ".repeat(gap_after_ln));
371+
// Build gutter (right-aligned line number plus spacer) as a dimmed span
372+
let gutter = format!("{ln_str:>gutter_width$} ");
337373
// Content with a sign ('+'/'-'/' ') styled per diff kind
338374
let content = format!("{sign_char}{chunk}");
339375
lines.push(RtLine::from(vec![
@@ -343,7 +379,7 @@ fn push_wrapped_diff_line(
343379
first = false;
344380
} else {
345381
// Continuation lines keep a space for the sign column so content aligns
346-
let gutter = format!("{indent}{} ", " ".repeat(ln_str.len() + gap_after_ln));
382+
let gutter = format!("{:gutter_width$} ", "");
347383
lines.push(RtLine::from(vec![
348384
RtSpan::styled(gutter, style_gutter()),
349385
RtSpan::styled(chunk.to_string(), line_style),
@@ -356,6 +392,14 @@ fn push_wrapped_diff_line(
356392
lines
357393
}
358394

395+
fn line_number_width(max_line_number: usize) -> usize {
396+
if max_line_number == 0 {
397+
1
398+
} else {
399+
max_line_number.to_string().len()
400+
}
401+
}
402+
359403
fn style_gutter() -> Style {
360404
Style::default().add_modifier(Modifier::DIM)
361405
}
@@ -421,7 +465,8 @@ mod tests {
421465
let long_line = "this is a very long line that should wrap across multiple terminal columns and continue";
422466

423467
// Call the wrapping function directly so we can precisely control the width
424-
let lines = push_wrapped_diff_line(1, DiffLineType::Insert, long_line, 80);
468+
let lines =
469+
push_wrapped_diff_line(1, DiffLineType::Insert, long_line, 80, line_number_width(1));
425470

426471
// Render into a small terminal to capture the visual layout
427472
snapshot_lines("wrap_behavior_insert", lines, 90, 8);
@@ -442,11 +487,9 @@ mod tests {
442487
},
443488
);
444489

445-
for name in ["apply_update_block", "apply_update_block_manual"] {
446-
let lines = diff_summary_for_tests(&changes);
490+
let lines = diff_summary_for_tests(&changes);
447491

448-
snapshot_lines(name, lines, 80, 12);
449-
}
492+
snapshot_lines("apply_update_block", lines, 80, 12);
450493
}
451494

452495
#[test]
@@ -573,14 +616,37 @@ mod tests {
573616
},
574617
);
575618

576-
let mut lines = create_diff_summary(&changes, &PathBuf::from("/"), 28);
577-
// Drop the combined header for this text-only snapshot
578-
if !lines.is_empty() {
579-
lines.remove(0);
580-
}
619+
let lines = create_diff_summary(&changes, &PathBuf::from("/"), 28);
581620
snapshot_lines_text("apply_update_block_wraps_long_lines_text", &lines);
582621
}
583622

623+
#[test]
624+
fn ui_snapshot_apply_update_block_line_numbers_three_digits_text() {
625+
let original = (1..=110).map(|i| format!("line {i}\n")).collect::<String>();
626+
let modified = (1..=110)
627+
.map(|i| {
628+
if i == 100 {
629+
format!("line {i} changed\n")
630+
} else {
631+
format!("line {i}\n")
632+
}
633+
})
634+
.collect::<String>();
635+
let patch = diffy::create_patch(&original, &modified).to_string();
636+
637+
let mut changes: HashMap<PathBuf, FileChange> = HashMap::new();
638+
changes.insert(
639+
PathBuf::from("hundreds.txt"),
640+
FileChange::Update {
641+
unified_diff: patch,
642+
move_path: None,
643+
},
644+
);
645+
646+
let lines = create_diff_summary(&changes, &PathBuf::from("/"), 80);
647+
snapshot_lines_text("apply_update_block_line_numbers_three_digits_text", &lines);
648+
}
649+
584650
#[test]
585651
fn ui_snapshot_apply_update_block_relativizes_path() {
586652
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("/"));

‎codex-rs/tui/src/render/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ pub mod highlight;
44
pub mod line_utils;
55
pub mod renderable;
66

7+
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
78
pub struct Insets {
89
pub left: u16,
910
pub top: u16,

‎codex-rs/tui/src/render/renderable.rs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ use ratatui::text::Line;
44
use ratatui::widgets::Paragraph;
55
use ratatui::widgets::WidgetRef;
66

7+
use crate::render::Insets;
8+
use crate::render::RectExt as _;
9+
710
pub trait Renderable {
811
fn render(&self, area: Rect, buf: &mut Buffer);
912
fn desired_height(&self, width: u16) -> u16;
@@ -100,3 +103,26 @@ impl ColumnRenderable {
100103
}
101104
}
102105
}
106+
107+
pub struct InsetRenderable {
108+
child: Box<dyn Renderable>,
109+
insets: Insets,
110+
}
111+
112+
impl Renderable for InsetRenderable {
113+
fn render(&self, area: Rect, buf: &mut Buffer) {
114+
self.child.render(area.inset(self.insets), buf);
115+
}
116+
fn desired_height(&self, width: u16) -> u16 {
117+
self.child
118+
.desired_height(width - self.insets.left - self.insets.right)
119+
+ self.insets.top
120+
+ self.insets.bottom
121+
}
122+
}
123+
124+
impl InsetRenderable {
125+
pub fn new(child: Box<dyn Renderable>, insets: Insets) -> Self {
126+
Self { child, insets }
127+
}
128+
}

‎codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_add_block.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ source: tui/src/diff_render.rs
33
expression: terminal.backend()
44
---
55
"• Added new_file.txt (+2 -0) "
6-
" 1 +alpha "
7-
" 2 +beta "
6+
" 1 +alpha "
7+
" 2 +beta "
88
" "
99
" "
1010
" "

‎codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_delete_block.snap‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ source: tui/src/diff_render.rs
33
expression: terminal.backend()
44
---
55
"• Deleted tmp_delete_example.txt (+0 -3) "
6-
" 1 -first "
7-
" 2 -second "
8-
" 3 -third "
6+
" 1 -first "
7+
" 2 -second "
8+
" 3 -third "
99
" "
1010
" "
1111
" "

‎codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_multiple_files_block.snap‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ expression: terminal.backend()
44
---
55
"• Edited 2 files (+2 -1) "
66
" └ a.txt (+1 -1) "
7-
" 1 -one "
8-
" 1 +one changed "
7+
" 1 -one "
8+
" 1 +one changed "
99
" "
1010
" └ b.txt (+1 -0) "
11-
" 1 +new "
11+
" 1 +new "
1212
" "
1313
" "
1414
" "

‎codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block.snap‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
---
22
source: tui/src/diff_render.rs
3-
assertion_line: 748
43
expression: terminal.backend()
54
---
65
"• Edited example.txt (+1 -1) "
7-
" 1 line one "
8-
" 2 -line two "
9-
" 2 +line two changed "
10-
" 3 line three "
6+
" 1 line one "
7+
" 2 -line two "
8+
" 2 +line two changed "
9+
" 3 line three "
1110
" "
1211
" "
1312
" "

0 commit comments

Comments
 (0)