mirror of
https://github.com/openai/codex.git
synced 2026-09-08 15:50:34 +00:00
Preserve hyperlink cell widths during terminal diffing (#35960)
## What changed - Mark OSC 8 hyperlink cells with their visible width so Ratatui's buffer diff handles wide and halfwidth characters correctly. - Use Ratatui's diff iterator while explicitly clearing styled trailing cells when a forced-width cell replaces a wider cell. ## Testing - Add coverage for forced-width hyperlink rendering, always-update cells, and styled trailing-cell cleanup. GitOrigin-RevId: 041128f43beb1e09d40179a3ac16dca1fbdcbc44
This commit is contained in:
@@ -48,39 +48,6 @@ use ratatui::style::Color;
|
||||
use ratatui::style::Modifier;
|
||||
use ratatui::widgets::WidgetRef;
|
||||
|
||||
/// Returns the display width of a cell symbol, ignoring OSC escape sequences.
|
||||
///
|
||||
/// OSC sequences (e.g. OSC 8 hyperlinks: `\x1B]8;;URL\x07`) are terminal
|
||||
/// control sequences that don't consume display columns. The standard
|
||||
/// `CellWidth::cell_width()` method incorrectly counts the printable
|
||||
/// characters inside OSC payloads (like `]`, `8`, `;`, and URL characters).
|
||||
/// This function strips them first so that only visible characters contribute
|
||||
/// to the width.
|
||||
fn display_width(s: &str) -> usize {
|
||||
// Fast path: no escape sequences present.
|
||||
if !s.contains('\x1B') {
|
||||
return usize::from(s.cell_width());
|
||||
}
|
||||
|
||||
// Strip OSC sequences: ESC ] ... BEL
|
||||
let mut visible = String::with_capacity(s.len());
|
||||
let mut chars = s.chars();
|
||||
while let Some(ch) = chars.next() {
|
||||
if ch == '\x1B' && chars.clone().next() == Some(']') {
|
||||
// Consume the ']' and everything up to and including BEL.
|
||||
chars.next(); // skip ']'
|
||||
for c in chars.by_ref() {
|
||||
if c == '\x07' {
|
||||
break;
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
visible.push(ch);
|
||||
}
|
||||
usize::from(visible.as_str().cell_width())
|
||||
}
|
||||
|
||||
fn osc8_hyperlink_parts(symbol: &str) -> Option<(&str, &str)> {
|
||||
let content = symbol.strip_prefix("\x1b]8;;")?;
|
||||
let destination_end = content.find('\x07')?;
|
||||
@@ -587,7 +554,6 @@ enum DrawCommand {
|
||||
}
|
||||
|
||||
fn diff_buffers(a: &Buffer, b: &Buffer) -> Vec<DrawCommand> {
|
||||
let previous_buffer = &a.content;
|
||||
let next_buffer = &b.content;
|
||||
|
||||
let mut updates = vec![];
|
||||
@@ -607,7 +573,7 @@ fn diff_buffers(a: &Buffer, b: &Buffer) -> Vec<DrawCommand> {
|
||||
let mut column = 0usize;
|
||||
while column < row.len() {
|
||||
let cell = &row[column];
|
||||
let width = display_width(cell.symbol());
|
||||
let width = usize::from(cell.cell_width());
|
||||
if cell.symbol() != " " || cell.bg != bg || cell.modifier != Modifier::empty() {
|
||||
last_nonblank_column = column + (width.saturating_sub(1));
|
||||
}
|
||||
@@ -622,34 +588,52 @@ fn diff_buffers(a: &Buffer, b: &Buffer) -> Vec<DrawCommand> {
|
||||
last_nonblank_columns[y as usize] = last_nonblank_column as u16;
|
||||
}
|
||||
|
||||
// Cells invalidated by drawing/replacing preceding multi-width characters:
|
||||
let mut invalidated: usize = 0;
|
||||
// Cells from the current buffer to skip due to preceding multi-width characters taking
|
||||
// their place (the skipped cells should be blank anyway), or due to per-cell-skipping:
|
||||
let mut to_skip: usize = 0;
|
||||
for (i, (current, previous)) in next_buffer.iter().zip(previous_buffer.iter()).enumerate() {
|
||||
if current.diff_option != CellDiffOption::Skip
|
||||
&& (current != previous || invalidated > 0)
|
||||
&& to_skip == 0
|
||||
let mut cell_updates = a.diff_iter(b).collect::<Vec<_>>();
|
||||
// Ratatui's ForcedWidth path skips trailing-cell invalidation when a styled wide cell shrinks.
|
||||
let visible_on_blank = Modifier::REVERSED
|
||||
.union(Modifier::UNDERLINED)
|
||||
.union(Modifier::SLOW_BLINK)
|
||||
.union(Modifier::RAPID_BLINK)
|
||||
.union(Modifier::CROSSED_OUT);
|
||||
for (i, (current, previous)) in next_buffer.iter().zip(a.content.iter()).enumerate() {
|
||||
let CellDiffOption::ForcedWidth(current_width) = current.diff_option else {
|
||||
continue;
|
||||
};
|
||||
let current_width = usize::from(current_width.get());
|
||||
let previous_width = usize::from(previous.cell_width());
|
||||
if previous_width <= current_width
|
||||
|| (previous.bg == Color::Reset && !previous.modifier.intersects(visible_on_blank))
|
||||
{
|
||||
let (x, y) = a.pos_of(i);
|
||||
let row = i / a.area.width as usize;
|
||||
if x <= last_nonblank_columns[row] {
|
||||
updates.push(DrawCommand::Put {
|
||||
x,
|
||||
y,
|
||||
cell: next_buffer[i].clone(),
|
||||
});
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
to_skip = display_width(current.symbol()).saturating_sub(1);
|
||||
for (index, cell) in next_buffer
|
||||
.iter()
|
||||
.enumerate()
|
||||
.skip(i + current_width)
|
||||
.take(previous_width - current_width)
|
||||
{
|
||||
#[allow(deprecated)]
|
||||
let is_skip = cell.diff_option == CellDiffOption::Skip
|
||||
|| (cell.skip && cell.diff_option == CellDiffOption::None);
|
||||
if !is_skip {
|
||||
let (x, y) = a.pos_of(index);
|
||||
cell_updates.push((x, y, cell));
|
||||
}
|
||||
}
|
||||
}
|
||||
cell_updates.sort_unstable_by_key(|(x, y, _)| (*y, *x));
|
||||
cell_updates.dedup_by_key(|(x, y, _)| (*y, *x));
|
||||
|
||||
let affected_width = std::cmp::max(
|
||||
display_width(current.symbol()),
|
||||
display_width(previous.symbol()),
|
||||
);
|
||||
invalidated = std::cmp::max(affected_width, invalidated).saturating_sub(1);
|
||||
for (x, y, cell) in cell_updates {
|
||||
let row = usize::from(y - a.area.y);
|
||||
if x <= last_nonblank_columns[row] {
|
||||
updates.push(DrawCommand::Put {
|
||||
x,
|
||||
y,
|
||||
cell: cell.clone(),
|
||||
});
|
||||
}
|
||||
}
|
||||
updates
|
||||
}
|
||||
@@ -806,6 +790,8 @@ impl ModifierDiff {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::num::NonZeroU16;
|
||||
|
||||
use pretty_assertions::assert_eq;
|
||||
use ratatui::backend::WindowSize;
|
||||
use ratatui::layout::Rect;
|
||||
@@ -1000,11 +986,50 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn display_width_handles_halfwidth_dakuten() {
|
||||
assert_eq!(display_width("ガ"), 2);
|
||||
assert_eq!(
|
||||
display_width("\x1b]8;;https://example.com\x07ガ\x1b]8;;\x07"),
|
||||
2
|
||||
fn diff_buffers_emits_always_update_cells() {
|
||||
use ratatui::buffer::CellDiffOption;
|
||||
|
||||
let mut previous = Buffer::with_lines(["abc"]);
|
||||
let mut next = Buffer::with_lines(["abc"]);
|
||||
previous[(1, 0)].set_diff_option(CellDiffOption::AlwaysUpdate);
|
||||
next[(1, 0)].set_diff_option(CellDiffOption::AlwaysUpdate);
|
||||
|
||||
let commands = diff_buffers(&previous, &next);
|
||||
assert!(
|
||||
commands
|
||||
.iter()
|
||||
.any(|command| matches!(command, DrawCommand::Put { x: 1, y: 0, .. })),
|
||||
"expected the always-update cell to be emitted; commands: {commands:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn diff_buffers_clears_styled_trailing_cell_replaced_by_forced_width_cell() {
|
||||
use ratatui::buffer::CellDiffOption;
|
||||
|
||||
let area = Rect::new(0, 0, 7, 1);
|
||||
let mut previous = Buffer::empty(area);
|
||||
let mut next = Buffer::empty(area);
|
||||
previous.set_string(
|
||||
0,
|
||||
0,
|
||||
"漢 tail",
|
||||
Style::default()
|
||||
.bg(Color::Blue)
|
||||
.add_modifier(Modifier::UNDERLINED),
|
||||
);
|
||||
next.set_string(0, 0, "a tail", Style::default());
|
||||
next[(0, 0)]
|
||||
.set_symbol("\x1b]8;;https://example.com\x07a\x1b]8;;\x07")
|
||||
.set_diff_option(CellDiffOption::ForcedWidth(NonZeroU16::MIN));
|
||||
|
||||
let commands = diff_buffers(&previous, &next);
|
||||
|
||||
assert!(
|
||||
commands
|
||||
.iter()
|
||||
.any(|command| matches!(command, DrawCommand::Put { x: 1, y: 0, .. })),
|
||||
"expected the styled trailing cell to be cleared; commands: {commands:?}"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
---
|
||||
source: tui/src/terminal_hyperlinks.rs
|
||||
expression: terminal.backend()
|
||||
---
|
||||
prefix 漢字 ガ
|
||||
tail
|
||||
@@ -3,6 +3,7 @@
|
||||
//! Layout code measures and wraps ordinary ratatui lines. Hyperlink annotations are applied only
|
||||
//! when text reaches a terminal buffer or scrollback writer so OSC 8 bytes never affect geometry.
|
||||
|
||||
use std::num::NonZeroU16;
|
||||
use std::ops::Range;
|
||||
|
||||
use ratatui::buffer::Buffer;
|
||||
@@ -567,7 +568,9 @@ pub(crate) fn mark_buffer_hyperlinks(
|
||||
format!("\x1b]8;;{destination}\x07{}\x1b]8;;\x07", cell.symbol())
|
||||
},
|
||||
);
|
||||
cell.set_symbol(&symbol);
|
||||
let width = NonZeroU16::new(cell.cell_width()).unwrap_or(NonZeroU16::MIN);
|
||||
cell.set_symbol(&symbol)
|
||||
.set_diff_option(CellDiffOption::ForcedWidth(width));
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -602,8 +605,10 @@ fn mark_matching_cells(
|
||||
&& !cell.symbol().trim().is_empty()
|
||||
&& matches(cell)
|
||||
{
|
||||
let width = NonZeroU16::new(cell.cell_width()).unwrap_or(NonZeroU16::MIN);
|
||||
let symbol = osc8_hyperlink(destination, cell.symbol());
|
||||
cell.set_symbol(&symbol);
|
||||
cell.set_symbol(&symbol)
|
||||
.set_diff_option(CellDiffOption::ForcedWidth(width));
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -612,6 +617,7 @@ fn mark_matching_cells(
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use ratatui::style::Style;
|
||||
|
||||
#[test]
|
||||
fn only_web_destinations_receive_osc8() {
|
||||
@@ -822,6 +828,116 @@ mod tests {
|
||||
assert_eq!(linked_text, "パlink");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn forced_width_hyperlinks_render_wide_and_halfwidth_cells_snapshot() {
|
||||
let destination = "https://example.com/rendered";
|
||||
let mut line = HyperlinkLine::new(Line::from("prefix "));
|
||||
line.push_span("漢字 ガ".into(), Some(destination));
|
||||
line.push_span(" tail".into(), /*destination*/ None);
|
||||
|
||||
let area = Rect::new(
|
||||
/*x*/ 0, /*y*/ 0, /*width*/ 14, /*height*/ 3,
|
||||
);
|
||||
let backend = crate::test_backend::VT100Backend::new(area.width, area.height);
|
||||
let mut terminal =
|
||||
crate::custom_terminal::Terminal::with_options(backend).expect("terminal");
|
||||
terminal.set_viewport_area(area);
|
||||
|
||||
terminal
|
||||
.draw(|frame| {
|
||||
Paragraph::new(Text::from(line.line.clone()))
|
||||
.wrap(Wrap { trim: false })
|
||||
.render(area, frame.buffer_mut());
|
||||
mark_buffer_hyperlinks(
|
||||
frame.buffer_mut(),
|
||||
area,
|
||||
&[line.clone()],
|
||||
/*scroll_rows*/ 0,
|
||||
);
|
||||
})
|
||||
.expect("render hyperlinks");
|
||||
|
||||
insta::assert_snapshot!(
|
||||
"forced_width_hyperlinks_render_wide_and_halfwidth_cells",
|
||||
terminal.backend()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn buffer_hyperlinks_preserve_visible_cell_width_for_ratatui_diff() {
|
||||
let destination = "https://example.com/dakuten";
|
||||
let mut line = HyperlinkLine::new(Line::from("ガ tail"));
|
||||
line.hyperlinks.push(TerminalHyperlink::web(
|
||||
/*columns*/ 0..2,
|
||||
destination.to_string(),
|
||||
));
|
||||
let area = Rect::new(
|
||||
/*x*/ 0, /*y*/ 0, /*width*/ 7, /*height*/ 1,
|
||||
);
|
||||
let previous = Buffer::with_lines([" "]);
|
||||
let mut next = Buffer::empty(area);
|
||||
|
||||
Paragraph::new(Text::from(line.line.clone())).render(area, &mut next);
|
||||
mark_buffer_hyperlinks(&mut next, area, &[line], /*scroll_rows*/ 0);
|
||||
|
||||
assert_eq!(next[(0, 0)].cell_width(), 2);
|
||||
assert!(matches!(
|
||||
next[(0, 0)].diff_option,
|
||||
CellDiffOption::ForcedWidth(width) if width.get() == 2
|
||||
));
|
||||
assert_eq!(
|
||||
previous
|
||||
.diff_iter(&next)
|
||||
.map(|(x, _, cell)| (x, strip_osc8(cell.symbol())))
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
(0, "ガ".to_string()),
|
||||
(3, "t".to_string()),
|
||||
(4, "a".to_string()),
|
||||
(5, "i".to_string()),
|
||||
(6, "l".to_string()),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn matching_hyperlinks_preserve_visible_cell_width_for_ratatui_diff() {
|
||||
let destination = "https://example.com/dakuten";
|
||||
let area = Rect::new(
|
||||
/*x*/ 0, /*y*/ 0, /*width*/ 7, /*height*/ 1,
|
||||
);
|
||||
let previous = Buffer::with_lines([" "]);
|
||||
let mut next = Buffer::empty(area);
|
||||
next.set_string(
|
||||
/*x*/ 0,
|
||||
/*y*/ 0,
|
||||
"ガ tail",
|
||||
Style::default().add_modifier(Modifier::UNDERLINED),
|
||||
);
|
||||
|
||||
mark_underlined_hyperlink(&mut next, area, destination);
|
||||
|
||||
assert_eq!(next[(0, 0)].cell_width(), 2);
|
||||
assert!(matches!(
|
||||
next[(0, 0)].diff_option,
|
||||
CellDiffOption::ForcedWidth(width) if width.get() == 2
|
||||
));
|
||||
assert_eq!(
|
||||
previous
|
||||
.diff_iter(&next)
|
||||
.map(|(x, _, cell)| (x, strip_osc8(cell.symbol())))
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
(0, "ガ".to_string()),
|
||||
(2, " ".to_string()),
|
||||
(3, "t".to_string()),
|
||||
(4, "a".to_string()),
|
||||
(5, "i".to_string()),
|
||||
(6, "l".to_string()),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn trusted_file_destination_receives_osc8_without_enabling_plain_file_links() {
|
||||
let temp_dir = tempfile::tempdir().expect("temp directory");
|
||||
|
||||
Reference in New Issue
Block a user