From a4fe57d607b9fde42804aa12a5eb8acf1b61f439 Mon Sep 17 00:00:00 2001 From: lex00 <121451605+lex00@users.noreply.github.com> Date: Mon, 2 Feb 2026 06:49:28 -0700 Subject: [PATCH] Fix vim method/comment navigation with expanded diff hunks (#47976) Previously, `]m`/`[m` (method) and `]/`/`[/` (comment) motions would navigate to incorrect positions when diff hunks were expanded. This was caused by extracting raw `usize` values from `MultiBufferOffset` and operating directly on the underlying buffer, which doesn't account for expanded diff hunk content. The fix properly uses `MultiBufferOffset` throughout and queries `text_object_ranges` on the `MultiBufferSnapshot` instead of the underlying buffer, ensuring correct coordinate mapping when diff content is displayed inline. Fixes #46612 Release Notes: - Fixed vim method and comment navigation (`] m`, `[ m`, `] shift-m`, `[ shift-m`, `] /`, `[ /`) incorrectly positioning cursor when diff hunks are expanded --------- Co-authored-by: Claude Opus 4.5 Co-authored-by: dino --- crates/vim/src/motion.rs | 236 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 218 insertions(+), 18 deletions(-) diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs index 7447ba14ea46b06887a7caf8a4fbb13d5360c6cf..d309b0f665e31d72f55ab9d458e703988855ced6 100644 --- a/crates/vim/src/motion.rs +++ b/crates/vim/src/motion.rs @@ -2962,20 +2962,22 @@ fn method_motion( direction: Direction, is_start: bool, ) -> DisplayPoint { - let Some((_, _, buffer)) = map.buffer_snapshot().as_singleton() else { + let snapshot = map.buffer_snapshot(); + if snapshot.as_singleton().is_none() { return display_point; - }; + } for _ in 0..times { - let point = map.display_point_to_point(display_point, Bias::Left); - let offset = point.to_offset(&map.buffer_snapshot()).0; + let offset = map + .display_point_to_point(display_point, Bias::Left) + .to_offset(&snapshot); let range = if direction == Direction::Prev { - 0..offset + MultiBufferOffset(0)..offset } else { - offset..buffer.len() + offset..snapshot.len() }; - let possibilities = buffer + let possibilities = snapshot .text_object_ranges(range, language::TreeSitterOptions::max_start_depth(4)) .filter_map(|(range, object)| { if !matches!(object, language::TextObject::AroundFunction) { @@ -2985,7 +2987,7 @@ fn method_motion( let relevant = if is_start { range.start } else { range.end }; if direction == Direction::Prev && relevant < offset { Some(relevant) - } else if direction == Direction::Next && relevant > offset + 1 { + } else if direction == Direction::Next && relevant > offset + 1usize { Some(relevant) } else { None @@ -2997,7 +2999,7 @@ fn method_motion( } else { possibilities.min().unwrap_or(offset) }; - let new_point = map.clip_point(MultiBufferOffset(dest).to_display_point(map), Bias::Left); + let new_point = map.clip_point(dest.to_display_point(map), Bias::Left); if new_point == display_point { break; } @@ -3012,20 +3014,22 @@ fn comment_motion( times: usize, direction: Direction, ) -> DisplayPoint { - let Some((_, _, buffer)) = map.buffer_snapshot().as_singleton() else { + let snapshot = map.buffer_snapshot(); + if snapshot.as_singleton().is_none() { return display_point; - }; + } for _ in 0..times { - let point = map.display_point_to_point(display_point, Bias::Left); - let offset = point.to_offset(&map.buffer_snapshot()).0; + let offset = map + .display_point_to_point(display_point, Bias::Left) + .to_offset(&snapshot); let range = if direction == Direction::Prev { - 0..offset + MultiBufferOffset(0)..offset } else { - offset..buffer.len() + offset..snapshot.len() }; - let possibilities = buffer + let possibilities = snapshot .text_object_ranges(range, language::TreeSitterOptions::max_start_depth(6)) .filter_map(|(range, object)| { if !matches!(object, language::TextObject::AroundComment) { @@ -3039,7 +3043,7 @@ fn comment_motion( }; if direction == Direction::Prev && relevant < offset { Some(relevant) - } else if direction == Direction::Next && relevant > offset + 1 { + } else if direction == Direction::Next && relevant > offset + 1usize { Some(relevant) } else { None @@ -3051,7 +3055,7 @@ fn comment_motion( } else { possibilities.min().unwrap_or(offset) }; - let new_point = map.clip_point(MultiBufferOffset(dest).to_display_point(map), Bias::Left); + let new_point = map.clip_point(dest.to_display_point(map), Bias::Left); if new_point == display_point { break; } @@ -5015,4 +5019,200 @@ mod test { cx.simulate_keystrokes("g e"); cx.assert_state("foˇo;bar", Mode::Normal); } + + #[gpui::test] + async fn test_method_motion_with_expanded_diff_hunks(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + + let diff_base = indoc! {r#" + fn first() { + println!("first"); + println!("removed line"); + } + + fn second() { + println!("second"); + } + + fn third() { + println!("third"); + } + "#}; + + let current_text = indoc! {r#" + fn first() { + println!("first"); + } + + fn second() { + println!("second"); + } + + fn third() { + println!("third"); + } + "#}; + + cx.set_state(&format!("ˇ{}", current_text), Mode::Normal); + cx.set_head_text(diff_base); + cx.update_editor(|editor, window, cx| { + editor.expand_all_diff_hunks(&editor::actions::ExpandAllDiffHunks, window, cx); + }); + + // When diff hunks are expanded, the deleted line from the diff base + // appears in the MultiBuffer. The method motion should correctly + // navigate to the second function even with this extra content. + cx.simulate_keystrokes("] m"); + cx.assert_editor_state(indoc! {r#" + fn first() { + println!("first"); + println!("removed line"); + } + + ˇfn second() { + println!("second"); + } + + fn third() { + println!("third"); + } + "#}); + + cx.simulate_keystrokes("] m"); + cx.assert_editor_state(indoc! {r#" + fn first() { + println!("first"); + println!("removed line"); + } + + fn second() { + println!("second"); + } + + ˇfn third() { + println!("third"); + } + "#}); + + cx.simulate_keystrokes("[ m"); + cx.assert_editor_state(indoc! {r#" + fn first() { + println!("first"); + println!("removed line"); + } + + ˇfn second() { + println!("second"); + } + + fn third() { + println!("third"); + } + "#}); + + cx.simulate_keystrokes("[ m"); + cx.assert_editor_state(indoc! {r#" + ˇfn first() { + println!("first"); + println!("removed line"); + } + + fn second() { + println!("second"); + } + + fn third() { + println!("third"); + } + "#}); + } + + #[gpui::test] + async fn test_comment_motion_with_expanded_diff_hunks(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + + let diff_base = indoc! {r#" + // first comment + fn first() { + // removed comment + println!("first"); + } + + // second comment + fn second() { println!("second"); } + "#}; + + let current_text = indoc! {r#" + // first comment + fn first() { + println!("first"); + } + + // second comment + fn second() { println!("second"); } + "#}; + + cx.set_state(&format!("ˇ{}", current_text), Mode::Normal); + cx.set_head_text(diff_base); + cx.update_editor(|editor, window, cx| { + editor.expand_all_diff_hunks(&editor::actions::ExpandAllDiffHunks, window, cx); + }); + + // The first `] /` (vim::NextComment) should go to the end of the first + // comment. + cx.simulate_keystrokes("] /"); + cx.assert_editor_state(indoc! {r#" + // first commenˇt + fn first() { + // removed comment + println!("first"); + } + + // second comment + fn second() { println!("second"); } + "#}); + + // The next `] /` (vim::NextComment) should go to the end of the second + // comment, skipping over the removed comment, since it's not in the + // actual buffer. + cx.simulate_keystrokes("] /"); + cx.assert_editor_state(indoc! {r#" + // first comment + fn first() { + // removed comment + println!("first"); + } + + // second commenˇt + fn second() { println!("second"); } + "#}); + + // Going back to previous comment with `[ /` (vim::PreviousComment) + // should go back to the start of the second comment. + cx.simulate_keystrokes("[ /"); + cx.assert_editor_state(indoc! {r#" + // first comment + fn first() { + // removed comment + println!("first"); + } + + ˇ// second comment + fn second() { println!("second"); } + "#}); + + // Going back again with `[ /` (vim::PreviousComment) should finally put + // the cursor at the start of the first comment. + cx.simulate_keystrokes("[ /"); + cx.assert_editor_state(indoc! {r#" + ˇ// first comment + fn first() { + // removed comment + println!("first"); + } + + // second comment + fn second() { println!("second"); } + "#}); + } }