From 5139cc2bfb6380ef0520727f2da57771018529e9 Mon Sep 17 00:00:00 2001
From: AidanV <84053180+AidanV@users.noreply.github.com>
Date: Tue, 25 Nov 2025 01:20:01 -0800
Subject: [PATCH] helix: Fix `Vim::NextWordEnd` off-by-one in `HelixSelect`
(#43234)
Closes #43209
Closes #38121
Starting on the first character.
Running `v e` before changes:
Running `v e` after changes:
Change Notes:
- Added helix selection sanitation code that directly mirrors the code
in the Vim
[`visual_motion`](https://github.com/AidanV/zed/blob/b6728c080c5d14ded7002d0276deb5c19d42ed8a/crates/vim/src/visual.rs#L237)
method. I kept the comments from the Vim section that explains its
purpose.
- The above change converted the problem from fixing `v e` to fixing `v
w`. Since `w` is treated differently in Helix than in Vim (i.e. `w` in
Vim goes to the first character of a word and `w` in Helix goes to the
character before a word. Commented
[here](https://github.com/AidanV/zed/blob/b6728c080c5d14ded7002d0276deb5c19d42ed8a/crates/vim/src/helix.rs#L132)),
the code treats `w` in `HelixSelect` as a motion that differs from the
Vim motion in the same way that the function
[`helix_move_cursor`](https://github.com/AidanV/zed/blob/b6728c080c5d14ded7002d0276deb5c19d42ed8a/crates/vim/src/helix.rs#L353)
separates these behaviors.
- Added a regression test
Release Notes:
- Fixes bug where `Vim::NextWordEnd` in `HelixSelect` would not select
whole word.
---
crates/vim/src/helix.rs | 185 ++++++++++++++++++++++++++++++----------
1 file changed, 139 insertions(+), 46 deletions(-)
diff --git a/crates/vim/src/helix.rs b/crates/vim/src/helix.rs
index 67c99ff6aea249692bddc38d3681be5c491a7437..fae2bda578c6844c33290d059248b895ebde4c3d 100644
--- a/crates/vim/src/helix.rs
+++ b/crates/vim/src/helix.rs
@@ -109,19 +109,76 @@ impl Vim {
};
s.move_with(|map, selection| {
- let current_head = selection.head();
-
- let Some((new_head, goal)) = motion.move_point(
- map,
- current_head,
- selection.goal,
- times,
- &text_layout_details,
- ) else {
- return;
+ let was_reversed = selection.reversed;
+ let mut current_head = selection.head();
+
+ // our motions assume the current character is after the cursor,
+ // but in (forward) visual mode the current character is just
+ // before the end of the selection.
+
+ // If the file ends with a newline (which is common) we don't do this.
+ // so that if you go to the end of such a file you can use "up" to go
+ // to the previous line and have it work somewhat as expected.
+ if !selection.reversed
+ && !selection.is_empty()
+ && !(selection.end.column() == 0 && selection.end == map.max_point())
+ {
+ current_head = movement::left(map, selection.end)
+ }
+
+ let (new_head, goal) = match motion {
+ // Going to next word start is special cased
+ // since Vim differs from Helix in that motion
+ // Vim: `w` goes to the first character of a word
+ // Helix: `w` goes to the character before a word
+ Motion::NextWordStart { ignore_punctuation } => {
+ let mut head = movement::right(map, current_head);
+ let classifier =
+ map.buffer_snapshot().char_classifier_at(head.to_point(map));
+ for _ in 0..times.unwrap_or(1) {
+ let (_, new_head) =
+ movement::find_boundary_trail(map, head, |left, right| {
+ Self::is_boundary_right(ignore_punctuation)(
+ left,
+ right,
+ &classifier,
+ )
+ });
+ head = new_head;
+ }
+ head = movement::left(map, head);
+ (head, SelectionGoal::None)
+ }
+ _ => motion
+ .move_point(
+ map,
+ current_head,
+ selection.goal,
+ times,
+ &text_layout_details,
+ )
+ .unwrap_or((current_head, selection.goal)),
};
selection.set_head(new_head, goal);
+
+ // ensure the current character is included in the selection.
+ if !selection.reversed {
+ let next_point = movement::right(map, selection.end);
+
+ if !(next_point.column() == 0 && next_point == map.max_point()) {
+ selection.end = next_point;
+ }
+ }
+
+ // vim always ensures the anchor character stays selected.
+ // if our selection has reversed, we need to move the opposite end
+ // to ensure the anchor is still selected.
+ if was_reversed && !selection.reversed {
+ selection.start = movement::left(map, selection.start);
+ } else if !was_reversed && selection.reversed {
+ selection.end = movement::right(map, selection.end);
+ }
})
});
});
@@ -255,6 +312,30 @@ impl Vim {
});
}
+ fn is_boundary_right(
+ ignore_punctuation: bool,
+ ) -> impl FnMut(char, char, &CharClassifier) -> bool {
+ move |left, right, classifier| {
+ let left_kind = classifier.kind_with(left, ignore_punctuation);
+ let right_kind = classifier.kind_with(right, ignore_punctuation);
+ let at_newline = (left == '\n') ^ (right == '\n');
+
+ (left_kind != right_kind && right_kind != CharKind::Whitespace) || at_newline
+ }
+ }
+
+ fn is_boundary_left(
+ ignore_punctuation: bool,
+ ) -> impl FnMut(char, char, &CharClassifier) -> bool {
+ move |left, right, classifier| {
+ let left_kind = classifier.kind_with(left, ignore_punctuation);
+ let right_kind = classifier.kind_with(right, ignore_punctuation);
+ let at_newline = (left == '\n') ^ (right == '\n');
+
+ (left_kind != right_kind && left_kind != CharKind::Whitespace) || at_newline
+ }
+ }
+
pub fn helix_move_cursor(
&mut self,
motion: Motion,
@@ -263,6 +344,30 @@ impl Vim {
cx: &mut Context,
) {
match motion {
+ Motion::NextWordStart { ignore_punctuation } => self.helix_find_range_forward(
+ times,
+ window,
+ cx,
+ Self::is_boundary_right(ignore_punctuation),
+ ),
+ Motion::NextWordEnd { ignore_punctuation } => self.helix_find_range_forward(
+ times,
+ window,
+ cx,
+ Self::is_boundary_left(ignore_punctuation),
+ ),
+ Motion::PreviousWordStart { ignore_punctuation } => self.helix_find_range_backward(
+ times,
+ window,
+ cx,
+ Self::is_boundary_left(ignore_punctuation),
+ ),
+ Motion::PreviousWordEnd { ignore_punctuation } => self.helix_find_range_backward(
+ times,
+ window,
+ cx,
+ Self::is_boundary_right(ignore_punctuation),
+ ),
Motion::EndOfLine { .. } => {
// In Helix mode, EndOfLine should position cursor ON the last character,
// not after it. We therefore need special handling for it.
@@ -288,42 +393,6 @@ impl Vim {
});
});
}
- Motion::NextWordStart { ignore_punctuation } => {
- self.helix_find_range_forward(times, window, cx, |left, right, classifier| {
- let left_kind = classifier.kind_with(left, ignore_punctuation);
- let right_kind = classifier.kind_with(right, ignore_punctuation);
- let at_newline = (left == '\n') ^ (right == '\n');
-
- (left_kind != right_kind && right_kind != CharKind::Whitespace) || at_newline
- })
- }
- Motion::NextWordEnd { ignore_punctuation } => {
- self.helix_find_range_forward(times, window, cx, |left, right, classifier| {
- let left_kind = classifier.kind_with(left, ignore_punctuation);
- let right_kind = classifier.kind_with(right, ignore_punctuation);
- let at_newline = (left == '\n') ^ (right == '\n');
-
- (left_kind != right_kind && left_kind != CharKind::Whitespace) || at_newline
- })
- }
- Motion::PreviousWordStart { ignore_punctuation } => {
- self.helix_find_range_backward(times, window, cx, |left, right, classifier| {
- let left_kind = classifier.kind_with(left, ignore_punctuation);
- let right_kind = classifier.kind_with(right, ignore_punctuation);
- let at_newline = (left == '\n') ^ (right == '\n');
-
- (left_kind != right_kind && left_kind != CharKind::Whitespace) || at_newline
- })
- }
- Motion::PreviousWordEnd { ignore_punctuation } => {
- self.helix_find_range_backward(times, window, cx, |left, right, classifier| {
- let left_kind = classifier.kind_with(left, ignore_punctuation);
- let right_kind = classifier.kind_with(right, ignore_punctuation);
- let at_newline = (left == '\n') ^ (right == '\n');
-
- (left_kind != right_kind && right_kind != CharKind::Whitespace) || at_newline
- })
- }
Motion::FindForward {
before,
char,
@@ -1394,6 +1463,30 @@ mod test {
cx.assert_state("«one ˇ»two", Mode::HelixNormal);
}
+ #[gpui::test]
+ async fn test_helix_select_motion(cx: &mut gpui::TestAppContext) {
+ let mut cx = VimTestContext::new(cx, true).await;
+ cx.enable_helix();
+
+ cx.set_state("«ˇ»one two three", Mode::HelixSelect);
+ cx.simulate_keystrokes("w");
+ cx.assert_state("«one ˇ»two three", Mode::HelixSelect);
+
+ cx.set_state("«ˇ»one two three", Mode::HelixSelect);
+ cx.simulate_keystrokes("e");
+ cx.assert_state("«oneˇ» two three", Mode::HelixSelect);
+ }
+
+ #[gpui::test]
+ async fn test_helix_full_cursor_selection(cx: &mut gpui::TestAppContext) {
+ let mut cx = VimTestContext::new(cx, true).await;
+ cx.enable_helix();
+
+ cx.set_state("ˇone two three", Mode::HelixNormal);
+ cx.simulate_keystrokes("l l v h h h");
+ cx.assert_state("«ˇone» two three", Mode::HelixSelect);
+ }
+
#[gpui::test]
async fn test_helix_select_regex(cx: &mut gpui::TestAppContext) {
let mut cx = VimTestContext::new(cx, true).await;