diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index ca2808afb2..a2d856760b 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -61,7 +61,7 @@ impl FileFinder { let abs_path = project .worktree_for_id(project_path.worktree_id, cx) .map(|worktree| worktree.read(cx).abs_path().join(&project_path.path)); - FoundPath::new(project_path, abs_path) + FoundPath::new_opened_file(project_path, abs_path) }); // if exists, bubble the currently opened path to the top @@ -139,7 +139,7 @@ pub struct FileFinderDelegate { latest_search_query: Option>, currently_opened_path: Option, matches: Matches, - selected_index: Option, + selected_index: usize, cancel_flag: Arc, history_items: Vec, } @@ -231,7 +231,15 @@ impl Matches { &mut self.history, history_items_to_show, 100, - |(_, a), (_, b)| b.cmp(a), + |(ap, a), (bp, b)| { + if ap.opened_file { + return cmp::Ordering::Less; + } + if bp.opened_file { + return cmp::Ordering::Greater; + } + b.cmp(a) + }, ); if extend_old_matches { @@ -305,11 +313,24 @@ fn matching_history_item_paths( struct FoundPath { project: ProjectPath, absolute: Option, + opened_file: bool, } impl FoundPath { fn new(project: ProjectPath, absolute: Option) -> Self { - Self { project, absolute } + Self { + project, + absolute, + opened_file: false, + } + } + + fn new_opened_file(project: ProjectPath, absolute: Option) -> Self { + Self { + project, + absolute, + opened_file: true, + } } } @@ -372,7 +393,7 @@ impl FileFinderDelegate { latest_search_query: None, currently_opened_path, matches: Matches::default(), - selected_index: None, + selected_index: 0, cancel_flag: Arc::new(AtomicBool::new(false)), history_items, } @@ -427,7 +448,6 @@ impl FileFinderDelegate { let did_cancel = cancel_flag.load(atomic::Ordering::Relaxed); picker .update(&mut cx, |picker, cx| { - picker.delegate.selected_index.take(); picker .delegate .set_search_matches(search_id, did_cancel, query, matches, cx) @@ -460,6 +480,7 @@ impl FileFinderDelegate { ); self.latest_search_query = Some(query); self.latest_search_did_cancel = did_cancel; + self.selected_index = self.calculate_selected_index(); cx.notify(); } } @@ -630,6 +651,20 @@ impl FileFinderDelegate { .log_err(); }) } + + /// Calculates selection index after the user performed search. + /// Prefers to return 1 if the top visible item corresponds to the currently opened file, otherwise returns 0. + fn calculate_selected_index(&self) -> usize { + let first = self.matches.history.get(0); + if let Some(first) = first { + if !first.0.opened_file { + return 0; + } + } else { + return 0; + } + (self.matches.len() - 1).min(1) + } } impl PickerDelegate for FileFinderDelegate { @@ -644,11 +679,11 @@ impl PickerDelegate for FileFinderDelegate { } fn selected_index(&self) -> usize { - self.selected_index.unwrap_or(0) + self.selected_index } fn set_selected_index(&mut self, ix: usize, cx: &mut ViewContext>) { - self.selected_index = Some(ix); + self.selected_index = ix; cx.notify(); } @@ -671,7 +706,6 @@ impl PickerDelegate for FileFinderDelegate { if raw_query.is_empty() { let project = self.project.read(cx); self.latest_search_id = post_inc(&mut self.search_count); - self.selected_index.take(); self.matches = Matches { history: self .history_items @@ -687,6 +721,7 @@ impl PickerDelegate for FileFinderDelegate { .collect(), search: Vec::new(), }; + self.selected_index = self.calculate_selected_index(); cx.notify(); Task::ready(()) } else { diff --git a/crates/file_finder/src/file_finder_tests.rs b/crates/file_finder/src/file_finder_tests.rs index 7702f71f46..66d1675227 100644 --- a/crates/file_finder/src/file_finder_tests.rs +++ b/crates/file_finder/src/file_finder_tests.rs @@ -1062,6 +1062,120 @@ async fn test_search_sorts_history_items(cx: &mut gpui::TestAppContext) { }); } +#[gpui::test] +async fn test_select_current_open_file_when_no_history(cx: &mut gpui::TestAppContext) { + let app_state = init_test(cx); + + app_state + .fs + .as_fake() + .insert_tree( + "/root", + json!({ + "test": { + "1_qw": "", + } + }), + ) + .await; + + let project = Project::test(app_state.fs.clone(), ["/root".as_ref()], cx).await; + let (workspace, cx) = cx.add_window_view(|cx| Workspace::test_new(project, cx)); + // Open new buffer + open_queried_buffer("1", 1, "1_qw", &workspace, cx).await; + + let picker = open_file_picker(&workspace, cx); + picker.update(cx, |finder, _| { + assert_match_selection(&finder, 0, "1_qw"); + }); +} + +#[gpui::test] +async fn test_keep_opened_file_on_top_of_search_results_and_select_next_one( + cx: &mut TestAppContext, +) { + let app_state = init_test(cx); + + app_state + .fs + .as_fake() + .insert_tree( + "/src", + json!({ + "test": { + "bar.rs": "// Bar file", + "lib.rs": "// Lib file", + "maaa.rs": "// Maaaaaaa", + "main.rs": "// Main file", + "moo.rs": "// Moooooo", + } + }), + ) + .await; + + let project = Project::test(app_state.fs.clone(), ["/src".as_ref()], cx).await; + let (workspace, cx) = cx.add_window_view(|cx| Workspace::test_new(project, cx)); + + open_close_queried_buffer("bar", 1, "bar.rs", &workspace, cx).await; + open_close_queried_buffer("lib", 1, "lib.rs", &workspace, cx).await; + open_queried_buffer("main", 1, "main.rs", &workspace, cx).await; + + // main.rs is on top, previously used is selected + let picker = open_file_picker(&workspace, cx); + picker.update(cx, |finder, _| { + assert_eq!(finder.delegate.matches.len(), 3); + assert_match_at_position(finder, 0, "main.rs"); + assert_match_selection(finder, 1, "lib.rs"); + }); + + // all files match, main.rs is still on top + picker + .update(cx, |finder, cx| { + finder.delegate.update_matches(".rs".to_string(), cx) + }) + .await; + picker.update(cx, |finder, _| { + assert_eq!(finder.delegate.matches.len(), 5); + assert_match_at_position(finder, 0, "main.rs"); + assert_match_selection(finder, 1, "bar.rs"); + }); + + // main.rs is not among matches, select top item + picker + .update(cx, |finder, cx| { + finder.delegate.update_matches("b".to_string(), cx) + }) + .await; + picker.update(cx, |finder, _| { + assert_eq!(finder.delegate.matches.len(), 2); + assert_match_at_position(finder, 0, "bar.rs"); + }); + + // main.rs is back, put it on top and select next item + picker + .update(cx, |finder, cx| { + finder.delegate.update_matches("m".to_string(), cx) + }) + .await; + picker.update(cx, |finder, _| { + assert_eq!(finder.delegate.matches.len(), 3); + assert_match_at_position(finder, 0, "main.rs"); + assert_match_selection(finder, 1, "moo.rs"); + }); + + // get back to the initial state + picker + .update(cx, |finder, cx| { + finder.delegate.update_matches("".to_string(), cx) + }) + .await; + picker.update(cx, |finder, _| { + assert_eq!(finder.delegate.matches.len(), 3); + assert_match_at_position(finder, 0, "main.rs"); + assert_match_selection(finder, 1, "lib.rs"); + }); +} + #[gpui::test] async fn test_history_items_vs_very_good_external_match(cx: &mut gpui::TestAppContext) { let app_state = init_test(cx); @@ -1172,6 +1286,27 @@ async fn open_close_queried_buffer( expected_editor_title: &str, workspace: &View, cx: &mut gpui::VisualTestContext, +) -> Vec { + let history_items = open_queried_buffer( + input, + expected_matches, + expected_editor_title, + workspace, + cx, + ) + .await; + + cx.dispatch_action(workspace::CloseActiveItem { save_intent: None }); + + history_items +} + +async fn open_queried_buffer( + input: &str, + expected_matches: usize, + expected_editor_title: &str, + workspace: &View, + cx: &mut gpui::VisualTestContext, ) -> Vec { let picker = open_file_picker(&workspace, cx); cx.simulate_input(input); @@ -1186,7 +1321,6 @@ async fn open_close_queried_buffer( finder.delegate.history_items.clone() }); - cx.dispatch_action(SelectNext); cx.dispatch_action(Confirm); cx.read(|cx| { @@ -1198,8 +1332,6 @@ async fn open_close_queried_buffer( ); }); - cx.dispatch_action(workspace::CloseActiveItem { save_intent: None }); - history_items } @@ -1313,3 +1445,30 @@ fn collect_search_matches(picker: &Picker) -> SearchEntries .collect(), } } + +fn assert_match_selection( + finder: &Picker, + expected_selection_index: usize, + expected_file_name: &str, +) { + assert_eq!(finder.delegate.selected_index(), expected_selection_index); + assert_match_at_position(finder, expected_selection_index, expected_file_name); +} + +fn assert_match_at_position( + finder: &Picker, + match_index: usize, + expected_file_name: &str, +) { + let match_item = finder + .delegate + .matches + .get(match_index) + .expect("Finder should have a match item with the given index"); + let match_file_name = match match_item { + Match::History(found_path, _) => found_path.absolute.as_deref().unwrap().file_name(), + Match::Search(path_match) => path_match.0.path.file_name(), + }; + let match_file_name = match_file_name.unwrap().to_string_lossy().to_string(); + assert_eq!(match_file_name.as_str(), expected_file_name); +}