Make buffer search aware of search direction (#24974)

Nico Lehmann created 10 months ago

This solves a couple of issues with Vim search by making the search
buffer and `SearchableItem` aware of the direction of the search. If
`SearchOptions::BACKWARDS` is set, all operations will be reversed. By
making `SearchableItem` aware of the direction, the correct active match
can be selected when searching backward.

Fixes #22506. This PR does not fix the last problem in that issue, but
that one is also tracked in #8049.

Release Notes:

- Fixes incorrect behavior of backward search in Vim mode

Change summary

crates/assistant_context_editor/src/context_editor.rs |  5 
crates/editor/src/items.rs                            | 16 +++
crates/language_tools/src/lsp_log.rs                  |  8 +
crates/search/src/buffer_search.rs                    | 11 ++
crates/search/src/project_search.rs                   |  1 
crates/search/src/search.rs                           |  2 
crates/terminal_view/src/terminal_view.rs             | 46 +++++++++---
crates/vim/src/normal/search.rs                       | 11 ++
crates/workspace/src/searchable.rs                    |  5 +
9 files changed, 80 insertions(+), 25 deletions(-)

Detailed changes

crates/assistant_context_editor/src/context_editor.rs

@@ -54,7 +54,7 @@ use ui::{
     Tooltip,
 };
 use util::{maybe, ResultExt};
-use workspace::searchable::SearchableItemHandle;
+use workspace::searchable::{Direction, SearchableItemHandle};
 use workspace::{
     item::{self, FollowableItem, Item, ItemHandle},
     notifications::NotificationId,
@@ -3060,12 +3060,13 @@ impl SearchableItem for ContextEditor {
 
     fn active_match_index(
         &mut self,
+        direction: Direction,
         matches: &[Self::Match],
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Option<usize> {
         self.editor.update(cx, |editor, cx| {
-            editor.active_match_index(matches, window, cx)
+            editor.active_match_index(direction, matches, window, cx)
         })
     }
 }

crates/editor/src/items.rs

@@ -1589,11 +1589,13 @@ impl SearchableItem for Editor {
 
     fn active_match_index(
         &mut self,
+        direction: Direction,
         matches: &[Range<Anchor>],
         _: &mut Window,
         cx: &mut Context<Self>,
     ) -> Option<usize> {
         active_match_index(
+            direction,
             matches,
             &self.selections.newest_anchor().head(),
             &self.buffer().read(cx).snapshot(cx),
@@ -1606,6 +1608,7 @@ impl SearchableItem for Editor {
 }
 
 pub fn active_match_index(
+    direction: Direction,
     ranges: &[Range<Anchor>],
     cursor: &Anchor,
     buffer: &MultiBufferSnapshot,
@@ -1613,7 +1616,7 @@ pub fn active_match_index(
     if ranges.is_empty() {
         None
     } else {
-        match ranges.binary_search_by(|probe| {
+        let r = ranges.binary_search_by(|probe| {
             if probe.end.cmp(cursor, buffer).is_lt() {
                 Ordering::Less
             } else if probe.start.cmp(cursor, buffer).is_gt() {
@@ -1621,8 +1624,15 @@ pub fn active_match_index(
             } else {
                 Ordering::Equal
             }
-        }) {
-            Ok(i) | Err(i) => Some(cmp::min(i, ranges.len() - 1)),
+        });
+        match direction {
+            Direction::Prev => match r {
+                Ok(i) => Some(i),
+                Err(i) => Some(i.saturating_sub(1)),
+            },
+            Direction::Next => match r {
+                Ok(i) | Err(i) => Some(cmp::min(i, ranges.len() - 1)),
+            },
         }
     }
 }

crates/language_tools/src/lsp_log.rs

@@ -16,7 +16,7 @@ use std::{borrow::Cow, sync::Arc};
 use ui::{prelude::*, Button, Checkbox, ContextMenu, Label, PopoverMenu, ToggleState};
 use workspace::{
     item::{Item, ItemHandle},
-    searchable::{SearchEvent, SearchableItem, SearchableItemHandle},
+    searchable::{Direction, SearchEvent, SearchableItem, SearchableItemHandle},
     SplitDirection, ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView, Workspace, WorkspaceId,
 };
 
@@ -1170,12 +1170,14 @@ impl SearchableItem for LspLogView {
     }
     fn active_match_index(
         &mut self,
+        direction: Direction,
         matches: &[Self::Match],
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Option<usize> {
-        self.editor
-            .update(cx, |e, cx| e.active_match_index(matches, window, cx))
+        self.editor.update(cx, |e, cx| {
+            e.active_match_index(direction, matches, window, cx)
+        })
     }
 }
 

crates/search/src/buffer_search.rs

@@ -1306,7 +1306,16 @@ impl BufferSearchBar {
         done_rx
     }
 
+    fn reverse_direction_if_backwards(&self, direction: Direction) -> Direction {
+        if self.search_options.contains(SearchOptions::BACKWARDS) {
+            direction.opposite()
+        } else {
+            direction
+        }
+    }
+
     pub fn update_match_index(&mut self, window: &mut Window, cx: &mut Context<Self>) {
+        let direction = self.reverse_direction_if_backwards(Direction::Next);
         let new_index = self
             .active_searchable_item
             .as_ref()
@@ -1314,7 +1323,7 @@ impl BufferSearchBar {
                 let matches = self
                     .searchable_items_with_matches
                     .get(&searchable_item.downgrade())?;
-                searchable_item.active_match_index(matches, window, cx)
+                searchable_item.active_match_index(direction, matches, window, cx)
             });
         if new_index != self.active_match_index {
             self.active_match_index = new_index;

crates/search/src/project_search.rs

@@ -1240,6 +1240,7 @@ impl ProjectSearchView {
     fn update_match_index(&mut self, cx: &mut Context<Self>) {
         let results_editor = self.results_editor.read(cx);
         let new_index = active_match_index(
+            Direction::Next,
             &self.entity.read(cx).match_ranges,
             &results_editor.selections.newest_anchor().head(),
             &results_editor.buffer().read(cx).snapshot(cx),

crates/search/src/search.rs

@@ -47,6 +47,8 @@ bitflags! {
         const CASE_SENSITIVE = 0b010;
         const INCLUDE_IGNORED = 0b100;
         const REGEX = 0b1000;
+        /// If set, reverse direction when finding the active match
+        const BACKWARDS = 0b10000;
     }
 }
 

crates/terminal_view/src/terminal_view.rs

@@ -41,7 +41,7 @@ use workspace::{
         BreadcrumbText, Item, ItemEvent, SerializableItem, TabContentParams, TabTooltipContent,
     },
     register_serializable_item,
-    searchable::{SearchEvent, SearchOptions, SearchableItem, SearchableItemHandle},
+    searchable::{Direction, SearchEvent, SearchOptions, SearchableItem, SearchableItemHandle},
     CloseActiveItem, NewCenterTerminal, NewTerminal, OpenVisible, ToolbarItemLocation, Workspace,
     WorkspaceId,
 };
@@ -1583,6 +1583,7 @@ impl SearchableItem for TerminalView {
     /// Reports back to the search toolbar what the active match should be (the selection)
     fn active_match_index(
         &mut self,
+        direction: Direction,
         matches: &[Self::Match],
         _: &mut Window,
         cx: &mut Context<Self>,
@@ -1593,19 +1594,36 @@ impl SearchableItem for TerminalView {
         let res = if !matches.is_empty() {
             if let Some(selection_head) = self.terminal().read(cx).selection_head {
                 // If selection head is contained in a match. Return that match
-                if let Some(ix) = matches
-                    .iter()
-                    .enumerate()
-                    .find(|(_, search_match)| {
-                        search_match.contains(&selection_head)
-                            || search_match.start() > &selection_head
-                    })
-                    .map(|(ix, _)| ix)
-                {
-                    Some(ix)
-                } else {
-                    // If no selection after selection head, return the last match
-                    Some(matches.len().saturating_sub(1))
+                match direction {
+                    Direction::Prev => {
+                        // If no selection before selection head, return the first match
+                        Some(
+                            matches
+                                .iter()
+                                .enumerate()
+                                .rev()
+                                .find(|(_, search_match)| {
+                                    search_match.contains(&selection_head)
+                                        || search_match.start() < &selection_head
+                                })
+                                .map(|(ix, _)| ix)
+                                .unwrap_or(0),
+                        )
+                    }
+                    Direction::Next => {
+                        // If no selection after selection head, return the last match
+                        Some(
+                            matches
+                                .iter()
+                                .enumerate()
+                                .find(|(_, search_match)| {
+                                    search_match.contains(&selection_head)
+                                        || search_match.start() > &selection_head
+                                })
+                                .map(|(ix, _)| ix)
+                                .unwrap_or(matches.len().saturating_sub(1)),
+                        )
+                    }
                 }
             } else {
                 // Matches found but no active selection, return the first last one (closest to cursor)

crates/vim/src/normal/search.rs

@@ -154,6 +154,9 @@ impl Vim {
                     if action.regex {
                         options |= SearchOptions::REGEX;
                     }
+                    if action.backwards {
+                        options |= SearchOptions::BACKWARDS;
+                    }
                     search_bar.set_search_options(options, cx);
                     let prior_mode = if self.temp_mode {
                         Mode::Insert
@@ -198,7 +201,7 @@ impl Vim {
                     .last()
                     .map_or(true, |range| range.start != new_head);
 
-                if is_different_head && self.search.direction == Direction::Next {
+                if is_different_head {
                     count = count.saturating_sub(1)
                 }
                 self.search.count = 1;
@@ -743,6 +746,12 @@ mod test {
         cx.simulate_keystrokes("*");
         cx.assert_state("one two ˇone", Mode::Normal);
 
+        // check that a backward search after last match works correctly
+        cx.set_state("aa\naa\nbbˇ", Mode::Normal);
+        cx.simulate_keystrokes("? a a");
+        cx.simulate_keystrokes("enter");
+        cx.assert_state("aa\nˇaa\nbb", Mode::Normal);
+
         // check that searching with unable search wrap
         cx.update_global(|store: &mut SettingsStore, cx| {
             store.update_user_settings::<EditorSettings>(cx, |s| s.search_wrap = Some(false));

crates/workspace/src/searchable.rs

@@ -150,6 +150,7 @@ pub trait SearchableItem: Item + EventEmitter<SearchEvent> {
     ) -> Task<Vec<Self::Match>>;
     fn active_match_index(
         &mut self,
+        direction: Direction,
         matches: &[Self::Match],
         window: &mut Window,
         cx: &mut Context<Self>,
@@ -208,6 +209,7 @@ pub trait SearchableItemHandle: ItemHandle {
     ) -> Task<AnyVec<dyn Send>>;
     fn active_match_index(
         &self,
+        direction: Direction,
         matches: &AnyVec<dyn Send>,
         window: &mut Window,
         cx: &mut App,
@@ -315,13 +317,14 @@ impl<T: SearchableItem> SearchableItemHandle for Entity<T> {
     }
     fn active_match_index(
         &self,
+        direction: Direction,
         matches: &AnyVec<dyn Send>,
         window: &mut Window,
         cx: &mut App,
     ) -> Option<usize> {
         let matches = matches.downcast_ref()?;
         self.update(cx, |this, cx| {
-            this.active_match_index(matches.as_slice(), window, cx)
+            this.active_match_index(direction, matches.as_slice(), window, cx)
         })
     }