helix: Fix insert line above/below with selection (#46492)

Fix Helix `o`/`O` behavior when a selection is active.

This updates `InsertLineAbove` and `InsertLineBelow` to use the
selection bounds correctly for Helix selections, including line
selections whose end is represented at column 0 of the following line.

It also adds Helix select-mode keybindings for `o` and `O`, and adds
tests covering both line selections and selections created via `v`.

Closes #43210

Release Notes:

- helix: Fixed insert line above/below behavior when a full line is
selected

---------

Co-authored-by: dino <dinojoaocosta@gmail.com>

assets/keymaps/vim.json

@@ -337,6 +337,8 @@
       "shift-j": "vim::JoinLines",
       "i": "vim::InsertBefore",
       "a": "vim::InsertAfter",
+      "o": "vim::InsertLineBelow",
+      "shift-o": "vim::InsertLineAbove",
       "p": "vim::Paste",
       "u": "vim::Undo",
       "r": "vim::PushReplace",

crates/vim/src/helix.rs

@@ -1898,6 +1898,91 @@ mod test {
         );
     }
 
+    #[gpui::test]
+    async fn test_helix_insert_before_after_select_lines(cx: &mut gpui::TestAppContext) {
+        let mut cx = VimTestContext::new(cx, true).await;
+
+        cx.set_state(
+            "line one\nline ˇtwo\nline three\nline four",
+            Mode::HelixNormal,
+        );
+        cx.simulate_keystrokes("2 x");
+        cx.assert_state(
+            "line one\n«line two\nline three\nˇ»line four",
+            Mode::HelixNormal,
+        );
+        cx.simulate_keystrokes("o");
+        cx.assert_state("line one\nline two\nline three\nˇ\nline four", Mode::Insert);
+
+        cx.set_state(
+            "line one\nline ˇtwo\nline three\nline four",
+            Mode::HelixNormal,
+        );
+        cx.simulate_keystrokes("2 x");
+        cx.assert_state(
+            "line one\n«line two\nline three\nˇ»line four",
+            Mode::HelixNormal,
+        );
+        cx.simulate_keystrokes("shift-o");
+        cx.assert_state("line one\nˇ\nline two\nline three\nline four", Mode::Insert);
+    }
+
+    #[gpui::test]
+    async fn test_helix_insert_before_after_helix_select(cx: &mut gpui::TestAppContext) {
+        let mut cx = VimTestContext::new(cx, true).await;
+        cx.enable_helix();
+
+        // Test new line in selection direction
+        cx.set_state(
+            "ˇline one\nline two\nline three\nline four",
+            Mode::HelixNormal,
+        );
+        cx.simulate_keystrokes("v j j");
+        cx.assert_state(
+            "«line one\nline two\nlˇ»ine three\nline four",
+            Mode::HelixSelect,
+        );
+        cx.simulate_keystrokes("o");
+        cx.assert_state("line one\nline two\nline three\nˇ\nline four", Mode::Insert);
+
+        cx.set_state(
+            "line one\nline two\nˇline three\nline four",
+            Mode::HelixNormal,
+        );
+        cx.simulate_keystrokes("v k k");
+        cx.assert_state(
+            "«ˇline one\nline two\nl»ine three\nline four",
+            Mode::HelixSelect,
+        );
+        cx.simulate_keystrokes("shift-o");
+        cx.assert_state("ˇ\nline one\nline two\nline three\nline four", Mode::Insert);
+
+        // Test new line in opposite selection direction
+        cx.set_state(
+            "ˇline one\nline two\nline three\nline four",
+            Mode::HelixNormal,
+        );
+        cx.simulate_keystrokes("v j j");
+        cx.assert_state(
+            "«line one\nline two\nlˇ»ine three\nline four",
+            Mode::HelixSelect,
+        );
+        cx.simulate_keystrokes("shift-o");
+        cx.assert_state("ˇ\nline one\nline two\nline three\nline four", Mode::Insert);
+
+        cx.set_state(
+            "line one\nline two\nˇline three\nline four",
+            Mode::HelixNormal,
+        );
+        cx.simulate_keystrokes("v k k");
+        cx.assert_state(
+            "«ˇline one\nline two\nl»ine three\nline four",
+            Mode::HelixSelect,
+        );
+        cx.simulate_keystrokes("o");
+        cx.assert_state("line one\nline two\nline three\nˇ\nline four", Mode::Insert);
+    }
+
     #[gpui::test]
     async fn test_helix_select_mode_motion(cx: &mut gpui::TestAppContext) {
         let mut cx = VimTestContext::new(cx, true).await;

crates/vim/src/normal.rs

@@ -731,10 +731,10 @@ impl Vim {
                     .collect::<Vec<_>>();
                 editor.edit_with_autoindent(edits, cx);
                 editor.change_selections(Default::default(), window, cx, |s| {
-                    s.move_cursors_with(&mut |map, cursor, _| {
-                        let previous_line = map.start_of_relative_buffer_row(cursor, -1);
+                    s.move_with(&mut |map, selection| {
+                        let previous_line = map.start_of_relative_buffer_row(selection.start, -1);
                         let insert_point = motion::end_of_line(map, false, previous_line, 1);
-                        (insert_point, SelectionGoal::None)
+                        selection.collapse_to(insert_point, SelectionGoal::None)
                     });
                 });
             });
@@ -750,14 +750,19 @@ impl Vim {
         self.start_recording(cx);
         self.switch_mode(Mode::Insert, false, window, cx);
         self.update_editor(cx, |_, editor, cx| {
-            let text_layout_details = editor.text_layout_details(window, cx);
             editor.transact(window, cx, |editor, window, cx| {
                 let selections = editor.selections.all::<Point>(&editor.display_snapshot(cx));
                 let snapshot = editor.buffer().read(cx).snapshot(cx);
 
                 let selection_end_rows: BTreeSet<u32> = selections
                     .into_iter()
-                    .map(|selection| selection.end.row)
+                    .map(|selection| {
+                        if !selection.is_empty() && selection.end.column == 0 {
+                            selection.end.row.saturating_sub(1)
+                        } else {
+                            selection.end.row
+                        }
+                    })
                     .collect();
                 let edits = selection_end_rows
                     .into_iter()
@@ -772,14 +777,17 @@ impl Vim {
                     })
                     .collect::<Vec<_>>();
                 editor.change_selections(Default::default(), window, cx, |s| {
-                    s.maybe_move_cursors_with(&mut |map, cursor, goal| {
-                        Motion::CurrentLine.move_point(
-                            map,
-                            cursor,
-                            goal,
-                            None,
-                            &text_layout_details,
-                        )
+                    s.move_with(&mut |map, selection| {
+                        let current_line = if !selection.is_empty() && selection.end.column() == 0 {
+                            // If this is an insert after a selection to the end of the line, the
+                            // cursor needs to be bumped back, because it'll be at the start of the
+                            // *next* line.
+                            map.start_of_relative_buffer_row(selection.end, -1)
+                        } else {
+                            selection.end
+                        };
+                        let insert_point = motion::end_of_line(map, false, current_line, 1);
+                        selection.collapse_to(insert_point, SelectionGoal::None)
                     });
                 });
                 editor.edit_with_autoindent(edits, cx);

crates/vim/src/vim.rs

@@ -1210,7 +1210,7 @@ impl Vim {
             return;
         }
 
-        if !mode.is_visual() && last_mode.is_visual() {
+        if !mode.is_visual() && last_mode.is_visual() && !last_mode.is_helix() {
             self.create_visual_marks(last_mode, window, cx);
         }
 
@@ -1277,7 +1277,7 @@ impl Vim {
                 }
 
                 s.move_with(&mut |map, selection| {
-                    if last_mode.is_visual() && !mode.is_visual() {
+                    if last_mode.is_visual() && !last_mode.is_helix() && !mode.is_visual() {
                         let mut point = selection.head();
                         if !selection.reversed && !selection.is_empty() {
                             point = movement::left(map, selection.head());