Avoid optional on select_match

Conrad Irwin created

Change summary

crates/ai/src/assistant.rs          |   4 
crates/editor/src/items.rs          |  70 +++++++++++----------
crates/search/src/buffer_search.rs  | 100 +++++++++---------------------
crates/search/src/project_search.rs |   3 
crates/vim/src/normal/search.rs     |   7 +
crates/workspace/src/searchable.rs  |  10 +-
6 files changed, 80 insertions(+), 114 deletions(-)

Detailed changes

crates/ai/src/assistant.rs 🔗

@@ -330,13 +330,13 @@ impl AssistantPanel {
 
     fn select_next_match(&mut self, _: &search::SelectNextMatch, cx: &mut ViewContext<Self>) {
         if let Some(search_bar) = self.toolbar.read(cx).item_of_type::<BufferSearchBar>() {
-            search_bar.update(cx, |bar, cx| bar.select_match(Direction::Next, None, cx));
+            search_bar.update(cx, |bar, cx| bar.select_match(Direction::Next, 1, cx));
         }
     }
 
     fn select_prev_match(&mut self, _: &search::SelectPrevMatch, cx: &mut ViewContext<Self>) {
         if let Some(search_bar) = self.toolbar.read(cx).item_of_type::<BufferSearchBar>() {
-            search_bar.update(cx, |bar, cx| bar.select_match(Direction::Prev, None, cx));
+            search_bar.update(cx, |bar, cx| bar.select_match(Direction::Prev, 1, cx));
         }
     }
 

crates/editor/src/items.rs 🔗

@@ -954,15 +954,19 @@ impl SearchableItem for Editor {
 
     fn select_matches(&mut self, matches: Vec<Self::Match>, cx: &mut ViewContext<Self>) {
         self.unfold_ranges(matches.clone(), false, false, cx);
-        self.change_selections(None, cx, |s| s.select_ranges(matches));
+        let mut ranges = Vec::new();
+        for m in &matches {
+            ranges.push(self.range_for_match(&m))
+        }
+        self.change_selections(None, cx, |s| s.select_ranges(ranges));
     }
 
     fn match_index_for_direction(
         &mut self,
         matches: &Vec<Range<Anchor>>,
-        mut current_index: usize,
+        current_index: usize,
         direction: Direction,
-        count: Option<usize>,
+        count: usize,
         cx: &mut ViewContext<Self>,
     ) -> usize {
         let buffer = self.buffer().read(cx).snapshot(cx);
@@ -971,39 +975,39 @@ impl SearchableItem for Editor {
         } else {
             matches[current_index].start
         };
-        if count.is_none()
-            && matches[current_index]
-                .start
-                .cmp(&current_index_position, &buffer)
-                .is_gt()
-        {
-            if direction == Direction::Prev {
-                if current_index == 0 {
-                    current_index = matches.len() - 1;
-                } else {
-                    current_index -= 1;
+
+        let mut count = count % matches.len();
+        if count == 0 {
+            return current_index;
+        }
+        match direction {
+            Direction::Next => {
+                if matches[current_index]
+                    .start
+                    .cmp(&current_index_position, &buffer)
+                    .is_gt()
+                {
+                    count = count - 1
                 }
+
+                (current_index + count) % matches.len()
             }
-        } else if count.is_none()
-            && matches[current_index]
-                .end
-                .cmp(&current_index_position, &buffer)
-                .is_lt()
-        {
-            if direction == Direction::Next {
-                current_index = 0;
-            }
-        } else if direction == Direction::Prev {
-            let count = count.unwrap_or(1) % matches.len();
-            if current_index >= count {
-                current_index = current_index - count;
-            } else {
-                current_index = matches.len() - (count - current_index);
+            Direction::Prev => {
+                if matches[current_index]
+                    .end
+                    .cmp(&current_index_position, &buffer)
+                    .is_lt()
+                {
+                    count = count - 1;
+                }
+
+                if current_index >= count {
+                    current_index - count
+                } else {
+                    matches.len() - (count - current_index)
+                }
             }
-        } else if direction == Direction::Next {
-            current_index = (current_index + count.unwrap_or(1)) % matches.len()
-        };
-        current_index
+        }
     }
 
     fn find_matches(

crates/search/src/buffer_search.rs 🔗

@@ -560,11 +560,11 @@ impl BufferSearchBar {
     }
 
     fn select_next_match(&mut self, _: &SelectNextMatch, cx: &mut ViewContext<Self>) {
-        self.select_match(Direction::Next, None, cx);
+        self.select_match(Direction::Next, 1, cx);
     }
 
     fn select_prev_match(&mut self, _: &SelectPrevMatch, cx: &mut ViewContext<Self>) {
-        self.select_match(Direction::Prev, None, cx);
+        self.select_match(Direction::Prev, 1, cx);
     }
 
     fn select_all_matches(&mut self, _: &SelectAllMatches, cx: &mut ViewContext<Self>) {
@@ -581,12 +581,7 @@ impl BufferSearchBar {
         }
     }
 
-    pub fn select_match(
-        &mut self,
-        direction: Direction,
-        count: Option<usize>,
-        cx: &mut ViewContext<Self>,
-    ) {
+    pub fn select_match(&mut self, direction: Direction, count: usize, cx: &mut ViewContext<Self>) {
         if let Some(index) = self.active_match_index {
             if let Some(searchable_item) = self.active_searchable_item.as_ref() {
                 if let Some(matches) = self
@@ -1086,15 +1081,17 @@ mod tests {
     }
 
     #[gpui::test]
-    async fn test_search_with_options(cx: &mut TestAppContext) {
+    async fn test_search_option_handling(cx: &mut TestAppContext) {
         let (editor, search_bar) = init_test(cx);
 
         // show with options should make current search case sensitive
-        search_bar.update(cx, |search_bar, cx| {
-            search_bar.show_with_options(false, false, SearchOptions::CASE_SENSITIVE, cx);
-            search_bar.search("us", cx);
-        });
-        editor.next_notification(cx).await;
+        search_bar
+            .update(cx, |search_bar, cx| {
+                search_bar.show(cx);
+                search_bar.search("us", Some(SearchOptions::CASE_SENSITIVE), cx)
+            })
+            .await
+            .unwrap();
         editor.update(cx, |editor, cx| {
             assert_eq!(
                 editor.all_background_highlights(cx),
@@ -1105,31 +1102,20 @@ mod tests {
             );
         });
 
-        // show should return to the default options (case insensitive)
+        // search_suggested should restore default options
         search_bar.update(cx, |search_bar, cx| {
-            search_bar.show(true, true, cx);
-        });
-        editor.next_notification(cx).await;
-        editor.update(cx, |editor, cx| {
-            assert_eq!(
-                editor.all_background_highlights(cx),
-                &[
-                    (
-                        DisplayPoint::new(2, 17)..DisplayPoint::new(2, 19),
-                        Color::red(),
-                    ),
-                    (
-                        DisplayPoint::new(2, 43)..DisplayPoint::new(2, 45),
-                        Color::red(),
-                    )
-                ]
-            );
+            search_bar.search_suggested(cx);
+            assert_eq!(search_bar.search_options, SearchOptions::NONE)
         });
 
-        // toggling a search option (even in show_with_options mode) should update the defaults
+        // toggling a search option should update the defaults
+        search_bar
+            .update(cx, |search_bar, cx| {
+                search_bar.search("regex", Some(SearchOptions::CASE_SENSITIVE), cx)
+            })
+            .await
+            .unwrap();
         search_bar.update(cx, |search_bar, cx| {
-            search_bar.search("regex", cx);
-            search_bar.show_with_options(false, false, SearchOptions::CASE_SENSITIVE, cx);
             search_bar.toggle_search_option(SearchOptions::WHOLE_WORD, cx)
         });
         editor.next_notification(cx).await;
@@ -1145,38 +1131,11 @@ mod tests {
 
         // defaults should still include whole word
         search_bar.update(cx, |search_bar, cx| {
-            search_bar.show(true, true, cx);
-        });
-        editor.next_notification(cx).await;
-        editor.update(cx, |editor, cx| {
-            assert_eq!(
-                editor.all_background_highlights(cx),
-                &[(
-                    DisplayPoint::new(0, 35)..DisplayPoint::new(0, 40),
-                    Color::red(),
-                ),]
-            );
-        });
-
-        // removing whole word changes the search again
-        search_bar.update(cx, |search_bar, cx| {
-            search_bar.toggle_search_option(SearchOptions::WHOLE_WORD, cx)
-        });
-        editor.next_notification(cx).await;
-        editor.update(cx, |editor, cx| {
+            search_bar.search_suggested(cx);
             assert_eq!(
-                editor.all_background_highlights(cx),
-                &[
-                    (
-                        DisplayPoint::new(0, 35)..DisplayPoint::new(0, 40),
-                        Color::red(),
-                    ),
-                    (
-                        DisplayPoint::new(0, 44)..DisplayPoint::new(0, 49),
-                        Color::red()
-                    )
-                ]
-            );
+                search_bar.search_options,
+                SearchOptions::CASE_SENSITIVE | SearchOptions::WHOLE_WORD
+            )
         });
     }
 
@@ -1207,15 +1166,18 @@ mod tests {
         let search_bar = cx.add_view(window_id, |cx| {
             let mut search_bar = BufferSearchBar::new(cx);
             search_bar.set_active_pane_item(Some(&editor), cx);
-            search_bar.show(false, true, cx);
+            search_bar.show(cx);
             search_bar
         });
 
+        search_bar
+            .update(cx, |search_bar, cx| search_bar.search("a", None, cx))
+            .await
+            .unwrap();
         search_bar.update(cx, |search_bar, cx| {
-            search_bar.set_query("a", cx);
+            search_bar.activate_current_match(cx);
         });
 
-        editor.next_notification(cx).await;
         let initial_selections = editor.update(cx, |editor, cx| {
             let initial_selections = editor.selections.display_ranges(cx);
             assert_eq!(

crates/search/src/project_search.rs 🔗

@@ -627,7 +627,7 @@ impl ProjectSearchView {
         if let Some(index) = self.active_match_index {
             let match_ranges = self.model.read(cx).match_ranges.clone();
             let new_index = self.results_editor.update(cx, |editor, cx| {
-                editor.match_index_for_direction(&match_ranges, index, direction, None, cx)
+                editor.match_index_for_direction(&match_ranges, index, direction, 1, cx)
             });
 
             let range_to_select = match_ranges[new_index].clone();
@@ -668,7 +668,6 @@ impl ProjectSearchView {
             self.active_match_index = None;
         } else {
             self.active_match_index = Some(0);
-            self.select_match(Direction::Next, cx);
             self.update_match_index(cx);
             let prev_search_id = mem::replace(&mut self.search_id, self.model.read(cx).search_id);
             let is_new_search = self.search_id != prev_search_id;

crates/vim/src/normal/search.rs 🔗

@@ -81,6 +81,7 @@ fn search(workspace: &mut Workspace, action: &Search, cx: &mut ViewContext<Works
     })
 }
 
+// hook into the existing to clear out any vim search state on cmd+f or edit -> find.
 fn search_deploy(_: &mut Pane, _: &buffer_search::Deploy, cx: &mut ViewContext<Pane>) {
     Vim::update(cx, |vim, _| vim.state.search = Default::default());
     cx.propagate_action();
@@ -100,9 +101,9 @@ fn search_submit(workspace: &mut Workspace, _: &SearchSubmit, cx: &mut ViewConte
                     if (search_bar.query(cx) != state.initial_query)
                         && state.direction == Direction::Next
                     {
-                        count = count.saturating_sub(1);
+                        count = count.saturating_sub(1)
                     }
-                    search_bar.select_match(state.direction, Some(count), cx);
+                    search_bar.select_match(state.direction, count, cx);
                     state.count = 1;
                     search_bar.focus_editor(&Default::default(), cx);
                 });
@@ -139,7 +140,7 @@ pub fn move_to_internal(
                     cx.spawn(|_, mut cx| async move {
                         search.await?;
                         search_bar.update(&mut cx, |search_bar, cx| {
-                            search_bar.select_match(direction, Some(count), cx)
+                            search_bar.select_match(direction, count, cx)
                         })?;
                         anyhow::Ok(())
                     })

crates/workspace/src/searchable.rs 🔗

@@ -57,19 +57,19 @@ pub trait SearchableItem: Item {
         matches: &Vec<Self::Match>,
         current_index: usize,
         direction: Direction,
-        count: Option<usize>,
+        count: usize,
         _: &mut ViewContext<Self>,
     ) -> usize {
         match direction {
             Direction::Prev => {
-                let count = count.unwrap_or(1) % matches.len();
+                let count = count % matches.len();
                 if current_index >= count {
                     current_index - count
                 } else {
                     matches.len() - (count - current_index)
                 }
             }
-            Direction::Next => (current_index + count.unwrap_or(1)) % matches.len(),
+            Direction::Next => (current_index + count) % matches.len(),
         }
     }
     fn find_matches(
@@ -108,7 +108,7 @@ pub trait SearchableItemHandle: ItemHandle {
         matches: &Vec<Box<dyn Any + Send>>,
         current_index: usize,
         direction: Direction,
-        count: Option<usize>,
+        count: usize,
         cx: &mut WindowContext,
     ) -> usize;
     fn find_matches(
@@ -179,7 +179,7 @@ impl<T: SearchableItem> SearchableItemHandle for ViewHandle<T> {
         matches: &Vec<Box<dyn Any + Send>>,
         current_index: usize,
         direction: Direction,
-        count: Option<usize>,
+        count: usize,
         cx: &mut WindowContext,
     ) -> usize {
         let matches = downcast_matches(matches);