search: Preserve `SearchOptions` across dismisses (#36954)

tidely created 2 months ago

Closes #36931 and #21956

Preserves `SearchOptions` across dismisses of the buffer search bar.
This behavior is consistent with VSCode, which seems reasonable. The
`configured_options` field is then no longer being used. The
configuration is still read during initialization of the
`BufferSearchBar`, but not after.

Something to consider is that there are other elements in the search bar
which are not kept across dismisses such as replace status. However
these are visually separated in the UI, leading me to believe this is a
okay change to make.

Release Notes:

- Preserve search options between buffer search dismisses

Change summary

crates/search/src/buffer_search.rs | 62 +++++++++++++++++++++----------
1 file changed, 41 insertions(+), 21 deletions(-)

Detailed changes

crates/search/src/buffer_search.rs

@@ -749,14 +749,16 @@ impl BufferSearchBar {
             return false;
         };
 
-        self.configured_options =
+        let configured_options =
             SearchOptions::from_settings(&EditorSettings::get_global(cx).search);
-        if self.dismissed
-            && (self.configured_options != self.default_options
-                || self.configured_options != self.search_options)
-        {
-            self.search_options = self.configured_options;
-            self.default_options = self.configured_options;
+        let settings_changed = configured_options != self.configured_options;
+
+        if self.dismissed && settings_changed {
+            // Only update configuration options when search bar is dismissed,
+            // so we don't miss updates even after calling show twice
+            self.configured_options = configured_options;
+            self.search_options = configured_options;
+            self.default_options = configured_options;
         }
 
         self.dismissed = false;
@@ -2750,11 +2752,6 @@ mod tests {
                 "Search bar should be present and visible"
             );
             search_bar.deploy(&deploy, window, cx);
-            assert_eq!(
-                search_bar.configured_options,
-                SearchOptions::NONE,
-                "Should have configured search options matching the settings"
-            );
             assert_eq!(
                 search_bar.search_options,
                 SearchOptions::WHOLE_WORD,
@@ -2765,21 +2762,22 @@ mod tests {
             search_bar.deploy(&deploy, window, cx);
             assert_eq!(
                 search_bar.search_options,
-                SearchOptions::NONE,
-                "After hiding and showing the search bar, default options should be used"
+                SearchOptions::WHOLE_WORD,
+                "After hiding and showing the search bar, search options should be preserved"
             );
 
             search_bar.toggle_search_option(SearchOptions::REGEX, window, cx);
             search_bar.toggle_search_option(SearchOptions::WHOLE_WORD, window, cx);
             assert_eq!(
                 search_bar.search_options,
-                SearchOptions::REGEX | SearchOptions::WHOLE_WORD,
+                SearchOptions::REGEX,
                 "Should enable the options toggled"
             );
             assert!(
                 !search_bar.dismissed,
                 "Search bar should be present and visible"
             );
+            search_bar.toggle_search_option(SearchOptions::WHOLE_WORD, window, cx);
         });
 
         update_search_settings(
@@ -2800,11 +2798,6 @@ mod tests {
             );
 
             search_bar.deploy(&deploy, window, cx);
-            assert_eq!(
-                search_bar.configured_options,
-                SearchOptions::CASE_SENSITIVE,
-                "Should have configured search options matching the settings"
-            );
             assert_eq!(
                 search_bar.search_options,
                 SearchOptions::REGEX | SearchOptions::WHOLE_WORD,
@@ -2812,10 +2805,37 @@ mod tests {
             );
             search_bar.dismiss(&Dismiss, window, cx);
             search_bar.deploy(&deploy, window, cx);
+            assert_eq!(
+                search_bar.configured_options,
+                SearchOptions::CASE_SENSITIVE,
+                "After a settings update and toggling the search bar, configured options should be updated"
+            );
             assert_eq!(
                 search_bar.search_options,
                 SearchOptions::CASE_SENSITIVE,
-                "After hiding and showing the search bar, default options should be used"
+                "After a settings update and toggling the search bar, configured options should be used"
+            );
+        });
+
+        update_search_settings(
+            SearchSettings {
+                button: true,
+                whole_word: true,
+                case_sensitive: true,
+                include_ignored: false,
+                regex: false,
+            },
+            cx,
+        );
+
+        search_bar.update_in(cx, |search_bar, window, cx| {
+            search_bar.deploy(&deploy, window, cx);
+            search_bar.dismiss(&Dismiss, window, cx);
+            search_bar.show(window, cx);
+            assert_eq!(
+                search_bar.search_options,
+                SearchOptions::CASE_SENSITIVE | SearchOptions::WHOLE_WORD,
+                "Calling deploy on an already deployed search bar should not prevent settings updates from being detected"
             );
         });
     }