From be6cd3e5f70438f36b5a483f30cd84977e78073e Mon Sep 17 00:00:00 2001 From: Josh Robson Chase Date: Thu, 26 Mar 2026 12:31:18 -0400 Subject: [PATCH] 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 --- assets/keymaps/vim.json | 2 + crates/vim/src/helix.rs | 85 ++++++++++++++++++++++++++++++++++++++++ crates/vim/src/normal.rs | 34 ++++++++++------ crates/vim/src/vim.rs | 4 +- 4 files changed, 110 insertions(+), 15 deletions(-) diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 5da15a1c1f304743c55e87ecc208fd6adbdc7cc2..ae0a0dd0f1ef3ba99814b39db6ec3932d0ef3730 100644 --- a/assets/keymaps/vim.json +++ b/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", diff --git a/crates/vim/src/helix.rs b/crates/vim/src/helix.rs index 56241275b5d8fa6de3645c6d00361b29dc49d259..c1e766c03a897facb3c7acf76b3ef7811e6910a8 100644 --- a/crates/vim/src/helix.rs +++ b/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; diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index 6763c5cddb8bf2cda6aa4fa0988ff6be67119d3c..118805586118e36269a1f0c1d1d619058133da30 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -731,10 +731,10 @@ impl Vim { .collect::>(); 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::(&editor.display_snapshot(cx)); let snapshot = editor.buffer().read(cx).snapshot(cx); let selection_end_rows: BTreeSet = 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::>(); 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); diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 1c2416dcdb4b9a4a06970c66aded0816faf21cd0..11cf59f590823068088308a74354badf3bacfbd1 100644 --- a/crates/vim/src/vim.rs +++ b/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());