From 86d22d4fa2da22c25d13a27fedf97b981b172650 Mon Sep 17 00:00:00 2001 From: hl-valdemar Date: Sun, 11 Oct 2026 03:02:47 +0200 Subject: [PATCH] fix: skip omitted diff lines instead of expanding excerpts during navigation --- docs/editing.md | 2 + src/diff.rs | 66 +++++++++---------- src/editor.rs | 172 ++++++++++++++++++++++++++++++++++++------------ src/main.rs | 3 +- tests/diff.rs | 138 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 302 insertions(+), 79 deletions(-) diff --git a/docs/editing.md b/docs/editing.md index 05552a9..4d4a1d6 100644 --- a/docs/editing.md +++ b/docs/editing.md @@ -232,6 +232,8 @@ Text panes show change excerpts with **10 unchanged lines above and below** each In **Normal mode** with a text pane focused, **`[` expands above** and **`]` expands below** the excerpt at the top of the viewport, revealing up to **15 lines** per keypress. In editable After's Insert mode these keys insert literal text. A hidden-line marker targets the following excerpt, or the preceding excerpt at the end of the file. Expansion clamps at file boundaries and merges excerpts when they meet. Existing visible source stays anchored; expanding above a leading/interior marker brings the newly revealed context into view. Opening a file resets its excerpts to the configured initial context. Live edits recompute changes while retaining expanded context; the caret stays visible even when the last text change is reverted. +Editable After navigation stays within the shown source lines. At an excerpt boundary, Up/Down or `j`/`k` jumps directly to the next/previous excerpt, and counts count visible lines rather than omitted lines. This also applies to remapped movement keys, Visual-mode movement, and Insert-mode arrows. Word motions, `gt`/`gb`, search, and occurrence selection use the visible excerpts; `gt`/`gb` target their first/last source line. Hidden short lines do not clamp the caret column. Moving or restoring a caret never reveals omitted context: only Normal-mode `[`/`]` expands existing excerpts. Edits still recompute changed regions and retain previously expanded context. + Configure these counts in Ruby with `config.diff_context_lines = 10` (nonnegative; `0` shows changes only) and `config.diff_expand_lines = 15` (positive). Changed text rows use muted red backgrounds in Before and muted green backgrounds in After, including blank tails and real empty lines. Foreground colors use the editor's bundled Tree-sitter syntax highlighting, parsed independently for each snapshot and its path (including the old path for renames). Editable After also uses its live buffer's semantic highlighting, selections, and diagnostics. Unknown languages retain the normal foreground; historical snapshots do not borrow live-buffer LSP tokens. diff --git a/src/diff.rs b/src/diff.rs index c8d6855..bc63b98 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -629,28 +629,24 @@ impl DiffState { }) } - pub fn reveal_after(&mut self, caret_row: usize) { - let Some(index) = self - .rows + pub(crate) fn after_rows(&self) -> impl DoubleEndedIterator + '_ { + self.excerpt_ranges .iter() - .position(|row| row.after == Some(caret_row)) - else { - return; - }; - if self.after_display_row(caret_row).is_some() { - return; + .flat_map(|&(start, end)| self.rows[start..end].iter().filter_map(|row| row.after)) + } + + pub(crate) fn clamp_after_row(&self, row: usize) -> Option { + let mut previous = None; + for visible in self.after_rows() { + if visible >= row { + return Some(match previous { + Some(previous) if row - previous < visible - row => previous, + _ => visible, + }); + } + previous = Some(visible); } - let anchor = self.display_rows.get(self.top).copied(); - self.excerpt_ranges.push(( - index.saturating_sub(self.context_lines), - index - .saturating_add(self.context_lines) - .saturating_add(1) - .min(self.rows.len()), - )); - self.excerpt_ranges.sort_unstable(); - self.rebuild_display_rows(); - self.restore_top(anchor); + previous } fn restore_top(&mut self, anchor: Option) { @@ -670,12 +666,11 @@ impl DiffState { } } - pub fn refresh_after(&mut self, lines: &[String], revision: u64, caret_row: usize) { + pub fn refresh_after(&mut self, lines: &[String], revision: u64) { if !self.after_editable() { return; } if self.live_revision == Some(revision) { - self.reveal_after(caret_row); return; } let file = &self.comparison.as_ref().unwrap().files[self.file]; @@ -761,7 +756,6 @@ impl DiffState { { self.restore_top(Some(DisplayRow::Line(index))); } - self.reveal_after(caret_row); if was_changed != changed { self.refresh_files(); } @@ -2039,7 +2033,7 @@ mod tests { } #[test] - fn live_reflow_keeps_separate_hunks_expansion_and_caret_after_revert() { + fn live_reflow_keeps_separate_hunks_and_expanded_context_after_revert() { let base = (0..100) .map(|row| format!("line {row}")) .collect::>(); @@ -2049,7 +2043,7 @@ mod tests { let mut state = live_state(base.clone(), changed.clone(), true); state.set_context_options(2, 5); changed.push(String::new()); - state.refresh_after(&changed, 1, 30); + state.refresh_after(&changed, 1); assert_eq!(state.comparison.as_ref().unwrap().files[0].hunks.len(), 2); assert!( state @@ -2061,7 +2055,7 @@ mod tests { state.handle(Input::Char('[')); assert!(state.after_display_row(23).is_some()); changed.insert(10, "inserted outside excerpts".into()); - state.refresh_after(&changed, 2, 31); + state.refresh_after(&changed, 2); assert!(state.after_display_row(24).is_some()); assert_eq!(state.comparison.as_ref().unwrap().files[0].hunks.len(), 3); assert!( @@ -2071,14 +2065,15 @@ mod tests { ); let mut reverted = base; reverted.push(String::new()); - state.refresh_after(&reverted, 3, 90); + state.refresh_after(&reverted, 3); assert!(state.comparison.as_ref().unwrap().files[0].hunks.is_empty()); assert!(state.rows.iter().all(|row| row.kind == RowKind::Equal)); - assert!(state.after_display_row(90).is_some()); + assert!(state.after_display_row(23).is_some()); + assert!(state.after_display_row(90).is_none()); assert_eq!(state.files.as_ref().unwrap().entries().len(), 0); assert_eq!(state.file, 0); reverted[50] = "changed again".into(); - state.refresh_after(&reverted, 4, 50); + state.refresh_after(&reverted, 4); assert_eq!(state.files.as_ref().unwrap().entries().len(), 1); assert!(state.after_display_row(50).is_some()); } @@ -2086,12 +2081,13 @@ mod tests { #[test] fn live_empty_and_eof_rows_match_buffer_without_false_eof_highlights() { let mut state = live_state(vec!["same".into()], vec!["changed".into()], true); - state.refresh_after(&["same".into(), "".into()], 1, 1); + state.refresh_after(&["same".into(), "".into()], 1); assert!(state.comparison.as_ref().unwrap().files[0].hunks.is_empty()); assert_eq!(state.rows.len(), 2); assert!(state.rows.iter().all(|row| row.kind == RowKind::Equal)); - assert!(state.after_display_row(1).is_some()); - state.refresh_after(&["same".into()], 2, 0); + assert!(state.after_display_row(0).is_some()); + assert!(state.after_display_row(1).is_none()); + state.refresh_after(&["same".into()], 2); assert_eq!(state.rows[0].kind, RowKind::Changed); assert!( state.comparison.as_ref().unwrap().files[0] @@ -2100,17 +2096,17 @@ mod tests { .unwrap() .contains("After missing") ); - state.refresh_after(&["".into()], 3, 0); + state.refresh_after(&["".into()], 3); assert!(state.after_display_row(0).is_some()); assert_eq!(state.comparison.as_ref().unwrap().files[0].after, [""]); let mut empty = live_state(vec![], vec!["text".into()], false); - empty.refresh_after(&["".into()], 1, 0); + empty.refresh_after(&["".into()], 1); assert!(empty.comparison.as_ref().unwrap().files[0].hunks.is_empty()); assert_eq!(empty.rows.len(), 1); assert_eq!(empty.rows[0].kind, RowKind::Equal); assert_eq!(empty.after_display_row(0), Some(0)); empty.after.as_mut().unwrap().target = RevisionTarget::Commit("historic".into()); - empty.refresh_after(&["must not apply".into()], 2, 0); + empty.refresh_after(&["must not apply".into()], 2); assert_eq!(empty.comparison.as_ref().unwrap().files[0].after, [""]); empty.handle(Input::Escape); assert_eq!(empty.phase, Phase::View); diff --git a/src/editor.rs b/src/editor.rs index 9ef56c7..cb7c187 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -1420,16 +1420,33 @@ impl Editor { } pub(crate) fn refresh_live_diff(&mut self) { - let row = self.cursor().row; let Some(diff) = &mut self.diff else { return }; if diff.live_buffer != Some(self.active.id) || !diff.after_editable() { return; } if diff.live_revision != Some(self.active.text_revision) { - diff.refresh_after(&self.active.lines, self.active.text_revision, row); + diff.refresh_after(&self.active.lines, self.active.text_revision); diff.follow_caret = true; } - diff.reveal_after(row); + let mut moved = false; + for index in 0..self.active.targets.len() { + let cursor = self.active.targets[index].active; + let Some(row) = self.diff.as_ref().unwrap().clamp_after_row(cursor.row) else { + continue; + }; + if row != cursor.row { + self.active.targets[index].active = Cursor { + row, + column: cursor.column.min(self.max_column(row)), + }; + self.active.targets[index].preferred_columns = None; + moved = true; + } + } + if moved { + self.active.search_match = None; + self.normalize_targets(); + } } pub(crate) fn scroll_diff(&mut self, focus: Focus, down: bool) { @@ -3805,11 +3822,9 @@ impl Editor { let query = self.active.search_query.as_str(); // ponytail: O(buffer graphemes × query bytes) scan; index matches if large buffers need it. let matches: Vec<_> = self - .active - .lines - .iter() - .enumerate() - .flat_map(|(row, line)| { + .navigable_rows(0, self.active.lines.len()) + .flat_map(|row| { + let line = &self.active.lines[row]; line.grapheme_indices(true).filter_map(move |(byte, _)| { (line[byte..].starts_with(query) && GraphemeCursor::new(byte + query.len(), line.len(), true) @@ -4749,10 +4764,10 @@ impl Editor { let target = self.active.targets[index]; let shift = |row: usize| { if down { - row.checked_add(1) - .filter(|row| *row < self.active.lines.len()) + self.navigable_rows(row.saturating_add(1), self.active.lines.len()) + .next() } else { - row.checked_sub(1) + self.navigable_rows(0, row).next_back() } }; let Some(anchor_row) = shift(target.anchor.row) else { @@ -4831,14 +4846,28 @@ impl Editor { }) .collect(); // ponytail: literal scan is O(buffer graphemes × phrase bytes); index only for large buffers. - let candidates: Vec<_> = text - .grapheme_indices(true) - .filter_map(|(byte, _)| { + let diff = self.diff.as_ref().filter(|_| self.diff_editable()); + let candidates: Vec<_> = self + .navigable_rows(0, self.active.lines.len()) + .flat_map(|row| { + let offset = offsets[row]; + let line = &self.active.lines[row]; + line.grapheme_indices(true) + .map(move |(byte, _)| offset + byte) + .chain(std::iter::once(offset + line.len())) + }) + .filter_map(|byte| { let end = byte + phrase.len(); (text[byte..].starts_with(phrase) && GraphemeCursor::new(end, text.len(), true) .is_boundary(&text, 0) - .expect("complete buffer supplies grapheme context")) + .expect("complete buffer supplies grapheme context") + && diff.is_none_or(|diff| { + let last = offsets + .partition_point(|offset| *offset < end) + .saturating_sub(1); + diff.after_display_row(last).is_some() + })) .then_some((byte, end)) }) .collect(); @@ -5784,6 +5813,19 @@ impl Editor { } } + fn navigable_rows( + &self, + start: usize, + end: usize, + ) -> impl DoubleEndedIterator + '_ { + let diff = self.diff.as_ref().filter(|_| self.diff_editable()); + (start..if diff.is_none() { end } else { start }).chain( + diff.into_iter() + .flat_map(DiffState::after_rows) + .filter(move |row| start <= *row && *row < end), + ) + } + fn move_targets(&mut self, action: Action, count: usize) { self.reveal_caret(); for index in 0..self.active.targets.len() { @@ -5797,30 +5839,54 @@ impl Editor { .min(self.max_column(cursor.row)) } Action::Down | Action::Up => { - let row = if matches!(action, Action::Down) { - cursor - .row - .saturating_add(count) - .min(self.active.lines.len() - 1) + if self.diff_editable() { + let step = |cursor: Cursor, row| Cursor { + row, + column: cursor.column.min(self.max_column(row)), + }; + cursor = if action == Action::Down { + self.navigable_rows( + cursor.row.saturating_add(1), + self.active.lines.len(), + ) + .take(count) + .fold(cursor, step) + } else { + self.navigable_rows(0, cursor.row) + .rev() + .take(count) + .fold(cursor, step) + }; } else { - cursor.row.saturating_sub(count) - }; - let first = cursor.row.min(row); - let last = cursor.row.max(row); - cursor.column = (first..=last) - .map(|row| self.max_column(row)) - .min() - .unwrap_or(0) - .min(cursor.column); - cursor.row = row; + let row = if matches!(action, Action::Down) { + cursor + .row + .saturating_add(count) + .min(self.active.lines.len() - 1) + } else { + cursor.row.saturating_sub(count) + }; + let first = cursor.row.min(row); + let last = cursor.row.max(row); + cursor.column = (first..=last) + .map(|row| self.max_column(row)) + .min() + .unwrap_or(0) + .min(cursor.column); + cursor.row = row; + } } Action::LineStart => cursor.column = 0, Action::LineEnd => cursor.column = self.max_column(cursor.row), Action::DocumentStart | Action::DocumentEnd => { cursor.row = if action == Action::DocumentStart { - 0 + self.navigable_rows(0, self.active.lines.len()) + .next() + .unwrap_or(cursor.row) } else { - self.active.lines.len() - 1 + self.navigable_rows(0, self.active.lines.len()) + .next_back() + .unwrap_or(cursor.row) }; cursor.column = first_nonblank(&self.active.lines[cursor.row]); } @@ -5838,17 +5904,34 @@ impl Editor { action, Action::BigWordNext | Action::BigWordPrevious | Action::BigWordEnd ); + let backward = matches!(action, Action::WordPrevious | Action::BigWordPrevious); + let start = if backward { 0 } else { row }; + let end = if backward { + row + 1 + } else { + self.active.lines.len() + }; + let tokens = self.navigable_rows(start, end).flat_map(move |source_row| { + word_tokens(std::slice::from_ref(&self.active.lines[source_row]), big) + .map(move |(_, byte, kind)| (source_row, byte, kind)) + }); let destination = match action { Action::WordPrevious | Action::BigWordPrevious => { - let mut tokens = word_tokens(&self.active.lines[..=row], big) + let mut tokens = tokens .rev() .skip_while(|(r, b, _)| (*r, *b) >= (row, byte)) .peekable(); - let mut destination = (0, 0); + let fallback = ( + self.navigable_rows(0, self.active.lines.len()) + .next() + .unwrap_or(row), + 0, + ); + let mut destination = fallback; for _ in 0..count { let Some(mut first) = tokens.find(|(_, _, kind)| *kind != WordKind::Whitespace) else { - destination = (0, 0); + destination = fallback; break; }; if first.2 != WordKind::EmptyLine { @@ -5861,14 +5944,17 @@ impl Editor { destination } _ => { - let mut tokens = word_tokens(&self.active.lines[row..], big) - .skip_while(|(r, b, _)| (*r, *b) < (0, byte)) + let mut tokens = tokens + .skip_while(|(r, b, _)| (*r, *b) < (row, byte)) .peekable(); - let mut current = tokens - .next() - .expect("cursor has a grapheme or line-end token") - .2; - let last = self.active.lines.len() - 1; + let Some(first) = tokens.next() else { + return cursor; + }; + let mut current = first.2; + let last = self + .navigable_rows(0, self.active.lines.len()) + .next_back() + .unwrap_or(row); let mut destination = (last, self.active.lines[last].len()); for _ in 0..count { let next_start = matches!(action, Action::WordNext | Action::BigWordNext); @@ -5890,7 +5976,7 @@ impl Editor { first = tokens.next().expect("peeked token"); } } - destination = (row + first.0, first.1); + destination = (first.0, first.1); } destination } diff --git a/src/main.rs b/src/main.rs index c419f3f..ea79d73 100644 --- a/src/main.rs +++ b/src/main.rs @@ -44,7 +44,8 @@ Buffers: :edit PATH, :buffers, :buffer N, :buffer_next, :buffer_previous, :buffe Markdown links: :follow_link (primary caret; local files only) Diff: :diff (Normal); choose Before then After; Working checkout After edits live unsaved buffers Before and historical/remote After stay read-only; Tab/Shift+Tab categories/Normal focus - Type to search IDs/refs; Normal [/] expands excerpts; Insert brackets remain literal text + Type to search IDs/refs; navigation skips omitted lines without expanding excerpts + Normal [/] explicitly expands context; Insert brackets remain literal text :diff.close exits explicitly; Esc cancels modes/prompts/filtering without closing the diff Remote refs/history load into temporary comparison repos without changing active refs/checkout Changed files: normal picker/explorer bindings, Ctrl+w close; A/M/D/R status and rename paths diff --git a/tests/diff.rs b/tests/diff.rs index 0af723b..5ba5034 100644 --- a/tests/diff.rs +++ b/tests/diff.rs @@ -182,3 +182,141 @@ fn typed_diff_edits_unsaved_working_after_and_preserves_undo_on_explicit_close() assert_eq!(fixture.git(&["rev-parse", "HEAD"]), base); assert_eq!(fixture.git(&["status", "--porcelain"]), "M buffer.rs"); } + +#[test] +fn editable_diff_navigation_skips_omitted_lines_without_expanding_excerpts() { + use vamp::diff::DisplayRow; + + let fixture = Fixture::new(); + fixture.git(&["init", "-q"]); + let source = fixture.0.join("repo/context.txt"); + let mut lines = (1..=100) + .map(|line| format!("line_{line:03} words")) + .collect::>(); + lines[49].clear(); // Hidden short lines must not clamp the visible caret column. + fs::write(&source, format!("{}\n", lines.join("\n"))).unwrap(); + fixture.git(&["add", "context.txt"]); + fixture.git(&["commit", "-qm", "base"]); + let base = fixture.git(&["rev-parse", "HEAD"]); + lines[19] = "changed_020 words".into(); + lines[79] = "changed_080 words".into(); + let text = format!("{}\n", lines.join("\n")); + fs::write(&source, &text).unwrap(); + let config = fixture.0.join("config.rb"); + fs::write(&config, "Vamp.configure do |config|\n config.clipboard_mode = 'separate'\n config.diff_context_lines = 1\n config.diff_expand_lines = 2\n config.bind 'ctrl+f', config.core.down\n config.bind 'ctrl+b', config.core.up\nend\n").unwrap(); + let mut session = + EditorSession::open(Some(source.clone()), vec!["text".into()], Some(&config)).unwrap(); + session.execute("diff"); + wait_until(&mut session, |session| { + session + .editor() + .diff() + .is_some_and(|diff| diff.catalog.is_some() && !diff.loading) + }); + type_query(&mut session, &base); + session.handle(Input::Down); + session.handle(Input::Enter); + session.handle(Input::Enter); + wait_until(&mut session, |session| { + session + .editor() + .diff() + .is_some_and(|diff| diff.phase == Phase::View) + }); + session.handle(Input::BackTab); + let shown = |session: &EditorSession| { + let diff = session.editor().diff().unwrap(); + diff.display_rows + .iter() + .filter_map(|row| match *row { + DisplayRow::Line(index) => diff.rows[index].after, + DisplayRow::Fold { .. } => None, + }) + .collect::>() + }; + let excerpts = vec![18, 19, 20, 78, 79, 80]; + assert_eq!(shown(&session), excerpts); + assert_eq!(session.editor().cursor().row, 18); + type_query(&mut session, "5l"); + for (keys, row) in [("2j", 20), ("j", 78), ("k", 20), ("3j", 80), ("5k", 18)] { + type_query(&mut session, keys); + assert_eq!(session.editor().cursor().row, row, "{keys}"); + assert_eq!(session.editor().cursor().column, 5, "{keys}"); + assert_eq!(shown(&session), excerpts, "{keys}"); + } + for (keys, row) in [ + ("gb", 80), + ("gt", 18), + ("2j$w", 78), + ("b", 20), + ("$2w", 78), + ("0B", 20), + ] { + type_query(&mut session, keys); + assert_eq!(session.editor().cursor().row, row, "{keys}"); + assert_eq!(shown(&session), excerpts, "{keys}"); + } + session.handle(Input::Ctrl('f')); + assert_eq!(session.editor().cursor().row, 78); + session.handle(Input::Ctrl('b')); + assert_eq!(session.editor().cursor().row, 20); + type_query(&mut session, "Vj"); + assert_eq!(session.editor().cursor().row, 78); + session.handle(Input::Escape); + session.handle(Input::Ctrl('k')); + assert_eq!( + session + .editor() + .carets() + .iter() + .map(|caret| caret.row) + .collect::>(), + [20, 78] + ); + type_query(&mut session, "j"); + assert_eq!( + session + .editor() + .carets() + .iter() + .map(|caret| caret.row) + .collect::>(), + [78, 79] + ); + session.handle(Input::Escape); + type_query(&mut session, "gt2j$v"); + session.handle(Input::Ctrl('a')); + assert_eq!( + session + .editor() + .carets() + .iter() + .map(|caret| caret.row) + .collect::>(), + excerpts + ); + assert_eq!(shown(&session), excerpts); + session.handle(Input::Escape); + session.handle(Input::Escape); + type_query(&mut session, "/line_040"); + session.handle(Input::Enter); + assert_eq!(session.editor().cursor().row, 20); + type_query(&mut session, "/line_079"); + session.handle(Input::Enter); + assert_eq!(session.editor().cursor().row, 78); + assert_eq!(shown(&session), excerpts); + type_query(&mut session, "gt2ji"); + session.handle(Input::Down); + assert_eq!(session.editor().cursor().row, 78); + session.handle(Input::Up); + assert_eq!(session.editor().cursor().row, 20); + session.handle(Input::Escape); + assert_eq!(shown(&session), excerpts); + assert_eq!(session.editor().context().text, text); + assert!(!session.editor().dirty()); + assert_eq!(fs::read_to_string(&source).unwrap(), text); + type_query(&mut session, "["); + assert_eq!(shown(&session), [16, 17, 18, 19, 20, 78, 79, 80]); + type_query(&mut session, "]"); + assert_eq!(shown(&session), [16, 17, 18, 19, 20, 21, 22, 78, 79, 80]); +}