From 976cfc9e6209d884596527b16cde163bfbe01326 Mon Sep 17 00:00:00 2001 From: hl-valdemar Date: Sun, 11 Oct 2026 10:50:54 +0200 Subject: [PATCH] fix: target caret excerpts and retain diff viewport offsets --- docs/editing.md | 4 +- src/diff.rs | 16 +++++- src/editor.rs | 84 ++++++++++++++++++++++++++++++++ src/session.rs | 11 +++++ src/tui.rs | 126 +++++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 237 insertions(+), 4 deletions(-) diff --git a/docs/editing.md b/docs/editing.md index 4d4a1d6..9d2d8ab 100644 --- a/docs/editing.md +++ b/docs/editing.md @@ -230,9 +230,9 @@ File rows show colored **A** (added, green), **M** (modified, yellow), **D** (re Text panes show change excerpts with **10 unchanged lines above and below** each change, preserving original file line numbers. Overlapping or touching excerpts merge; omitted stretches show an unchanged-line count in both panes. Files with only metadata changes show their notice or “No text changes” rather than unrelated source. -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. +In **Normal mode** with a text pane focused, **`[` expands above** and **`]` expands below**, revealing up to **15 lines** per keypress. Editable After targets the excerpt containing the caret; read-only panes target the excerpt at the top of the viewport. In editable After's Insert mode these keys insert literal text. In read-only panes, 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. +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. The viewport retains its vertical and horizontal offsets when movement reverses, scrolling only when the caret crosses a visible edge, as in a normal buffer. 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). diff --git a/src/diff.rs b/src/diff.rs index bc63b98..4b8d689 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -939,11 +939,25 @@ impl DiffState { } } + pub(crate) fn expand_after_excerpt(&mut self, below: bool, source_row: usize) { + let Some(display_row) = self.after_display_row(source_row) else { + return; + }; + self.expand_excerpt_at(below, self.display_rows[display_row]); + } + fn expand_excerpt(&mut self, below: bool) { + let Some(target) = self.display_rows.get(self.top).copied() else { + return; + }; + self.expand_excerpt_at(below, target); + } + + fn expand_excerpt_at(&mut self, below: bool, target: DisplayRow) { let Some(top) = self.display_rows.get(self.top).copied() else { return; }; - let index = match top { + let index = match target { DisplayRow::Line(row) => self .excerpt_ranges .iter() diff --git a/src/editor.rs b/src/editor.rs index cb7c187..f8c4123 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -1462,6 +1462,22 @@ impl Editor { } } + pub(crate) fn set_diff_viewport( + &mut self, + comparison_id: u64, + file: usize, + top: usize, + left_after: usize, + ) { + if let Some(diff) = &mut self.diff + && diff.comparison_id == Some(comparison_id) + && diff.file == file + { + diff.top = top; + diff.left_after = left_after; + } + } + fn handle_diff(&mut self, input: Input) -> Option { if input == Input::Escape && (self.count.is_some() || self.g_pending || self.line_pending.is_some()) @@ -1565,6 +1581,14 @@ impl Editor { if live && !matches!(input, Input::Char('\t' | '[' | ']') | Input::BackTab) { return self.handle_input(input); } + if live && matches!(input, Input::Char('[' | ']')) { + let row = self.cursor().row; + self.diff + .as_mut() + .expect("active diff") + .expand_after_excerpt(input == Input::Char(']'), row); + return None; + } if !files_focused && self .diff @@ -7050,6 +7074,66 @@ mod tests { buffers } + #[test] + fn editable_diff_brackets_expand_caret_excerpt_without_moving_source_anchor() { + for selected in [30, 50] { + let directory = Directory::new(); + let path = directory.path("file.txt"); + let before: Vec = (0..80).map(|row| format!("line {row}")).collect(); + fs::write(&path, before.join("\n")).unwrap(); + let mut editor = Editor::open(Some(path), vec![]).unwrap(); + let mut after = before; + for row in [10, 30, 50] { + after[row] = format!("changed {row}"); + } + editor + .apply(vec![Effect::Replace(after.join("\n"))]) + .unwrap(); + editor.set_diff_context_options(1, 4); + working_diff(&mut editor, &directory.0, &["file.txt"]); + editor.handle(Input::BackTab); + assert!(editor.diff_editing()); + editor.active.targets = vec![Target::caret(Cursor { + row: selected, + column: 2, + })]; + let top = editor.diff().unwrap().after_display_row(9).unwrap(); + editor.diff.as_mut().unwrap().top = top; + let cursor = editor.cursor(); + let visible = |editor: &Editor| editor.diff().unwrap().after_rows().collect::>(); + assert_eq!( + visible(&editor), + (9..12).chain(29..32).chain(49..52).collect::>() + ); + editor.handle(Input::Char('[')); + editor.handle(Input::Char(']')); + let expected = if selected == 30 { + (9..12).chain(25..36).chain(49..52).collect::>() + } else { + (9..12).chain(29..32).chain(45..56).collect::>() + }; + assert_eq!(visible(&editor), expected); + assert_eq!(editor.cursor(), cursor); + assert_eq!( + editor.diff().unwrap().after_display_row(9), + Some(editor.diff().unwrap().top) + ); + if selected == 30 { + // The caret keeps targeting the second excerpt after it merges + // with the third, rather than expanding the first excerpt. + for _ in 0..5 { + editor.handle(Input::Char(']')); + } + assert_eq!(visible(&editor), (9..12).chain(25..56).collect::>()); + assert_eq!(editor.cursor(), cursor); + assert_eq!( + editor.diff().unwrap().after_display_row(9), + Some(editor.diff().unwrap().top) + ); + } + } + } + #[test] fn working_diff_reuses_dirty_buffers_and_normal_edit_undo_save_paths() { let directory = Directory::new(); diff --git a/src/session.rs b/src/session.rs index 0442315..6296c8e 100644 --- a/src/session.rs +++ b/src/session.rs @@ -258,6 +258,17 @@ impl EditorSession { &self.editor } + pub(crate) fn set_diff_viewport( + &mut self, + comparison_id: u64, + file: usize, + top: usize, + left_after: usize, + ) { + self.editor + .set_diff_viewport(comparison_id, file, top, left_after); + } + pub(crate) fn scroll_diff(&mut self, focus: Focus, down: bool) { self.editor.scroll_diff(focus, down); } diff --git a/src/tui.rs b/src/tui.rs index 89fbb6e..53e9bec 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -172,6 +172,18 @@ impl BufferViewports { ); } + fn draw_session(&mut self, frame: &mut Frame, session: &mut EditorSession) { + self.draw(frame, session.editor()); + if let Some((comparison_id, file, top, left_after)) = self + .panels + .diff + .as_ref() + .and_then(|geometry| geometry.viewport) + { + session.set_diff_viewport(comparison_id, file, top, left_after); + } + } + fn scroll(&mut self, session: &mut EditorSession, mouse: MouseEvent) -> bool { if !matches!( mouse.kind, @@ -274,7 +286,7 @@ pub fn run(session: &mut EditorSession) -> io::Result<()> { redraw |= session.poll(); if redraw { draw_frame(&mut terminal, &mut output, |frame| { - viewports.draw(frame, session.editor()) + viewports.draw_session(frame, session) })?; redraw = false; } @@ -708,6 +720,7 @@ fn draw( struct DiffGeometry { regions: [(Rect, Focus); 3], + viewport: Option<(u64, usize, usize, usize)>, } fn diff_panel(frame: &mut Frame, area: Rect, title: &str, focused: bool) -> Rect { @@ -1195,6 +1208,7 @@ fn draw_diff( (Rect::default(), Focus::Before), (Rect::default(), Focus::After), ], + viewport: None, }); if let Some(editor) = editor { draw_diff_popups(frame, editor, area, None, panels); @@ -1348,6 +1362,9 @@ fn draw_diff( (before_area, Focus::Before), (after_area, Focus::After), ], + viewport: diff + .comparison_id + .map(|id| (id, diff.file, top, after_left)), }); if let Some(picker) = &diff.files && diff.files_visible @@ -3445,6 +3462,113 @@ mod tests { assert!(editor.dirty()); } + #[test] + fn session_diff_draw_retains_rendered_offsets_until_caret_crosses_edges() { + let directory = Directory::new(); + let original = (0..80) + .map(|row| format!("old row {row:03} {}", "x".repeat(100))) + .collect::>() + .join("\n"); + let path = directory.file("source.txt", &original); + let git = |args: &[&str]| { + let output = std::process::Command::new("git") + .args(args) + .current_dir(&directory.0) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_AUTHOR_NAME", "Diff fixture") + .env("GIT_AUTHOR_EMAIL", "fixture@example.invalid") + .env("GIT_COMMITTER_NAME", "Diff fixture") + .env("GIT_COMMITTER_EMAIL", "fixture@example.invalid") + .env("GIT_AUTHOR_DATE", "2000-01-01T00:00:00Z") + .env("GIT_COMMITTER_DATE", "2000-01-01T00:00:00Z") + .output() + .unwrap(); + assert!(output.status.success(), "{output:?}"); + }; + git(&["init", "-q"]); + git(&["add", "."]); + git(&["commit", "-qm", "base"]); + directory.file("source.txt", &original.replace("old row", "new row")); + let mut session = EditorSession::open(Some(path), vec![], None).unwrap(); + let ready = |session: &mut EditorSession, phase| { + let deadline = std::time::Instant::now() + Duration::from_secs(10); + loop { + session.poll(); + let diff = session.editor().diff().unwrap(); + assert!(diff.error.is_none(), "{:?}", diff.error); + if diff.phase == phase && !diff.loading { + break; + } + assert!(std::time::Instant::now() < deadline, "{diff:?}"); + std::thread::sleep(Duration::from_millis(1)); + } + }; + session.execute("diff"); + ready(&mut session, Phase::Before); + session.handle(Input::Down); + session.handle(Input::Enter); + session.handle(Input::Enter); + ready(&mut session, Phase::View); + session.handle(Input::Char('\t')); + session.handle(Input::Char('\t')); + assert!(session.editor().diff_editing()); + let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(120, 18)).unwrap(); + let mut views = BufferViewports::default(); + let mut render = |session: &mut EditorSession| { + terminal + .draw(|frame| views.draw_session(frame, session)) + .unwrap(); + terminal.get_cursor_position().unwrap() + }; + render(&mut session); + for _ in 0..40 { + session.handle(Input::Down); + render(&mut session); + } + let top = session.editor().diff().unwrap().top; + assert!(top > 0); + let bottom_caret = render(&mut session); + session.handle(Input::Up); + let reversed_caret = render(&mut session); + assert_eq!(session.editor().diff().unwrap().top, top); + assert_eq!(reversed_caret.y + 1, bottom_caret.y); + while session + .editor() + .diff() + .unwrap() + .after_display_row(session.editor().cursor().row) + .unwrap() + > top + { + session.handle(Input::Up); + render(&mut session); + assert_eq!(session.editor().diff().unwrap().top, top); + } + session.handle(Input::Up); + render(&mut session); + assert!(session.editor().diff().unwrap().top < top); + for _ in 0..80 { + session.handle(Input::Right); + render(&mut session); + } + let left = session.editor().diff().unwrap().left_after; + assert!(left > 0); + let right_caret = render(&mut session); + session.handle(Input::Left); + let reversed_caret = render(&mut session); + assert_eq!(session.editor().diff().unwrap().left_after, left); + assert_eq!(reversed_caret.x + 1, right_caret.x); + while session.editor().cursor().column > left { + session.handle(Input::Left); + render(&mut session); + assert_eq!(session.editor().diff().unwrap().left_after, left); + } + session.handle(Input::Left); + render(&mut session); + assert!(session.editor().diff().unwrap().left_after < left); + } + #[test] fn diff_wheel_routes_right_tree_and_both_panes_without_touching_document() { let directory = Directory::new();