From 65e75508d31432e1be632ba64362c4877a05caa7 Mon Sep 17 00:00:00 2001 From: Finn Eitreim <48069764+feitreim@users.noreply.github.com> Date: Thu, 2 Apr 2026 02:42:04 -0400 Subject: [PATCH] helix: Fix crash while moving through selections (#52922) Closes #51573 Closes #52852 * I believe, looking for verification of that. A very similar issue was belived to be fixed with #51642 , however it seems like there is still some edge cases that were causing crashes. These issues appeared to me to be caused by the dedup method failing to catch sub-ranges (created by entering SelectMode after the search) as duplicates. two new tests that isolate the case in 51573 and 52852 respectively. fixed vid: https://github.com/user-attachments/assets/f62d5210-6cb3-4bdf-a061-efc265eb2804 Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - helix: Fix search selection range deduplication logic --- crates/vim/src/helix.rs | 102 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 92 insertions(+), 10 deletions(-) diff --git a/crates/vim/src/helix.rs b/crates/vim/src/helix.rs index d2c8f4b78dcde8c4f2135b63ee3d07f04e01ebd5..923bd8c6a057819129b29b86e559c79a30f011f9 100644 --- a/crates/vim/src/helix.rs +++ b/crates/vim/src/helix.rs @@ -12,7 +12,6 @@ use editor::{ }; use gpui::actions; use gpui::{Context, Window}; -use itertools::Itertools as _; use language::{CharClassifier, CharKind, Point}; use search::{BufferSearchBar, SearchOptions}; use settings::Settings; @@ -941,19 +940,15 @@ impl Vim { editor.change_selections(SelectionEffects::default(), window, cx, |s| { let buffer = snapshot.buffer_snapshot(); - s.select_anchor_ranges( + s.select_ranges( prior_selections .iter() .cloned() .chain(s.all_anchors(&snapshot).iter().map(|s| s.range())) - .sorted_by(|a, b| { - a.start - .cmp(&b.start, buffer) - .then_with(|| a.end.cmp(&b.end, buffer)) - }) - .dedup_by(|a, b| { - a.start.cmp(&b.start, buffer).is_eq() - && a.end.cmp(&b.end, buffer).is_eq() + .map(|range| { + let start = range.start.to_offset(buffer); + let end = range.end.to_offset(buffer); + start..end }), ); }) @@ -2152,6 +2147,93 @@ mod test { cx.assert_state("hello two «oneˇ» two «oneˇ» two «oneˇ»", Mode::HelixSelect); } + #[gpui::test] + async fn test_helix_select_next_match_wrapping_from_normal(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + cx.enable_helix(); + + // Exact repro for #51573: start in HelixNormal, search, then `v` to + // enter HelixSelect, then `n` past last match. + // + // In HelixNormal, search collapses the cursor to the match start. + // Pressing `v` expands by only one character, creating a partial + // selection that overlaps the full match range when the search wraps. + // The overlapping ranges must be merged (not just deduped) to avoid + // a backward-seeking rope cursor panic. + cx.set_state( + indoc! {" + searˇch term + stuff + search term + other stuff + "}, + Mode::HelixNormal, + ); + cx.simulate_keystrokes("/ t e r m"); + cx.simulate_keystrokes("enter"); + cx.simulate_keystrokes("v"); + cx.simulate_keystrokes("n"); + cx.simulate_keystrokes("n"); + // Should not panic when wrapping past last match. + cx.assert_state( + indoc! {" + search «termˇ» + stuff + search «termˇ» + other stuff + "}, + Mode::HelixSelect, + ); + } + + #[gpui::test] + async fn test_helix_select_star_then_match(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + cx.enable_helix(); + + // Repro attempts for #52852: `*` searches for word under cursor, + // `v` enters select, `n` accumulates matches, `m` triggers match mode. + // Try multiple cursor positions and match counts. + + // Cursor on first occurrence, 3 more occurrences to select through + cx.set_state( + indoc! {" + ˇone two one three one four one + "}, + Mode::HelixNormal, + ); + cx.simulate_keystrokes("*"); + cx.simulate_keystrokes("v"); + cx.simulate_keystrokes("n n n"); + // Should not panic on wrapping `n`. + + // Cursor in the middle of text before matches + cx.set_state( + indoc! {" + heˇllo one two one three one + "}, + Mode::HelixNormal, + ); + cx.simulate_keystrokes("*"); + cx.simulate_keystrokes("v"); + cx.simulate_keystrokes("n"); + // Should not panic. + + // The original #52852 sequence: * v n n n then m m + cx.set_state( + indoc! {" + fn ˇfoo() { bar(foo()) } + fn baz() { foo() } + "}, + Mode::HelixNormal, + ); + cx.simulate_keystrokes("*"); + cx.simulate_keystrokes("v"); + cx.simulate_keystrokes("n n n"); + cx.simulate_keystrokes("m m"); + // Should not panic. + } + #[gpui::test] async fn test_helix_substitute(cx: &mut gpui::TestAppContext) { let mut cx = VimTestContext::new(cx, true).await;