From 4511bc2df9afe141a2cce31aef8174803857e81c Mon Sep 17 00:00:00 2001 From: Valdemar Lorenzen Date: Fri, 16 Oct 2026 17:32:04 +0200 Subject: [PATCH] refactor: hide logical comparison rows from painting --- docs/architecture.md | 2 + src/diff.rs | 190 +++++++++++++++++++++++++++++++-- src/tui.rs | 245 +++++++++++++++++++++++-------------------- tests/unit/diff.rs | 153 ++++++++++++++++++++++++--- tests/unit/tui.rs | 5 +- 5 files changed, 462 insertions(+), 133 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index a15fdc9..08c6a8d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -86,6 +86,8 @@ Resolution exposes its projected comparison through the same active DiffState an Conflict History alignment lives in `diff::HistoryComparison`: it joins the selected inputs at shared base rows, combines base change ranges, retains native EOF rows, and exposes immutable column projections with source/display mapping. The frontend consumes each projection's source side rather than assigning Before/After conventions itself. `HistoryDiff` retains only presentation state: geometry, wrapping, shared scrolling, and caret visibility. Ordinary comparisons and Draft resolution continue to use their existing `DiffState` behavior. +Content painting and native caret overlays consume a borrowed `diff::ColumnView` for both ordinary comparisons and Conflict History. It resolves folds, gaps, source text and coordinates, native terminal rows, newline state, and side-specific change ranges without copying text or building a row catalog. Native caret mapping remains separate from review membership: unified equal context can carry anchors from either original source. Review freshness/filtering, pane chrome, viewport policy, wrapping, scope labels, and colors stay with their existing owners; the content painter receives only the view and already-filtered review annotations. History retains its internal pairwise `DiffState` storage. + `Editor::scroll_distance()` supplies the session-wide optional edge margin. The TUI applies it to document/diff visual rows and unwrapped text columns, and retains independent picker offsets across navigation frames. Picker windows exclude their fixed chrome and align whole multi-row entries. File projections carry a monotonic viewport revision, so catalog/filter/expansion changes invalidate picker identity without hashing the entire catalog on every redraw. Metadata-only status changes do not move the viewport. Explicit LSP/wrap-toggle anchors and detached wheel views retain precedence. Keyboard viewport actions queue per-view row and configured-step counts without moving carets. `Editor::take_viewport_scroll(height)` resolves step counts using the session-wide `scroll_step_lines` preference, or half the focused viewport when unset, and saturates multiplication before the frontend clamps movement to content bounds. Ordinary, diff, and conflict-history rendering share this resolution. diff --git a/src/diff.rs b/src/diff.rs index 618713f..1521ffd 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -240,6 +240,163 @@ pub enum DisplayRow { Fold { start: usize, end: usize }, } +/// Borrowed painting and native-coordinate projection of a loaded comparison. +#[derive(Clone, Copy, Debug)] +pub(crate) struct ColumnView<'a> { + diff: &'a DiffState, + file: &'a ChangedFile, + pub info: ColumnInfo<'a>, +} + +#[derive(Clone, Copy, Debug)] +pub(crate) struct ColumnInfo<'a> { + pub focus: Focus, + pub unified: bool, + pub display_len: usize, + pub source_extents: [usize; 2], + pub stored_lengths: [usize; 2], + pub empty: ColumnEmpty<'a>, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum ColumnEmpty<'a> { + EmptyFile, + NoChanges, + Notice(&'a str), +} + +#[derive(Debug)] +pub(crate) enum ColumnRow<'a> { + Fold { hidden: usize }, + Gap, + Text(ColumnLine<'a>), +} + +#[derive(Debug)] +pub(crate) struct ColumnLine<'a> { + pub text: Option<&'a str>, + pub side: Focus, + pub source_row: usize, + pub numbers: [Option; 2], + pub changed: bool, + pub emphasize: bool, + pub newline: bool, + changes: &'a [[std::ops::Range; 2]], +} + +impl<'a> ColumnLine<'a> { + pub fn changes(&self) -> impl Iterator> + 'a { + let side = usize::from(self.side == Focus::After); + let emphasize = self.emphasize; + self.changes + .iter() + .filter(move |_| emphasize) + .map(move |change| &change[side]) + } +} + +impl<'a> ColumnView<'a> { + fn new(diff: &'a DiffState, file: &'a ChangedFile, focus: Focus) -> Self { + let stored_lengths = [file.before.len(), file.after.len()]; + let last = diff.rows.last(); + let source_extents = [ + stored_lengths[0].max(last.and_then(|row| row.before).map_or(0, |row| row + 1)), + stored_lengths[1].max(last.and_then(|row| row.after).map_or(0, |row| row + 1)), + ]; + Self { + diff, + file, + info: ColumnInfo { + focus, + unified: diff.view_mode == ViewMode::Unified, + display_len: diff.display_rows.len(), + source_extents, + stored_lengths, + empty: match file.notice.as_deref() { + Some(notice) => ColumnEmpty::Notice(notice), + None if diff.rows.is_empty() => ColumnEmpty::EmptyFile, + None => ColumnEmpty::NoChanges, + }, + }, + } + } + + pub fn row(&self, display: usize) -> Option> { + let index = match *self.diff.display_rows.get(display)? { + DisplayRow::Fold { start, end } => { + return Some(ColumnRow::Fold { + hidden: end - start, + }); + } + DisplayRow::Line(index) => index, + }; + let row = self.diff.rows.get(index)?; + let side = if self.info.unified { + self.diff.display_focus(display).unwrap_or(self.info.focus) + } else { + self.info.focus + }; + let Some(source_row) = self.diff.display_source_row(side, display) else { + return Some(ColumnRow::Gap); + }; + let before = side == Focus::Before; + let side_index = usize::from(!before); + let (source, eof) = if before { + (&self.file.before, self.file.before_eof) + } else { + (&self.file.after, self.file.after_eof) + }; + let text = source + .get(source_row) + .map(String::as_str) + .or_else(|| (self.diff.terminal_rows[side_index] == Some(source_row)).then_some("")); + Some(ColumnRow::Text(ColumnLine { + text, + side, + source_row, + numbers: [ + self.diff.display_source_row(Focus::Before, display), + self.diff.display_source_row(Focus::After, display), + ], + changed: row.kind == RowKind::Changed + || (before && row.kind == RowKind::Removed) + || (!before && row.kind == RowKind::Added), + emphasize: !matches!(self.file.status, FileStatus::Added | FileStatus::Removed), + newline: source_row + 1 < source.len() || eof, + changes: &row.changes, + })) + } + + /// Caret mapping stays on this view's native side, even in unified display. + pub fn native_row(&self, display: usize) -> Option { + self.diff.display_source_row(self.info.focus, display) + } + + /// Finds the first visible native match; never reveals folded source rows. + pub fn display_row(&self, native: usize) -> Option { + self.diff.source_display_row(self.info.focus, native) + } + + /// Review anchors may address either coordinate of unified equal context, + /// independently of the focused native caret mapping. + pub fn review_row(&self, side: Focus, display: usize) -> Option { + self.diff.display_source_row(side, display).or_else(|| { + let DisplayRow::Line(index) = *self.diff.display_rows.get(display)? else { + return None; + }; + let row = self.diff.rows.get(index)?; + if !self.info.unified || row.kind != RowKind::Equal { + return None; + } + match side { + Focus::Before => row.before, + Focus::After => row.after, + Focus::Files => None, + } + }) + } +} + /// Immutable left/base/right conflict History, aligned at shared base rows. pub(crate) struct HistoryComparison { columns: [DiffState; 3], @@ -253,13 +410,13 @@ pub(crate) struct HistoryColumn<'a> { pub lines: &'a [String], } -impl HistoryColumn<'_> { - pub fn source_row(&self, display: usize) -> Option { - self.diff.display_source_row(self.focus, display) +impl<'a> HistoryColumn<'a> { + pub fn view(&self) -> ColumnView<'a> { + ColumnView::new(self.diff, self.file, self.focus) } - pub fn display_row(&self, source: usize) -> Option { - self.diff.source_display_row(self.focus, source) + pub fn source_row(&self, display: usize) -> Option { + self.diff.display_source_row(self.focus, display) } } @@ -404,6 +561,8 @@ pub struct DiffState { pub rows: Vec, pub display_rows: Vec, display_sides: Vec>, + /// Native empty terminal rows explicitly supplied by resolver text. + terminal_rows: [Option; 2], excerpt_ranges: Vec<(usize, usize)>, pub live_buffer: Option, pub live_revision: Option, @@ -462,6 +621,7 @@ impl DiffState { rows: Vec::new(), display_rows: Vec::new(), display_sides: Vec::new(), + terminal_rows: [None; 2], excerpt_ranges: Vec::new(), live_buffer: None, live_revision: None, @@ -538,7 +698,10 @@ impl DiffState { } let (before, before_eof) = content(draft); let (after, after_eof) = content(alternative); + let before_terminal = (draft.len() > before.len()).then_some(before.len()); + let after_terminal = (alternative.len() > after.len()).then_some(after.len()); if self.phase == Phase::View + && self.terminal_rows == [before_terminal, after_terminal] && self.comparison.as_ref().is_some_and(|comparison| { let file = &comparison.files[self.file]; file.before == before @@ -567,8 +730,7 @@ impl DiffState { file.after_eof = after_eof; file.hunks = line_hunks(before, before_eof, after, after_eof); self.align_file(); - let before_terminal = (draft.len() > before.len()).then_some(before.len()); - let after_terminal = (alternative.len() > after.len()).then_some(after.len()); + self.terminal_rows = [before_terminal, after_terminal]; if before_terminal.is_some() || after_terminal.is_some() { self.rows.push(AlignedRow { before: before_terminal, @@ -1650,6 +1812,19 @@ impl DiffState { .is_some_and(|file| file.loaded) } + pub(crate) fn column_view(&self, focus: Focus) -> Option> { + if focus == Focus::Files { + return None; + } + let file = self + .comparison + .as_ref()? + .files + .get(self.file) + .filter(|file| file.loaded)?; + Some(ColumnView::new(self, file, focus)) + } + /// Source rows without the editor's virtual terminal row, preserving real empty lines. pub(crate) fn after_source_lines<'a>(&self, file: &'a ChangedFile) -> &'a [String] { if self.live_files.contains(&self.file) { @@ -2079,6 +2254,7 @@ impl DiffState { fn rebuild_file(&mut self, reset: bool) { self.normalize_focus(); self.rows.clear(); + self.terminal_rows = [None; 2]; self.display_rows.clear(); self.display_sides.clear(); self.excerpt_ranges.clear(); diff --git a/src/tui.rs b/src/tui.rs index 581cdf4..e7295d6 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -41,8 +41,8 @@ use unicode_width::UnicodeWidthStr; use crate::{ chrome::{ChromeLine, PopupAnchor, SegmentConfig}, diff::{ - ChangedFile, DiffState, DisplayRow, FileStatus, Focus, HistoryComparison, PaneLayout, - Phase, RowKind, ViewMode, + ChangedFile, ColumnEmpty, ColumnRow, ColumnView, DiffState, DisplayRow, FileStatus, Focus, + HistoryComparison, PaneLayout, Phase, ViewMode, }, editor::{ Completion, Cursor, Editor, Hover, Input, Mode, Palette, ResolverLayout, ResolverStatus, @@ -911,7 +911,11 @@ impl BufferViewports { ); if !editor.file_picker().is_some_and(|picker| picker.focused) && (history.caret != Some(identity) || previous_widths != history.widths) - && let Some(display) = history.comparison.column(column).display_row(cursor.row) + && let Some(display) = history + .comparison + .column(column) + .view() + .display_row(cursor.row) { let column_offset = display_column(&editor.lines()[cursor.row], cursor.column); let continuation = if history.wrapping[column] { @@ -1012,9 +1016,8 @@ impl BufferViewports { self.panels.caret = paint_diff_editor( frame, view, - projection.diff, + projection.view(), text_area, - projection.focus, (history.top.display, history.left[column]), Some(&history.visible[column]), ) @@ -3225,9 +3228,8 @@ fn paint_editor_line( fn paint_diff_editor( frame: &mut Frame, editor: &Editor, - diff: &DiffState, + view: ColumnView<'_>, area: Rect, - focus: Focus, viewport: (usize, usize), wrapped: Option<&[WrappedDiffRow]>, ) -> Option { @@ -3237,8 +3239,11 @@ fn paint_diff_editor( let source_cursor = editor.cursor(); let cursor_column = display_column(&editor.lines()[source_cursor.row], source_cursor.column); let mut cursor = None; - for (screen, display, segments) in diff_screen_rows(diff, top, area.height, wrapped) { - let Some(source) = diff.display_source_row(focus, display) else { + let focus = view.info.focus; + for (screen, display, segments) in + diff_screen_rows(view.info.display_len, top, area.height, wrapped) + { + let Some(source) = view.native_row(display) else { continue; }; let mut line_area = Rect::new(area.x, area.y + screen as u16, area.width, 1); @@ -3895,9 +3900,8 @@ fn draw_diff( document_caret = paint_diff_editor( frame, editor, - diff, + diff.column_view(Focus::Before).unwrap(), text_area, - Focus::Before, (top, before_left), wrapped, ) @@ -3936,9 +3940,8 @@ fn draw_diff( document_caret = paint_diff_editor( frame, editor, - diff, + diff.column_view(Focus::After).unwrap(), text_area, - Focus::After, (top, after_left), wrapped, ) @@ -4269,25 +4272,13 @@ fn draw_diff_pane( } else { diff_pane_panel(frame, diff, Some(file), area, before, resolver) }; - if diff.display_rows.is_empty() { - frame.render_widget( - Paragraph::new(file.notice.as_deref().map(display_text).unwrap_or_else(|| { - if diff.rows.is_empty() { - "Empty file" - } else { - "No text changes" - } - .into() - })), - content, - ); - return content; - } + let view = diff.column_view(focus).expect("prepared comparison column"); let unified = diff.view_mode == ViewMode::Unified; let (top, left, previous_width) = viewport; let gutter = diff_gutter(diff, file, usize::from(content.width), focus); let width = usize::from(content.width).saturating_sub(gutter); - if !diff.word_wrap + if view.info.display_len > 0 + && !diff.word_wrap && !content_only && previous_width.is_some_and(|previous| usize::from(previous) != width) { @@ -4310,68 +4301,95 @@ fn draw_diff_pane( && comment.anchor.path == *path && diff.review_anchor_current(&comment.anchor) }); + paint_diff_content( + frame, + view, + content, + syntax, + semantic, + (top, left), + wrapped, + colors, + highlight_trailing_whitespace, + whitespace_characters, + whitespace_scope, + review_comments, + diff.local_review.selected, + ) +} + +#[allow(clippy::too_many_arguments)] +fn paint_diff_content<'a>( + frame: &mut Frame, + view: ColumnView<'a>, + content: Rect, + syntax: [&SyntaxCache; 2], + semantic: [&[SemanticToken]; 2], + viewport: (usize, usize), + wrapped: Option<&[WrappedDiffRow]>, + colors: DiffColors, + highlight_trailing_whitespace: bool, + whitespace_characters: Option<&BTreeMap>, + whitespace_scope: WhitespaceScope, + review_comments: impl Iterator + Clone, + selected_review: Option, +) -> Rect { + if view.info.display_len == 0 { + let notice = match view.info.empty { + ColumnEmpty::Notice(text) => display_text(text), + ColumnEmpty::EmptyFile => "Empty file".into(), + ColumnEmpty::NoChanges => "No text changes".into(), + }; + frame.render_widget(Paragraph::new(notice), content); + return content; + } + let unified = view.info.unified; + let (top, left) = viewport; + let gutter = column_gutter( + unified, + view.info.source_extents, + usize::from(content.width), + view.info.focus, + ); + let width = usize::from(content.width).saturating_sub(gutter); let selected_anchor = review_comments .clone() - .find(|comment| diff.local_review.selected == Some(comment.id)) + .find(|comment| selected_review == Some(comment.id)) .map(|comment| &comment.anchor); - let review_source_row = |side, display| { - diff.display_source_row(side, display).or_else(|| { - let DisplayRow::Line(index) = diff.display_rows[display] else { - return None; - }; - let row = &diff.rows[index]; - (unified && row.kind == RowKind::Equal) - .then_some(match side { - Focus::Before => row.before, - Focus::After => row.after, - Focus::Files => None, - }) - .flatten() - }) - }; - for (screen, display_row, segments) in diff_screen_rows(diff, top, content.height, wrapped) { - let row = &diff.display_rows[display_row]; + for (screen, display_row, segments) in + diff_screen_rows(view.info.display_len, top, content.height, wrapped) + { let row_area = Rect::new(content.x, content.y + screen as u16, content.width, 1); - let aligned_row = match row { - DisplayRow::Line(index) => *index, - DisplayRow::Fold { start, end } => { + let line = match view.row(display_row) { + Some(ColumnRow::Fold { hidden }) => { frame.render_widget( Paragraph::new(format!( "… {} unchanged lines hidden · [/] expand context", - end - start + hidden )) .style(Style::default().fg(Color::DarkGray)), row_area, ); continue; } - }; - let row = &diff.rows[aligned_row]; - let before = if unified { - diff.display_focus(display_row).unwrap_or(focus) == Focus::Before - } else { - before - }; - let source = if before { &file.before } else { &file.after }; - let index = diff.display_source_row( - if before { Focus::Before } else { Focus::After }, - display_row, - ); - let Some(index) = index else { - let style = Style::default().fg(Color::DarkGray).bg(Color::Reset); - let buffer = frame.buffer_mut(); - for x in 0..content.width { - buffer[(content.x + x, row_area.y)] - .set_symbol("╱") - .set_style(style); + Some(ColumnRow::Text(line)) => line, + Some(ColumnRow::Gap) => { + let style = Style::default().fg(Color::DarkGray).bg(Color::Reset); + let buffer = frame.buffer_mut(); + for x in 0..content.width { + buffer[(content.x + x, row_area.y)] + .set_symbol("╱") + .set_style(style); + } + continue; } - continue; - }; - let source_text = match source.get(index) { - Some(text) => text.as_str(), - None if resolver.is_some() && index == source.len() => "", None => continue, }; + let Some(source_text) = line.text else { + continue; + }; + let before = line.side == Focus::Before; + let index = line.source_row; let segment = segments.and_then(|segments| segments[usize::from(!before)]); let left = segment.map_or(left, |segment| segment.start); let text_width = segment.map_or(width, |segment| segment.end - segment.start); @@ -4381,15 +4399,12 @@ fn draw_diff_pane( segment_paint_width(segment, final_segment, width) }); let text_area = Rect::new(content.x + gutter as u16, row_area.y, paint_width as u16, 1); - let changed = matches!(row.kind, RowKind::Changed) - || (before && row.kind == RowKind::Removed) - || (!before && row.kind == RowKind::Added); - let background = - if changed && !matches!(file.status, FileStatus::Added | FileStatus::Removed) { - colors.row[usize::from(!before)] - } else { - Color::Reset - }; + let changed = line.changed; + let background = if changed && line.emphasize { + colors.row[usize::from(!before)] + } else { + Color::Reset + }; frame .buffer_mut() .set_style(row_area, Style::default().bg(background)); @@ -4413,14 +4428,11 @@ fn draw_diff_pane( Color::DarkGray }; let label = if unified { - let old = diff - .display_source_row(Focus::Before, display_row) - .map(|row| row + 1); - let new = diff - .display_source_row(Focus::After, display_row) - .map(|row| row + 1); - let old_digits = file.before.len().max(1).ilog10() as usize + 1; - let new_digits = file.after.len().max(1).ilog10() as usize + 1; + let [old, new] = line.numbers.map(|row| row.map(|row| row + 1)); + let [old_digits, new_digits] = view + .info + .stored_lengths + .map(|len| len.max(1).ilog10() as usize + 1); match (old, new) { (Some(old), Some(new)) => { format!("{old:>old_digits$} {new:>new_digits$}{marker} ") @@ -4430,7 +4442,10 @@ fn draw_diff_pane( (None, None) => unreachable!("displayed diff line has a source"), } } else { - let digits = diff_source_len(diff, file, before).max(1).ilog10() as usize + 1; + let digits = view.info.source_extents[usize::from(!before)] + .max(1) + .ilog10() as usize + + 1; format!("{:>digits$}{marker} ", index + 1) }; frame.render_widget( @@ -4462,19 +4477,13 @@ fn draw_diff_pane( characters, whitespace_scope, syntax[usize::from(!before)].string_spans(index), - index + 1 < source.len() - || if before { - file.before_eof - } else { - file.after_eof - }, + line.newline, true, ); } - if !matches!(file.status, FileStatus::Added | FileStatus::Removed) { + if line.emphasize { let style = Style::default().bg(colors.character[usize::from(!before)]); - for change in &row.changes { - let range = &change[usize::from(!before)]; + for range in line.changes() { if !range.is_empty() { paint_grapheme_range(frame, text_area, left, source_text, range.clone(), style); } @@ -4504,7 +4513,8 @@ fn draw_diff_pane( crate::review::Side::Before => Focus::Before, crate::review::Side::After => Focus::After, }; - if let Some(source_row) = review_source_row(side, display_row) + if let Some(source_row) = view + .review_row(side, display_row) .filter(|source_row| anchor.rows().contains(source_row)) { let start = if source_row == anchor.start.row { @@ -4534,9 +4544,11 @@ fn draw_diff_pane( crate::review::Side::Before => Focus::Before, crate::review::Side::After => Focus::After, }; - let selected = diff.local_review.selected == Some(comment.id); - for (screen, display, _) in diff_screen_rows(diff, top, content.height, wrapped) { - let source = review_source_row(side, display); + let selected = selected_review == Some(comment.id); + for (screen, display, _) in + diff_screen_rows(view.info.display_len, top, content.height, wrapped) + { + let source = view.review_row(side, display); if wrapped.is_some_and(|rows| rows[screen].position.continuation != 0) || !source.is_some_and(|row| comment.anchor.rows().contains(&row)) { @@ -4687,9 +4699,20 @@ fn diff_source_len(diff: &DiffState, file: &ChangedFile, before: bool) -> usize } fn diff_gutter(diff: &DiffState, file: &ChangedFile, width: usize, focus: Focus) -> usize { - let old = diff_source_len(diff, file, true).max(1).ilog10() as usize + 1; - let new = diff_source_len(diff, file, false).max(1).ilog10() as usize + 1; - let gutter = if diff.view_mode == ViewMode::Unified { + column_gutter( + diff.view_mode == ViewMode::Unified, + [ + diff_source_len(diff, file, true), + diff_source_len(diff, file, false), + ], + width, + focus, + ) +} + +fn column_gutter(unified: bool, extents: [usize; 2], width: usize, focus: Focus) -> usize { + let [old, new] = extents.map(|len| len.max(1).ilog10() as usize + 1); + let gutter = if unified { old + new + 3 } else { (if focus == Focus::Before { old } else { new }) + 2 @@ -5103,7 +5126,7 @@ impl DiffWrapViewport { } fn diff_screen_rows<'a>( - diff: &'a DiffState, + display_len: usize, top: usize, height: u16, wrapped: Option<&'a [WrappedDiffRow]>, @@ -5120,7 +5143,7 @@ fn diff_screen_rows<'a>( ) }) } else { - (top + screen < diff.display_rows.len()).then_some((screen, top + screen, None)) + (top + screen < display_len).then_some((screen, top + screen, None)) } }) } @@ -5268,7 +5291,7 @@ fn diff_visible_rows( } else { pane.height }; - for (_, display, _) in diff_screen_rows(diff, top, height, wrapped) { + for (_, display, _) in diff_screen_rows(diff.display_rows.len(), top, height, wrapped) { let source = diff.display_source_row(if before { Focus::Before } else { Focus::After }, display); if ranges diff --git a/tests/unit/diff.rs b/tests/unit/diff.rs index cd18c4d..cf5263a 100644 --- a/tests/unit/diff.rs +++ b/tests/unit/diff.rs @@ -13,7 +13,7 @@ fn conflict_history_preserves_source_gaps_base_changes_and_native_eof_rows() { ]); let rows: Vec<_> = (0..history.row_count()) .map(|row| { - std::array::from_fn::<_, 3, _>(|column| history.column(column).source_row(row)) + std::array::from_fn::<_, 3, _>(|column| history.column(column).view().native_row(row)) }) .collect(); assert_eq!( @@ -26,7 +26,8 @@ fn conflict_history_preserves_source_gaps_base_changes_and_native_eof_rows() { ] ); for column in 0..3 { - let projection = history.column(column); + let history_column = history.column(column); + let projection = history_column.view(); for (display, row) in rows.iter().enumerate() { if let Some(source) = row[column] { assert_eq!(projection.display_row(source), Some(display)); @@ -34,14 +35,26 @@ fn conflict_history_preserves_source_gaps_base_changes_and_native_eof_rows() { } } let base = history.column(1); - let changed = &base.diff.rows[0].changes; + let base = base.view(); + let ColumnRow::Text(line) = base.row(0).unwrap() else { + panic!("changed base row must contain text"); + }; + assert!(line.changed); for column in [0, 6] { - assert!(changed.iter().any(|range| range[0].contains(&column))); + assert!(line.changes().any(|range| range.contains(&column))); } - assert!(!changed.iter().any(|range| range[0].contains(&4))); - assert_eq!(base.diff.rows[2].kind, RowKind::Equal); - assert!(base.diff.rows[2].changes.is_empty()); - assert_eq!(history.column(2).lines[1], "added"); + assert!(!line.changes().any(|range| range.contains(&4))); + let ColumnRow::Text(line) = base.row(2).unwrap() else { + panic!("unchanged base row must contain text"); + }; + assert!(!line.changed); + assert_eq!(line.changes().count(), 0); + let right = history.column(2); + let ColumnRow::Text(line) = right.view().row(1).unwrap() else { + panic!("inserted right row must contain text"); + }; + assert_eq!(line.text, Some("added")); + assert!(matches!(base.row(1), Some(ColumnRow::Gap))); // Rebuilding for a new input/base choice must not retain old source mappings. let history = HistoryComparison::new([ @@ -49,17 +62,17 @@ fn conflict_history_preserves_source_gaps_base_changes_and_native_eof_rows() { "abc deg\nadded\nkeep\n", "xbc def\nkeep\n", ]); - assert_eq!(history.column(1).source_row(1), Some(1)); - assert_eq!(history.column(0).source_row(1), None); - assert_eq!(history.column(2).source_row(1), None); + assert_eq!(history.column(1).view().native_row(1), Some(1)); + assert_eq!(history.column(0).view().native_row(1), None); + assert_eq!(history.column(2).view().native_row(1), None); for text in ["", "\n", "same", "same\n"] { let history = HistoryComparison::new([text; 3]); let native_rows = text.split('\n').count(); assert_eq!(history.row_count(), native_rows); for column in 0..3 { for row in 0..native_rows { - assert_eq!(history.column(column).source_row(row), Some(row)); - assert_eq!(history.column(column).display_row(row), Some(row)); + assert_eq!(history.column(column).view().native_row(row), Some(row)); + assert_eq!(history.column(column).view().display_row(row), Some(row)); } } } @@ -1815,6 +1828,120 @@ fn live_state(before: Vec, after: Vec, eof: bool) -> DiffState { state } +#[test] +fn column_view_preserves_split_gaps_unified_sides_and_folded_coordinates() { + let mut state = live_state( + ["head", "tail"].map(str::to_owned).to_vec(), + ["head", "added", "tail"].map(str::to_owned).to_vec(), + false, + ); + let before = state.column_view(Focus::Before).unwrap(); + assert!(matches!(before.row(1), Some(ColumnRow::Gap))); + assert_eq!(before.native_row(1), None); + assert_eq!(before.display_row(1), Some(2)); + state.set_view_mode(ViewMode::Unified); + let before = state.column_view(Focus::Before).unwrap(); + let ColumnRow::Text(line) = before.row(1).unwrap() else { + panic!("unified insertion must paint the after source"); + }; + assert_eq!((line.text, line.side, line.source_row), (Some("added"), Focus::After, 1)); + assert_eq!(line.numbers, [None, Some(1)]); + assert!(line.changed); + assert_eq!(before.native_row(1), None); + assert_eq!(before.review_row(Focus::Before, 1), None); + assert_eq!(before.review_row(Focus::After, 1), Some(1)); + let ColumnRow::Text(line) = before.row(2).unwrap() else { + panic!("equal context must contain text"); + }; + assert_eq!(line.numbers, [Some(1), Some(2)]); + assert_eq!(before.native_row(2), Some(1)); + assert_eq!(before.review_row(Focus::Before, 2), Some(1)); + assert_eq!(before.review_row(Focus::After, 2), Some(2)); + assert_eq!(before.display_row(1), Some(2)); + for invalid in [before.info.display_len, usize::MAX] { + assert!(before.row(invalid).is_none()); + assert_eq!(before.native_row(invalid), None); + assert_eq!(before.review_row(Focus::After, invalid), None); + } + assert_eq!(before.display_row(usize::MAX), None); + + let lines: Vec<_> = (0..30).map(|row| format!("line {row}")).collect(); + let mut changed = lines.clone(); + changed[15] = "changed".into(); + let mut state = live_state(lines, changed, false); + state.set_context_options(0, 1); + for mode in [ViewMode::Split, ViewMode::Unified] { + state.set_view_mode(mode); + let view = state.column_view(Focus::After).unwrap(); + assert!(matches!(view.row(0), Some(ColumnRow::Fold { hidden: 15 }))); + assert_eq!(view.native_row(0), None); + assert_eq!(view.review_row(Focus::After, 0), None); + assert_eq!(view.display_row(0), None); + let display = view.display_row(15).unwrap(); + let ColumnRow::Text(line) = view.row(display).unwrap() else { + panic!("visible changed source must contain text"); + }; + assert_eq!((line.text, line.source_row), (Some("changed"), 15)); + assert_eq!(view.native_row(display), Some(15)); + } +} + +#[test] +fn column_view_distinguishes_native_terminal_rows_from_missing_ordinary_text() { + for text in ["", "same\n"] { + let lines: Vec<_> = text.split('\n').map(str::to_owned).collect(); + let mut state = DiffState::new(); + state.set_resolver_text(&lines, &lines); + for focus in [Focus::Before, Focus::After] { + let view = state.column_view(focus).unwrap(); + let native = lines.len() - 1; + let display = view.display_row(native).unwrap(); + let ColumnRow::Text(line) = view.row(display).unwrap() else { + panic!("native terminal row must contain text"); + }; + assert_eq!((line.text, line.source_row), (Some(""), native)); + assert_eq!(line.newline, text.ends_with('\n')); + assert_eq!(view.native_row(display), Some(native)); + assert_eq!(view.info.source_extents, [lines.len(); 2]); + assert_eq!(view.info.stored_lengths, [lines.len() - 1; 2]); + } + } + let mut state = live_state(vec!["old".into()], vec!["new".into()], false); + // A mapped but unavailable ordinary source is not a synthetic terminal row. + state.comparison.as_mut().unwrap().files[0].after.clear(); + let view = state.column_view(Focus::After).unwrap(); + let ColumnRow::Text(line) = view.row(0).unwrap() else { + panic!("missing source text must retain its mapped row"); + }; + assert_eq!(line.text, None); + assert_eq!(line.source_row, 0); +} + +#[test] +fn column_view_suppresses_whole_file_emphasis_without_losing_change_markers() { + let mut state = live_state(vec!["abc".into()], vec!["aXYbc".into()], false); + for (focus, expected) in [(Focus::Before, 1..1), (Focus::After, 1..3)] { + let view = state.column_view(focus).unwrap(); + let ColumnRow::Text(line) = view.row(0).unwrap() else { + panic!("changed row must contain text"); + }; + assert!(line.changed && line.emphasize); + assert_eq!(line.changes().cloned().collect::>(), [expected]); + } + for status in [FileStatus::Added, FileStatus::Removed] { + state.comparison.as_mut().unwrap().files[0].status = status; + for focus in [Focus::Before, Focus::After] { + let view = state.column_view(focus).unwrap(); + let ColumnRow::Text(line) = view.row(0).unwrap() else { + panic!("whole-file status must retain source text"); + }; + assert!(line.changed); + assert!(!line.emphasize); + assert_eq!(line.changes().count(), 0); + } + } +} + #[test] fn working_refresh_preserves_noops_errors_and_view_state_but_invalidates_old_identity() { let mut state = live_state(vec!["old".into()], vec!["new".into()], false); diff --git a/tests/unit/tui.rs b/tests/unit/tui.rs index d405a9c..b7dc161 100644 --- a/tests/unit/tui.rs +++ b/tests/unit/tui.rs @@ -1,4 +1,5 @@ use super::*; +use crate::diff::RowKind; use crate::plugin::TextPosition; use std::{ fs, @@ -2403,7 +2404,7 @@ fn diff_scope_labels_identify_multiple_scopes_and_sticky_renamed_sides() { assert_eq!(rows[0].scope, [true; 2]); assert_eq!(rows[1].scope, [false; 2]); assert_eq!(rows[0].position, rows[1].position); - let first = diff_screen_rows(&diff, diff.top, 48, Some(rows)) + let first = diff_screen_rows(diff.display_rows.len(), diff.top, 48, Some(rows)) .next() .unwrap(); assert_eq!((first.0, first.1), (1, diff.top)); @@ -3054,7 +3055,7 @@ fn local_review_highlights_exact_graphemes_newlines_clips_and_wrap_continuations let gutter = diff_gutter(&diff, file, content.width as usize, focus); let width = content.width as usize - gutter; for (screen, display, segments) in - diff_screen_rows(&diff, 0, content.height, wrapped) + diff_screen_rows(diff.display_rows.len(), 0, content.height, wrapped) { let mut expected = vec![false; width]; if let DisplayRow::Line(aligned) = diff.display_rows[display] {