From c9bf4074311dcb2cf23da842bb19c354efca026d Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 17 Jul 2023 12:02:10 -0600 Subject: [PATCH] Avoid optional on select_match --- crates/ai/src/assistant.rs | 4 +- crates/editor/src/items.rs | 70 ++++++++++--------- crates/search/src/buffer_search.rs | 100 +++++++++------------------- crates/search/src/project_search.rs | 3 +- crates/vim/src/normal/search.rs | 7 +- crates/workspace/src/searchable.rs | 10 +-- 6 files changed, 80 insertions(+), 114 deletions(-) diff --git a/crates/ai/src/assistant.rs b/crates/ai/src/assistant.rs index 5d587401e5c05e24447b0600630a4239919c0add..8a4c04d3387784e0e2b5e4e7d745c690f72c02aa 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, None, cx)); + search_bar.update(cx, |bar, cx| bar.select_match(Direction::Next, 1, 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, None, cx)); + search_bar.update(cx, |bar, cx| bar.select_match(Direction::Prev, 1, cx)); } } diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index c7a93e754acd94695279974d11abf3831158079f..0ce41a97c96416af9bd4d45b1255c8f4b7ae0301 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -954,15 +954,19 @@ impl SearchableItem for Editor { fn select_matches(&mut self, matches: Vec, cx: &mut ViewContext) { self.unfold_ranges(matches.clone(), false, false, cx); - self.change_selections(None, cx, |s| s.select_ranges(matches)); + let mut ranges = Vec::new(); + for m in &matches { + ranges.push(self.range_for_match(&m)) + } + self.change_selections(None, cx, |s| s.select_ranges(ranges)); } fn match_index_for_direction( &mut self, matches: &Vec>, - mut current_index: usize, + current_index: usize, direction: Direction, - count: Option, + count: usize, cx: &mut ViewContext, ) -> usize { let buffer = self.buffer().read(cx).snapshot(cx); @@ -971,39 +975,39 @@ impl SearchableItem for Editor { } else { matches[current_index].start }; - if count.is_none() - && matches[current_index] - .start - .cmp(¤t_index_position, &buffer) - .is_gt() - { - if direction == Direction::Prev { - if current_index == 0 { - current_index = matches.len() - 1; - } else { - current_index -= 1; + + let mut count = count % matches.len(); + if count == 0 { + return current_index; + } + match direction { + Direction::Next => { + if matches[current_index] + .start + .cmp(¤t_index_position, &buffer) + .is_gt() + { + count = count - 1 } + + (current_index + count) % matches.len() } - } else if count.is_none() - && matches[current_index] - .end - .cmp(¤t_index_position, &buffer) - .is_lt() - { - if direction == Direction::Next { - current_index = 0; - } - } else if direction == Direction::Prev { - let count = count.unwrap_or(1) % matches.len(); - if current_index >= count { - current_index = current_index - count; - } else { - current_index = matches.len() - (count - current_index); + Direction::Prev => { + if matches[current_index] + .end + .cmp(¤t_index_position, &buffer) + .is_lt() + { + count = count - 1; + } + + if current_index >= count { + current_index - count + } else { + matches.len() - (count - current_index) + } } - } else if direction == Direction::Next { - current_index = (current_index + count.unwrap_or(1)) % matches.len() - }; - current_index + } } fn find_matches( diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index 7d50794108cf55b10c62c2e43e46f8da99b7bf63..7fade13a509dabd633b7da26601f90b770df3b6f 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -560,11 +560,11 @@ impl BufferSearchBar { } fn select_next_match(&mut self, _: &SelectNextMatch, cx: &mut ViewContext) { - self.select_match(Direction::Next, None, cx); + self.select_match(Direction::Next, 1, cx); } fn select_prev_match(&mut self, _: &SelectPrevMatch, cx: &mut ViewContext) { - self.select_match(Direction::Prev, None, cx); + self.select_match(Direction::Prev, 1, cx); } fn select_all_matches(&mut self, _: &SelectAllMatches, cx: &mut ViewContext) { @@ -581,12 +581,7 @@ impl BufferSearchBar { } } - pub fn select_match( - &mut self, - direction: Direction, - count: Option, - cx: &mut ViewContext, - ) { + pub fn select_match(&mut self, direction: Direction, count: usize, 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 @@ -1086,15 +1081,17 @@ mod tests { } #[gpui::test] - async fn test_search_with_options(cx: &mut TestAppContext) { + async fn test_search_option_handling(cx: &mut TestAppContext) { let (editor, search_bar) = init_test(cx); // show with options should make current search case sensitive - search_bar.update(cx, |search_bar, cx| { - search_bar.show_with_options(false, false, SearchOptions::CASE_SENSITIVE, cx); - search_bar.search("us", cx); - }); - editor.next_notification(cx).await; + search_bar + .update(cx, |search_bar, cx| { + search_bar.show(cx); + search_bar.search("us", Some(SearchOptions::CASE_SENSITIVE), cx) + }) + .await + .unwrap(); editor.update(cx, |editor, cx| { assert_eq!( editor.all_background_highlights(cx), @@ -1105,31 +1102,20 @@ mod tests { ); }); - // show should return to the default options (case insensitive) + // search_suggested should restore default options search_bar.update(cx, |search_bar, cx| { - search_bar.show(true, true, cx); - }); - editor.next_notification(cx).await; - editor.update(cx, |editor, cx| { - assert_eq!( - editor.all_background_highlights(cx), - &[ - ( - DisplayPoint::new(2, 17)..DisplayPoint::new(2, 19), - Color::red(), - ), - ( - DisplayPoint::new(2, 43)..DisplayPoint::new(2, 45), - Color::red(), - ) - ] - ); + search_bar.search_suggested(cx); + assert_eq!(search_bar.search_options, SearchOptions::NONE) }); - // toggling a search option (even in show_with_options mode) should update the defaults + // toggling a search option should update the defaults + search_bar + .update(cx, |search_bar, cx| { + search_bar.search("regex", Some(SearchOptions::CASE_SENSITIVE), cx) + }) + .await + .unwrap(); search_bar.update(cx, |search_bar, cx| { - search_bar.search("regex", cx); - search_bar.show_with_options(false, false, SearchOptions::CASE_SENSITIVE, cx); search_bar.toggle_search_option(SearchOptions::WHOLE_WORD, cx) }); editor.next_notification(cx).await; @@ -1145,38 +1131,11 @@ mod tests { // defaults should still include whole word search_bar.update(cx, |search_bar, cx| { - search_bar.show(true, true, cx); - }); - editor.next_notification(cx).await; - editor.update(cx, |editor, cx| { - assert_eq!( - editor.all_background_highlights(cx), - &[( - DisplayPoint::new(0, 35)..DisplayPoint::new(0, 40), - Color::red(), - ),] - ); - }); - - // removing whole word changes the search again - search_bar.update(cx, |search_bar, cx| { - search_bar.toggle_search_option(SearchOptions::WHOLE_WORD, cx) - }); - editor.next_notification(cx).await; - editor.update(cx, |editor, cx| { + search_bar.search_suggested(cx); assert_eq!( - editor.all_background_highlights(cx), - &[ - ( - DisplayPoint::new(0, 35)..DisplayPoint::new(0, 40), - Color::red(), - ), - ( - DisplayPoint::new(0, 44)..DisplayPoint::new(0, 49), - Color::red() - ) - ] - ); + search_bar.search_options, + SearchOptions::CASE_SENSITIVE | SearchOptions::WHOLE_WORD + ) }); } @@ -1207,15 +1166,18 @@ mod tests { let search_bar = cx.add_view(window_id, |cx| { let mut search_bar = BufferSearchBar::new(cx); search_bar.set_active_pane_item(Some(&editor), cx); - search_bar.show(false, true, cx); + search_bar.show(cx); search_bar }); + search_bar + .update(cx, |search_bar, cx| search_bar.search("a", None, cx)) + .await + .unwrap(); search_bar.update(cx, |search_bar, cx| { - search_bar.set_query("a", cx); + search_bar.activate_current_match(cx); }); - editor.next_notification(cx).await; let initial_selections = editor.update(cx, |editor, cx| { let initial_selections = editor.selections.display_ranges(cx); assert_eq!( diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 5bdcf72a97391aa928056a0f0a5c751d942e1a0f..abebb9a48f1a853097248d796550f00bb3e6cd70 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, None, cx) + editor.match_index_for_direction(&match_ranges, index, direction, 1, cx) }); let range_to_select = match_ranges[new_index].clone(); @@ -668,7 +668,6 @@ impl ProjectSearchView { self.active_match_index = None; } else { self.active_match_index = Some(0); - self.select_match(Direction::Next, cx); self.update_match_index(cx); let prev_search_id = mem::replace(&mut self.search_id, self.model.read(cx).search_id); let is_new_search = self.search_id != prev_search_id; diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index 70e397bcb0b88081fdbb1c86f2b881302af27f90..cae64a40a6b0cdb290014539b7c10ead6a7a0b82 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -81,6 +81,7 @@ fn search(workspace: &mut Workspace, action: &Search, cx: &mut ViewContext find. fn search_deploy(_: &mut Pane, _: &buffer_search::Deploy, cx: &mut ViewContext) { Vim::update(cx, |vim, _| vim.state.search = Default::default()); cx.propagate_action(); @@ -100,9 +101,9 @@ fn search_submit(workspace: &mut Workspace, _: &SearchSubmit, cx: &mut ViewConte if (search_bar.query(cx) != state.initial_query) && state.direction == Direction::Next { - count = count.saturating_sub(1); + count = count.saturating_sub(1) } - search_bar.select_match(state.direction, Some(count), cx); + search_bar.select_match(state.direction, count, cx); state.count = 1; search_bar.focus_editor(&Default::default(), cx); }); @@ -139,7 +140,7 @@ pub fn move_to_internal( cx.spawn(|_, mut cx| async move { search.await?; search_bar.update(&mut cx, |search_bar, cx| { - search_bar.select_match(direction, Some(count), cx) + search_bar.select_match(direction, count, cx) })?; anyhow::Ok(()) }) diff --git a/crates/workspace/src/searchable.rs b/crates/workspace/src/searchable.rs index 4f5e7099fae05fb691c78c505ee4b27adc76f345..ae95838a742bb623a9376947e905d6054a82fff4 100644 --- a/crates/workspace/src/searchable.rs +++ b/crates/workspace/src/searchable.rs @@ -57,19 +57,19 @@ pub trait SearchableItem: Item { matches: &Vec, current_index: usize, direction: Direction, - count: Option, + count: usize, _: &mut ViewContext, ) -> usize { match direction { Direction::Prev => { - let count = count.unwrap_or(1) % matches.len(); + let count = count % matches.len(); if current_index >= count { current_index - count } else { matches.len() - (count - current_index) } } - Direction::Next => (current_index + count.unwrap_or(1)) % matches.len(), + Direction::Next => (current_index + count) % matches.len(), } } fn find_matches( @@ -108,7 +108,7 @@ pub trait SearchableItemHandle: ItemHandle { matches: &Vec>, current_index: usize, direction: Direction, - count: Option, + count: usize, cx: &mut WindowContext, ) -> usize; fn find_matches( @@ -179,7 +179,7 @@ impl SearchableItemHandle for ViewHandle { matches: &Vec>, current_index: usize, direction: Direction, - count: Option, + count: usize, cx: &mut WindowContext, ) -> usize { let matches = downcast_matches(matches);