Buffer search channel notes (#3608)

Piotr Osiewicz created 2 years ago

We were registering `deploy` only on editors, which did succeed for
channel notes; however, channel note does not have an associated
workspace (that we pulled from the editor). It made more sense to just
register these actions for a workspace, notwithstanding the editor.

This PR also fixes a bunch of cx.dispatch_action calls to call the
handler directly instead (e.g. instead of dispatching ReplaceNext we
just call buffer_search_bar.replace_next instead) as otherwise these
actions are not handled if the buffer search bar does not have the
focus.

Release Notes:

- N/A

Change summary

crates/search2/src/buffer_search.rs | 177 ++++++++++++++----------------
crates/search2/src/search.rs        |  28 ++--
2 files changed, 95 insertions(+), 110 deletions(-)

Detailed changes

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,
+    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;
@@ -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();
 }
 
@@ -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()
@@ -158,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);
 
@@ -209,16 +216,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 +240,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(
@@ -315,17 +331,9 @@ impl ToolbarItemView for BufferSearchBar {
 }
 
 impl BufferSearchBar {
-    pub fn register(editor: &mut Editor, cx: &mut ViewContext<Editor>) {
-        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 +349,11 @@ impl BufferSearchBar {
             });
         });
         fn register_action<A: Action>(
-            editor: &mut Editor,
-            handle: WeakView<Editor>,
+            workspace: &mut Workspace,
             update: fn(&mut BufferSearchBar, &A, &mut ViewContext<BufferSearchBar>),
         ) {
-            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::<BufferSearchBar>() {
@@ -361,71 +365,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;
@@ -609,6 +588,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<Self>) {
         assert_ne!(
             mode,

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))
@@ -122,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)
@@ -129,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)
 }