search: Fix buffer search history navigation (#37924)

crates/search/src/buffer_search.rs 🔗

@@ -489,7 +489,7 @@ impl ToolbarItemView for BufferSearchBar {
 
             let is_project_search = searchable_item_handle.supported_options(cx).find_in_results;
             self.active_searchable_item = Some(searchable_item_handle);
-            drop(self.update_matches(true, window, cx));
+            drop(self.update_matches(true, false, window, cx));
             if !self.dismissed {
                 if is_project_search {
                     self.dismiss(&Default::default(), window, cx);
@@ -778,9 +778,9 @@ impl BufferSearchBar {
     }
 
     pub fn search_suggested(&mut self, window: &mut Window, cx: &mut Context<Self>) {
-        let search = self
-            .query_suggestion(window, cx)
-            .map(|suggestion| self.search(&suggestion, Some(self.default_options), window, cx));
+        let search = self.query_suggestion(window, cx).map(|suggestion| {
+            self.search(&suggestion, Some(self.default_options), true, window, cx)
+        });
 
         if let Some(search) = search {
             cx.spawn_in(window, async move |this, cx| {
@@ -855,6 +855,7 @@ impl BufferSearchBar {
         &mut self,
         query: &str,
         options: Option<SearchOptions>,
+        add_to_history: bool,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> oneshot::Receiver<()> {
@@ -871,7 +872,7 @@ impl BufferSearchBar {
             self.clear_matches(window, cx);
             cx.notify();
         }
-        self.update_matches(!updated, window, cx)
+        self.update_matches(!updated, add_to_history, window, cx)
     }
 
     pub fn focus_editor(&mut self, _: &FocusEditor, window: &mut Window, cx: &mut Context<Self>) {
@@ -889,7 +890,7 @@ impl BufferSearchBar {
     ) {
         self.search_options.toggle(search_option);
         self.default_options = self.search_options;
-        drop(self.update_matches(false, window, cx));
+        drop(self.update_matches(false, false, window, cx));
         self.adjust_query_regex_language(cx);
         cx.notify();
     }
@@ -1033,7 +1034,7 @@ impl BufferSearchBar {
             editor::EditorEvent::Edited { .. } => {
                 self.smartcase(window, cx);
                 self.clear_matches(window, cx);
-                let search = self.update_matches(false, window, cx);
+                let search = self.update_matches(false, true, window, cx);
 
                 let width = editor.update(cx, |editor, cx| {
                     let text_layout_details = editor.text_layout_details(window);
@@ -1078,7 +1079,7 @@ impl BufferSearchBar {
     ) {
         match event {
             SearchEvent::MatchesInvalidated => {
-                drop(self.update_matches(false, window, cx));
+                drop(self.update_matches(false, false, window, cx));
             }
             SearchEvent::ActiveMatchChanged => self.update_match_index(window, cx),
         }
@@ -1111,7 +1112,7 @@ impl BufferSearchBar {
         if let Some(active_item) = self.active_searchable_item.as_mut() {
             self.selection_search_enabled = !self.selection_search_enabled;
             active_item.toggle_filtered_search_ranges(self.selection_search_enabled, window, cx);
-            drop(self.update_matches(false, window, cx));
+            drop(self.update_matches(false, false, window, cx));
             cx.notify();
         }
     }
@@ -1154,6 +1155,7 @@ impl BufferSearchBar {
     fn update_matches(
         &mut self,
         reuse_existing_query: bool,
+        add_to_history: bool,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> oneshot::Receiver<()> {
@@ -1234,8 +1236,10 @@ impl BufferSearchBar {
                                 .insert(active_searchable_item.downgrade(), matches);
 
                             this.update_match_index(window, cx);
-                            this.search_history
-                                .add(&mut this.search_history_cursor, query_text);
+                            if add_to_history {
+                                this.search_history
+                                    .add(&mut this.search_history_cursor, query_text);
+                            }
                             if !this.dismissed {
                                 let matches = this
                                     .searchable_items_with_matches
@@ -1324,10 +1328,10 @@ impl BufferSearchBar {
             .next(&mut self.search_history_cursor)
             .map(str::to_string)
         {
-            drop(self.search(&new_query, Some(self.search_options), window, cx));
+            drop(self.search(&new_query, Some(self.search_options), false, window, cx));
         } else {
             self.search_history_cursor.reset();
-            drop(self.search("", Some(self.search_options), window, cx));
+            drop(self.search("", Some(self.search_options), false, window, cx));
         }
     }
 
@@ -1343,7 +1347,7 @@ impl BufferSearchBar {
                 .current(&self.search_history_cursor)
                 .map(str::to_string)
         {
-            drop(self.search(&new_query, Some(self.search_options), window, cx));
+            drop(self.search(&new_query, Some(self.search_options), false, window, cx));
             return;
         }
 
@@ -1352,7 +1356,7 @@ impl BufferSearchBar {
             .previous(&mut self.search_history_cursor)
             .map(str::to_string)
         {
-            drop(self.search(&new_query, Some(self.search_options), window, cx));
+            drop(self.search(&new_query, Some(self.search_options), false, window, cx));
         }
     }
 
@@ -1548,7 +1552,7 @@ mod tests {
         // By default, search is case-insensitive.
         search_bar
             .update_in(cx, |search_bar, window, cx| {
-                search_bar.search("us", None, window, cx)
+                search_bar.search("us", None, true, window, cx)
             })
             .await
             .unwrap();
@@ -1579,7 +1583,7 @@ mod tests {
         // within other words. By default, all results are found.
         search_bar
             .update_in(cx, |search_bar, window, cx| {
-                search_bar.search("or", None, window, cx)
+                search_bar.search("or", None, true, window, cx)
             })
             .await
             .unwrap();
@@ -1823,7 +1827,7 @@ mod tests {
         search_bar
             .update_in(cx, |search_bar, window, cx| {
                 search_bar.show(window, cx);
-                search_bar.search("us", Some(SearchOptions::CASE_SENSITIVE), window, cx)
+                search_bar.search("us", Some(SearchOptions::CASE_SENSITIVE), true, window, cx)
             })
             .await
             .unwrap();
@@ -1843,7 +1847,13 @@ mod tests {
         // toggling a search option should update the defaults
         search_bar
             .update_in(cx, |search_bar, window, cx| {
-                search_bar.search("regex", Some(SearchOptions::CASE_SENSITIVE), window, cx)
+                search_bar.search(
+                    "regex",
+                    Some(SearchOptions::CASE_SENSITIVE),
+                    true,
+                    window,
+                    cx,
+                )
             })
             .await
             .unwrap();
@@ -1904,7 +1914,7 @@ mod tests {
         window
             .update(cx, |_, window, cx| {
                 search_bar.update(cx, |search_bar, cx| {
-                    search_bar.search("a", None, window, cx)
+                    search_bar.search("a", None, true, window, cx)
                 })
             })
             .unwrap()
@@ -2046,7 +2056,7 @@ mod tests {
                 search_bar.update(cx, |search_bar, cx| {
                     let handle = search_bar.query_editor.focus_handle(cx);
                     window.focus(&handle);
-                    search_bar.search("abas_nonexistent_match", None, window, cx)
+                    search_bar.search("abas_nonexistent_match", None, true, window, cx)
                 })
             })
             .unwrap()
@@ -2114,6 +2124,7 @@ mod tests {
                 search_bar.search(
                     "edit\\(",
                     Some(SearchOptions::WHOLE_WORD | SearchOptions::REGEX),
+                    true,
                     window,
                     cx,
                 )
@@ -2139,6 +2150,7 @@ mod tests {
                 search_bar.search(
                     "edit(",
                     Some(SearchOptions::WHOLE_WORD | SearchOptions::CASE_SENSITIVE),
+                    true,
                     window,
                     cx,
                 )
@@ -2162,43 +2174,24 @@ mod tests {
 
     #[gpui::test]
     async fn test_search_query_history(cx: &mut TestAppContext) {
-        init_globals(cx);
-        let buffer_text = r#"
-        A regular expression (shortened as regex or regexp;[1] also referred to as
-        rational expression[2][3]) is a sequence of characters that specifies a search
-        pattern in text. Usually such patterns are used by string-searching algorithms
-        for "find" or "find and replace" operations on strings, or for input validation.
-        "#
-        .unindent();
-        let buffer = cx.new(|cx| Buffer::local(buffer_text, cx));
-        let cx = cx.add_empty_window();
-
-        let editor =
-            cx.new_window_entity(|window, cx| Editor::for_buffer(buffer.clone(), None, window, cx));
-
-        let search_bar = cx.new_window_entity(|window, cx| {
-            let mut search_bar = BufferSearchBar::new(None, window, cx);
-            search_bar.set_active_pane_item(Some(&editor), window, cx);
-            search_bar.show(window, cx);
-            search_bar
-        });
+        let (_editor, search_bar, cx) = init_test(cx);
 
         // Add 3 search items into the history.
         search_bar
             .update_in(cx, |search_bar, window, cx| {
-                search_bar.search("a", None, window, cx)
+                search_bar.search("a", None, true, window, cx)
             })
             .await
             .unwrap();
         search_bar
             .update_in(cx, |search_bar, window, cx| {
-                search_bar.search("b", None, window, cx)
+                search_bar.search("b", None, true, window, cx)
             })
             .await
             .unwrap();
         search_bar
             .update_in(cx, |search_bar, window, cx| {
-                search_bar.search("c", Some(SearchOptions::CASE_SENSITIVE), window, cx)
+                search_bar.search("c", Some(SearchOptions::CASE_SENSITIVE), true, window, cx)
             })
             .await
             .unwrap();
@@ -2212,6 +2205,7 @@ mod tests {
         search_bar.update_in(cx, |search_bar, window, cx| {
             search_bar.next_history_query(&NextHistoryQuery, window, cx);
         });
+        cx.background_executor.run_until_parked();
         search_bar.update(cx, |search_bar, cx| {
             assert_eq!(search_bar.query(cx), "");
             assert_eq!(search_bar.search_options, SearchOptions::CASE_SENSITIVE);
@@ -2219,6 +2213,7 @@ mod tests {
         search_bar.update_in(cx, |search_bar, window, cx| {
             search_bar.next_history_query(&NextHistoryQuery, window, cx);
         });
+        cx.background_executor.run_until_parked();
         search_bar.update(cx, |search_bar, cx| {
             assert_eq!(search_bar.query(cx), "");
             assert_eq!(search_bar.search_options, SearchOptions::CASE_SENSITIVE);
@@ -2228,6 +2223,7 @@ mod tests {
         search_bar.update_in(cx, |search_bar, window, cx| {
             search_bar.previous_history_query(&PreviousHistoryQuery, window, cx);
         });
+        cx.background_executor.run_until_parked();
         search_bar.update(cx, |search_bar, cx| {
             assert_eq!(search_bar.query(cx), "c");
             assert_eq!(search_bar.search_options, SearchOptions::CASE_SENSITIVE);
@@ -2237,6 +2233,7 @@ mod tests {
         search_bar.update_in(cx, |search_bar, window, cx| {
             search_bar.previous_history_query(&PreviousHistoryQuery, window, cx);
         });
+        cx.background_executor.run_until_parked();
         search_bar.update(cx, |search_bar, cx| {
             assert_eq!(search_bar.query(cx), "b");
             assert_eq!(search_bar.search_options, SearchOptions::CASE_SENSITIVE);
@@ -2246,6 +2243,7 @@ mod tests {
         search_bar.update_in(cx, |search_bar, window, cx| {
             search_bar.previous_history_query(&PreviousHistoryQuery, window, cx);
         });
+        cx.background_executor.run_until_parked();
         search_bar.update(cx, |search_bar, cx| {
             assert_eq!(search_bar.query(cx), "a");
             assert_eq!(search_bar.search_options, SearchOptions::CASE_SENSITIVE);
@@ -2253,6 +2251,7 @@ mod tests {
         search_bar.update_in(cx, |search_bar, window, cx| {
             search_bar.previous_history_query(&PreviousHistoryQuery, window, cx);
         });
+        cx.background_executor.run_until_parked();
         search_bar.update(cx, |search_bar, cx| {
             assert_eq!(search_bar.query(cx), "a");
             assert_eq!(search_bar.search_options, SearchOptions::CASE_SENSITIVE);
@@ -2262,6 +2261,7 @@ mod tests {
         search_bar.update_in(cx, |search_bar, window, cx| {
             search_bar.next_history_query(&NextHistoryQuery, window, cx);
         });
+        cx.background_executor.run_until_parked();
         search_bar.update(cx, |search_bar, cx| {
             assert_eq!(search_bar.query(cx), "b");
             assert_eq!(search_bar.search_options, SearchOptions::CASE_SENSITIVE);
@@ -2269,7 +2269,7 @@ mod tests {
 
         search_bar
             .update_in(cx, |search_bar, window, cx| {
-                search_bar.search("ba", None, window, cx)
+                search_bar.search("ba", None, true, window, cx)
             })
             .await
             .unwrap();
@@ -2282,6 +2282,7 @@ mod tests {
         search_bar.update_in(cx, |search_bar, window, cx| {
             search_bar.previous_history_query(&PreviousHistoryQuery, window, cx);
         });
+        cx.background_executor.run_until_parked();
         search_bar.update(cx, |search_bar, cx| {
             assert_eq!(search_bar.query(cx), "c");
             assert_eq!(search_bar.search_options, SearchOptions::NONE);
@@ -2289,6 +2290,7 @@ mod tests {
         search_bar.update_in(cx, |search_bar, window, cx| {
             search_bar.previous_history_query(&PreviousHistoryQuery, window, cx);
         });
+        cx.background_executor.run_until_parked();
         search_bar.update(cx, |search_bar, cx| {
             assert_eq!(search_bar.query(cx), "b");
             assert_eq!(search_bar.search_options, SearchOptions::NONE);
@@ -2296,6 +2298,7 @@ mod tests {
         search_bar.update_in(cx, |search_bar, window, cx| {
             search_bar.next_history_query(&NextHistoryQuery, window, cx);
         });
+        cx.background_executor.run_until_parked();
         search_bar.update(cx, |search_bar, cx| {
             assert_eq!(search_bar.query(cx), "c");
             assert_eq!(search_bar.search_options, SearchOptions::NONE);
@@ -2303,6 +2306,7 @@ mod tests {
         search_bar.update_in(cx, |search_bar, window, cx| {
             search_bar.next_history_query(&NextHistoryQuery, window, cx);
         });
+        cx.background_executor.run_until_parked();
         search_bar.update(cx, |search_bar, cx| {
             assert_eq!(search_bar.query(cx), "ba");
             assert_eq!(search_bar.search_options, SearchOptions::NONE);
@@ -2310,6 +2314,7 @@ mod tests {
         search_bar.update_in(cx, |search_bar, window, cx| {
             search_bar.next_history_query(&NextHistoryQuery, window, cx);
         });
+        cx.background_executor.run_until_parked();
         search_bar.update(cx, |search_bar, cx| {
             assert_eq!(search_bar.query(cx), "");
             assert_eq!(search_bar.search_options, SearchOptions::NONE);
@@ -2322,7 +2327,7 @@ mod tests {
 
         search_bar
             .update_in(cx, |search_bar, window, cx| {
-                search_bar.search("expression", None, window, cx)
+                search_bar.search("expression", None, true, window, cx)
             })
             .await
             .unwrap();
@@ -2348,7 +2353,7 @@ mod tests {
         // Search for word boundaries and replace just a single one.
         search_bar
             .update_in(cx, |search_bar, window, cx| {
-                search_bar.search("or", Some(SearchOptions::WHOLE_WORD), window, cx)
+                search_bar.search("or", Some(SearchOptions::WHOLE_WORD), true, window, cx)
             })
             .await
             .unwrap();
@@ -2373,7 +2378,13 @@ mod tests {
         // Let's turn on regex mode.
         search_bar
             .update_in(cx, |search_bar, window, cx| {
-                search_bar.search("\\[([^\\]]+)\\]", Some(SearchOptions::REGEX), window, cx)
+                search_bar.search(
+                    "\\[([^\\]]+)\\]",
+                    Some(SearchOptions::REGEX),
+                    true,
+                    window,
+                    cx,
+                )
             })
             .await
             .unwrap();
@@ -2399,6 +2410,7 @@ mod tests {
                 search_bar.search(
                     "a\\w+s",
                     Some(SearchOptions::REGEX | SearchOptions::WHOLE_WORD),
+                    true,
                     window,
                     cx,
                 )
@@ -2443,7 +2455,13 @@ mod tests {
                 if let Some(options) = options.search_options {
                     search_bar.set_search_options(options, cx);
                 }
-                search_bar.search(options.search_text, options.search_options, window, cx)
+                search_bar.search(
+                    options.search_text,
+                    options.search_options,
+                    true,
+                    window,
+                    cx,
+                )
             })
             .await
             .unwrap();
@@ -2582,7 +2600,7 @@ mod tests {
 
         search_bar
             .update_in(cx, |search_bar, window, cx| {
-                search_bar.search("aaa", None, window, cx)
+                search_bar.search("aaa", None, true, window, cx)
             })
             .await
             .unwrap();
@@ -2668,7 +2686,7 @@ mod tests {
 
         search_bar
             .update_in(cx, |search_bar, window, cx| {
-                search_bar.search("aaa", None, window, cx)
+                search_bar.search("aaa", None, true, window, cx)
             })
             .await
             .unwrap();
@@ -2692,7 +2710,7 @@ mod tests {
         search_bar
             .update_in(cx, |search_bar, window, cx| {
                 search_bar.enable_search_option(SearchOptions::REGEX, window, cx);
-                search_bar.search("expression", None, window, cx)
+                search_bar.search("expression", None, true, window, cx)
             })
             .await
             .unwrap();
@@ -2709,7 +2727,7 @@ mod tests {
         // Now, the expression is invalid
         search_bar
             .update_in(cx, |search_bar, window, cx| {
-                search_bar.search("expression (", None, window, cx)
+                search_bar.search("expression (", None, true, window, cx)
             })
             .await
             .unwrap_err();

crates/search/src/project_search.rs 🔗

@@ -4114,7 +4114,7 @@ pub mod tests {
         buffer_search_bar
             .update_in(&mut cx, |buffer_search_bar, window, cx| {
                 buffer_search_bar.focus_handle(cx).focus(window);
-                buffer_search_bar.search(buffer_search_query, None, window, cx)
+                buffer_search_bar.search(buffer_search_query, None, true, window, cx)
             })
             .await
             .unwrap();

crates/vim/src/command.rs 🔗

@@ -1585,6 +1585,7 @@ impl OnMatchingLines {
                             let _ = search_bar.search(
                                 &last_pattern,
                                 Some(SearchOptions::REGEX | SearchOptions::CASE_SENSITIVE),
+                                false,
                                 window,
                                 cx,
                             );

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

@@ -360,12 +360,12 @@ impl Vim {
                     .query_suggestion(window, cx)
                     .or_else(|| cursor_word)
                 else {
-                    drop(search_bar.search("", None, window, cx));
+                    drop(search_bar.search("", None, false, window, cx));
                     return None;
                 };
 
                 let query = regex::escape(&query);
-                Some(search_bar.search(&query, Some(options), window, cx))
+                Some(search_bar.search(&query, Some(options), true, window, cx))
             });
 
             let Some(search) = search else { return false };
@@ -425,7 +425,7 @@ impl Vim {
                         );
                     }
 
-                    Some(search_bar.search(&query, Some(options), window, cx))
+                    Some(search_bar.search(&query, Some(options), true, window, cx))
                 });
                 let Some(search) = search else { return };
                 let search_bar = search_bar.downgrade();
@@ -509,7 +509,7 @@ impl Vim {
             if replacement.flag_c {
                 search_bar.focus_replace(window, cx);
             }
-            Some(search_bar.search(&search, Some(options), window, cx))
+            Some(search_bar.search(&search, Some(options), true, window, cx))
         });
         if replacement.flag_n {
             self.move_cursor(
@@ -534,7 +534,7 @@ impl Vim {
                 search_bar.select_last_match(window, cx);
                 search_bar.replace_all(&Default::default(), window, cx);
                 editor.update(cx, |editor, cx| editor.clear_search_within_ranges(cx));
-                let _ = search_bar.search(&search_bar.query(cx), None, window, cx);
+                let _ = search_bar.search(&search_bar.query(cx), None, false, window, cx);
                 vim.update(cx, |vim, cx| {
                     vim.move_cursor(
                         Motion::StartOfLine {

search: Fix buffer search history navigation (#37924)

Change summary

Detailed changes

crates/search/src/buffer_search.rs 🔗

crates/search/src/project_search.rs 🔗

crates/vim/src/command.rs 🔗

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