From 3a3c1c5a5bc1ffbb3761f441d9ef8bd47b62d0db Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sat, 27 May 2023 00:06:09 +0300 Subject: [PATCH] Add a test co-authored-by: Mikayla --- crates/file_finder/src/file_finder.rs | 256 ++++++++++++++++++++++---- crates/project/src/project.rs | 2 +- crates/workspace/src/pane.rs | 8 +- crates/workspace/src/workspace.rs | 24 +-- 4 files changed, 235 insertions(+), 55 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 6617bd2888..d00704d6b9 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -32,6 +32,7 @@ pub struct FileFinderDelegate { history_items: Vec, } +#[derive(Debug)] enum Matches { History(Vec), Search(Vec), @@ -114,6 +115,12 @@ fn toggle_file_finder(workspace: &mut Workspace, _: &Toggle, cx: &mut ViewContex .as_ref() .map(|found_path| &found_path.project) }) + .filter(|(_, history_abs_path)| { + history_abs_path.as_ref() + != currently_opened_path + .as_ref() + .and_then(|found_path| found_path.absolute.as_ref()) + }) .map(|(history_path, abs_path)| FoundPath::new(history_path, abs_path)), ) .collect(); @@ -330,7 +337,6 @@ impl FileFinderDelegate { ) -> (String, Vec, String, Vec) { let path = &path_match.path; let path_string = path.to_string_lossy(); - // TODO kb full path could be very long, trim it to fit the panel width let full_path = [path_match.path_prefix.as_ref(), path_string.as_ref()].join(""); let path_positions = path_match.positions.clone(); @@ -437,7 +443,7 @@ impl PickerDelegate for FileFinderDelegate { } else { match history_match.absolute.as_ref() { Some(abs_path) => { - workspace.open_abs_path(abs_path.to_path_buf(), true, cx) + workspace.open_abs_path(abs_path.to_path_buf(), false, cx) } None => workspace.open_path( ProjectPath { @@ -1192,10 +1198,13 @@ mod tests { .await; assert_eq!( history_after_first, - vec![dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/first.rs")), - })], + vec![FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/first.rs")), + }, + Some(PathBuf::from("/src/test/first.rs")) + )], "Should show 1st opened item in the history when opening the 2nd item" ); @@ -1212,14 +1221,20 @@ mod tests { assert_eq!( history_after_second, vec![ - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/second.rs")), - }), - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/first.rs")), - }), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/second.rs")), + }, + Some(PathBuf::from("/src/test/second.rs")) + ), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/first.rs")), + }, + Some(PathBuf::from("/src/test/first.rs")) + ), ], "Should show 1st and 2nd opened items in the history when opening the 3rd item. \ 2nd item should be the first in the history, as the last opened." @@ -1238,18 +1253,27 @@ mod tests { assert_eq!( history_after_third, vec![ - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/third.rs")), - }), - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/second.rs")), - }), - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/first.rs")), - }), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/third.rs")), + }, + Some(PathBuf::from("/src/test/third.rs")) + ), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/second.rs")), + }, + Some(PathBuf::from("/src/test/second.rs")) + ), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/first.rs")), + }, + Some(PathBuf::from("/src/test/first.rs")) + ), ], "Should show 1st, 2nd and 3rd opened items in the history when opening the 2nd item again. \ 3rd item should be the first in the history, as the last opened." @@ -1268,24 +1292,162 @@ mod tests { assert_eq!( history_after_second_again, vec![ - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/second.rs")), - }), - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/third.rs")), - }), - dummy_found_path(ProjectPath { - worktree_id, - path: Arc::from(Path::new("test/first.rs")), - }), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/second.rs")), + }, + Some(PathBuf::from("/src/test/second.rs")) + ), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/third.rs")), + }, + Some(PathBuf::from("/src/test/third.rs")) + ), + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/first.rs")), + }, + Some(PathBuf::from("/src/test/first.rs")) + ), ], "Should show 1st, 2nd and 3rd opened items in the history when opening the 3rd item again. \ 2nd item, as the last opened, 3rd item should go next as it was opened right before." ); } + #[gpui::test] + async fn test_external_files_history( + deterministic: Arc, + cx: &mut gpui::TestAppContext, + ) { + let app_state = init_test(cx); + + app_state + .fs + .as_fake() + .insert_tree( + "/src", + json!({ + "test": { + "first.rs": "// First Rust file", + "second.rs": "// Second Rust file", + } + }), + ) + .await; + + app_state + .fs + .as_fake() + .insert_tree( + "/external-src", + json!({ + "test": { + "third.rs": "// Third Rust file", + "fourth.rs": "// Fourth Rust file", + } + }), + ) + .await; + + let project = Project::test(app_state.fs.clone(), ["/src".as_ref()], cx).await; + cx.update(|cx| { + project.update(cx, |project, cx| { + project.find_or_create_local_worktree("/external-src", false, cx) + }) + }) + .detach(); + deterministic.run_until_parked(); + + let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx)); + let worktree_id = cx.read(|cx| { + let worktrees = workspace.read(cx).worktrees(cx).collect::>(); + assert_eq!(worktrees.len(), 1,); + + WorktreeId::from_usize(worktrees[0].id()) + }); + workspace + .update(cx, |workspace, cx| { + workspace.open_abs_path(PathBuf::from("/external-src/test/third.rs"), false, cx) + }) + .detach(); + deterministic.run_until_parked(); + let external_worktree_id = cx.read(|cx| { + let worktrees = workspace.read(cx).worktrees(cx).collect::>(); + assert_eq!( + worktrees.len(), + 2, + "External file should get opened in a new worktree" + ); + + WorktreeId::from_usize( + worktrees + .into_iter() + .find(|worktree| worktree.id() != worktree_id.to_usize()) + .expect("New worktree should have a different id") + .id(), + ) + }); + close_active_item(&workspace, &deterministic, cx).await; + + let initial_history_items = open_close_queried_buffer( + "sec", + 1, + "second.rs", + window_id, + &workspace, + &deterministic, + cx, + ) + .await; + assert_eq!( + initial_history_items, + vec![FoundPath::new( + ProjectPath { + worktree_id: external_worktree_id, + path: Arc::from(Path::new("")), + }, + Some(PathBuf::from("/external-src/test/third.rs")) + )], + "Should show external file with its full path in the history after it was open" + ); + + let updated_history_items = open_close_queried_buffer( + "fir", + 1, + "first.rs", + window_id, + &workspace, + &deterministic, + cx, + ) + .await; + assert_eq!( + updated_history_items, + vec![ + FoundPath::new( + ProjectPath { + worktree_id, + path: Arc::from(Path::new("test/second.rs")), + }, + Some(PathBuf::from("/src/test/second.rs")) + ), + FoundPath::new( + ProjectPath { + worktree_id: external_worktree_id, + path: Arc::from(Path::new("")), + }, + Some(PathBuf::from("/external-src/test/third.rs")) + ), + ], + "Should keep external file with history updates", + ); + } + async fn open_close_queried_buffer( input: &str, expected_matches: usize, @@ -1332,6 +1494,16 @@ mod tests { ); }); + close_active_item(workspace, deterministic, cx).await; + + history_items + } + + async fn close_active_item( + workspace: &ViewHandle, + deterministic: &gpui::executor::Deterministic, + cx: &mut TestAppContext, + ) { let mut original_items = HashMap::new(); cx.read(|cx| { for pane in workspace.read(cx).panes() { @@ -1341,6 +1513,8 @@ mod tests { assert!(insertion_result.is_none(), "Pane id {pane_id} collision"); } }); + + let active_pane = cx.read(|cx| workspace.read(cx).active_pane().clone()); active_pane .update(cx, |pane, cx| { pane.close_active_item(&workspace::CloseActiveItem, cx) @@ -1365,8 +1539,10 @@ mod tests { } } }); - - history_items + assert!( + original_items.len() <= 1, + "At most one panel should got closed" + ); } fn init_test(cx: &mut TestAppContext) -> Arc { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 6a6f878978..c5442221eb 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -3218,7 +3218,7 @@ impl Project { ) -> Result<(), anyhow::Error> { let (worktree, relative_path) = self .find_local_worktree(&abs_path, cx) - .ok_or_else(|| anyhow!("no worktree found for diagnostics"))?; + .ok_or_else(|| anyhow!("no worktree found for diagnostics path {abs_path:?}"))?; let project_path = ProjectPath { worktree_id: worktree.read(cx).id(), diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index b8d08ee2d4..cf42013e8d 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -1018,9 +1018,11 @@ impl Pane { if let Some(path) = item.project_path(cx) { let abs_path = self - .workspace() - .upgrade(cx) - .and_then(|workspace| workspace.read(cx).absolute_path(&path, cx)); + .nav_history + .borrow() + .paths_by_item + .get(&item.id()) + .and_then(|(_, abs_path)| abs_path.clone()); self.nav_history .borrow_mut() .paths_by_item diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 1fe1c3ac93..38e1dbc9ac 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -975,23 +975,25 @@ impl Workspace { }); } - let project = self.project.read(cx); history .into_iter() .sorted_by_key(|(_, (_, timestamp))| *timestamp) .map(|(project_path, (fs_path, _))| (project_path, fs_path)) .rev() .filter(|(history_path, abs_path)| { - project - .worktree_for_id(history_path.worktree_id, cx) - .is_some() - || abs_path - .as_ref() - .and_then(|abs_path| { - let buffers_opened = abs_paths_opened.get(abs_path)?; - Some(buffers_opened.len() < 2) - }) - .unwrap_or(false) + let latest_project_path_opened = abs_path + .as_ref() + .and_then(|abs_path| abs_paths_opened.get(abs_path)) + .and_then(|project_paths| { + project_paths + .iter() + .max_by(|b1, b2| b1.worktree_id.cmp(&b2.worktree_id)) + }); + + match latest_project_path_opened { + Some(latest_project_path_opened) => latest_project_path_opened == history_path, + None => true, + } }) .take(limit.unwrap_or(usize::MAX)) .collect()