From 229e8538741c3ce5b8672b710aa6030c380de7ab Mon Sep 17 00:00:00 2001 From: Nico Lehmann Date: Tue, 4 Mar 2025 18:27:37 -0800 Subject: [PATCH] Make buffer search aware of search direction (#24974) This solves a couple of issues with Vim search by making the search buffer and `SearchableItem` aware of the direction of the search. If `SearchOptions::BACKWARDS` is set, all operations will be reversed. By making `SearchableItem` aware of the direction, the correct active match can be selected when searching backward. Fixes #22506. This PR does not fix the last problem in that issue, but that one is also tracked in #8049. Release Notes: - Fixes incorrect behavior of backward search in Vim mode --- .../src/context_editor.rs | 5 +- crates/editor/src/items.rs | 16 +++++-- crates/language_tools/src/lsp_log.rs | 8 ++-- crates/search/src/buffer_search.rs | 11 ++++- crates/search/src/project_search.rs | 1 + crates/search/src/search.rs | 2 + crates/terminal_view/src/terminal_view.rs | 46 +++++++++++++------ crates/vim/src/normal/search.rs | 11 ++++- crates/workspace/src/searchable.rs | 5 +- 9 files changed, 80 insertions(+), 25 deletions(-) diff --git a/crates/assistant_context_editor/src/context_editor.rs b/crates/assistant_context_editor/src/context_editor.rs index bce25821af..d7dd9e4d83 100644 --- a/crates/assistant_context_editor/src/context_editor.rs +++ b/crates/assistant_context_editor/src/context_editor.rs @@ -54,7 +54,7 @@ use ui::{ Tooltip, }; use util::{maybe, ResultExt}; -use workspace::searchable::SearchableItemHandle; +use workspace::searchable::{Direction, SearchableItemHandle}; use workspace::{ item::{self, FollowableItem, Item, ItemHandle}, notifications::NotificationId, @@ -3060,12 +3060,13 @@ impl SearchableItem for ContextEditor { fn active_match_index( &mut self, + direction: Direction, matches: &[Self::Match], window: &mut Window, cx: &mut Context, ) -> Option { self.editor.update(cx, |editor, cx| { - editor.active_match_index(matches, window, cx) + editor.active_match_index(direction, matches, window, cx) }) } } diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index bbb8943145..aa27905f1e 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -1589,11 +1589,13 @@ impl SearchableItem for Editor { fn active_match_index( &mut self, + direction: Direction, matches: &[Range], _: &mut Window, cx: &mut Context, ) -> Option { active_match_index( + direction, matches, &self.selections.newest_anchor().head(), &self.buffer().read(cx).snapshot(cx), @@ -1606,6 +1608,7 @@ impl SearchableItem for Editor { } pub fn active_match_index( + direction: Direction, ranges: &[Range], cursor: &Anchor, buffer: &MultiBufferSnapshot, @@ -1613,7 +1616,7 @@ pub fn active_match_index( if ranges.is_empty() { None } else { - match ranges.binary_search_by(|probe| { + let r = ranges.binary_search_by(|probe| { if probe.end.cmp(cursor, buffer).is_lt() { Ordering::Less } else if probe.start.cmp(cursor, buffer).is_gt() { @@ -1621,8 +1624,15 @@ pub fn active_match_index( } else { Ordering::Equal } - }) { - Ok(i) | Err(i) => Some(cmp::min(i, ranges.len() - 1)), + }); + match direction { + Direction::Prev => match r { + Ok(i) => Some(i), + Err(i) => Some(i.saturating_sub(1)), + }, + Direction::Next => match r { + Ok(i) | Err(i) => Some(cmp::min(i, ranges.len() - 1)), + }, } } } diff --git a/crates/language_tools/src/lsp_log.rs b/crates/language_tools/src/lsp_log.rs index dcb0c5b595..12ab583d96 100644 --- a/crates/language_tools/src/lsp_log.rs +++ b/crates/language_tools/src/lsp_log.rs @@ -16,7 +16,7 @@ use std::{borrow::Cow, sync::Arc}; use ui::{prelude::*, Button, Checkbox, ContextMenu, Label, PopoverMenu, ToggleState}; use workspace::{ item::{Item, ItemHandle}, - searchable::{SearchEvent, SearchableItem, SearchableItemHandle}, + searchable::{Direction, SearchEvent, SearchableItem, SearchableItemHandle}, SplitDirection, ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView, Workspace, WorkspaceId, }; @@ -1170,12 +1170,14 @@ impl SearchableItem for LspLogView { } fn active_match_index( &mut self, + direction: Direction, matches: &[Self::Match], window: &mut Window, cx: &mut Context, ) -> Option { - self.editor - .update(cx, |e, cx| e.active_match_index(matches, window, cx)) + self.editor.update(cx, |e, cx| { + e.active_match_index(direction, matches, window, cx) + }) } } diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index 97a2e31ac1..9ff0daaa79 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -1306,7 +1306,16 @@ impl BufferSearchBar { done_rx } + fn reverse_direction_if_backwards(&self, direction: Direction) -> Direction { + if self.search_options.contains(SearchOptions::BACKWARDS) { + direction.opposite() + } else { + direction + } + } + pub fn update_match_index(&mut self, window: &mut Window, cx: &mut Context) { + let direction = self.reverse_direction_if_backwards(Direction::Next); let new_index = self .active_searchable_item .as_ref() @@ -1314,7 +1323,7 @@ impl BufferSearchBar { let matches = self .searchable_items_with_matches .get(&searchable_item.downgrade())?; - searchable_item.active_match_index(matches, window, cx) + searchable_item.active_match_index(direction, matches, window, cx) }); if new_index != self.active_match_index { self.active_match_index = new_index; diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index bee19b3f47..e702ac25b4 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -1240,6 +1240,7 @@ impl ProjectSearchView { fn update_match_index(&mut self, cx: &mut Context) { let results_editor = self.results_editor.read(cx); let new_index = active_match_index( + Direction::Next, &self.entity.read(cx).match_ranges, &results_editor.selections.newest_anchor().head(), &results_editor.buffer().read(cx).snapshot(cx), diff --git a/crates/search/src/search.rs b/crates/search/src/search.rs index 4bfea34c78..6745167c3f 100644 --- a/crates/search/src/search.rs +++ b/crates/search/src/search.rs @@ -47,6 +47,8 @@ bitflags! { const CASE_SENSITIVE = 0b010; const INCLUDE_IGNORED = 0b100; const REGEX = 0b1000; + /// If set, reverse direction when finding the active match + const BACKWARDS = 0b10000; } } diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 28a926c611..bf371773b7 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -41,7 +41,7 @@ use workspace::{ BreadcrumbText, Item, ItemEvent, SerializableItem, TabContentParams, TabTooltipContent, }, register_serializable_item, - searchable::{SearchEvent, SearchOptions, SearchableItem, SearchableItemHandle}, + searchable::{Direction, SearchEvent, SearchOptions, SearchableItem, SearchableItemHandle}, CloseActiveItem, NewCenterTerminal, NewTerminal, OpenVisible, ToolbarItemLocation, Workspace, WorkspaceId, }; @@ -1583,6 +1583,7 @@ impl SearchableItem for TerminalView { /// Reports back to the search toolbar what the active match should be (the selection) fn active_match_index( &mut self, + direction: Direction, matches: &[Self::Match], _: &mut Window, cx: &mut Context, @@ -1593,19 +1594,36 @@ impl SearchableItem for TerminalView { let res = if !matches.is_empty() { if let Some(selection_head) = self.terminal().read(cx).selection_head { // If selection head is contained in a match. Return that match - if let Some(ix) = matches - .iter() - .enumerate() - .find(|(_, search_match)| { - search_match.contains(&selection_head) - || search_match.start() > &selection_head - }) - .map(|(ix, _)| ix) - { - Some(ix) - } else { - // If no selection after selection head, return the last match - Some(matches.len().saturating_sub(1)) + match direction { + Direction::Prev => { + // If no selection before selection head, return the first match + Some( + matches + .iter() + .enumerate() + .rev() + .find(|(_, search_match)| { + search_match.contains(&selection_head) + || search_match.start() < &selection_head + }) + .map(|(ix, _)| ix) + .unwrap_or(0), + ) + } + Direction::Next => { + // If no selection after selection head, return the last match + Some( + matches + .iter() + .enumerate() + .find(|(_, search_match)| { + search_match.contains(&selection_head) + || search_match.start() > &selection_head + }) + .map(|(ix, _)| ix) + .unwrap_or(matches.len().saturating_sub(1)), + ) + } } } else { // Matches found but no active selection, return the first last one (closest to cursor) diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index dcd9c2ad1c..24adfabdfb 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -154,6 +154,9 @@ impl Vim { if action.regex { options |= SearchOptions::REGEX; } + if action.backwards { + options |= SearchOptions::BACKWARDS; + } search_bar.set_search_options(options, cx); let prior_mode = if self.temp_mode { Mode::Insert @@ -198,7 +201,7 @@ impl Vim { .last() .map_or(true, |range| range.start != new_head); - if is_different_head && self.search.direction == Direction::Next { + if is_different_head { count = count.saturating_sub(1) } self.search.count = 1; @@ -743,6 +746,12 @@ mod test { cx.simulate_keystrokes("*"); cx.assert_state("one two ˇone", Mode::Normal); + // check that a backward search after last match works correctly + cx.set_state("aa\naa\nbbˇ", Mode::Normal); + cx.simulate_keystrokes("? a a"); + cx.simulate_keystrokes("enter"); + cx.assert_state("aa\nˇaa\nbb", Mode::Normal); + // check that searching with unable search wrap cx.update_global(|store: &mut SettingsStore, cx| { store.update_user_settings::(cx, |s| s.search_wrap = Some(false)); diff --git a/crates/workspace/src/searchable.rs b/crates/workspace/src/searchable.rs index 080307538b..63f572931c 100644 --- a/crates/workspace/src/searchable.rs +++ b/crates/workspace/src/searchable.rs @@ -150,6 +150,7 @@ pub trait SearchableItem: Item + EventEmitter { ) -> Task>; fn active_match_index( &mut self, + direction: Direction, matches: &[Self::Match], window: &mut Window, cx: &mut Context, @@ -208,6 +209,7 @@ pub trait SearchableItemHandle: ItemHandle { ) -> Task>; fn active_match_index( &self, + direction: Direction, matches: &AnyVec, window: &mut Window, cx: &mut App, @@ -315,13 +317,14 @@ impl SearchableItemHandle for Entity { } fn active_match_index( &self, + direction: Direction, matches: &AnyVec, window: &mut Window, cx: &mut App, ) -> Option { let matches = matches.downcast_ref()?; self.update(cx, |this, cx| { - this.active_match_index(matches.as_slice(), window, cx) + this.active_match_index(direction, matches.as_slice(), window, cx) }) }