From b4b0f622de531a58a6fa6b1f8c80743105e6ec32 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Fri, 7 Jul 2023 11:33:15 -0600 Subject: [PATCH] Rebuild vim search experience on refactored code --- assets/keymaps/vim.json | 2 +- crates/ai/src/assistant.rs | 4 +- crates/editor/src/items.rs | 22 ++-- crates/search/src/buffer_search.rs | 99 +++++++++------- crates/search/src/project_search.rs | 2 +- crates/vim/src/normal/search.rs | 172 ++++++++++++++++++++-------- crates/vim/src/state.rs | 18 +++ crates/workspace/src/searchable.rs | 23 ++-- 8 files changed, 226 insertions(+), 116 deletions(-) diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 4a215dcef3..40ebe13558 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -357,7 +357,7 @@ { "context": "BufferSearchBar", "bindings": { - "enter": "buffer_search::FocusEditor", + "enter": "vim::SearchSubmit", "escape": "buffer_search::Dismiss" } } diff --git a/crates/ai/src/assistant.rs b/crates/ai/src/assistant.rs index 3cc97468c3..4ca771ebcb 100644 --- a/crates/ai/src/assistant.rs +++ b/crates/ai/src/assistant.rs @@ -330,13 +330,13 @@ impl AssistantPanel { fn select_next_match(&mut self, _: &search::SelectNextMatch, cx: &mut ViewContext) { if let Some(search_bar) = self.toolbar.read(cx).item_of_type::() { - search_bar.update(cx, |bar, cx| bar.select_match(Direction::Next, cx)); + search_bar.update(cx, |bar, cx| bar.select_match(Direction::Next, None, cx)); } } fn select_prev_match(&mut self, _: &search::SelectPrevMatch, cx: &mut ViewContext) { if let Some(search_bar) = self.toolbar.read(cx).item_of_type::() { - search_bar.update(cx, |bar, cx| bar.select_match(Direction::Prev, cx)); + search_bar.update(cx, |bar, cx| bar.select_match(Direction::Prev, None, cx)); } } diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 86f26bcf94..6b2cdacaa2 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -937,7 +937,9 @@ impl SearchableItem for Editor { ) { self.unfold_ranges([matches[index].clone()], false, true, cx); let range = self.range_for_match(&matches[index]); - self.change_selections(Some(Autoscroll::fit()), cx, |s| s.select_ranges([range])); + self.change_selections(Some(Autoscroll::fit()), cx, |s| { + s.select_ranges([range]); + }) } fn match_index_for_direction( @@ -945,11 +947,12 @@ impl SearchableItem for Editor { matches: &Vec>, mut current_index: usize, direction: Direction, + count: Option, cx: &mut ViewContext, ) -> usize { let buffer = self.buffer().read(cx).snapshot(cx); let cursor = self.selections.newest_anchor().head(); - if matches[current_index].start.cmp(&cursor, &buffer).is_gt() { + if count.is_none() && matches[current_index].start.cmp(&cursor, &buffer).is_gt() { if direction == Direction::Prev { if current_index == 0 { current_index = matches.len() - 1; @@ -957,22 +960,19 @@ impl SearchableItem for Editor { current_index -= 1; } } - } else if matches[current_index].end.cmp(&cursor, &buffer).is_lt() { + } else if count.is_none() && matches[current_index].end.cmp(&cursor, &buffer).is_lt() { if direction == Direction::Next { current_index = 0; } } else if direction == Direction::Prev { - if current_index == 0 { - current_index = matches.len() - 1; + let count = count.unwrap_or(1) % matches.len(); + if current_index >= count { + current_index = current_index - count; } else { - current_index -= 1; + current_index = matches.len() - (count - current_index); } } else if direction == Direction::Next { - if current_index == matches.len() - 1 { - current_index = 0 - } else { - current_index += 1; - } + current_index = (current_index + count.unwrap_or(1)) % matches.len() }; current_index } diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index c8d1c58b6f..2bd765f8bb 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -66,7 +66,6 @@ pub struct BufferSearchBar { pub query_editor: ViewHandle, active_searchable_item: Option>, active_match_index: Option, - pending_match_direction: Option, active_searchable_item_subscription: Option, seachable_items_with_matches: HashMap, Vec>>, @@ -254,7 +253,6 @@ impl BufferSearchBar { default_options: SearchOptions::NONE, search_options: SearchOptions::NONE, pending_search: None, - pending_match_direction: None, query_contains_error: false, dismissed: true, } @@ -281,12 +279,9 @@ impl BufferSearchBar { } pub fn show(&mut self, cx: &mut ViewContext) -> bool { - let searchable_item = if let Some(searchable_item) = &self.active_searchable_item { - SearchableItemHandle::boxed_clone(searchable_item.as_ref()) - } else { + if self.active_searchable_item.is_none() { return false; - }; - + } self.dismissed = false; cx.notify(); cx.emit(Event::UpdateLocation); @@ -296,44 +291,53 @@ impl BufferSearchBar { pub fn search_suggested(&mut self, cx: &mut ViewContext) { let search = self .query_suggestion(cx) - .map(|suggestion| self.search(&suggestion, self.default_options, cx)); + .map(|suggestion| self.search(&suggestion, Some(self.default_options), cx)); if let Some(search) = search { cx.spawn(|this, mut cx| async move { search.await?; - this.update(&mut cx, |this, cx| { - if let Some(match_ix) = this.active_match_index { - if let Some(active_searchable_item) = this.active_searchable_item.as_ref() { - if let Some(matches) = this - .seachable_items_with_matches - .get(&active_searchable_item.downgrade()) - { - active_searchable_item.activate_match(match_ix, matches, cx); - } - } - } - }) + this.update(&mut cx, |this, cx| this.activate_current_match(cx)) }) .detach_and_log_err(cx); } } + pub fn activate_current_match(&mut self, cx: &mut ViewContext) { + if let Some(match_ix) = self.active_match_index { + if let Some(active_searchable_item) = self.active_searchable_item.as_ref() { + if let Some(matches) = self + .seachable_items_with_matches + .get(&active_searchable_item.downgrade()) + { + active_searchable_item.activate_match(match_ix, matches, cx) + } + } + } + } + pub fn select_query(&mut self, cx: &mut ViewContext) { self.query_editor.update(cx, |query_editor, cx| { query_editor.select_all(&Default::default(), cx); }); } - pub fn query_suggestion(&self, cx: &mut ViewContext) -> Option { - Some(self.active_searchable_item.as_ref()?.query_suggestion(cx)) + pub fn query(&self, cx: &WindowContext) -> String { + self.query_editor.read(cx).text(cx) } - fn search( + pub fn query_suggestion(&mut self, cx: &mut ViewContext) -> Option { + self.active_searchable_item + .as_ref() + .map(|searchable_item| searchable_item.query_suggestion(cx)) + } + + pub fn search( &mut self, query: &str, - options: SearchOptions, + options: Option, cx: &mut ViewContext, ) -> oneshot::Receiver<()> { + let options = options.unwrap_or(self.default_options); if query != self.query_editor.read(cx).text(cx) || self.search_options != options { self.query_editor.update(cx, |query_editor, cx| { query_editor.buffer().update(cx, |query_buffer, cx| { @@ -499,7 +503,7 @@ impl BufferSearchBar { cx.propagate_action(); } - fn focus_editor(&mut self, _: &FocusEditor, cx: &mut ViewContext) { + pub fn focus_editor(&mut self, _: &FocusEditor, cx: &mut ViewContext) { if let Some(active_editor) = self.active_searchable_item.as_ref() { cx.focus(active_editor.as_any()); } @@ -512,23 +516,37 @@ impl BufferSearchBar { cx.notify(); } + pub fn set_search_options( + &mut self, + search_options: SearchOptions, + cx: &mut ViewContext, + ) { + self.search_options = search_options; + cx.notify(); + } + fn select_next_match(&mut self, _: &SelectNextMatch, cx: &mut ViewContext) { - self.select_match(Direction::Next, cx); + self.select_match(Direction::Next, None, cx); } fn select_prev_match(&mut self, _: &SelectPrevMatch, cx: &mut ViewContext) { - self.select_match(Direction::Prev, cx); + self.select_match(Direction::Prev, None, cx); } - pub fn select_match(&mut self, direction: Direction, cx: &mut ViewContext) { + pub fn select_match( + &mut self, + direction: Direction, + count: Option, + cx: &mut ViewContext, + ) { if let Some(index) = self.active_match_index { if let Some(searchable_item) = self.active_searchable_item.as_ref() { if let Some(matches) = self .seachable_items_with_matches .get(&searchable_item.downgrade()) { - let new_match_index = - searchable_item.match_index_for_direction(matches, index, direction, cx); + let new_match_index = searchable_item + .match_index_for_direction(matches, index, direction, count, cx); searchable_item.update_matches(matches, cx); searchable_item.activate_match(new_match_index, matches, cx); } @@ -563,15 +581,12 @@ impl BufferSearchBar { cx: &mut ViewContext, ) { if let editor::Event::Edited { .. } = event { - let query = self.query_editor.read(cx).text(cx); - let search = self.search(&query, self.search_options, cx); self.query_contains_error = false; self.clear_matches(cx); let search = self.update_matches(cx); cx.spawn(|this, mut cx| async move { search.await?; - this.update(&mut cx, |this, cx| this.select_match(Direction::Next, cx))?; - anyhow::Ok(()) + this.update(&mut cx, |this, cx| this.activate_current_match(cx)) }) .detach_and_log_err(cx); } @@ -611,7 +626,6 @@ impl BufferSearchBar { if let Some(active_searchable_item) = self.active_searchable_item.as_ref() { if query.is_empty() { self.active_match_index.take(); - self.pending_match_direction.take(); active_searchable_item.clear_matches(cx); let _ = done_tx.send(()); } else { @@ -733,10 +747,9 @@ mod tests { // Search for a string that appears with different casing. // By default, search is case-insensitive. search_bar - .update(cx, |search_bar, cx| { - search_bar.search("us", search_bar.default_options, cx) - }) - .await; + .update(cx, |search_bar, cx| search_bar.search("us", None, cx)) + .await + .unwrap(); editor.update(cx, |editor, cx| { assert_eq!( editor.all_background_highlights(cx), @@ -770,10 +783,10 @@ mod tests { // Search for a string that appears both as a whole word and // within other words. By default, all results are found. - search_bar.update(cx, |search_bar, cx| { - search_bar.search("or", search_bar.default_options, cx); - }); - editor.next_notification(cx).await; + search_bar + .update(cx, |search_bar, cx| search_bar.search("or", None, cx)) + .await + .unwrap(); editor.update(cx, |editor, cx| { assert_eq!( editor.all_background_highlights(cx), diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 4e485eaaab..76350f1812 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -627,7 +627,7 @@ impl ProjectSearchView { if let Some(index) = self.active_match_index { let match_ranges = self.model.read(cx).match_ranges.clone(); let new_index = self.results_editor.update(cx, |editor, cx| { - editor.match_index_for_direction(&match_ranges, index, direction, cx) + editor.match_index_for_direction(&match_ranges, index, direction, None, cx) }); let range_to_select = match_ranges[new_index].clone(); diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index f23de53c37..70e397bcb0 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -1,9 +1,9 @@ -use gpui::{impl_actions, AppContext, ViewContext}; -use search::{BufferSearchBar, SearchOptions}; +use gpui::{actions, impl_actions, AppContext, ViewContext}; +use search::{buffer_search, BufferSearchBar, SearchOptions}; use serde_derive::Deserialize; -use workspace::{searchable::Direction, Workspace}; +use workspace::{searchable::Direction, Pane, Workspace}; -use crate::Vim; +use crate::{state::SearchState, Vim}; #[derive(Clone, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] @@ -26,11 +26,14 @@ pub(crate) struct Search { } impl_actions!(vim, [MoveToNext, MoveToPrev, Search]); +actions!(vim, [SearchSubmit]); pub(crate) fn init(cx: &mut AppContext) { cx.add_action(move_to_next); cx.add_action(move_to_prev); cx.add_action(search); + cx.add_action(search_submit); + cx.add_action(search_deploy); } fn move_to_next(workspace: &mut Workspace, action: &MoveToNext, cx: &mut ViewContext) { @@ -43,19 +46,68 @@ fn move_to_prev(workspace: &mut Workspace, action: &MoveToPrev, cx: &mut ViewCon fn search(workspace: &mut Workspace, action: &Search, cx: &mut ViewContext) { let pane = workspace.active_pane().clone(); - pane.update(cx, |pane, cx| { - if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::() { - search_bar.update(cx, |search_bar, cx| { - let options = SearchOptions::CASE_SENSITIVE | SearchOptions::REGEX; - let direction = if action.backwards { - Direction::Prev - } else { - Direction::Next - }; - search_bar.select_match(direction, cx); - // search_bar.show_with_options(true, false, options, cx); - }) - } + let direction = if action.backwards { + Direction::Prev + } else { + Direction::Next + }; + Vim::update(cx, |vim, cx| { + let count = vim.pop_number_operator(cx).unwrap_or(1); + pane.update(cx, |pane, cx| { + if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::() { + search_bar.update(cx, |search_bar, cx| { + if !search_bar.show(cx) { + return; + } + let query = search_bar.query(cx); + + search_bar.select_query(cx); + cx.focus_self(); + + if query.is_empty() { + search_bar.set_search_options( + SearchOptions::CASE_SENSITIVE | SearchOptions::REGEX, + cx, + ); + } + vim.state.search = SearchState { + direction, + count, + initial_query: query, + }; + }); + } + }) + }) +} + +fn search_deploy(_: &mut Pane, _: &buffer_search::Deploy, cx: &mut ViewContext) { + Vim::update(cx, |vim, _| vim.state.search = Default::default()); + cx.propagate_action(); +} + +fn search_submit(workspace: &mut Workspace, _: &SearchSubmit, cx: &mut ViewContext) { + Vim::update(cx, |vim, cx| { + let pane = workspace.active_pane().clone(); + pane.update(cx, |pane, cx| { + if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::() { + search_bar.update(cx, |search_bar, cx| { + let mut state = &mut vim.state.search; + let mut count = state.count; + + // in the case that the query has changed, the search bar + // will have selected the next match already. + if (search_bar.query(cx) != state.initial_query) + && state.direction == Direction::Next + { + count = count.saturating_sub(1); + } + search_bar.select_match(state.direction, Some(count), cx); + state.count = 1; + search_bar.focus_editor(&Default::default(), cx); + }); + } + }); }) } @@ -67,18 +119,32 @@ pub fn move_to_internal( ) { Vim::update(cx, |vim, cx| { let pane = workspace.active_pane().clone(); + let count = vim.pop_number_operator(cx).unwrap_or(1); pane.update(cx, |pane, cx| { if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::() { - search_bar.update(cx, |search_bar, cx| { - // let mut options = SearchOptions::CASE_SENSITIVE; - // options.set(SearchOptions::WHOLE_WORD, whole_word); - // search_bar.show(false, false, cx); - // let word = search_bar.query_suggestion(); - // search_bar.show() - // search_bar.search(word, options) - - // search_bar.select_word_under_cursor(direction, options, cx); + let search = search_bar.update(cx, |search_bar, cx| { + let mut options = SearchOptions::CASE_SENSITIVE; + options.set(SearchOptions::WHOLE_WORD, whole_word); + if search_bar.show(cx) { + search_bar + .query_suggestion(cx) + .map(|query| search_bar.search(&query, Some(options), cx)) + } else { + None + } }); + + if let Some(search) = search { + let search_bar = search_bar.downgrade(); + cx.spawn(|_, mut cx| async move { + search.await?; + search_bar.update(&mut cx, |search_bar, cx| { + search_bar.select_match(direction, Some(count), cx) + })?; + anyhow::Ok(()) + }) + .detach_and_log_err(cx); + } } }); vim.clear_operator(cx); @@ -100,15 +166,6 @@ mod test { deterministic: Arc, ) { let mut cx = VimTestContext::new(cx, true).await; - let search_bar = cx.workspace(|workspace, cx| { - workspace - .active_pane() - .read(cx) - .toolbar() - .read(cx) - .item_of_type::() - .expect("Buffer search bar should be deployed") - }); cx.set_state("ˇhi\nhigh\nhi\n", Mode::Normal); cx.simulate_keystrokes(["*"]); @@ -127,6 +184,10 @@ mod test { deterministic.run_until_parked(); cx.assert_state("ˇhi\nhigh\nhi\n", Mode::Normal); + cx.simulate_keystrokes(["2", "*"]); + deterministic.run_until_parked(); + cx.assert_state("ˇhi\nhigh\nhi\n", Mode::Normal); + cx.simulate_keystrokes(["g", "*"]); deterministic.run_until_parked(); cx.assert_state("hi\nˇhigh\nhi\n", Mode::Normal); @@ -140,7 +201,10 @@ mod test { } #[gpui::test] - async fn test_search(cx: &mut gpui::TestAppContext) { + async fn test_search( + cx: &mut gpui::TestAppContext, + deterministic: Arc, + ) { let mut cx = VimTestContext::new(cx, true).await; cx.set_state("aa\nbˇb\ncc\ncc\ncc\n", Mode::Normal); @@ -160,8 +224,7 @@ mod test { assert_eq!(bar.query_editor.read(cx).text(cx), "cc"); }); - // wait for the query editor change event to fire. - search_bar.next_notification(&cx).await; + deterministic.run_until_parked(); cx.update_editor(|editor, cx| { let highlights = editor.all_background_highlights(cx); @@ -173,30 +236,49 @@ mod test { }); cx.simulate_keystrokes(["enter"]); + cx.assert_state("aa\nbb\nˇcc\ncc\ncc\n", Mode::Normal); // n to go to next/N to go to previous - cx.assert_state("aa\nbb\nˇcc\ncc\ncc\n", Mode::Normal); cx.simulate_keystrokes(["n"]); cx.assert_state("aa\nbb\ncc\nˇcc\ncc\n", Mode::Normal); cx.simulate_keystrokes(["shift-n"]); + cx.assert_state("aa\nbb\nˇcc\ncc\ncc\n", Mode::Normal); // ? to go to previous - cx.assert_state("aa\nbb\nˇcc\ncc\ncc\n", Mode::Normal); cx.simulate_keystrokes(["?", "enter"]); + deterministic.run_until_parked(); cx.assert_state("aa\nbb\ncc\ncc\nˇcc\n", Mode::Normal); cx.simulate_keystrokes(["?", "enter"]); + deterministic.run_until_parked(); + cx.assert_state("aa\nbb\ncc\nˇcc\ncc\n", Mode::Normal); // / to go to next - cx.assert_state("aa\nbb\ncc\nˇcc\ncc\n", Mode::Normal); cx.simulate_keystrokes(["/", "enter"]); + deterministic.run_until_parked(); cx.assert_state("aa\nbb\ncc\ncc\nˇcc\n", Mode::Normal); // ?{search} to search backwards cx.simulate_keystrokes(["?", "b", "enter"]); - - // wait for the query editor change event to fire. - search_bar.next_notification(&cx).await; - + deterministic.run_until_parked(); cx.assert_state("aa\nbˇb\ncc\ncc\ncc\n", Mode::Normal); + + // works with counts + cx.simulate_keystrokes(["4", "/", "c"]); + deterministic.run_until_parked(); + cx.simulate_keystrokes(["enter"]); + cx.assert_state("aa\nbb\ncc\ncˇc\ncc\n", Mode::Normal); + + // check that searching resumes from cursor, not previous match + cx.set_state("ˇaa\nbb\ndd\ncc\nbb\n", Mode::Normal); + cx.simulate_keystrokes(["/", "d"]); + deterministic.run_until_parked(); + cx.simulate_keystrokes(["enter"]); + cx.assert_state("aa\nbb\nˇdd\ncc\nbb\n", Mode::Normal); + cx.update_editor(|editor, cx| editor.move_to_beginning(&Default::default(), cx)); + cx.assert_state("ˇaa\nbb\ndd\ncc\nbb\n", Mode::Normal); + cx.simulate_keystrokes(["/", "b"]); + deterministic.run_until_parked(); + cx.simulate_keystrokes(["enter"]); + cx.assert_state("aa\nˇbb\ndd\ncc\nbb\n", Mode::Normal); } } diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index e1a06fce59..6434b710b2 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -1,6 +1,7 @@ use gpui::keymap_matcher::KeymapContext; use language::CursorShape; use serde::{Deserialize, Serialize}; +use workspace::searchable::Direction; #[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)] pub enum Mode { @@ -38,6 +39,23 @@ pub enum Operator { pub struct VimState { pub mode: Mode, pub operator_stack: Vec, + pub search: SearchState, +} + +pub struct SearchState { + pub direction: Direction, + pub count: usize, + pub initial_query: String, +} + +impl Default for SearchState { + fn default() -> Self { + Self { + direction: Direction::Next, + count: 1, + initial_query: "".to_string(), + } + } } impl VimState { diff --git a/crates/workspace/src/searchable.rs b/crates/workspace/src/searchable.rs index 7e3f7227b0..bdbed072b0 100644 --- a/crates/workspace/src/searchable.rs +++ b/crates/workspace/src/searchable.rs @@ -50,26 +50,21 @@ pub trait SearchableItem: Item { fn match_index_for_direction( &mut self, matches: &Vec, - mut current_index: usize, + current_index: usize, direction: Direction, + count: Option, _: &mut ViewContext, ) -> usize { match direction { Direction::Prev => { - if current_index == 0 { - matches.len() - 1 + let count = count.unwrap_or(1) % matches.len(); + if current_index >= count { + current_index - count } else { - current_index - 1 - } - } - Direction::Next => { - current_index += 1; - if current_index == matches.len() { - 0 - } else { - current_index + matches.len() - (count - current_index) } } + Direction::Next => (current_index + count.unwrap_or(1)) % matches.len(), } } fn find_matches( @@ -107,6 +102,7 @@ pub trait SearchableItemHandle: ItemHandle { matches: &Vec>, current_index: usize, direction: Direction, + count: Option, cx: &mut WindowContext, ) -> usize; fn find_matches( @@ -170,11 +166,12 @@ impl SearchableItemHandle for ViewHandle { matches: &Vec>, current_index: usize, direction: Direction, + count: Option, cx: &mut WindowContext, ) -> usize { let matches = downcast_matches(matches); self.update(cx, |this, cx| { - this.match_index_for_direction(&matches, current_index, direction, cx) + this.match_index_for_direction(&matches, current_index, direction, count, cx) }) } fn find_matches(