From 0a26418e12f0a9b3bd6ef25c80b34248f48cba0a Mon Sep 17 00:00:00 2001 From: CodeWhale Bot Date: Tue, 1 Sep 2026 18:16:19 -0700 Subject: [PATCH 1/4] feat(tui): diff cards emphasise the changed words within a line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A deleted run followed by an added run of the same length is a set of replaced lines. Each pair is now diffed at unicode word boundaries (similar's `from_unicode_words`, so punctuation stays out of the emphasis) and the words that differ are painted bold + reversed on the existing −rose / +green line; unchanged words keep the plain line style. Any other shape — unequal runs, or a pair sharing less than half its words — renders exactly as before, so a rewrite does not light up the whole row. Emphasis flags are carried across the wrapper's breaks, and the row text is unchanged. `similar` gains its `unicode` feature; unicode-segmentation was already in the tree (ratatui), so no new crate. syntect is left out: colouring hunk lines by file extension is not a small, obviously-safe addition to this renderer today. Evidence: `scripts/dev-test.sh tui diff_render` — test result: ok. 9 passed; 0 failed; 0 ignored. `scripts/dev-test.sh tui` — 11854 tests run: 11854 passed, 13 skipped (no golden changed). clippy -D warnings with CI's allow flags: clean. Signed-off-by: CodeWhale Bot --- Cargo.lock | 2 + crates/tui/Cargo.toml | 2 +- crates/tui/src/tui/diff_render.rs | 315 +++++++++++++++++++++++++++--- 3 files changed, 289 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9d8487934..b0f7aab39 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -538,6 +538,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1f7dc094d718f2e1c1559ad110e27eeaae14a5465d3d56dd6dbd793079fbd530" dependencies = [ "memchr", + "regex-automata", "serde_core", ] @@ -5131,6 +5132,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4f66ca1f7aca2474dc10c942eb22feffc897735f54cd1db90138c2fddb490987" dependencies = [ "bstr", + "unicode-segmentation", ] [[package]] diff --git a/crates/tui/Cargo.toml b/crates/tui/Cargo.toml index 65027b368..48ea75282 100644 --- a/crates/tui/Cargo.toml +++ b/crates/tui/Cargo.toml @@ -70,7 +70,7 @@ rusqlite.workspace = true rmcp = { version = "2.2.0", default-features = false, features = ["auth", "client"] } rustls.workspace = true qrcode = { version = "0.14", default-features = false } -similar = "3" +similar = { version = "3", features = ["unicode"] } syntect = { version = "5.2", default-features = false, features = ["default-fancy"] } serde.workspace = true serde_json = { workspace = true, features = ["preserve_order"] } diff --git a/crates/tui/src/tui/diff_render.rs b/crates/tui/src/tui/diff_render.rs index 49c25343e..d1b6a40a5 100644 --- a/crates/tui/src/tui/diff_render.rs +++ b/crates/tui/src/tui/diff_render.rs @@ -2,12 +2,20 @@ use ratatui::style::{Modifier, Style}; use ratatui::text::{Line, Span}; +use similar::{ChangeTag, TextDiff}; use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; use crate::palette; const LINE_NUMBER_WIDTH: usize = 4; +/// Below this word-level similarity a replaced line pair is rewritten, not +/// edited, and emphasising the changed words would light up the whole row. +const INTRALINE_MIN_RATIO: f32 = 0.5; + +/// A run of text inside a changed line and whether it is part of the change. +type Segment = (String, bool); + #[derive(Debug, Clone, PartialEq, Eq)] pub struct DiffFileSummary { pub path: String, @@ -57,7 +65,8 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun let mut old_line: Option = None; let mut new_line: Option = None; - for raw in diff.lines() { + let mut lines = diff.lines().peekable(); + while let Some(raw) = lines.next() { if raw.starts_with("diff --git") || raw.starts_with("index ") { collector.extend(render_header_line(raw, width)); continue; @@ -77,7 +86,7 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun continue; } - if raw.starts_with('+') && !raw.starts_with("+++") { + if is_added(raw) { let content = raw.trim_start_matches('+'); collector.extend(render_diff_line( content, @@ -85,9 +94,8 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun old_line, new_line, '+', - Style::default() - .fg(palette::DIFF_ADDED) - .bg(palette::DIFF_ADDED_BG), + added_style(), + None, )); if let Some(line) = new_line.as_mut() { *line = line.saturating_add(1); @@ -95,20 +103,61 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun continue; } - if raw.starts_with('-') && !raw.starts_with("---") { - let content = raw.trim_start_matches('-'); - collector.extend(render_diff_line( - content, - width, - old_line, - new_line, - '-', - Style::default() - .fg(palette::STATUS_ERROR) - .bg(palette::DIFF_DELETED_BG), - )); - if let Some(line) = old_line.as_mut() { - *line = line.saturating_add(1); + if is_deleted(raw) { + // A deleted run followed by an added run of the same length is a + // set of replaced lines: emphasise the words that changed within + // each pair. Any other shape renders line by line as before. + let mut removed = vec![raw.trim_start_matches('-')]; + while let Some(next) = lines.next_if(|next| is_deleted(next)) { + removed.push(next.trim_start_matches('-')); + } + let mut added = Vec::new(); + while let Some(next) = lines.next_if(|next| is_added(next)) { + added.push(next.trim_start_matches('+')); + } + let pairs: Vec, Vec)>> = if removed.len() == added.len() { + removed + .iter() + .zip(&added) + .map(|(old, new)| intraline_segments(old, new)) + .collect() + } else { + Vec::new() + }; + + for (idx, content) in removed.iter().enumerate() { + let emphasis = pairs + .get(idx) + .and_then(|pair| pair.as_ref().map(|(old, _)| old.as_slice())); + collector.extend(render_diff_line( + content, + width, + old_line, + new_line, + '-', + deleted_style(), + emphasis, + )); + if let Some(line) = old_line.as_mut() { + *line = line.saturating_add(1); + } + } + for (idx, content) in added.iter().enumerate() { + let emphasis = pairs + .get(idx) + .and_then(|pair| pair.as_ref().map(|(_, new)| new.as_slice())); + collector.extend(render_diff_line( + content, + width, + old_line, + new_line, + '+', + added_style(), + emphasis, + )); + if let Some(line) = new_line.as_mut() { + *line = line.saturating_add(1); + } } continue; } @@ -122,6 +171,7 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun new_line, ' ', Style::default().fg(palette::TEXT_PRIMARY), + None, )); if let Some(line) = old_line.as_mut() { *line = line.saturating_add(1); @@ -338,6 +388,102 @@ fn render_hunk_header(line: &str, width: u16) -> Vec> { wrap_with_style(line, style, width) } +fn is_added(raw: &str) -> bool { + raw.starts_with('+') && !raw.starts_with("+++") +} + +fn is_deleted(raw: &str) -> bool { + raw.starts_with('-') && !raw.starts_with("---") +} + +fn added_style() -> Style { + Style::default() + .fg(palette::DIFF_ADDED) + .bg(palette::DIFF_ADDED_BG) +} + +fn deleted_style() -> Style { + Style::default() + .fg(palette::STATUS_ERROR) + .bg(palette::DIFF_DELETED_BG) +} + +/// Split a replaced line pair into word runs (unicode word boundaries, so +/// punctuation stays out of the emphasis), flagging the runs that differ. +/// +/// Returns `None` when the pair shares too little to read as an edit, so the +/// caller paints the whole line the way it always has. +fn intraline_segments(old: &str, new: &str) -> Option<(Vec, Vec)> { + let diff = TextDiff::from_unicode_words(old, new); + if diff.ratio() < INTRALINE_MIN_RATIO { + return None; + } + let mut old_segments: Vec = Vec::new(); + let mut new_segments: Vec = Vec::new(); + let mut changed = false; + for change in diff.iter_all_changes() { + let text = change.value(); + match change.tag() { + ChangeTag::Equal => { + push_segment(&mut old_segments, text, false); + push_segment(&mut new_segments, text, false); + } + ChangeTag::Delete => { + changed = true; + push_segment(&mut old_segments, text, true); + } + ChangeTag::Insert => { + changed = true; + push_segment(&mut new_segments, text, true); + } + } + } + changed.then_some((old_segments, new_segments)) +} + +fn push_segment(segments: &mut Vec, text: &str, emphasised: bool) { + match segments.last_mut() { + Some((run, flag)) if *flag == emphasised => run.push_str(text), + _ => segments.push((text.to_string(), emphasised)), + } +} + +/// Paint one wrapped chunk of a changed line, carrying the emphasis flags +/// across the wrap. `wrap_text` only drops or collapses whitespace, so every +/// chunk character is matched forward against the source characters. +fn emphasised_spans( + chunk: &str, + style: Style, + source: &[(char, bool)], + cursor: &mut usize, +) -> Vec> { + let emphasis = style.add_modifier(Modifier::BOLD | Modifier::REVERSED); + let mut spans = Vec::new(); + let mut run = String::new(); + let mut run_flag = false; + for ch in chunk.chars() { + while *cursor < source.len() + && !(source[*cursor].0 == ch + || (source[*cursor].0.is_whitespace() && ch.is_whitespace())) + { + *cursor += 1; + } + let flag = source.get(*cursor).map(|(_, flag)| *flag).unwrap_or(false); + *cursor = cursor.saturating_add(1).min(source.len()); + if flag != run_flag && !run.is_empty() { + let painted = if run_flag { emphasis } else { style }; + spans.push(Span::styled(std::mem::take(&mut run), painted)); + } + run_flag = flag; + run.push(ch); + } + if !run.is_empty() { + let painted = if run_flag { emphasis } else { style }; + spans.push(Span::styled(run, painted)); + } + spans +} + fn render_diff_line( content: &str, width: u16, @@ -345,25 +491,36 @@ fn render_diff_line( new_line: Option, marker: char, style: Style, + emphasis: Option<&[Segment]>, ) -> Vec> { let prefix = format_line_numbers(old_line, new_line, marker); let prefix_width = prefix.width(); let available = width.saturating_sub(prefix_width as u16).max(1) as usize; let wrapped = wrap_text(content, available); + let source: Vec<(char, bool)> = emphasis + .map(|segments| { + segments + .iter() + .flat_map(|(run, flag)| run.chars().map(move |ch| (ch, *flag))) + .collect() + }) + .unwrap_or_default(); + let mut cursor = 0usize; let mut out = Vec::new(); for (idx, chunk) in wrapped.into_iter().enumerate() { - if idx == 0 { - out.push(Line::from(vec![ - Span::styled(prefix.clone(), Style::default().fg(palette::TEXT_MUTED)), - Span::styled(chunk, style), - ])); + let gutter = if idx == 0 { + Span::styled(prefix.clone(), Style::default().fg(palette::TEXT_MUTED)) } else { - out.push(Line::from(vec![ - Span::raw(" ".repeat(prefix_width)), - Span::styled(chunk, style), - ])); + Span::raw(" ".repeat(prefix_width)) + }; + let mut spans = vec![gutter]; + if emphasis.is_some() { + spans.extend(emphasised_spans(&chunk, style, &source, &mut cursor)); + } else { + spans.push(Span::styled(chunk, style)); } + out.push(Line::from(spans)); } if out.is_empty() { @@ -491,7 +648,107 @@ mod tests { } fn diff_content_text(line: &Line<'static>) -> Option { - line.spans.get(1).map(|span| span.content.to_string()) + line.spans + .get(1..) + .filter(|rest| !rest.is_empty()) + .map(|rest| rest.iter().map(|span| span.content.as_ref()).collect()) + } + + fn emphasised_text(line: &Line<'static>) -> String { + line.spans + .iter() + .filter(|span| span.style.add_modifier.contains(Modifier::REVERSED)) + .map(|span| span.content.as_ref()) + .collect() + } + + fn rendered_body(diff: &str, width: u16) -> Vec> { + render_diff_body_bounded(diff, width, usize::MAX).lines + } + + #[test] + fn replaced_line_pair_emphasises_only_the_changed_words() { + let diff = "\ +@@ -1,1 +1,1 @@ +- let total = price * quantity; ++ let total = price * count; +"; + let rendered = rendered_body(diff, 80); + let emphasised = rendered + .iter() + .map(emphasised_text) + .filter(|text| !text.is_empty()) + .collect::>(); + assert_eq!( + emphasised, + vec!["quantity".to_string(), "count".to_string()] + ); + let content = rendered + .iter() + .filter_map(diff_content_text) + .collect::>(); + assert_eq!( + content, + vec![ + " let total = price * quantity;".to_string(), + " let total = price * count;".to_string() + ], + "emphasis must not alter the line text" + ); + } + + #[test] + fn unequal_runs_and_rewrites_render_without_emphasis() { + let unequal = "\ +@@ -1,2 +1,1 @@ +-let a = 1; +-let b = 2; ++let a = 1; let b = 2; +"; + assert!( + rendered_body(unequal, 80) + .iter() + .all(|line| emphasised_text(line).is_empty()), + "two deletions against one insertion are not a replaced pair" + ); + + let rewrite = "\ +@@ -1,1 +1,1 @@ +-fn render(width: u16) -> Vec ++return None; +"; + assert!( + rendered_body(rewrite, 80) + .iter() + .all(|line| emphasised_text(line).is_empty()), + "a line sharing almost nothing with its replacement is painted whole" + ); + } + + #[test] + fn emphasis_survives_wrapping_without_changing_text() { + let diff = "\ +@@ -1,1 +1,1 @@ +-alpha beta gamma delta epsilon zeta eta theta iota kappa ++alpha beta gamma delta epsilon zeta eta THETA iota kappa +"; + let rendered = rendered_body(diff, 30); + assert!(rendered.len() > 2, "narrow width should wrap: {rendered:?}"); + let joined: String = rendered.iter().map(emphasised_text).collect(); + assert_eq!(joined, "thetaTHETA"); + // Wrapping drops the space at each break; everything else survives. + let body: String = rendered + .iter() + .skip(1) + .filter_map(diff_content_text) + .collect::() + .split_whitespace() + .collect(); + assert_eq!( + body, + "alphabetagammadeltaepsilonzetaetathetaiotakappa\ + alphabetagammadeltaepsilonzetaetaTHETAiotakappa" + ); } #[test] From a708452d6c25ada3631e5207145d8feb41bedafd Mon Sep 17 00:00:00 2001 From: CodeWhale Bot Date: Tue, 1 Sep 2026 18:51:08 -0700 Subject: [PATCH 2/4] fix(tui): intraline emphasis maps by word characters and counts words only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on #5813: - Emphasis desynced on wrapped, indented lines: `wrap_text` re-inserts the indent lead on every continuation chunk and the greedy forward match drifted past it. Chunks now map each non-whitespace character back to the source by position (the invariant the wrapper keeps); whitespace joins an emphasised run only when both neighbours are in it. A `debug_assert!` pins the order, and any mismatch falls back to whole-line styling — never a wrong-word emphasis. - The similarity threshold counted whitespace and punctuation tokens, so lines agreeing only on syntax passed. The ratio now counts word tokens only (tokens bearing an alphanumeric character). - `\ No newline at end of file` markers are lifted out of a `-`/`+` run and painted after it, so files without a trailing newline still pair. - The buffered run is capped at `INTRALINE_MAX_RUN` (64) replaced pairs and the `render_diff_body_bounded` doc states the bound honestly: the retained preview, one wrapped source line, and the current run's borrowed slices plus word segments for at most 64 pairs. - Tests: multiple pairs in one run; a rewrite next to an edit in one run; syntax-only sharing stays plain; unicode word boundaries; the no-newline marker; an indented line whose change lands on the third wrapped chunk, asserting the exact text before the emphasis; and the wrapping test now asserts emphasis position per chunk. Evidence: `scripts/dev-test.sh tui diff_render` — test result: ok. 15 passed; 0 failed; 0 ignored. `scripts/dev-test.sh tui` — 11860 tests run: 11859 passed, 1 failed (chatgpt_oauth::tests::callback_is_accepted_on_either_loopback_family, untouched by this diff; passes alone: 1 passed; 0 failed — loopback port contention under six concurrent builds). clippy -D warnings with CI's allow flags: clean. Signed-off-by: CodeWhale Bot --- crates/tui/src/tui/diff_render.rs | 330 ++++++++++++++++++++++-------- 1 file changed, 239 insertions(+), 91 deletions(-) diff --git a/crates/tui/src/tui/diff_render.rs b/crates/tui/src/tui/diff_render.rs index d1b6a40a5..5bb43d858 100644 --- a/crates/tui/src/tui/diff_render.rs +++ b/crates/tui/src/tui/diff_render.rs @@ -13,6 +13,10 @@ const LINE_NUMBER_WIDTH: usize = 4; /// edited, and emphasising the changed words would light up the whole row. const INTRALINE_MIN_RATIO: f32 = 0.5; +/// Pairing stops here so a huge hunk cannot buffer word segments for every +/// replaced line; longer runs render line by line. +const INTRALINE_MAX_RUN: usize = 64; + /// A run of text inside a changed line and whether it is part of the change. type Segment = (String, bool); @@ -57,8 +61,11 @@ pub fn render_diff_bounded(diff: &str, width: u16, max_body_rows: usize) -> Boun /// this form so the bounded preview budget is spent on the actual red/green /// evidence instead of a second, generic summary. /// Render at most `max_rows` body rows while counting every omitted wrapped -/// row. Allocation is bounded by the retained preview plus one source line's -/// wrapped representation, rather than by the size of the complete diff. +/// row. Allocation is bounded by the retained preview, one source line's +/// wrapped representation, and the current `-`/`+` run: its line slices +/// (borrowed, pointer-sized) plus word segments for at most +/// `INTRALINE_MAX_RUN` replaced pairs — never by the size of the complete +/// diff. #[must_use] pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> BoundedDiffRender { let mut collector = BoundedLineCollector::new(max_rows); @@ -107,23 +114,35 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun // A deleted run followed by an added run of the same length is a // set of replaced lines: emphasise the words that changed within // each pair. Any other shape renders line by line as before. + // `\ No newline at end of file` markers are lifted out of the run + // (and painted after it) so files without a trailing newline + // still pair. let mut removed = vec![raw.trim_start_matches('-')]; - while let Some(next) = lines.next_if(|next| is_deleted(next)) { - removed.push(next.trim_start_matches('-')); + let mut added: Vec<&str> = Vec::new(); + let mut markers: Vec<&str> = Vec::new(); + loop { + if let Some(marker) = lines.next_if(|next| is_no_newline_marker(next)) { + markers.push(marker); + } else if added.is_empty() + && let Some(next) = lines.next_if(|next| is_deleted(next)) + { + removed.push(next.trim_start_matches('-')); + } else if let Some(next) = lines.next_if(|next| is_added(next)) { + added.push(next.trim_start_matches('+')); + } else { + break; + } } - let mut added = Vec::new(); - while let Some(next) = lines.next_if(|next| is_added(next)) { - added.push(next.trim_start_matches('+')); - } - let pairs: Vec, Vec)>> = if removed.len() == added.len() { - removed - .iter() - .zip(&added) - .map(|(old, new)| intraline_segments(old, new)) - .collect() - } else { - Vec::new() - }; + let pairs: Vec, Vec)>> = + if removed.len() == added.len() && removed.len() <= INTRALINE_MAX_RUN { + removed + .iter() + .zip(&added) + .map(|(old, new)| intraline_segments(old, new)) + .collect() + } else { + Vec::new() + }; for (idx, content) in removed.iter().enumerate() { let emphasis = pairs @@ -159,6 +178,9 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun *line = line.saturating_add(1); } } + for marker in markers { + collector.extend(render_header_line(marker, width)); + } continue; } @@ -396,6 +418,10 @@ fn is_deleted(raw: &str) -> bool { raw.starts_with('-') && !raw.starts_with("---") } +fn is_no_newline_marker(raw: &str) -> bool { + raw.starts_with("\\ No newline") +} + fn added_style() -> Style { Style::default() .fg(palette::DIFF_ADDED) @@ -411,34 +437,45 @@ fn deleted_style() -> Style { /// Split a replaced line pair into word runs (unicode word boundaries, so /// punctuation stays out of the emphasis), flagging the runs that differ. /// -/// Returns `None` when the pair shares too little to read as an edit, so the -/// caller paints the whole line the way it always has. +/// Returns `None` when the pair shares too few *words* to read as an edit — +/// whitespace and punctuation tokens do not count, so two lines that agree +/// only on syntax are painted whole, the way they always were. fn intraline_segments(old: &str, new: &str) -> Option<(Vec, Vec)> { let diff = TextDiff::from_unicode_words(old, new); - if diff.ratio() < INTRALINE_MIN_RATIO { - return None; - } let mut old_segments: Vec = Vec::new(); let mut new_segments: Vec = Vec::new(); let mut changed = false; + let mut shared_words = 0usize; + let mut total_words = 0usize; for change in diff.iter_all_changes() { let text = change.value(); + let is_word = text.chars().any(char::is_alphanumeric); match change.tag() { ChangeTag::Equal => { + if is_word { + shared_words += 2; + total_words += 2; + } push_segment(&mut old_segments, text, false); push_segment(&mut new_segments, text, false); } ChangeTag::Delete => { changed = true; + total_words += usize::from(is_word); push_segment(&mut old_segments, text, true); } ChangeTag::Insert => { changed = true; + total_words += usize::from(is_word); push_segment(&mut new_segments, text, true); } } } - changed.then_some((old_segments, new_segments)) + if !changed || total_words == 0 { + return None; + } + let ratio = shared_words as f32 / total_words as f32; + (ratio >= INTRALINE_MIN_RATIO).then_some((old_segments, new_segments)) } fn push_segment(segments: &mut Vec, text: &str, emphasised: bool) { @@ -448,40 +485,69 @@ fn push_segment(segments: &mut Vec, text: &str, emphasised: bool) { } } -/// Paint one wrapped chunk of a changed line, carrying the emphasis flags -/// across the wrap. `wrap_text` only drops or collapses whitespace, so every -/// chunk character is matched forward against the source characters. -fn emphasised_spans( - chunk: &str, +/// Paint the wrapped chunks of a changed line, mapping every non-whitespace +/// character back to its source flag by position. `wrap_text` only drops, +/// collapses, or re-inserts whitespace (the indent lead comes back on every +/// continuation chunk), so the non-whitespace sequence is the invariant. +/// Whitespace joins an emphasised run only when both its neighbours are in +/// it. Returns `None` if the invariant ever fails, and the caller paints the +/// whole line plainly rather than emphasising the wrong word. +fn emphasised_chunks( + chunks: &[String], style: Style, - source: &[(char, bool)], - cursor: &mut usize, -) -> Vec> { + segments: &[Segment], +) -> Option>>> { + let source: Vec<(char, bool)> = segments + .iter() + .flat_map(|(run, flag)| { + run.chars() + .filter(|ch| !ch.is_whitespace()) + .map(move |ch| (ch, *flag)) + }) + .collect(); let emphasis = style.add_modifier(Modifier::BOLD | Modifier::REVERSED); - let mut spans = Vec::new(); - let mut run = String::new(); - let mut run_flag = false; - for ch in chunk.chars() { - while *cursor < source.len() - && !(source[*cursor].0 == ch - || (source[*cursor].0.is_whitespace() && ch.is_whitespace())) - { - *cursor += 1; + let mut cursor = 0usize; + let mut out = Vec::with_capacity(chunks.len()); + for chunk in chunks { + let mut spans = Vec::new(); + let mut run = String::new(); + let mut run_flag = false; + let mut prev_flag = false; + for ch in chunk.chars() { + let flag = if ch.is_whitespace() { + prev_flag && source.get(cursor).is_some_and(|(_, next)| *next) + } else { + let (expected, flag) = *source.get(cursor)?; + debug_assert_eq!( + expected, ch, + "wrapped chunk diverged from its source line at {cursor}" + ); + if expected != ch { + return None; + } + cursor += 1; + prev_flag = flag; + flag + }; + if flag != run_flag && !run.is_empty() { + let painted = if run_flag { emphasis } else { style }; + spans.push(Span::styled(std::mem::take(&mut run), painted)); + } + run_flag = flag; + run.push(ch); } - let flag = source.get(*cursor).map(|(_, flag)| *flag).unwrap_or(false); - *cursor = cursor.saturating_add(1).min(source.len()); - if flag != run_flag && !run.is_empty() { + if !run.is_empty() { let painted = if run_flag { emphasis } else { style }; - spans.push(Span::styled(std::mem::take(&mut run), painted)); + spans.push(Span::styled(run, painted)); } - run_flag = flag; - run.push(ch); + out.push(spans); } - if !run.is_empty() { - let painted = if run_flag { emphasis } else { style }; - spans.push(Span::styled(run, painted)); - } - spans + debug_assert_eq!( + cursor, + source.len(), + "wrapped chunks did not consume the whole source line" + ); + (cursor == source.len()).then_some(out) } fn render_diff_line( @@ -497,15 +563,7 @@ fn render_diff_line( let prefix_width = prefix.width(); let available = width.saturating_sub(prefix_width as u16).max(1) as usize; let wrapped = wrap_text(content, available); - let source: Vec<(char, bool)> = emphasis - .map(|segments| { - segments - .iter() - .flat_map(|(run, flag)| run.chars().map(move |ch| (ch, *flag))) - .collect() - }) - .unwrap_or_default(); - let mut cursor = 0usize; + let mut painted = emphasis.and_then(|segments| emphasised_chunks(&wrapped, style, segments)); let mut out = Vec::new(); for (idx, chunk) in wrapped.into_iter().enumerate() { @@ -515,10 +573,9 @@ fn render_diff_line( Span::raw(" ".repeat(prefix_width)) }; let mut spans = vec![gutter]; - if emphasis.is_some() { - spans.extend(emphasised_spans(&chunk, style, &source, &mut cursor)); - } else { - spans.push(Span::styled(chunk, style)); + match painted.as_mut() { + Some(rows) => spans.append(&mut rows[idx]), + None => spans.push(Span::styled(chunk, style)), } out.push(Line::from(spans)); } @@ -666,6 +723,25 @@ mod tests { render_diff_body_bounded(diff, width, usize::MAX).lines } + /// Text between the gutter and the first emphasised span, so a test can + /// pin where on the row the emphasis starts. + fn text_before_emphasis(line: &Line<'static>) -> String { + line.spans + .iter() + .skip(1) + .take_while(|span| !span.style.add_modifier.contains(Modifier::REVERSED)) + .map(|span| span.content.as_ref()) + .collect() + } + + fn emphasis_per_row(diff: &str, width: u16) -> Vec { + rendered_body(diff, width) + .iter() + .skip(1) // hunk header + .map(emphasised_text) + .collect() + } + #[test] fn replaced_line_pair_emphasises_only_the_changed_words() { let diff = "\ @@ -674,15 +750,7 @@ mod tests { + let total = price * count; "; let rendered = rendered_body(diff, 80); - let emphasised = rendered - .iter() - .map(emphasised_text) - .filter(|text| !text.is_empty()) - .collect::>(); - assert_eq!( - emphasised, - vec!["quantity".to_string(), "count".to_string()] - ); + assert_eq!(emphasis_per_row(diff, 80), vec!["quantity", "count"]); let content = rendered .iter() .filter_map(diff_content_text) @@ -698,30 +766,107 @@ mod tests { } #[test] - fn unequal_runs_and_rewrites_render_without_emphasis() { + fn every_pair_in_a_replaced_run_is_emphasised_in_order() { + let diff = "\ +@@ -1,2 +1,2 @@ +-let alpha = 1; +-let beta = 2; ++let alpha = 10; ++let beta = 20; +"; + assert_eq!(emphasis_per_row(diff, 80), vec!["1", "2", "10", "20"]); + } + + #[test] + fn a_rewrite_inside_a_replaced_run_stays_plain_while_its_neighbour_is_emphasised() { + let diff = "\ +@@ -1,2 +1,2 @@ +-let x = 1; +-fn old_name() {} ++let x = 2; ++return None; +"; + assert_eq!(emphasis_per_row(diff, 80), vec!["1", "", "2", ""]); + } + + #[test] + fn lines_sharing_only_syntax_are_not_emphasised() { + // Whitespace, `=` and `;` agree; every word differs but `let`. + let diff = "\ +@@ -1,1 +1,1 @@ +-let alpha = beta; ++let gamma = delta; +"; + assert_eq!(emphasis_per_row(diff, 80), vec!["", ""]); + } + + #[test] + fn unequal_runs_render_without_emphasis() { let unequal = "\ @@ -1,2 +1,1 @@ -let a = 1; -let b = 2; +let a = 1; let b = 2; "; - assert!( - rendered_body(unequal, 80) - .iter() - .all(|line| emphasised_text(line).is_empty()), - "two deletions against one insertion are not a replaced pair" - ); + assert_eq!(emphasis_per_row(unequal, 80), vec!["", "", ""]); + } - let rewrite = "\ + #[test] + fn emphasis_follows_unicode_word_boundaries() { + let diff = "\ @@ -1,1 +1,1 @@ --fn render(width: u16) -> Vec -+return None; +-café au lait, naïve ++café au thé, naïve "; - assert!( - rendered_body(rewrite, 80) - .iter() - .all(|line| emphasised_text(line).is_empty()), - "a line sharing almost nothing with its replacement is painted whole" + assert_eq!(emphasis_per_row(diff, 80), vec!["lait", "thé"]); + } + + #[test] + fn no_newline_marker_does_not_break_pairing() { + let diff = "\ +@@ -1,1 +1,1 @@ +-old line +\\ No newline at end of file ++new line +\\ No newline at end of file +"; + let rendered = rendered_body(diff, 80); + let rows: Vec = rendered.iter().map(emphasised_text).collect(); + assert_eq!(rows, vec!["", "old", "new", "", ""]); + let text: Vec = rendered.iter().map(line_text).collect(); + assert_eq!( + text.iter() + .filter(|row| row.contains("No newline at end of file")) + .count(), + 2, + "markers are still shown: {text:?}" + ); + } + + #[test] + fn emphasis_lands_on_the_right_chunk_of_a_wrapped_indented_line() { + // Gutter is 12 columns; width 40 leaves 28 for text. With the 8-space + // indent re-inserted on every continuation chunk, the line wraps as + // " let total =" / " compute(alpha, beta," / + // " gamma, quantity);" — the change sits on the third chunk. + let diff = "\ +@@ -1,1 +1,1 @@ +- let total = compute(alpha, beta, gamma, quantity); ++ let total = compute(alpha, beta, gamma, count); +"; + let rendered = rendered_body(diff, 40); + let rows: Vec = rendered.iter().map(emphasised_text).collect(); + assert_eq!(rows, vec!["", "", "", "quantity", "", "", "count"]); + assert_eq!( + diff_content_text(&rendered[3]).as_deref(), + Some(" gamma, quantity);") + ); + assert_eq!(text_before_emphasis(&rendered[3]), " gamma, "); + assert_eq!(text_before_emphasis(&rendered[6]), " gamma, "); + // Row text is untouched by the emphasis. + assert_eq!( + diff_content_text(&rendered[6]).as_deref(), + Some(" gamma, count);") ); } @@ -733,9 +878,12 @@ mod tests { +alpha beta gamma delta epsilon zeta eta THETA iota kappa "; let rendered = rendered_body(diff, 30); - assert!(rendered.len() > 2, "narrow width should wrap: {rendered:?}"); - let joined: String = rendered.iter().map(emphasised_text).collect(); - assert_eq!(joined, "thetaTHETA"); + let rows: Vec = rendered.iter().map(emphasised_text).collect(); + // 18 text columns: "alpha beta gamma" / "delta epsilon zeta" / + // "eta theta iota" / "kappa" — the change sits on the third chunk. + assert_eq!(rows, vec!["", "", "", "theta", "", "", "", "THETA", ""]); + assert_eq!(text_before_emphasis(&rendered[3]), "eta "); + assert_eq!(text_before_emphasis(&rendered[7]), "eta "); // Wrapping drops the space at each break; everything else survives. let body: String = rendered .iter() From 0a21f0a314d9a9dfb7b41a665142858186961dad Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 03:16:32 +0000 Subject: [PATCH 3/4] fix(tui): bound intraline diff buffering Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-Authored-By: Hunter Bown --- crates/tui/src/tui/diff_render.rs | 186 ++++++++++++++++++++++++++++-- 1 file changed, 175 insertions(+), 11 deletions(-) diff --git a/crates/tui/src/tui/diff_render.rs b/crates/tui/src/tui/diff_render.rs index 5bb43d858..e1bdd2b27 100644 --- a/crates/tui/src/tui/diff_render.rs +++ b/crates/tui/src/tui/diff_render.rs @@ -93,6 +93,11 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun continue; } + if is_no_newline_marker(raw) { + collector.extend(render_header_line(raw, width)); + continue; + } + if is_added(raw) { let content = raw.trim_start_matches('+'); collector.extend(render_diff_line( @@ -114,25 +119,121 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun // A deleted run followed by an added run of the same length is a // set of replaced lines: emphasise the words that changed within // each pair. Any other shape renders line by line as before. - // `\ No newline at end of file` markers are lifted out of the run - // (and painted after it) so files without a trailing newline - // still pair. let mut removed = vec![raw.trim_start_matches('-')]; let mut added: Vec<&str> = Vec::new(); - let mut markers: Vec<&str> = Vec::new(); + let mut removed_markers: Vec<(usize, &str)> = Vec::new(); + let mut added_markers: Vec<(usize, &str)> = Vec::new(); + let mut oversized = false; loop { if let Some(marker) = lines.next_if(|next| is_no_newline_marker(next)) { - markers.push(marker); + if added.is_empty() { + removed_markers.push((removed.len() - 1, marker)); + } else { + added_markers.push((added.len() - 1, marker)); + } } else if added.is_empty() && let Some(next) = lines.next_if(|next| is_deleted(next)) { - removed.push(next.trim_start_matches('-')); + if removed.len() < INTRALINE_MAX_RUN { + removed.push(next.trim_start_matches('-')); + } else { + for (idx, content) in removed.drain(..).enumerate() { + render_plain_diff_line( + &mut collector, + content, + width, + &mut old_line, + &mut new_line, + '-', + ); + for &(marker_idx, marker) in &removed_markers { + if marker_idx == idx { + collector.extend(render_header_line(marker, width)); + } + } + } + render_plain_diff_line( + &mut collector, + next.trim_start_matches('-'), + width, + &mut old_line, + &mut new_line, + '-', + ); + oversized = true; + break; + } } else if let Some(next) = lines.next_if(|next| is_added(next)) { - added.push(next.trim_start_matches('+')); + if added.len() < INTRALINE_MAX_RUN { + added.push(next.trim_start_matches('+')); + } else { + for (idx, content) in removed.drain(..).enumerate() { + render_plain_diff_line( + &mut collector, + content, + width, + &mut old_line, + &mut new_line, + '-', + ); + for &(marker_idx, marker) in &removed_markers { + if marker_idx == idx { + collector.extend(render_header_line(marker, width)); + } + } + } + for (idx, content) in added.drain(..).enumerate() { + render_plain_diff_line( + &mut collector, + content, + width, + &mut old_line, + &mut new_line, + '+', + ); + for &(marker_idx, marker) in &added_markers { + if marker_idx == idx { + collector.extend(render_header_line(marker, width)); + } + } + } + render_plain_diff_line( + &mut collector, + next.trim_start_matches('+'), + width, + &mut old_line, + &mut new_line, + '+', + ); + oversized = true; + break; + } } else { break; } } + + if oversized { + while let Some(next) = lines.next_if(|next| { + is_no_newline_marker(next) || is_deleted(next) || is_added(next) + }) { + if is_no_newline_marker(next) { + collector.extend(render_header_line(next, width)); + continue; + } + let marker = if is_deleted(next) { '-' } else { '+' }; + render_plain_diff_line( + &mut collector, + next.trim_start_matches(marker), + width, + &mut old_line, + &mut new_line, + marker, + ); + } + continue; + } + let pairs: Vec, Vec)>> = if removed.len() == added.len() && removed.len() <= INTRALINE_MAX_RUN { removed @@ -160,6 +261,11 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun if let Some(line) = old_line.as_mut() { *line = line.saturating_add(1); } + for &(marker_idx, marker) in &removed_markers { + if marker_idx == idx { + collector.extend(render_header_line(marker, width)); + } + } } for (idx, content) in added.iter().enumerate() { let emphasis = pairs @@ -177,9 +283,11 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun if let Some(line) = new_line.as_mut() { *line = line.saturating_add(1); } - } - for marker in markers { - collector.extend(render_header_line(marker, width)); + for &(marker_idx, marker) in &added_markers { + if marker_idx == idx { + collector.extend(render_header_line(marker, width)); + } + } } continue; } @@ -550,6 +658,37 @@ fn emphasised_chunks( (cursor == source.len()).then_some(out) } +fn render_plain_diff_line( + collector: &mut BoundedLineCollector, + content: &str, + width: u16, + old_line: &mut Option, + new_line: &mut Option, + marker: char, +) { + let style = match marker { + '-' => deleted_style(), + '+' => added_style(), + _ => Style::default().fg(palette::TEXT_PRIMARY), + }; + collector.extend(render_diff_line( + content, width, *old_line, *new_line, marker, style, None, + )); + match marker { + '-' => { + if let Some(line) = old_line.as_mut() { + *line = line.saturating_add(1); + } + } + '+' => { + if let Some(line) = new_line.as_mut() { + *line = line.saturating_add(1); + } + } + _ => {} + } +} + fn render_diff_line( content: &str, width: u16, @@ -832,7 +971,7 @@ mod tests { "; let rendered = rendered_body(diff, 80); let rows: Vec = rendered.iter().map(emphasised_text).collect(); - assert_eq!(rows, vec!["", "old", "new", "", ""]); + assert_eq!(rows, vec!["", "old", "", "new", ""]); let text: Vec = rendered.iter().map(line_text).collect(); assert_eq!( text.iter() @@ -843,6 +982,31 @@ mod tests { ); } + #[test] + fn pure_insertion_no_newline_marker_is_a_header_row() { + let diff = "\ +@@ -1,0 +1,1 @@ ++inserted +\\ No newline at end of file + context +"; + let rendered = rendered_body(diff, 80); + let marker = rendered + .iter() + .find(|line| line_text(line).contains("No newline at end of file")) + .expect("marker row"); + assert!(marker.spans[0].style.add_modifier.contains(Modifier::BOLD)); + + let context = rendered + .iter() + .find(|line| line_text(line).contains("context")) + .expect("context row"); + assert!( + line_text(context).starts_with(" 1 2 "), + "context numbering was changed by the marker: {context:?}" + ); + } + #[test] fn emphasis_lands_on_the_right_chunk_of_a_wrapped_indented_line() { // Gutter is 12 columns; width 40 leaves 28 for text. With the 8-space From b2ccdde16fd96569ea15bf6abfa9334204da17f0 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 03:39:26 +0000 Subject: [PATCH 4/4] fix(tui): simplify bounded diff rendering Signed-off-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-Authored-By: Hunter Bown --- crates/tui/src/tui/diff_render.rs | 121 ++++++++++++------------------ 1 file changed, 49 insertions(+), 72 deletions(-) diff --git a/crates/tui/src/tui/diff_render.rs b/crates/tui/src/tui/diff_render.rs index e1bdd2b27..76e1a1e4a 100644 --- a/crates/tui/src/tui/diff_render.rs +++ b/crates/tui/src/tui/diff_render.rs @@ -73,7 +73,7 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun let mut new_line: Option = None; let mut lines = diff.lines().peekable(); - while let Some(raw) = lines.next() { + 'line: while let Some(raw) = lines.next() { if raw.starts_with("diff --git") || raw.starts_with("index ") { collector.extend(render_header_line(raw, width)); continue; @@ -123,7 +123,6 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun let mut added: Vec<&str> = Vec::new(); let mut removed_markers: Vec<(usize, &str)> = Vec::new(); let mut added_markers: Vec<(usize, &str)> = Vec::new(); - let mut oversized = false; loop { if let Some(marker) = lines.next_if(|next| is_no_newline_marker(next)) { if added.is_empty() { @@ -137,21 +136,15 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun if removed.len() < INTRALINE_MAX_RUN { removed.push(next.trim_start_matches('-')); } else { - for (idx, content) in removed.drain(..).enumerate() { - render_plain_diff_line( - &mut collector, - content, - width, - &mut old_line, - &mut new_line, - '-', - ); - for &(marker_idx, marker) in &removed_markers { - if marker_idx == idx { - collector.extend(render_header_line(marker, width)); - } - } - } + flush_plain_run( + &mut collector, + &removed, + &removed_markers, + '-', + width, + &mut old_line, + &mut new_line, + ); render_plain_diff_line( &mut collector, next.trim_start_matches('-'), @@ -160,43 +153,30 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun &mut new_line, '-', ); - oversized = true; - break; + continue 'line; } } else if let Some(next) = lines.next_if(|next| is_added(next)) { if added.len() < INTRALINE_MAX_RUN { added.push(next.trim_start_matches('+')); } else { - for (idx, content) in removed.drain(..).enumerate() { - render_plain_diff_line( - &mut collector, - content, - width, - &mut old_line, - &mut new_line, - '-', - ); - for &(marker_idx, marker) in &removed_markers { - if marker_idx == idx { - collector.extend(render_header_line(marker, width)); - } - } - } - for (idx, content) in added.drain(..).enumerate() { - render_plain_diff_line( - &mut collector, - content, - width, - &mut old_line, - &mut new_line, - '+', - ); - for &(marker_idx, marker) in &added_markers { - if marker_idx == idx { - collector.extend(render_header_line(marker, width)); - } - } - } + flush_plain_run( + &mut collector, + &removed, + &removed_markers, + '-', + width, + &mut old_line, + &mut new_line, + ); + flush_plain_run( + &mut collector, + &added, + &added_markers, + '+', + width, + &mut old_line, + &mut new_line, + ); render_plain_diff_line( &mut collector, next.trim_start_matches('+'), @@ -205,35 +185,13 @@ pub fn render_diff_body_bounded(diff: &str, width: u16, max_rows: usize) -> Boun &mut new_line, '+', ); - oversized = true; - break; + continue 'line; } } else { break; } } - if oversized { - while let Some(next) = lines.next_if(|next| { - is_no_newline_marker(next) || is_deleted(next) || is_added(next) - }) { - if is_no_newline_marker(next) { - collector.extend(render_header_line(next, width)); - continue; - } - let marker = if is_deleted(next) { '-' } else { '+' }; - render_plain_diff_line( - &mut collector, - next.trim_start_matches(marker), - width, - &mut old_line, - &mut new_line, - marker, - ); - } - continue; - } - let pairs: Vec, Vec)>> = if removed.len() == added.len() && removed.len() <= INTRALINE_MAX_RUN { removed @@ -689,6 +647,25 @@ fn render_plain_diff_line( } } +fn flush_plain_run( + collector: &mut BoundedLineCollector, + lines: &[&str], + markers: &[(usize, &str)], + sign: char, + width: u16, + old_line: &mut Option, + new_line: &mut Option, +) { + for (idx, content) in lines.iter().enumerate() { + render_plain_diff_line(collector, content, width, old_line, new_line, sign); + for &(marker_idx, marker) in markers { + if marker_idx == idx { + collector.extend(render_header_line(marker, width)); + } + } + } +} + fn render_diff_line( content: &str, width: u16,