From 3af7b644a6114eb325ec361b780bcc80007cc29d Mon Sep 17 00:00:00 2001 From: Xiaobo Liu Date: Wed, 18 Mar 2026 18:59:46 +0800 Subject: [PATCH] vim: Fix visual block Shift-I jumping to original cursor after Ctrl-D scroll (#50822) Ensure that the `vim::visual::Vim::visual_block_motion` method is called when `scroll_editor` is called while in Visual Block mode. This fixes an issue where, after getting into `Visual Block` mode, if the user used `ctrl-d` or `ctrl-u` to scroll half of the page, the wrong selection would be made and, if `shift-i` was used to start inserting at the start of the line, a single cursor would be placed at the cursor position where the selection was started, instead of one cursor per each of the selected lines. These changes ensure that we now match Neovim's behavior for the same flow. --------- Signed-off-by: Xiaobo Liu Co-authored-by: dino --- crates/vim/src/normal/scroll.rs | 164 ++++++++++-------- crates/vim/src/visual.rs | 32 ++++ ...sual_block_insert_after_ctrl_d_scroll.json | 10 ++ 3 files changed, 130 insertions(+), 76 deletions(-) create mode 100644 crates/vim/test_data/test_visual_block_insert_after_ctrl_d_scroll.json diff --git a/crates/vim/src/normal/scroll.rs b/crates/vim/src/normal/scroll.rs index 9d61aea9525b939271631feb6d493df2871a9607..01719cd59325f35474f10775488fd2aea4f38e41 100644 --- a/crates/vim/src/normal/scroll.rs +++ b/crates/vim/src/normal/scroll.rs @@ -88,82 +88,74 @@ pub fn register(editor: &mut Editor, cx: &mut Context) { impl Vim { fn scroll( &mut self, - move_cursor: bool, + preserve_cursor_position: bool, window: &mut Window, cx: &mut Context, by: fn(c: Option) -> ScrollAmount, ) { let amount = by(Vim::take_count(cx).map(|c| c as f32)); - let mode = self.mode; Vim::take_forced_motion(cx); self.exit_temporary_normal(window, cx); - self.update_editor(cx, |_, editor, cx| { - scroll_editor(editor, mode, move_cursor, amount, window, cx) - }); + self.scroll_editor(preserve_cursor_position, amount, window, cx); } -} -fn scroll_editor( - editor: &mut Editor, - mode: Mode, - preserve_cursor_position: bool, - amount: ScrollAmount, - window: &mut Window, - cx: &mut Context, -) { - let should_move_cursor = editor.newest_selection_on_screen(cx).is_eq(); - let display_snapshot = editor.display_map.update(cx, |map, cx| map.snapshot(cx)); - let old_top = editor - .scroll_manager - .scroll_top_display_point(&display_snapshot, cx); - - if editor.scroll_hover(amount, window, cx) { - return; - } + fn scroll_editor( + &mut self, + preserve_cursor_position: bool, + amount: ScrollAmount, + window: &mut Window, + cx: &mut Context, + ) { + self.update_editor(cx, |vim, editor, cx| { + let should_move_cursor = editor.newest_selection_on_screen(cx).is_eq(); + let display_snapshot = editor.display_map.update(cx, |map, cx| map.snapshot(cx)); + let old_top = editor + .scroll_manager + .scroll_top_display_point(&display_snapshot, cx); + + if editor.scroll_hover(amount, window, cx) { + return; + } - let full_page_up = amount.is_full_page() && amount.direction().is_upwards(); - let amount = match (amount.is_full_page(), editor.visible_line_count()) { - (true, Some(visible_line_count)) => { - if amount.direction().is_upwards() { - ScrollAmount::Line((amount.lines(visible_line_count) + 1.0) as f32) - } else { - ScrollAmount::Line((amount.lines(visible_line_count) - 1.0) as f32) + let full_page_up = amount.is_full_page() && amount.direction().is_upwards(); + let amount = match (amount.is_full_page(), editor.visible_line_count()) { + (true, Some(visible_line_count)) => { + if amount.direction().is_upwards() { + ScrollAmount::Line((amount.lines(visible_line_count) + 1.0) as f32) + } else { + ScrollAmount::Line((amount.lines(visible_line_count) - 1.0) as f32) + } + } + _ => amount, + }; + + editor.scroll_screen(&amount, window, cx); + if !should_move_cursor { + return; } - } - _ => amount, - }; - editor.scroll_screen(&amount, window, cx); - if !should_move_cursor { - return; - } + let Some(visible_line_count) = editor.visible_line_count() else { + return; + }; - let Some(visible_line_count) = editor.visible_line_count() else { - return; - }; + let Some(visible_column_count) = editor.visible_column_count() else { + return; + }; - let Some(visible_column_count) = editor.visible_column_count() else { - return; - }; + let display_snapshot = editor.display_map.update(cx, |map, cx| map.snapshot(cx)); + let top = editor + .scroll_manager + .scroll_top_display_point(&display_snapshot, cx); + let vertical_scroll_margin = EditorSettings::get_global(cx).vertical_scroll_margin; - let display_snapshot = editor.display_map.update(cx, |map, cx| map.snapshot(cx)); - let top = editor - .scroll_manager - .scroll_top_display_point(&display_snapshot, cx); - let vertical_scroll_margin = EditorSettings::get_global(cx).vertical_scroll_margin; - - editor.change_selections( - SelectionEffects::no_scroll().nav_history(false), - window, - cx, - |s| { - s.move_with(&mut |map, selection| { + let mut move_cursor = |map: &editor::display_map::DisplaySnapshot, + mut head: DisplayPoint, + goal: SelectionGoal| { // TODO: Improve the logic and function calls below to be dependent on // the `amount`. If the amount is vertical, we don't care about // columns, while if it's horizontal, we don't care about rows, // so we don't need to calculate both and deal with logic for // both. - let mut head = selection.head(); let max_point = map.max_point(); let starting_column = head.column(); @@ -171,17 +163,18 @@ fn scroll_editor( (vertical_scroll_margin as u32).min(visible_line_count as u32 / 2); if preserve_cursor_position { - let new_row = if old_top.row() == top.row() { - DisplayRow( - head.row() - .0 - .saturating_add_signed(amount.lines(visible_line_count) as i32), - ) - } else { - DisplayRow(top.row().0.saturating_add_signed( - selection.head().row().0 as i32 - old_top.row().0 as i32, - )) - }; + let new_row = + if old_top.row() == top.row() { + DisplayRow( + head.row() + .0 + .saturating_add_signed(amount.lines(visible_line_count) as i32), + ) + } else { + DisplayRow(top.row().0.saturating_add_signed( + head.row().0 as i32 - old_top.row().0 as i32, + )) + }; head = map.clip_point(DisplayPoint::new(new_row, head.column()), Bias::Left) } @@ -259,17 +252,36 @@ fn scroll_editor( let new_head = map.clip_point(DisplayPoint::new(new_row, new_column), Bias::Left); let goal = match amount { ScrollAmount::Column(_) | ScrollAmount::PageWidth(_) => SelectionGoal::None, - _ => selection.goal, + _ => goal, }; - if selection.is_empty() || !mode.is_visual() { - selection.collapse_to(new_head, goal) - } else { - selection.set_head(new_head, goal) - }; - }) - }, - ); + Some((new_head, goal)) + }; + + if vim.mode == Mode::VisualBlock { + vim.visual_block_motion(true, editor, window, cx, &mut move_cursor); + } else { + editor.change_selections( + SelectionEffects::no_scroll().nav_history(false), + window, + cx, + |s| { + s.move_with(&mut |map, selection| { + if let Some((new_head, goal)) = + move_cursor(map, selection.head(), selection.goal) + { + if selection.is_empty() || !vim.mode.is_visual() { + selection.collapse_to(new_head, goal) + } else { + selection.set_head(new_head, goal) + } + } + }) + }, + ); + } + }); + } } #[cfg(test)] diff --git a/crates/vim/src/visual.rs b/crates/vim/src/visual.rs index 889e3468f2ef6eaa290b6e0aec1971cd2e9ad813..502aa756b67889b1171464fde11be08ff0ccd508 100644 --- a/crates/vim/src/visual.rs +++ b/crates/vim/src/visual.rs @@ -1561,6 +1561,38 @@ mod test { }); } + #[gpui::test] + async fn test_visual_block_insert_after_ctrl_d_scroll(cx: &mut gpui::TestAppContext) { + let mut cx = NeovimBackedTestContext::new(cx).await; + let shared_state_lines = (1..=10) + .map(|line_number| format!("{line_number:02}")) + .collect::>() + .join("\n"); + let shared_state = format!("ˇ{shared_state_lines}\n"); + + cx.set_scroll_height(5).await; + cx.set_shared_state(&shared_state).await; + + cx.simulate_shared_keystrokes("ctrl-v ctrl-d").await; + cx.shared_state().await.assert_matches(); + + cx.simulate_shared_keystrokes("shift-i x escape").await; + cx.shared_state().await.assert_eq(indoc! { + " + ˇx01 + x02 + x03 + x04 + x05 + 06 + 07 + 08 + 09 + 10 + " + }); + } + #[gpui::test] async fn test_visual_block_wrapping_selection(cx: &mut gpui::TestAppContext) { let mut cx = NeovimBackedTestContext::new(cx).await; diff --git a/crates/vim/test_data/test_visual_block_insert_after_ctrl_d_scroll.json b/crates/vim/test_data/test_visual_block_insert_after_ctrl_d_scroll.json new file mode 100644 index 0000000000000000000000000000000000000000..ddad34e9ad6b11d5f29670f46903e6daf4082215 --- /dev/null +++ b/crates/vim/test_data/test_visual_block_insert_after_ctrl_d_scroll.json @@ -0,0 +1,10 @@ +{"SetOption":{"value":"scrolloff=3"}} +{"SetOption":{"value":"lines=7"}} +{"Put":{"state":"ˇ01\n02\n03\n04\n05\n06\n07\n08\n09\n10\n"}} +{"Key":"ctrl-v"} +{"Key":"ctrl-d"} +{"Get":{"state":"«0ˇ»1\n«0ˇ»2\n«0ˇ»3\n«0ˇ»4\n«0ˇ»5\n06\n07\n08\n09\n10\n","mode":"VisualBlock"}} +{"Key":"shift-i"} +{"Key":"x"} +{"Key":"escape"} +{"Get":{"state":"ˇx01\nx02\nx03\nx04\nx05\n06\n07\n08\n09\n10\n","mode":"Normal"}}