From 610968815c1bac8e1bcc6d19d2ad4cbc623223e0 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 3 May 2024 10:29:30 -0600 Subject: [PATCH] Fix backwards mouse selection in vim mode (#11329) Fixes: #8492 Release Notes: - vim: Fixed last character of reversed mouse selections (#8492) --- crates/editor/src/hover_links.rs | 26 +++++++++---------- crates/gpui/src/app/test_context.rs | 40 +++++++++++++++++++++++++++-- crates/vim/src/test.rs | 18 ++++++++++++- crates/vim/src/vim.rs | 32 +++++++++++++---------- 4 files changed, 87 insertions(+), 29 deletions(-) diff --git a/crates/editor/src/hover_links.rs b/crates/editor/src/hover_links.rs index fb6a5c3126..07d41db38b 100644 --- a/crates/editor/src/hover_links.rs +++ b/crates/editor/src/hover_links.rs @@ -732,7 +732,7 @@ mod tests { cx.cx .cx - .simulate_mouse_move(screen_coord.unwrap(), Modifiers::command_shift()); + .simulate_mouse_move(screen_coord.unwrap(), None, Modifiers::command_shift()); requests.next().await; cx.run_until_parked(); @@ -802,7 +802,7 @@ mod tests { ]))) }); - cx.simulate_mouse_move(hover_point, Modifiers::secondary_key()); + cx.simulate_mouse_move(hover_point, None, Modifiers::secondary_key()); requests.next().await; cx.background_executor.run_until_parked(); cx.assert_editor_text_highlights::(indoc! {" @@ -828,7 +828,7 @@ mod tests { ]))) }); - cx.simulate_mouse_move(hover_point, Modifiers::secondary_key()); + cx.simulate_mouse_move(hover_point, None, Modifiers::secondary_key()); requests.next().await; cx.background_executor.run_until_parked(); cx.assert_editor_text_highlights::(indoc! {" @@ -847,7 +847,7 @@ mod tests { // No definitions returned Ok(Some(lsp::GotoDefinitionResponse::Link(vec![]))) }); - cx.simulate_mouse_move(hover_point, Modifiers::secondary_key()); + cx.simulate_mouse_move(hover_point, None, Modifiers::secondary_key()); requests.next().await; cx.background_executor.run_until_parked(); @@ -863,7 +863,7 @@ mod tests { fn test() { do_work(); } fn do_work() { teˇst(); } "}); - cx.simulate_mouse_move(hover_point, Modifiers::none()); + cx.simulate_mouse_move(hover_point, None, Modifiers::none()); // Assert no link highlights cx.assert_editor_text_highlights::(indoc! {" @@ -907,7 +907,7 @@ mod tests { fn do_work() { test(); } "}); - cx.simulate_mouse_move(hover_point, Modifiers::secondary_key()); + cx.simulate_mouse_move(hover_point, None, Modifiers::secondary_key()); cx.background_executor.run_until_parked(); cx.assert_editor_text_highlights::(indoc! {" fn test() { do_work(); } @@ -919,7 +919,7 @@ mod tests { fn test() { do_work(); } fn do_work() { tesˇt(); } "}); - cx.simulate_mouse_move(hover_point, Modifiers::secondary_key()); + cx.simulate_mouse_move(hover_point, None, Modifiers::secondary_key()); cx.background_executor.run_until_parked(); cx.assert_editor_text_highlights::(indoc! {" fn test() { do_work(); } @@ -1009,7 +1009,7 @@ mod tests { s.set_pending_anchor_range(anchor_range, crate::SelectMode::Character) }); }); - cx.simulate_mouse_move(hover_point, Modifiers::secondary_key()); + cx.simulate_mouse_move(hover_point, None, Modifiers::secondary_key()); cx.background_executor.run_until_parked(); assert!(requests.try_next().is_err()); cx.assert_editor_text_highlights::(indoc! {" @@ -1123,7 +1123,7 @@ mod tests { }); // Press cmd to trigger highlight let hover_point = cx.pixel_position_for(midpoint); - cx.simulate_mouse_move(hover_point, Modifiers::secondary_key()); + cx.simulate_mouse_move(hover_point, None, Modifiers::secondary_key()); cx.background_executor.run_until_parked(); cx.update_editor(|editor, cx| { let snapshot = editor.snapshot(cx); @@ -1142,7 +1142,7 @@ mod tests { assert_set_eq!(actual_highlights, vec![&expected_highlight]); }); - cx.simulate_mouse_move(hover_point, Modifiers::none()); + cx.simulate_mouse_move(hover_point, None, Modifiers::none()); // Assert no link highlights cx.update_editor(|editor, cx| { let snapshot = editor.snapshot(cx); @@ -1186,7 +1186,7 @@ mod tests { Let's test a [complex](https://zed.dev/channel/had-(ˇoops)) case. "}); - cx.simulate_mouse_move(screen_coord, Modifiers::secondary_key()); + cx.simulate_mouse_move(screen_coord, None, Modifiers::secondary_key()); cx.assert_editor_text_highlights::(indoc! {" Let's test a [complex](«https://zed.dev/channel/had-(oops)ˇ») case. "}); @@ -1214,7 +1214,7 @@ mod tests { let screen_coord = cx.pixel_position(indoc! {"https://zed.dev/relˇeases is a cool webpage."}); - cx.simulate_mouse_move(screen_coord, Modifiers::secondary_key()); + cx.simulate_mouse_move(screen_coord, None, Modifiers::secondary_key()); cx.assert_editor_text_highlights::( indoc! {"«https://zed.dev/releasesˇ» is a cool webpage."}, ); @@ -1239,7 +1239,7 @@ mod tests { let screen_coord = cx.pixel_position(indoc! {"A cool webpage is https://zed.dev/releˇases"}); - cx.simulate_mouse_move(screen_coord, Modifiers::secondary_key()); + cx.simulate_mouse_move(screen_coord, None, Modifiers::secondary_key()); cx.assert_editor_text_highlights::( indoc! {"A cool webpage is «https://zed.dev/releasesˇ»"}, ); diff --git a/crates/gpui/src/app/test_context.rs b/crates/gpui/src/app/test_context.rs index df622b5382..98512e6d81 100644 --- a/crates/gpui/src/app/test_context.rs +++ b/crates/gpui/src/app/test_context.rs @@ -685,11 +685,47 @@ impl VisualTestContext { } /// Simulate a mouse move event to the given point - pub fn simulate_mouse_move(&mut self, position: Point, modifiers: Modifiers) { + pub fn simulate_mouse_move( + &mut self, + position: Point, + button: impl Into>, + modifiers: Modifiers, + ) { self.simulate_event(MouseMoveEvent { position, modifiers, - pressed_button: None, + pressed_button: button.into(), + }) + } + + /// Simulate a mouse down event to the given point + pub fn simulate_mouse_down( + &mut self, + position: Point, + button: MouseButton, + modifiers: Modifiers, + ) { + self.simulate_event(MouseDownEvent { + position, + modifiers, + button, + click_count: 1, + first_mouse: false, + }) + } + + /// Simulate a mouse up event to the given point + pub fn simulate_mouse_up( + &mut self, + position: Point, + button: MouseButton, + modifiers: Modifiers, + ) { + self.simulate_event(MouseUpEvent { + position, + modifiers, + button, + click_count: 1, }) } diff --git a/crates/vim/src/test.rs b/crates/vim/src/test.rs index d2b516b613..29aefbae76 100644 --- a/crates/vim/src/test.rs +++ b/crates/vim/src/test.rs @@ -8,7 +8,7 @@ use std::time::Duration; use command_palette::CommandPalette; use editor::DisplayPoint; use futures::StreamExt; -use gpui::KeyBinding; +use gpui::{KeyBinding, Modifiers, MouseButton, TestAppContext}; pub use neovim_backed_binding_test_context::*; pub use neovim_backed_test_context::*; pub use vim_test_context::*; @@ -1057,3 +1057,19 @@ async fn test_undo(cx: &mut gpui::TestAppContext) { 3"}) .await; } + +#[gpui::test] +async fn test_mouse_selection(cx: &mut TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + + cx.set_state("ˇone two three", Mode::Normal); + + let start_point = cx.pixel_position("one twˇo three"); + let end_point = cx.pixel_position("one ˇtwo three"); + + cx.simulate_mouse_down(start_point, MouseButton::Left, Modifiers::none()); + cx.simulate_mouse_move(end_point, MouseButton::Left, Modifiers::none()); + cx.simulate_mouse_up(end_point, MouseButton::Left, Modifiers::none()); + + cx.assert_state("one «ˇtwo» three", Mode::Visual) +} diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 4d0837247d..95645f83de 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -21,13 +21,13 @@ use collections::HashMap; use command_palette_hooks::{CommandPaletteFilter, CommandPaletteInterceptor}; use editor::{ movement::{self, FindRange}, - Anchor, Editor, EditorEvent, EditorMode, + Anchor, Bias, Editor, EditorEvent, EditorMode, ToPoint, }; use gpui::{ actions, impl_actions, Action, AppContext, EntityId, FocusableView, Global, KeystrokeEvent, Subscription, View, ViewContext, WeakView, WindowContext, }; -use language::{CursorShape, Point, Selection, SelectionGoal, TransactionId}; +use language::{CursorShape, Point, SelectionGoal, TransactionId}; pub use mode_indicator::ModeIndicator; use motion::Motion; use normal::normal_replace; @@ -239,12 +239,9 @@ impl Vim { self.active_editor = Some(editor.clone().downgrade()); self.editor_subscription = Some(cx.subscribe(&editor, |editor, event, cx| match event { EditorEvent::SelectionsChanged { local: true } => { - let editor = editor.read(cx); - if editor.leader_peer_id().is_none() { - let newest = editor.selections.newest_anchor().clone(); - let is_multicursor = editor.selections.count() > 1; + if editor.read(cx).leader_peer_id().is_none() { Vim::update(cx, |vim, cx| { - vim.local_selections_changed(newest, is_multicursor, cx); + vim.local_selections_changed(editor, cx); }) } } @@ -455,6 +452,17 @@ impl Vim { s.select_anchor_ranges(vec![pos..pos]) } + let snapshot = s.display_map(); + if let Some(pending) = s.pending.as_mut() { + if pending.selection.reversed && mode.is_visual() && !last_mode.is_visual() { + let mut end = pending.selection.end.to_point(&snapshot.buffer_snapshot); + end = snapshot + .buffer_snapshot + .clip_point(end + Point::new(0, 1), Bias::Right); + pending.selection.end = snapshot.buffer_snapshot.anchor_before(end); + } + } + s.move_with(|map, selection| { if last_mode.is_visual() && !mode.is_visual() { let mut point = selection.head(); @@ -601,12 +609,10 @@ impl Vim { self.switch_mode(Mode::Normal, true, cx) } - fn local_selections_changed( - &mut self, - newest: Selection, - is_multicursor: bool, - cx: &mut WindowContext, - ) { + fn local_selections_changed(&mut self, editor: View, cx: &mut WindowContext) { + let newest = editor.read(cx).selections.newest_anchor().clone(); + let is_multicursor = editor.read(cx).selections.count() > 1; + let state = self.state(); if state.mode == Mode::Insert && state.current_tx.is_some() { if state.current_anchor.is_none() {