From 98c6dfa53711c499a989309c30b0fe38bfa54590 Mon Sep 17 00:00:00 2001 From: MomentDerek <40252940+MomentDerek@users.noreply.github.com> Date: Sat, 18 Oct 2025 10:03:36 +0800 Subject: [PATCH] fix: diff_buffers clear-to-end when deleting wide graphemes (#4921) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #4870 #4717 #3260 #4431 #2718 #4898 #5036 - Fix the chat composer “phantom space” bug that appeared when backspacing CJK (and other double-width) characters after the composer got a uniform background in 43b63ccae89c…. - Pull diff_buffers’s clear-to-end logic forward to iterate by display width, so wide graphemes are counted correctly when computing the trailing column. - Keep modifier-aware detection so styled cells are still flushed, and add a regression test (diff_buffers_clear_to_end_starts_after_wide_char) that covers the CJK deletion scenario. --------- Co-authored-by: Josh McKinney --- codex-rs/tui/src/custom_terminal.rs | 58 +++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/codex-rs/tui/src/custom_terminal.rs b/codex-rs/tui/src/custom_terminal.rs index bbd89006..bd37fe81 100644 --- a/codex-rs/tui/src/custom_terminal.rs +++ b/codex-rs/tui/src/custom_terminal.rs @@ -411,28 +411,35 @@ fn diff_buffers<'a>(a: &'a Buffer, b: &'a Buffer) -> Vec> { let next_buffer = &b.content; let mut updates = vec![]; - let mut last_nonblank_column = vec![0; a.area.height as usize]; + let mut last_nonblank_columns = vec![0; a.area.height as usize]; for y in 0..a.area.height { let row_start = y as usize * a.area.width as usize; let row_end = row_start + a.area.width as usize; let row = &next_buffer[row_start..row_end]; let bg = row.last().map(|cell| cell.bg).unwrap_or(Color::Reset); - let x = row - .iter() - .rposition(|cell| { - cell.symbol() != " " || cell.bg != bg || cell.modifier != Modifier::empty() - }) - .unwrap_or(0); - last_nonblank_column[y as usize] = x as u16; - if x < (a.area.width as usize).saturating_sub(1) { - let (x_abs, y_abs) = a.pos_of(row_start + x + 1); - updates.push(DrawCommand::ClearToEnd { - x: x_abs, - y: y_abs, - bg, - }); + // Scan the row to find the rightmost column that still matters: any non-space glyph, + // any cell whose bg differs from the row’s trailing bg, or any cell with modifiers. + // Multi-width glyphs extend that region through their full displayed width. + // After that point the rest of the row can be cleared with a single ClearToEnd, a perf win + // versus emitting multiple space Put commands. + let mut last_nonblank_column = 0usize; + let mut column = 0usize; + while column < row.len() { + let cell = &row[column]; + let width = cell.symbol().width(); + if cell.symbol() != " " || cell.bg != bg || cell.modifier != Modifier::empty() { + last_nonblank_column = column + (width.saturating_sub(1)); + } + column += width.max(1); // treat zero-width symbols as width 1 } + + if last_nonblank_column + 1 < row.len() { + let (x, y) = a.pos_of(row_start + last_nonblank_column + 1); + updates.push(DrawCommand::ClearToEnd { x, y, bg }); + } + + last_nonblank_columns[y as usize] = last_nonblank_column as u16; } // Cells invalidated by drawing/replacing preceding multi-width characters: @@ -444,7 +451,7 @@ fn diff_buffers<'a>(a: &'a Buffer, b: &'a Buffer) -> Vec> { if !current.skip && (current != previous || invalidated > 0) && to_skip == 0 { let (x, y) = a.pos_of(i); let row = i / a.area.width as usize; - if x <= last_nonblank_column[row] { + if x <= last_nonblank_columns[row] { updates.push(DrawCommand::Put { x, y, @@ -592,6 +599,7 @@ mod tests { use super::*; use pretty_assertions::assert_eq; use ratatui::layout::Rect; + use ratatui::style::Style; #[test] fn diff_buffers_does_not_emit_clear_to_end_for_full_width_row() { @@ -620,4 +628,22 @@ mod tests { "expected diff_buffers to update the final cell; commands: {commands:?}", ); } + + #[test] + fn diff_buffers_clear_to_end_starts_after_wide_char() { + let area = Rect::new(0, 0, 10, 1); + let mut previous = Buffer::empty(area); + let mut next = Buffer::empty(area); + + previous.set_string(0, 0, "中文", Style::default()); + next.set_string(0, 0, "中", Style::default()); + + let commands = diff_buffers(&previous, &next); + assert!( + commands + .iter() + .any(|command| matches!(command, DrawCommand::ClearToEnd { x: 2, y: 0, .. })), + "expected clear-to-end to start after the remaining wide char; commands: {commands:?}" + ); + } }