From 1c437a2b9238dd4f094a53e50d4ef3b0dbb041b0 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 12 Dec 2023 14:23:52 +0100 Subject: [PATCH 1/3] Register buffer search listener on new workspaces and not editors --- crates/search2/src/buffer_search.rs | 113 ++++++++++------------------ 1 file changed, 38 insertions(+), 75 deletions(-) diff --git a/crates/search2/src/buffer_search.rs b/crates/search2/src/buffer_search.rs index 14c046e0d4..f3d2548703 100644 --- a/crates/search2/src/buffer_search.rs +++ b/crates/search2/src/buffer_search.rs @@ -7,12 +7,12 @@ use crate::{ ToggleCaseSensitive, ToggleReplace, ToggleWholeWord, }; use collections::HashMap; -use editor::{Editor, EditorMode}; +use editor::Editor; use futures::channel::oneshot; use gpui::{ actions, div, impl_actions, red, Action, AppContext, Div, EventEmitter, FocusableView, InteractiveElement as _, IntoElement, KeyContext, ParentElement as _, Render, Styled, - Subscription, Task, View, ViewContext, VisualContext as _, WeakView, WindowContext, + Subscription, Task, View, ViewContext, VisualContext as _, WindowContext, }; use project::search::SearchQuery; use serde::Deserialize; @@ -23,7 +23,7 @@ use util::ResultExt; use workspace::{ item::ItemHandle, searchable::{Direction, SearchEvent, SearchableItemHandle, WeakSearchableItemHandle}, - ToolbarItemLocation, ToolbarItemView, + ToolbarItemLocation, ToolbarItemView, Workspace, }; #[derive(PartialEq, Clone, Deserialize)] @@ -40,7 +40,7 @@ pub enum Event { } pub fn init(cx: &mut AppContext) { - cx.observe_new_views(|editor: &mut Editor, cx| BufferSearchBar::register(editor, cx)) + cx.observe_new_views(|editor: &mut Workspace, _| BufferSearchBar::register(editor)) .detach(); } @@ -315,17 +315,9 @@ impl ToolbarItemView for BufferSearchBar { } impl BufferSearchBar { - pub fn register(editor: &mut Editor, cx: &mut ViewContext) { - if editor.mode() != EditorMode::Full { - return; - }; - - let handle = cx.view().downgrade(); - - editor.register_action(move |deploy: &Deploy, cx| { - let Some(pane) = handle.upgrade().and_then(|editor| editor.read(cx).pane(cx)) else { - return; - }; + fn register(workspace: &mut Workspace) { + workspace.register_action(move |workspace, deploy: &Deploy, cx| { + let pane = workspace.active_pane(); pane.update(cx, |this, cx| { this.toolbar().update(cx, |this, cx| { @@ -341,15 +333,11 @@ impl BufferSearchBar { }); }); fn register_action( - editor: &mut Editor, - handle: WeakView, + workspace: &mut Workspace, update: fn(&mut BufferSearchBar, &A, &mut ViewContext), ) { - editor.register_action(move |action: &A, cx| { - let Some(pane) = handle.upgrade().and_then(|editor| editor.read(cx).pane(cx)) - else { - return; - }; + workspace.register_action(move |workspace, action: &A, cx| { + let pane = workspace.active_pane(); pane.update(cx, move |this, cx| { this.toolbar().update(cx, move |this, cx| { if let Some(search_bar) = this.item_of_type::() { @@ -361,71 +349,46 @@ impl BufferSearchBar { }); } - let handle = cx.view().downgrade(); - register_action( - editor, - handle.clone(), - |this, action: &ToggleCaseSensitive, cx| { - if this.supported_options().case { - this.toggle_case_sensitive(action, cx); - } - }, - ); - register_action( - editor, - handle.clone(), - |this, action: &ToggleWholeWord, cx| { - if this.supported_options().word { - this.toggle_whole_word(action, cx); - } - }, - ); - register_action( - editor, - handle.clone(), - |this, action: &ToggleReplace, cx| { - if this.supported_options().replacement { - this.toggle_replace(action, cx); - } - }, - ); - register_action(editor, handle.clone(), |this, _: &ActivateRegexMode, cx| { + register_action(workspace, |this, action: &ToggleCaseSensitive, cx| { + if this.supported_options().case { + this.toggle_case_sensitive(action, cx); + } + }); + register_action(workspace, |this, action: &ToggleWholeWord, cx| { + if this.supported_options().word { + this.toggle_whole_word(action, cx); + } + }); + register_action(workspace, |this, action: &ToggleReplace, cx| { + if this.supported_options().replacement { + this.toggle_replace(action, cx); + } + }); + register_action(workspace, |this, _: &ActivateRegexMode, cx| { if this.supported_options().regex { this.activate_search_mode(SearchMode::Regex, cx); } }); - register_action(editor, handle.clone(), |this, _: &ActivateTextMode, cx| { + register_action(workspace, |this, _: &ActivateTextMode, cx| { this.activate_search_mode(SearchMode::Text, cx); }); - register_action(editor, handle.clone(), |this, action: &CycleMode, cx| { + register_action(workspace, |this, action: &CycleMode, cx| { if this.supported_options().regex { // If regex is not supported then search has just one mode (text) - in that case there's no point in supporting // cycling. this.cycle_mode(action, cx) } }); - register_action( - editor, - handle.clone(), - |this, action: &SelectNextMatch, cx| { - this.select_next_match(action, cx); - }, - ); - register_action( - editor, - handle.clone(), - |this, action: &SelectPrevMatch, cx| { - this.select_prev_match(action, cx); - }, - ); - register_action( - editor, - handle.clone(), - |this, action: &SelectAllMatches, cx| { - this.select_all_matches(action, cx); - }, - ); - register_action(editor, handle.clone(), |this, _: &editor::Cancel, cx| { + register_action(workspace, |this, action: &SelectNextMatch, cx| { + this.select_next_match(action, cx); + }); + register_action(workspace, |this, action: &SelectPrevMatch, cx| { + this.select_prev_match(action, cx); + }); + register_action(workspace, |this, action: &SelectAllMatches, cx| { + this.select_all_matches(action, cx); + }); + register_action(workspace, |this, _: &editor::Cancel, cx| { if !this.dismissed { this.dismiss(&Dismiss, cx); return; From be5705919533a07363eaddc14ba3c4a170e96e6a Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 12 Dec 2023 16:36:00 +0100 Subject: [PATCH 2/3] Move away from using cx.dispatch_action in buffer search --- crates/search2/src/buffer_search.rs | 49 ++++++++++++++++++----------- crates/search2/src/search.rs | 23 +++++++------- 2 files changed, 42 insertions(+), 30 deletions(-) diff --git a/crates/search2/src/buffer_search.rs b/crates/search2/src/buffer_search.rs index f3d2548703..481bc757eb 100644 --- a/crates/search2/src/buffer_search.rs +++ b/crates/search2/src/buffer_search.rs @@ -10,9 +10,9 @@ use collections::HashMap; use editor::Editor; use futures::channel::oneshot; use gpui::{ - actions, div, impl_actions, red, Action, AppContext, Div, EventEmitter, FocusableView, - InteractiveElement as _, IntoElement, KeyContext, ParentElement as _, Render, Styled, - Subscription, Task, View, ViewContext, VisualContext as _, WindowContext, + actions, div, impl_actions, red, Action, AppContext, ClickEvent, Div, EventEmitter, + FocusableView, InteractiveElement as _, IntoElement, KeyContext, ParentElement as _, Render, + Styled, Subscription, Task, View, ViewContext, VisualContext as _, WindowContext, }; use project::search::SearchQuery; use serde::Deserialize; @@ -135,10 +135,6 @@ impl Render for BufferSearchBar { render_search_mode_button(mode, is_active) }; - let search_option_button = |option| { - let is_active = self.search_options.contains(option); - option.as_button(is_active) - }; let match_count = self .active_searchable_item .as_ref() @@ -209,16 +205,20 @@ impl Render for BufferSearchBar { .items_center() .child(IconElement::new(Icon::MagnifyingGlass)) .child(self.query_editor.clone()) - .children( - supported_options - .case - .then(|| search_option_button(SearchOptions::CASE_SENSITIVE)), - ) - .children( - supported_options - .word - .then(|| search_option_button(SearchOptions::WHOLE_WORD)), - ), + .children(supported_options.case.then(|| { + self.render_search_option_button( + SearchOptions::CASE_SENSITIVE, + cx.listener(|this, _, cx| { + this.toggle_case_sensitive(&ToggleCaseSensitive, cx) + }), + ) + })) + .children(supported_options.word.then(|| { + self.render_search_option_button( + SearchOptions::WHOLE_WORD, + cx.listener(|this, _, cx| this.toggle_whole_word(&ToggleWholeWord, cx)), + ) + })), ) .child( h_stack() @@ -229,7 +229,12 @@ impl Render for BufferSearchBar { .child(search_button_for_mode(SearchMode::Regex)), ) .when(supported_options.replacement, |this| { - this.child(super::toggle_replace_button(self.replace_enabled)) + this.child(super::toggle_replace_button( + self.replace_enabled, + cx.listener(|this, _: &ClickEvent, cx| { + this.toggle_replace(&ToggleReplace, cx); + }), + )) }), ) .child( @@ -572,6 +577,14 @@ impl BufferSearchBar { .tooltip(|cx| Tooltip::for_action("Select all matches", &SelectAllMatches, cx)) } + fn render_search_option_button( + &self, + option: SearchOptions, + action: impl Fn(&ClickEvent, &mut WindowContext) + 'static, + ) -> impl IntoElement { + let is_active = self.search_options.contains(option); + option.as_button(is_active, action) + } pub fn activate_search_mode(&mut self, mode: SearchMode, cx: &mut ViewContext) { assert_ne!( mode, diff --git a/crates/search2/src/search.rs b/crates/search2/src/search.rs index 1118f197a5..41be2508b3 100644 --- a/crates/search2/src/search.rs +++ b/crates/search2/src/search.rs @@ -88,14 +88,13 @@ impl SearchOptions { options } - pub fn as_button(&self, active: bool) -> impl IntoElement { + pub fn as_button( + &self, + active: bool, + action: impl Fn(&gpui::ClickEvent, &mut WindowContext) + 'static, + ) -> impl IntoElement { IconButton::new(self.label(), self.icon()) - .on_click({ - let action = self.to_toggle_action(); - move |_, cx| { - cx.dispatch_action(action.boxed_clone()); - } - }) + .on_click(action) .style(ButtonStyle::Subtle) .when(active, |button| button.style(ButtonStyle::Filled)) .tooltip({ @@ -106,13 +105,13 @@ impl SearchOptions { } } -fn toggle_replace_button(active: bool) -> impl IntoElement { +fn toggle_replace_button( + active: bool, + action: impl Fn(&gpui::ClickEvent, &mut WindowContext) + 'static, +) -> impl IntoElement { // todo: add toggle_replace button IconButton::new("buffer-search-bar-toggle-replace-button", Icon::Replace) - .on_click(|_, cx| { - cx.dispatch_action(Box::new(ToggleReplace)); - cx.notify(); - }) + .on_click(action) .style(ButtonStyle::Subtle) .when(active, |button| button.style(ButtonStyle::Filled)) .tooltip(|cx| Tooltip::for_action("Toggle replace", &ToggleReplace, cx)) From 13f9fec0bd916bb1dfd79c87520b7ee124536ff7 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 12 Dec 2023 16:38:58 +0100 Subject: [PATCH 3/3] fixup! Move away from using cx.dispatch_action in buffer search --- crates/search2/src/buffer_search.rs | 17 ++++++++++++++--- crates/search2/src/search.rs | 5 ++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/crates/search2/src/buffer_search.rs b/crates/search2/src/buffer_search.rs index 481bc757eb..2598049ce6 100644 --- a/crates/search2/src/buffer_search.rs +++ b/crates/search2/src/buffer_search.rs @@ -154,10 +154,21 @@ impl Render for BufferSearchBar { Some(ui::Label::new(message)) }); let should_show_replace_input = self.replace_enabled && supported_options.replacement; - let replace_all = should_show_replace_input - .then(|| super::render_replace_button(ReplaceAll, ui::Icon::ReplaceAll, "Replace all")); + let replace_all = should_show_replace_input.then(|| { + super::render_replace_button( + ReplaceAll, + ui::Icon::ReplaceAll, + "Replace all", + cx.listener(|this, _, cx| this.replace_all(&ReplaceAll, cx)), + ) + }); let replace_next = should_show_replace_input.then(|| { - super::render_replace_button(ReplaceNext, ui::Icon::ReplaceNext, "Replace next") + super::render_replace_button( + ReplaceNext, + ui::Icon::ReplaceNext, + "Replace next", + cx.listener(|this, _, cx| this.replace_next(&ReplaceNext, cx)), + ) }); let in_replace = self.replacement_editor.focus_handle(cx).is_focused(cx); diff --git a/crates/search2/src/search.rs b/crates/search2/src/search.rs index 41be2508b3..015c126aa1 100644 --- a/crates/search2/src/search.rs +++ b/crates/search2/src/search.rs @@ -121,6 +121,7 @@ fn render_replace_button( action: impl Action + 'static + Send + Sync, icon: Icon, tooltip: &'static str, + on_click: impl Fn(&gpui::ClickEvent, &mut WindowContext) + 'static, ) -> impl IntoElement { let id: SharedString = format!("search-replace-{}", action.name()).into(); IconButton::new(id, icon) @@ -128,7 +129,5 @@ fn render_replace_button( let action = action.boxed_clone(); move |cx| Tooltip::for_action(tooltip, &*action, cx) }) - .on_click(move |_, cx| { - cx.dispatch_action(action.boxed_clone()); - }) + .on_click(on_click) }