Make tab cycling behave symmetrical in search buffer

Lukas Wirth created

Change summary

crates/search/src/buffer_search.rs  | 52 ++++++++++++++++--------------
crates/search/src/project_search.rs | 47 ++++++++-------------------
crates/workspace/src/workspace.rs   |  8 +---
3 files changed, 44 insertions(+), 63 deletions(-)

Detailed changes

crates/search/src/buffer_search.rs 🔗

@@ -697,7 +697,7 @@ impl BufferSearchBar {
             active_editor.search_bar_visibility_changed(false, window, cx);
             active_editor.toggle_filtered_search_ranges(false, window, cx);
             let handle = active_editor.item_focus_handle(cx);
-            self.focus(&handle, window, cx);
+            self.focus(&handle, window);
         }
         cx.emit(Event::UpdateLocation);
         cx.emit(ToolbarItemEvent::ChangeLocation(
@@ -853,7 +853,7 @@ impl BufferSearchBar {
     }
 
     pub fn focus_replace(&mut self, window: &mut Window, cx: &mut Context<Self>) {
-        self.focus(&self.replacement_editor.focus_handle(cx), window, cx);
+        self.focus(&self.replacement_editor.focus_handle(cx), window);
         cx.notify();
     }
 
@@ -1295,28 +1295,32 @@ impl BufferSearchBar {
     }
 
     fn tab(&mut self, _: &Tab, window: &mut Window, cx: &mut Context<Self>) {
-        // Search -> Replace -> Editor
-        let focus_handle = if self.replace_enabled && self.query_editor_focused {
-            self.replacement_editor.focus_handle(cx)
-        } else if let Some(item) = self.active_searchable_item.as_ref() {
-            item.item_focus_handle(cx)
-        } else {
-            return;
-        };
-        self.focus(&focus_handle, window, cx);
-        cx.stop_propagation();
+        self.cycle_field(Direction::Next, window, cx);
     }
 
     fn backtab(&mut self, _: &Backtab, window: &mut Window, cx: &mut Context<Self>) {
-        // Search -> Replace -> Search
-        let focus_handle = if self.replace_enabled && self.query_editor_focused {
-            self.replacement_editor.focus_handle(cx)
-        } else if self.replacement_editor_focused {
-            self.query_editor.focus_handle(cx)
-        } else {
-            return;
+        self.cycle_field(Direction::Prev, window, cx);
+    }
+    fn cycle_field(&mut self, direction: Direction, window: &mut Window, cx: &mut Context<Self>) {
+        let mut handles = vec![self.query_editor.focus_handle(cx)];
+        if self.replace_enabled {
+            handles.push(self.replacement_editor.focus_handle(cx));
+        }
+        if let Some(item) = self.active_searchable_item.as_ref() {
+            handles.push(item.item_focus_handle(cx));
+        }
+        let current_index = match handles.iter().position(|focus| focus.is_focused(window)) {
+            Some(index) => index,
+            None => return,
         };
-        self.focus(&focus_handle, window, cx);
+
+        let new_index = match direction {
+            Direction::Next => (current_index + 1) % handles.len(),
+            Direction::Prev if current_index == 0 => handles.len() - 1,
+            Direction::Prev => (current_index - 1) % handles.len(),
+        };
+        let next_focus_handle = &handles[new_index];
+        self.focus(next_focus_handle, window);
         cx.stop_propagation();
     }
 
@@ -1364,10 +1368,8 @@ impl BufferSearchBar {
         }
     }
 
-    fn focus(&self, handle: &gpui::FocusHandle, window: &mut Window, cx: &mut Context<Self>) {
-        cx.on_next_frame(window, |_, window, _| {
-            window.invalidate_character_coordinates();
-        });
+    fn focus(&self, handle: &gpui::FocusHandle, window: &mut Window) {
+        window.invalidate_character_coordinates();
         window.focus(handle);
     }
 
@@ -1379,7 +1381,7 @@ impl BufferSearchBar {
             } else {
                 self.query_editor.focus_handle(cx)
             };
-            self.focus(&handle, window, cx);
+            self.focus(&handle, window);
             cx.notify();
         }
     }

crates/search/src/project_search.rs 🔗

@@ -11,7 +11,8 @@ use anyhow::Context as _;
 use collections::{HashMap, HashSet};
 use editor::{
     Anchor, Editor, EditorEvent, EditorSettings, MAX_TAB_TITLE_LEN, MultiBuffer, SelectionEffects,
-    actions::SelectAll, items::active_match_index,
+    actions::{Backtab, SelectAll, Tab},
+    items::active_match_index,
 };
 use futures::{StreamExt, stream::FuturesOrdered};
 use gpui::{
@@ -1613,16 +1614,11 @@ impl ProjectSearchBar {
         }
     }
 
-    fn tab(&mut self, _: &editor::actions::Tab, window: &mut Window, cx: &mut Context<Self>) {
+    fn tab(&mut self, _: &Tab, window: &mut Window, cx: &mut Context<Self>) {
         self.cycle_field(Direction::Next, window, cx);
     }
 
-    fn backtab(
-        &mut self,
-        _: &editor::actions::Backtab,
-        window: &mut Window,
-        cx: &mut Context<Self>,
-    ) {
+    fn backtab(&mut self, _: &Backtab, window: &mut Window, cx: &mut Context<Self>) {
         self.cycle_field(Direction::Prev, window, cx);
     }
 
@@ -1637,29 +1633,22 @@ impl ProjectSearchBar {
     fn cycle_field(&mut self, direction: Direction, window: &mut Window, cx: &mut Context<Self>) {
         let active_project_search = match &self.active_project_search {
             Some(active_project_search) => active_project_search,
-
-            None => {
-                return;
-            }
+            None => return,
         };
 
         active_project_search.update(cx, |project_view, cx| {
-            let mut views = vec![&project_view.query_editor];
+            let mut views = vec![project_view.query_editor.focus_handle(cx)];
             if project_view.replace_enabled {
-                views.push(&project_view.replacement_editor);
+                views.push(project_view.replacement_editor.focus_handle(cx));
             }
             if project_view.filters_enabled {
                 views.extend([
-                    &project_view.included_files_editor,
-                    &project_view.excluded_files_editor,
+                    project_view.included_files_editor.focus_handle(cx),
+                    project_view.excluded_files_editor.focus_handle(cx),
                 ]);
             }
-            let current_index = match views
-                .iter()
-                .enumerate()
-                .find(|(_, editor)| editor.focus_handle(cx).is_focused(window))
-            {
-                Some((index, _)) => index,
+            let current_index = match views.iter().position(|focus| focus.is_focused(window)) {
+                Some(index) => index,
                 None => return,
             };
 
@@ -1668,8 +1657,8 @@ impl ProjectSearchBar {
                 Direction::Prev if current_index == 0 => views.len() - 1,
                 Direction::Prev => (current_index - 1) % views.len(),
             };
-            let next_focus_handle = views[new_index].focus_handle(cx);
-            window.focus(&next_focus_handle);
+            let next_focus_handle = &views[new_index];
+            window.focus(next_focus_handle);
             cx.stop_propagation();
         });
     }
@@ -2213,14 +2202,8 @@ impl Render for ProjectSearchBar {
             .on_action(cx.listener(|this, _: &ToggleFilters, window, cx| {
                 this.toggle_filters(window, cx);
             }))
-            .capture_action(cx.listener(|this, action, window, cx| {
-                this.tab(action, window, cx);
-                cx.stop_propagation();
-            }))
-            .capture_action(cx.listener(|this, action, window, cx| {
-                this.backtab(action, window, cx);
-                cx.stop_propagation();
-            }))
+            .capture_action(cx.listener(Self::tab))
+            .capture_action(cx.listener(Self::backtab))
             .on_action(cx.listener(|this, action, window, cx| this.confirm(action, window, cx)))
             .on_action(cx.listener(|this, action, window, cx| {
                 this.toggle_replace(action, window, cx);

crates/workspace/src/workspace.rs 🔗

@@ -3880,9 +3880,7 @@ impl Workspace {
                 local,
                 focus_changed,
             } => {
-                cx.on_next_frame(window, |_, window, _| {
-                    window.invalidate_character_coordinates();
-                });
+                window.invalidate_character_coordinates();
 
                 pane.update(cx, |pane, _| {
                     pane.track_alternate_file_items();
@@ -3923,9 +3921,7 @@ impl Workspace {
                 }
             }
             pane::Event::Focus => {
-                cx.on_next_frame(window, |_, window, _| {
-                    window.invalidate_character_coordinates();
-                });
+                window.invalidate_character_coordinates();
                 self.handle_pane_focused(pane.clone(), window, cx);
             }
             pane::Event::ZoomIn => {