From 32ff9e89ed06e6bbb5bb580f363de361a6ce8edf Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sun, 8 Feb 2026 23:38:52 -0300 Subject: [PATCH] fix(tui): fix fallback phantom line and diff wrap at span boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes: 1. highlight.rs: The fallback path used split('\n') which produces a phantom empty trailing element for inputs ending in '\n' (as pulldown-cmark emits for code blocks). Switch to lines() which handles trailing newlines correctly. 2. diff_render.rs: wrap_styled_spans only flushed the current line when col >= max_cols AND the current span had remaining content. This missed the case where one span ends exactly at max_cols and the next span has content — the next span's first character was appended to the already-full line. Remove the !remaining.is_empty() guard so the line flushes at span boundaries too. --- codex-rs/tui/src/diff_render.rs | 32 +++++++++++++++++++++++++++- codex-rs/tui/src/render/highlight.rs | 25 +++++++++++++++++++--- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/codex-rs/tui/src/diff_render.rs b/codex-rs/tui/src/diff_render.rs index d3c0d6b50e..937e42e394 100644 --- a/codex-rs/tui/src/diff_render.rs +++ b/codex-rs/tui/src/diff_render.rs @@ -609,7 +609,9 @@ fn wrap_styled_spans(spans: &[RtSpan<'static>], max_cols: usize) -> Vec= max_cols && !remaining.is_empty() { + // Do not gate on !remaining.is_empty() — the next span in the + // outer loop may still have content that must start on a fresh line. + if col >= max_cols { result.push(std::mem::take(&mut current_line)); col = 0; } @@ -997,6 +999,34 @@ mod tests { ); } + #[test] + fn wrap_styled_spans_flushes_at_span_boundary() { + // When span A fills exactly to max_cols and span B follows, the line + // must be flushed before B starts. Otherwise B's first character lands + // on an already-full line, producing over-width output. + let style_a = Style::default().fg(Color::Red); + let style_b = Style::default().fg(Color::Blue); + let spans = vec![ + RtSpan::styled("aaaa", style_a), // 4 cols, fills line exactly at max_cols=4 + RtSpan::styled("bb", style_b), // should start on a new line + ]; + let result = wrap_styled_spans(&spans, 4); + assert_eq!( + result.len(), + 2, + "span ending exactly at max_cols should flush before next span: {result:?}" + ); + // First line should only contain the 'a' span. + let first_width: usize = result[0] + .iter() + .map(|s| s.content.chars().count()) + .sum(); + assert!( + first_width <= 4, + "first line should be at most 4 cols wide, got {first_width}" + ); + } + #[test] fn wrap_styled_spans_preserves_styles() { // Verify that styles survive split boundaries. diff --git a/codex-rs/tui/src/render/highlight.rs b/codex-rs/tui/src/render/highlight.rs index 039f56a5ef..22f6d73018 100644 --- a/codex-rs/tui/src/render/highlight.rs +++ b/codex-rs/tui/src/render/highlight.rs @@ -219,9 +219,13 @@ pub(crate) fn highlight_code_to_lines(code: &str, lang: &str) -> Vec> = code.lines().map(|l| Line::from(l.to_string())).collect(); + if result.is_empty() { + result.push(Line::from(String::new())); + } + result } } @@ -291,6 +295,21 @@ mod tests { } } + #[test] + fn fallback_trailing_newline_no_phantom_line() { + // pulldown-cmark sends code block text ending with '\n'. + // The fallback path (unknown language) must not produce a phantom + // empty trailing line from that newline. + let code = "hello world\n"; + let lines = highlight_code_to_lines(code, "xyzlang"); + assert_eq!( + lines.len(), + 1, + "trailing newline should not produce phantom blank line, got {lines:?}" + ); + assert_eq!(reconstructed(&lines), "hello world"); + } + #[test] fn highlight_empty_string() { let lines = highlight_code_to_lines("", "rust");