valknar/llmx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

dynamic width for line numbers in diffs (#4664)

Browse Source 
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"
/>
This commit is contained in:
Jeremy Rose
2025-10-07 11:32:07 -07:00
committed by GitHub
parent 12fd2b4160
commit 75176dae70
18 changed files with 182 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
codex-rs/tui/src/bottom_pane/approval_overlay.rs
View File

@@ -315,15 +315,16 @@ impl From<ApprovalRequest> for ApprovalRequestState {
changes, changes,
} => { } => {
let mut header: Vec<Box<dyn Renderable>> = Vec::new(); let mut header: Vec<Box<dyn Renderable>> = Vec::new();
header.push(DiffSummary::new(changes, cwd).into());
if let Some(reason) = reason if let Some(reason) = reason
&& !reason.is_empty() && !reason.is_empty()
{ {
header.push(Box::new(Line::from("")));
header.push(Box::new( header.push(Box::new(
Paragraph::new(reason.italic()).wrap(Wrap { trim: false }), Paragraph::new(Line::from_iter(["Reason: ".into(), reason.italic()]))
.wrap(Wrap { trim: false }),
)); ));
header.push(Box::new(Line::from("")));
} }
header.push(DiffSummary::new(changes, cwd).into());
Self { Self {
variant: ApprovalVariant::ApplyPatch { id }, variant: ApprovalVariant::ApplyPatch { id },
header: Box::new(ColumnRenderable::new(header)), header: Box::new(ColumnRenderable::new(header)),

2
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__apply_patch_manual_flow_history_approved.snap
View File

@@ -3,4 +3,4 @@ source: tui/src/chatwidget/tests.rs
expression: lines_to_single_string(&approved_lines) expression: lines_to_single_string(&approved_lines)
--- ---
• Added foo.txt (+1 -0) • Added foo.txt (+1 -0)
1 +hello 1 +hello

8
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap
View File

@@ -4,12 +4,12 @@ expression: terminal.backend().vt100().screen().contents()
--- ---
Would you like to make the following edits? Would you like to make the following edits?
Reason: The model wants to apply changes
README.md (+2 -0) README.md (+2 -0)
1 +hello 1 +hello
2 +world 2 +world
The model wants to apply changes
› 1. Yes, proceed › 1. Yes, proceed
2. No, and tell Codex what to do differently esc 2. No, and tell Codex what to do differently esc

116
codex-rs/tui/src/diff_render.rs
View File

@@ -13,13 +13,14 @@ use std::path::Path;
use std::path::PathBuf; use std::path::PathBuf;
use crate::exec_command::relativize_to_home; use crate::exec_command::relativize_to_home;
use crate::render::Insets;
use crate::render::line_utils::prefix_lines;
use crate::render::renderable::ColumnRenderable; use crate::render::renderable::ColumnRenderable;
use crate::render::renderable::InsetRenderable;
use crate::render::renderable::Renderable; use crate::render::renderable::Renderable;
use codex_core::git_info::get_git_repo_root; use codex_core::git_info::get_git_repo_root;
use codex_core::protocol::FileChange; use codex_core::protocol::FileChange;
const SPACES_AFTER_LINE_NUMBER: usize = 6;
// Internal representation for diff line rendering // Internal representation for diff line rendering
enum DiffLineType { enum DiffLineType {
Insert, Insert,
@@ -65,7 +66,10 @@ impl From<DiffSummary> for Box<dyn Renderable> {
path.extend(render_line_count_summary(row.added, row.removed)); path.extend(render_line_count_summary(row.added, row.removed));
rows.push(Box::new(path)); rows.push(Box::new(path));
rows.push(Box::new(RtLine::from(""))); rows.push(Box::new(RtLine::from("")));
rows.push(Box::new(row.change)); rows.push(Box::new(InsetRenderable::new(
Box::new(row.change),
Insets::tlbr(0, 2, 0, 0),
)));
} }
Box::new(ColumnRenderable::new(rows)) Box::new(ColumnRenderable::new(rows))
@@ -181,7 +185,9 @@ fn render_changes_block(rows: Vec<Row>, wrap_cols: usize, cwd: &Path) -> Vec<RtL
out.push(RtLine::from(header)); out.push(RtLine::from(header));
} }
render_change(&r.change, &mut out, wrap_cols); let mut lines = vec![];
render_change(&r.change, &mut lines, wrap_cols - 4);
out.extend(prefix_lines(lines, " ".into(), " ".into()));
} }
out out
@@ -190,31 +196,60 @@ fn render_changes_block(rows: Vec<Row>, wrap_cols: usize, cwd: &Path) -> Vec<RtL
fn render_change(change: &FileChange, out: &mut Vec<RtLine<'static>>, width: usize) { fn render_change(change: &FileChange, out: &mut Vec<RtLine<'static>>, width: usize) {
match change { match change {
FileChange::Add { content } => { FileChange::Add { content } => {
let line_number_width = line_number_width(content.lines().count());
for (i, raw) in content.lines().enumerate() { for (i, raw) in content.lines().enumerate() {
out.extend(push_wrapped_diff_line( out.extend(push_wrapped_diff_line(
i + 1, i + 1,
DiffLineType::Insert, DiffLineType::Insert,
raw, raw,
width, width,
line_number_width,
)); ));
} }
} }
FileChange::Delete { content } => { FileChange::Delete { content } => {
let line_number_width = line_number_width(content.lines().count());
for (i, raw) in content.lines().enumerate() { for (i, raw) in content.lines().enumerate() {
out.extend(push_wrapped_diff_line( out.extend(push_wrapped_diff_line(
i + 1, i + 1,
DiffLineType::Delete, DiffLineType::Delete,
raw, raw,
width, width,
line_number_width,
)); ));
} }
} }
FileChange::Update { unified_diff, .. } => { FileChange::Update { unified_diff, .. } => {
if let Ok(patch) = diffy::Patch::from_str(unified_diff) { if let Ok(patch) = diffy::Patch::from_str(unified_diff) {
let mut max_line_number = 0;
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(_) => {
max_line_number = max_line_number.max(new_ln);
new_ln += 1;
}
diffy::Line::Delete(_) => {
max_line_number = max_line_number.max(old_ln);
old_ln += 1;
}
diffy::Line::Context(_) => {
max_line_number = max_line_number.max(new_ln);
old_ln += 1;
new_ln += 1;
}
}
}
}
let line_number_width = line_number_width(max_line_number);
let mut is_first_hunk = true; let mut is_first_hunk = true;
for h in patch.hunks() { for h in patch.hunks() {
if !is_first_hunk { if !is_first_hunk {
out.push(RtLine::from(vec![" ".into(), "⋮".dim()])); let spacer = format!("{:width$} ", "", width = line_number_width.max(1));
let spacer_span = RtSpan::styled(spacer, style_gutter());
out.push(RtLine::from(vec![spacer_span, "⋮".dim()]));
} }
is_first_hunk = false; is_first_hunk = false;
@@ -229,6 +264,7 @@ fn render_change(change: &FileChange, out: &mut Vec<RtLine<'static>>, width: usi
DiffLineType::Insert, DiffLineType::Insert,
s, s,
width, width,
line_number_width,
)); ));
new_ln += 1; new_ln += 1;
} }
@@ -239,6 +275,7 @@ fn render_change(change: &FileChange, out: &mut Vec<RtLine<'static>>, width: usi
DiffLineType::Delete, DiffLineType::Delete,
s, s,
width, width,
line_number_width,
)); ));
old_ln += 1; old_ln += 1;
} }
@@ -249,6 +286,7 @@ fn render_change(change: &FileChange, out: &mut Vec<RtLine<'static>>, width: usi
DiffLineType::Context, DiffLineType::Context,
s, s,
width, width,
line_number_width,
)); ));
old_ln += 1; old_ln += 1;
new_ln += 1; new_ln += 1;
@@ -298,17 +336,15 @@ fn push_wrapped_diff_line(
kind: DiffLineType, kind: DiffLineType,
text: &str, text: &str,
width: usize, width: usize,
line_number_width: usize,
) -> Vec<RtLine<'static>> { ) -> Vec<RtLine<'static>> {
let indent = " ";
let ln_str = line_number.to_string(); let ln_str = line_number.to_string();
let mut remaining_text: &str = text; let mut remaining_text: &str = text;
// Reserve a fixed number of spaces after the line number so that content starts // Reserve a fixed number of spaces (equal to the widest line number plus a
// at a consistent column. Content includes a 1-character diff sign prefix // trailing spacer) so the sign column stays aligned across the diff block.
// ("+"/"-" for inserts/deletes, or a space for context lines) so alignment let gutter_width = line_number_width.max(1);
// stays consistent across all diff lines. let prefix_cols = gutter_width + 1;
let gap_after_ln = SPACES_AFTER_LINE_NUMBER.saturating_sub(ln_str.len());
let prefix_cols = indent.len() + ln_str.len() + gap_after_ln;
let mut first = true; let mut first = true;
let (sign_char, line_style) = match kind { let (sign_char, line_style) = match kind {
@@ -332,8 +368,8 @@ fn push_wrapped_diff_line(
remaining_text = rest; remaining_text = rest;
if first { if first {
// Build gutter (indent + line number + spacing) as a dimmed span // Build gutter (right-aligned line number plus spacer) as a dimmed span
let gutter = format!("{indent}{ln_str}{}", " ".repeat(gap_after_ln)); let gutter = format!("{ln_str:>gutter_width$} ");
// Content with a sign ('+'/'-'/' ') styled per diff kind // Content with a sign ('+'/'-'/' ') styled per diff kind
let content = format!("{sign_char}{chunk}"); let content = format!("{sign_char}{chunk}");
lines.push(RtLine::from(vec![ lines.push(RtLine::from(vec![
@@ -343,7 +379,7 @@ fn push_wrapped_diff_line(
first = false; first = false;
} else { } else {
// Continuation lines keep a space for the sign column so content aligns // Continuation lines keep a space for the sign column so content aligns
let gutter = format!("{indent}{} ", " ".repeat(ln_str.len() + gap_after_ln)); let gutter = format!("{:gutter_width$} ", "");
lines.push(RtLine::from(vec![ lines.push(RtLine::from(vec![
RtSpan::styled(gutter, style_gutter()), RtSpan::styled(gutter, style_gutter()),
RtSpan::styled(chunk.to_string(), line_style), RtSpan::styled(chunk.to_string(), line_style),
@@ -356,6 +392,14 @@ fn push_wrapped_diff_line(
lines lines
} }
fn line_number_width(max_line_number: usize) -> usize {
if max_line_number == 0 {
1
} else {
max_line_number.to_string().len()
}
}
fn style_gutter() -> Style { fn style_gutter() -> Style {
Style::default().add_modifier(Modifier::DIM) Style::default().add_modifier(Modifier::DIM)
} }
@@ -421,7 +465,8 @@ mod tests {
let long_line = "this is a very long line that should wrap across multiple terminal columns and continue"; 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 // Call the wrapping function directly so we can precisely control the width
let lines = push_wrapped_diff_line(1, DiffLineType::Insert, long_line, 80); let lines =
push_wrapped_diff_line(1, DiffLineType::Insert, long_line, 80, line_number_width(1));
// Render into a small terminal to capture the visual layout // Render into a small terminal to capture the visual layout
snapshot_lines("wrap_behavior_insert", lines, 90, 8); snapshot_lines("wrap_behavior_insert", lines, 90, 8);
@@ -442,11 +487,9 @@ mod tests {
}, },
); );
for name in ["apply_update_block", "apply_update_block_manual"] { let lines = diff_summary_for_tests(&changes);
let lines = diff_summary_for_tests(&changes);
snapshot_lines(name, lines, 80, 12); snapshot_lines("apply_update_block", lines, 80, 12);
}
} }
#[test] #[test]
@@ -573,14 +616,37 @@ mod tests {
}, },
); );
let mut lines = create_diff_summary(&changes, &PathBuf::from("/"), 28); let lines = create_diff_summary(&changes, &PathBuf::from("/"), 28);
// Drop the combined header for this text-only snapshot
if !lines.is_empty() {
lines.remove(0);
}
snapshot_lines_text("apply_update_block_wraps_long_lines_text", &lines); snapshot_lines_text("apply_update_block_wraps_long_lines_text", &lines);
} }
#[test]
fn ui_snapshot_apply_update_block_line_numbers_three_digits_text() {
let original = (1..=110).map(|i| format!("line {i}\n")).collect::<String>();
let modified = (1..=110)
.map(|i| {
if i == 100 {
format!("line {i} changed\n")
} else {
format!("line {i}\n")
}
})
.collect::<String>();
let patch = diffy::create_patch(&original, &modified).to_string();
let mut changes: HashMap<PathBuf, FileChange> = HashMap::new();
changes.insert(
PathBuf::from("hundreds.txt"),
FileChange::Update {
unified_diff: patch,
move_path: None,
},
);
let lines = create_diff_summary(&changes, &PathBuf::from("/"), 80);
snapshot_lines_text("apply_update_block_line_numbers_three_digits_text", &lines);
}
#[test] #[test]
fn ui_snapshot_apply_update_block_relativizes_path() { fn ui_snapshot_apply_update_block_relativizes_path() {
let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("/")); let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("/"));

1
codex-rs/tui/src/render/mod.rs
View File

@@ -4,6 +4,7 @@ pub mod highlight;
pub mod line_utils; pub mod line_utils;
pub mod renderable; pub mod renderable;
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct Insets { pub struct Insets {
pub left: u16, pub left: u16,
pub top: u16, pub top: u16,

26
codex-rs/tui/src/render/renderable.rs
View File

@@ -4,6 +4,9 @@ use ratatui::text::Line;
use ratatui::widgets::Paragraph; use ratatui::widgets::Paragraph;
use ratatui::widgets::WidgetRef; use ratatui::widgets::WidgetRef;
use crate::render::Insets;
use crate::render::RectExt as _;
pub trait Renderable { pub trait Renderable {
fn render(&self, area: Rect, buf: &mut Buffer); fn render(&self, area: Rect, buf: &mut Buffer);
fn desired_height(&self, width: u16) -> u16; fn desired_height(&self, width: u16) -> u16;
@@ -100,3 +103,26 @@ impl ColumnRenderable {
} }
} }
} }
pub struct InsetRenderable {
child: Box<dyn Renderable>,
insets: Insets,
}
impl Renderable for InsetRenderable {
fn render(&self, area: Rect, buf: &mut Buffer) {
self.child.render(area.inset(self.insets), buf);
}
fn desired_height(&self, width: u16) -> u16 {
self.child
.desired_height(width - self.insets.left - self.insets.right)
+ self.insets.top
+ self.insets.bottom
}
}
impl InsetRenderable {
pub fn new(child: Box<dyn Renderable>, insets: Insets) -> Self {
Self { child, insets }
}
}

4
codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_add_block.snap
View File

@@ -3,8 +3,8 @@ source: tui/src/diff_render.rs
expression: terminal.backend() expression: terminal.backend()
--- ---
"• Added new_file.txt (+2 -0) " "• Added new_file.txt (+2 -0) "
" 1 +alpha " " 1 +alpha "
" 2 +beta " " 2 +beta "
" " " "
" " " "
" " " "

6
codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_delete_block.snap
View File

@@ -3,9 +3,9 @@ source: tui/src/diff_render.rs
expression: terminal.backend() expression: terminal.backend()
--- ---
"• Deleted tmp_delete_example.txt (+0 -3) " "• Deleted tmp_delete_example.txt (+0 -3) "
" 1 -first " " 1 -first "
" 2 -second " " 2 -second "
" 3 -third " " 3 -third "
" " " "
" " " "
" " " "

6
codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_multiple_files_block.snap
View File

@@ -4,11 +4,11 @@ expression: terminal.backend()
--- ---
"• Edited 2 files (+2 -1) " "• Edited 2 files (+2 -1) "
" └ a.txt (+1 -1) " " └ a.txt (+1 -1) "
" 1 -one " " 1 -one "
" 1 +one changed " " 1 +one changed "
" " " "
" └ b.txt (+1 -0) " " └ b.txt (+1 -0) "
" 1 +new " " 1 +new "
" " " "
" " " "
" " " "

9
codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block.snap
View File

@@ -1,13 +1,12 @@
--- ---
source: tui/src/diff_render.rs source: tui/src/diff_render.rs
assertion_line: 748
expression: terminal.backend() expression: terminal.backend()
--- ---
"• Edited example.txt (+1 -1) " "• Edited example.txt (+1 -1) "
" 1 line one " " 1 line one "
" 2 -line two " " 2 -line two "
" 2 +line two changed " " 2 +line two changed "
" 3 line three " " 3 line three "
" " " "
" " " "
" " " "

13
codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_line_numbers_three_digits_text.snap Normal file
View File

@@ -0,0 +1,13 @@
---
source: tui/src/diff_render.rs
expression: text
---
• Edited hundreds.txt (+1 -1)
97 line 97
98 line 98
99 line 99
100 -line 100
100 +line 100 changed
101 line 101
102 line 102
103 line 103

16
codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_manual.snap
View File

@@ -1,16 +0,0 @@
---
source: tui/src/diff_render.rs
expression: terminal.backend()
---
"• Edited example.txt (+1 -1) "
" 1 line one "
" 2 -line two "
" 2 +line two changed "
" 3 line three "
" "
" "
" "
" "
" "
" "
" "

7
codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_relativizes_path.snap
View File

@@ -1,12 +1,11 @@
--- ---
source: tui/src/diff_render.rs source: tui/src/diff_render.rs
assertion_line: 748
expression: terminal.backend() expression: terminal.backend()
--- ---
"• Edited abs_old.rs → abs_new.rs (+1 -1) " "• Edited abs_old.rs → abs_new.rs (+1 -1) "
" 1 -X " " 1 -X "
" 1 +X changed " " 1 +X changed "
" 2 Y " " 2 Y "
" " " "
" " " "
" " " "

13
codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines.snap
View File

@@ -1,15 +1,14 @@
--- ---
source: tui/src/diff_render.rs source: tui/src/diff_render.rs
assertion_line: 748
expression: terminal.backend() expression: terminal.backend()
--- ---
"• Edited long_example.txt (+1 -1) " "• Edited long_example.txt (+1 -1) "
" 1 line 1 " " 1 line 1 "
" 2 -short " " 2 -short "
" 2 +short this_is_a_very_long_modified_line_that_should_wrap_acro " " 2 +short this_is_a_very_long_modified_line_that_should_wrap_across_m "
" ss_multiple_terminal_columns_and_continue_even_further_beyond " " ultiple_terminal_columns_and_continue_even_further_beyond_eighty_ "
" _eighty_columns_to_force_multiple_wraps " " columns_to_force_multiple_wraps "
" 3 line 3 " " 3 line 3 "
" " " "
" " " "
" " " "

23
codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_block_wraps_long_lines_text.snap
View File

@@ -2,15 +2,14 @@
source: tui/src/diff_render.rs source: tui/src/diff_render.rs
expression: text expression: text
--- ---
1 1 • Edited wrap_demo.txt (+2 -2)
2 -2 1 1
2 +added long line w 2 -2
hich wraps and_if 2 +added long line which
_there_is_a_long_ wraps and_if_there_i
token_it_will_be_ s_a_long_token_it_wil
broken l_be_broken
3 3 3 3
4 -4 4 -4
4 +4 context line wh 4 +4 context line which
ich also wraps ac also wraps across
ross

9
codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__apply_update_with_rename_block.snap
View File

@@ -1,13 +1,12 @@
--- ---
source: tui/src/diff_render.rs source: tui/src/diff_render.rs
assertion_line: 748
expression: terminal.backend() expression: terminal.backend()
--- ---
"• Edited old_name.rs → new_name.rs (+1 -1) " "• Edited old_name.rs → new_name.rs (+1 -1) "
" 1 A " " 1 A "
" 2 -B " " 2 -B "
" 2 +B changed " " 2 +B changed "
" 3 C " " 3 C "
" " " "
" " " "
" " " "

4
codex-rs/tui/src/snapshots/codex_tui__diff_render__tests__wrap_behavior_insert.snap
View File

@@ -2,8 +2,8 @@
source: tui/src/diff_render.rs source: tui/src/diff_render.rs
expression: terminal.backend() expression: terminal.backend()
--- ---
" 1 +this is a very long line that should wrap across multiple terminal co " "1 +this is a very long line that should wrap across multiple terminal columns an "
" lumns and continue " " d continue "
" " " "
" " " "
" " " "

8
codex-rs/tui/src/snapshots/codex_tui__pager_overlay__tests__transcript_overlay_apply_patch_scroll_vt100.snap
View File

@@ -4,12 +4,12 @@ expression: snapshot
--- ---
/ T R A N S C R I P T / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / T R A N S C R I P T / / / / / / / / / / / / / / / / / / / / / / / / / / / / /
• Added foo.txt (+2 -0) • Added foo.txt (+2 -0)
1 +hello 1 +hello
2 +world 2 +world
• Added foo.txt (+2 -0) • Added foo.txt (+2 -0)
1 +hello 1 +hello
2 +world 2 +world
─────────────────────────────────────────────────────────────────────────── 0% ─ ─────────────────────────────────────────────────────────────────────────── 0% ─
↑/↓ to scroll pgup/pgdn to page home/end to jump ↑/↓ to scroll pgup/pgdn to page home/end to jump
q to quit esc to edit prev q to quit esc to edit prev