From 48299b5b2402748a7501047cc11204e452289fcf Mon Sep 17 00:00:00 2001 From: tidely <43219534+tidely@users.noreply.github.com> Date: Thu, 28 Aug 2025 00:17:21 +0300 Subject: [PATCH] search: Preserve `SearchOptions` across dismisses (#36954) 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 --- crates/search/src/buffer_search.rs | 62 ++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index a38dc8c35b3a0caef230247505a6131d40170bca..b2096d48ef5ba4e3b74eb0955bb11ef32cb5498a 100644 --- a/crates/search/src/buffer_search.rs +++ b/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" ); }); }