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

@@ -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::<HoveredLinkState>(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::<HoveredLinkState>(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::<HoveredLinkState>(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::<HoveredLinkState>(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::<HoveredLinkState>(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::<HoveredLinkState>(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::<HoveredLinkState>(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::<HoveredLinkState>(
             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::<HoveredLinkState>(
             indoc! {"A cool webpage is «https://zed.dev/releasesˇ»"},
         );

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<Pixels>, modifiers: Modifiers) {
+    pub fn simulate_mouse_move(
+        &mut self,
+        position: Point<Pixels>,
+        button: impl Into<Option<MouseButton>>,
+        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<Pixels>,
+        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<Pixels>,
+        button: MouseButton,
+        modifiers: Modifiers,
+    ) {
+        self.simulate_event(MouseUpEvent {
+            position,
+            modifiers,
+            button,
+            click_count: 1,
         })
     }
 

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)
+}

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<editor::Anchor>,
-        is_multicursor: bool,
-        cx: &mut WindowContext,
-    ) {
+    fn local_selections_changed(&mut self, editor: View<Editor>, 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() {