mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Fix TUI layout for halfwidth Japanese sound marks (#35962)
## What changed - Use Ratatui-compatible terminal widths throughout TUI layout, preserving full-width counters for long lines. - Wrap and truncate on grapheme boundaries so halfwidth kana sound marks and emoji sequences stay intact. - Apply the corrected sizing to text input, selection popups, status and history cells, Markdown, hyperlinks, and scrollable cloud diffs. ## Testing - Add focused unit and snapshot coverage for wrapping, truncation, cursor and mask alignment, popup columns, status output, session headers, Markdown tables, process output, and scrollable diffs. GitOrigin-RevId: 294ec0bcff743c6bb16d55ec26308524ce741879
This commit is contained in:
@@ -37,7 +37,9 @@ tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
|
||||
tokio-stream = { workspace = true }
|
||||
tracing = { workspace = true, features = ["log"] }
|
||||
tracing-subscriber = { workspace = true, features = ["env-filter"] }
|
||||
unicode-segmentation = { workspace = true }
|
||||
unicode-width = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
insta = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
use unicode_width::UnicodeWidthChar;
|
||||
use unicode_segmentation::UnicodeSegmentation;
|
||||
use unicode_width::UnicodeWidthStr;
|
||||
|
||||
/// Scroll position and geometry for a vertical scroll view.
|
||||
@@ -131,38 +131,57 @@ impl ScrollableDiff {
|
||||
let mut line = String::new();
|
||||
let mut line_cols = 0usize;
|
||||
let mut last_soft_idx: Option<usize> = None; // last whitespace or punctuation break
|
||||
for (_i, ch) in raw.char_indices() {
|
||||
if ch == '\n' {
|
||||
for grapheme in raw
|
||||
.graphemes(/*is_extended*/ true)
|
||||
.flat_map(|grapheme| grapheme.split_inclusive(char::is_whitespace))
|
||||
{
|
||||
if grapheme == "\n" {
|
||||
out.push(std::mem::take(&mut line));
|
||||
out_idx.push(raw_idx);
|
||||
line_cols = 0;
|
||||
last_soft_idx = None;
|
||||
continue;
|
||||
}
|
||||
let w = UnicodeWidthChar::width(ch).unwrap_or(0);
|
||||
if line_cols.saturating_add(w) > max_cols {
|
||||
let grapheme_width = display_width(grapheme);
|
||||
if line_cols.saturating_add(grapheme_width) > max_cols {
|
||||
if let Some(split) = last_soft_idx {
|
||||
let (prefix, rest) = line.split_at(split);
|
||||
out.push(prefix.trim_end().to_string());
|
||||
out_idx.push(raw_idx);
|
||||
let prefix = prefix.trim_end();
|
||||
if !prefix.is_empty() {
|
||||
out.push(prefix.to_string());
|
||||
out_idx.push(raw_idx);
|
||||
}
|
||||
line = rest.trim_start().to_string();
|
||||
last_soft_idx = None;
|
||||
// retry add current ch now that line may be shorter
|
||||
// Retry adding the current grapheme now that the line may be shorter.
|
||||
} else if !line.is_empty() {
|
||||
out.push(std::mem::take(&mut line));
|
||||
out_idx.push(raw_idx);
|
||||
}
|
||||
}
|
||||
if ch.is_whitespace()
|
||||
|| matches!(
|
||||
ch,
|
||||
',' | ';' | '.' | ':' | ')' | ']' | '}' | '|' | '/' | '?' | '!' | '-' | '_'
|
||||
)
|
||||
if grapheme.chars().all(char::is_whitespace)
|
||||
|| grapheme.chars().any(|ch| {
|
||||
matches!(
|
||||
ch,
|
||||
',' | ';'
|
||||
| '.'
|
||||
| ':'
|
||||
| ')'
|
||||
| ']'
|
||||
| '}'
|
||||
| '|'
|
||||
| '/'
|
||||
| '?'
|
||||
| '!'
|
||||
| '-'
|
||||
| '_'
|
||||
)
|
||||
})
|
||||
{
|
||||
last_soft_idx = Some(line.len());
|
||||
}
|
||||
line.push(ch);
|
||||
line_cols = UnicodeWidthStr::width(line.as_str());
|
||||
line.push_str(grapheme);
|
||||
line_cols = display_width(&line);
|
||||
}
|
||||
if !line.is_empty() {
|
||||
out.push(line);
|
||||
@@ -174,3 +193,16 @@ impl ScrollableDiff {
|
||||
self.state.content_h = self.wrapped.len() as u16;
|
||||
}
|
||||
}
|
||||
|
||||
/// Counts terminal cells, including the halfwidth sound marks omitted by `unicode-width`.
|
||||
fn display_width(text: &str) -> usize {
|
||||
UnicodeWidthStr::width(text)
|
||||
+ text
|
||||
.chars()
|
||||
.filter(|ch| matches!(ch, '\u{FF9E}' | '\u{FF9F}'))
|
||||
.count()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "scrollable_diff_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
use super::ScrollableDiff;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn wraps_halfwidth_sound_marks_using_ratatui_width() {
|
||||
let mut diff = ScrollableDiff::new();
|
||||
diff.set_content(vec![
|
||||
"aガbパc".to_string(),
|
||||
"a ゙b".to_string(),
|
||||
"a ゚b".to_string(),
|
||||
]);
|
||||
diff.set_width(/*width*/ 2);
|
||||
|
||||
insta::assert_snapshot!(
|
||||
diff.wrapped_lines().join("\n"),
|
||||
@r"
|
||||
a
|
||||
ガ
|
||||
b
|
||||
パ
|
||||
c
|
||||
a
|
||||
゙b
|
||||
a
|
||||
゚b
|
||||
"
|
||||
);
|
||||
assert_eq!(diff.wrapped_src_indices(), [0, 0, 0, 0, 0, 1, 1, 2, 2]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn does_not_emit_empty_rows_after_full_width_sound_mark_graphemes() {
|
||||
for grapheme in ["ガ", "パ"] {
|
||||
let mut diff = ScrollableDiff::new();
|
||||
diff.set_content(vec![format!("a {grapheme} tail")]);
|
||||
diff.set_width(/*width*/ 2);
|
||||
|
||||
let expected = ["a", grapheme, "ta", "il"];
|
||||
let actual = diff
|
||||
.wrapped_lines()
|
||||
.iter()
|
||||
.map(String::as_str)
|
||||
.collect::<Vec<_>>();
|
||||
assert_eq!(actual, expected);
|
||||
assert_eq!(diff.wrapped_src_indices(), [0, 0, 0, 0]);
|
||||
assert_eq!(diff.state.content_h, 4);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user