Preserve serach index for multicaret selection editor events

Kirill Bulatov created

Change summary

crates/editor/src/items.rs                | 32 ++++++++-
crates/feedback/src/feedback_editor.rs    |  9 ++
crates/language_tools/src/lsp_log.rs      |  9 ++
crates/search/src/buffer_search.rs        | 79 ++++++++++++++++++++++++
crates/terminal_view/src/terminal_view.rs |  6 +
crates/workspace/src/searchable.rs        | 11 ++
6 files changed, 130 insertions(+), 16 deletions(-)

Detailed changes

crates/editor/src/items.rs 🔗

@@ -887,10 +887,20 @@ pub(crate) enum BufferSearchHighlights {}
 impl SearchableItem for Editor {
     type Match = Range<Anchor>;
 
-    fn to_search_event(event: &Self::Event) -> Option<SearchEvent> {
+    fn to_search_event(
+        &mut self,
+        event: &Self::Event,
+        _: &mut ViewContext<Self>,
+    ) -> Option<SearchEvent> {
         match event {
             Event::BufferEdited => Some(SearchEvent::MatchesInvalidated),
-            Event::SelectionsChanged { .. } => Some(SearchEvent::ActiveMatchChanged),
+            Event::SelectionsChanged { .. } => {
+                if self.selections.disjoint_anchors().len() == 1 {
+                    Some(SearchEvent::ActiveMatchChanged)
+                } else {
+                    None
+                }
+            }
             _ => None,
         }
     }
@@ -954,8 +964,16 @@ impl SearchableItem for Editor {
         cx: &mut ViewContext<Self>,
     ) -> usize {
         let buffer = self.buffer().read(cx).snapshot(cx);
-        let cursor = self.selections.newest_anchor().head();
-        if matches[current_index].start.cmp(&cursor, &buffer).is_gt() {
+        let current_index_position = if self.selections.disjoint_anchors().len() == 1 {
+            self.selections.newest_anchor().head()
+        } else {
+            matches[current_index].start
+        };
+        if matches[current_index]
+            .start
+            .cmp(&current_index_position, &buffer)
+            .is_gt()
+        {
             if direction == Direction::Prev {
                 if current_index == 0 {
                     current_index = matches.len() - 1;
@@ -963,7 +981,11 @@ impl SearchableItem for Editor {
                     current_index -= 1;
                 }
             }
-        } else if matches[current_index].end.cmp(&cursor, &buffer).is_lt() {
+        } else if matches[current_index]
+            .end
+            .cmp(&current_index_position, &buffer)
+            .is_lt()
+        {
             if direction == Direction::Next {
                 current_index = 0;
             }

crates/feedback/src/feedback_editor.rs 🔗

@@ -362,8 +362,13 @@ impl Item for FeedbackEditor {
 impl SearchableItem for FeedbackEditor {
     type Match = Range<Anchor>;
 
-    fn to_search_event(event: &Self::Event) -> Option<workspace::searchable::SearchEvent> {
-        Editor::to_search_event(event)
+    fn to_search_event(
+        &mut self,
+        event: &Self::Event,
+        cx: &mut ViewContext<Self>,
+    ) -> Option<workspace::searchable::SearchEvent> {
+        self.editor
+            .update(cx, |editor, cx| editor.to_search_event(event, cx))
     }
 
     fn clear_matches(&mut self, cx: &mut ViewContext<Self>) {

crates/language_tools/src/lsp_log.rs 🔗

@@ -467,8 +467,13 @@ impl Item for LspLogView {
 impl SearchableItem for LspLogView {
     type Match = <Editor as SearchableItem>::Match;
 
-    fn to_search_event(event: &Self::Event) -> Option<workspace::searchable::SearchEvent> {
-        Editor::to_search_event(event)
+    fn to_search_event(
+        &mut self,
+        event: &Self::Event,
+        cx: &mut ViewContext<Self>,
+    ) -> Option<workspace::searchable::SearchEvent> {
+        self.editor
+            .update(cx, |editor, cx| editor.to_search_event(event, cx))
     }
 
     fn clear_matches(&mut self, cx: &mut ViewContext<Self>) {

crates/search/src/buffer_search.rs 🔗

@@ -1029,12 +1029,16 @@ mod tests {
         });
 
         editor.next_notification(cx).await;
-        editor.update(cx, |editor, cx| {
-            let initial_selections =   editor.selections.display_ranges(cx);
+        let initial_selections = editor.update(cx, |editor, cx| {
+            let initial_selections = editor.selections.display_ranges(cx);
             assert_eq!(
                 initial_selections.len(), 1,
                 "Expected to have only one selection before adding carets to all matches, but got: {initial_selections:?}",
-            )
+            );
+            initial_selections
+        });
+        search_bar.update(cx, |search_bar, _| {
+            assert_eq!(search_bar.active_match_index, Some(0));
         });
 
         search_bar.update(cx, |search_bar, cx| {
@@ -1047,5 +1051,74 @@ mod tests {
                 "Should select all `a` characters in the buffer, but got: {all_selections:?}"
             );
         });
+        search_bar.update(cx, |search_bar, _| {
+            assert_eq!(
+                search_bar.active_match_index,
+                Some(0),
+                "Match index should not change after selecting all matches"
+            );
+        });
+
+        search_bar.update(cx, |search_bar, cx| {
+            search_bar.select_next_match(&SelectNextMatch, cx);
+            let all_selections =
+                editor.update(cx, |editor, cx| editor.selections.display_ranges(cx));
+            assert_eq!(
+                all_selections.len(),
+                1,
+                "On next match, should deselect items and select the next match"
+            );
+            assert_ne!(
+                all_selections, initial_selections,
+                "Next match should be different from the first selection"
+            );
+        });
+        search_bar.update(cx, |search_bar, _| {
+            assert_eq!(
+                search_bar.active_match_index,
+                Some(1),
+                "Match index should be updated to the next one"
+            );
+        });
+
+        search_bar.update(cx, |search_bar, cx| {
+            search_bar.select_all_matches(&SelectAllMatches, cx);
+            let all_selections =
+                editor.update(cx, |editor, cx| editor.selections.display_ranges(cx));
+            assert_eq!(
+                all_selections.len(),
+                expected_query_matches_count,
+                "Should select all `a` characters in the buffer, but got: {all_selections:?}"
+            );
+        });
+        search_bar.update(cx, |search_bar, _| {
+            assert_eq!(
+                search_bar.active_match_index,
+                Some(1),
+                "Match index should not change after selecting all matches"
+            );
+        });
+
+        search_bar.update(cx, |search_bar, cx| {
+            search_bar.select_prev_match(&SelectPrevMatch, cx);
+            let all_selections =
+                editor.update(cx, |editor, cx| editor.selections.display_ranges(cx));
+            assert_eq!(
+                all_selections.len(),
+                1,
+                "On previous match, should deselect items and select the previous item"
+            );
+            assert_eq!(
+                all_selections, initial_selections,
+                "Previous match should be the same as the first selection"
+            );
+        });
+        search_bar.update(cx, |search_bar, _| {
+            assert_eq!(
+                search_bar.active_match_index,
+                Some(0),
+                "Match index should be updated to the previous one"
+            );
+        });
     }
 }

crates/terminal_view/src/terminal_view.rs 🔗

@@ -647,7 +647,11 @@ impl SearchableItem for TerminalView {
     }
 
     /// Convert events raised by this item into search-relevant events (if applicable)
-    fn to_search_event(event: &Self::Event) -> Option<SearchEvent> {
+    fn to_search_event(
+        &mut self,
+        event: &Self::Event,
+        _: &mut ViewContext<Self>,
+    ) -> Option<SearchEvent> {
         match event {
             Event::Wakeup => Some(SearchEvent::MatchesInvalidated),
             Event::SelectionsChanged => Some(SearchEvent::ActiveMatchChanged),

crates/workspace/src/searchable.rs 🔗

@@ -37,7 +37,11 @@ pub trait SearchableItem: Item {
             regex: true,
         }
     }
-    fn to_search_event(event: &Self::Event) -> Option<SearchEvent>;
+    fn to_search_event(
+        &mut self,
+        event: &Self::Event,
+        cx: &mut ViewContext<Self>,
+    ) -> Option<SearchEvent>;
     fn clear_matches(&mut self, cx: &mut ViewContext<Self>);
     fn update_matches(&mut self, matches: Vec<Self::Match>, cx: &mut ViewContext<Self>);
     fn query_suggestion(&mut self, cx: &mut ViewContext<Self>) -> String;
@@ -141,8 +145,9 @@ impl<T: SearchableItem> SearchableItemHandle for ViewHandle<T> {
         cx: &mut WindowContext,
         handler: Box<dyn Fn(SearchEvent, &mut WindowContext)>,
     ) -> Subscription {
-        cx.subscribe(self, move |_, event, cx| {
-            if let Some(search_event) = T::to_search_event(event) {
+        cx.subscribe(self, move |handle, event, cx| {
+            let search_event = handle.update(cx, |handle, cx| handle.to_search_event(event, cx));
+            if let Some(search_event) = search_event {
                 handler(search_event, cx)
             }
         })