Fix ssh project history (#19683)

Kirill Bulatov created

Use `Fs` instead of `std::fs` and do entry existence checks better:
* first, check the worktree entry existence without any FS checks
* then, only for local cases, use `Fs` to check for abs_path existence
of items, in case those came from single-filed worktrees that got closed
and removed.

Remote entries do not get file existence checks, so might try opening
previously removed buffers for now.

Release Notes:

- N/A

Change summary

crates/collab/src/tests/channel_guest_tests.rs |  4 
crates/collab/src/tests/following_tests.rs     | 19 +++-
crates/file_finder/src/file_finder.rs          | 81 ++++++++++++-------
crates/file_finder/src/file_finder_tests.rs    | 11 ++
crates/project/src/project.rs                  | 19 ---
5 files changed, 81 insertions(+), 53 deletions(-)

Detailed changes

crates/collab/src/tests/channel_guest_tests.rs 🔗

@@ -95,7 +95,9 @@ async fn test_channel_guest_promotion(cx_a: &mut TestAppContext, cx_b: &mut Test
     let room_b = cx_b
         .read(ActiveCall::global)
         .update(cx_b, |call, _| call.room().unwrap().clone());
-    cx_b.simulate_keystrokes("cmd-p 1 enter");
+    cx_b.simulate_keystrokes("cmd-p");
+    cx_a.run_until_parked();
+    cx_b.simulate_keystrokes("1 enter");
 
     let (project_b, editor_b) = workspace_b.update(cx_b, |workspace, cx| {
         (

crates/collab/src/tests/following_tests.rs 🔗

@@ -1589,8 +1589,9 @@ async fn test_following_stops_on_unshare(cx_a: &mut TestAppContext, cx_b: &mut T
         .await;
     let (workspace_b, cx_b) = client_b.join_workspace(channel_id, cx_b).await;
 
-    cx_a.simulate_keystrokes("cmd-p 2 enter");
+    cx_a.simulate_keystrokes("cmd-p");
     cx_a.run_until_parked();
+    cx_a.simulate_keystrokes("2 enter");
 
     let editor_a = workspace_a.update(cx_a, |workspace, cx| {
         workspace.active_item_as::<Editor>(cx).unwrap()
@@ -2041,7 +2042,9 @@ async fn test_following_to_channel_notes_other_workspace(
     share_workspace(&workspace_a, cx_a).await.unwrap();
 
     // a opens 1.txt
-    cx_a.simulate_keystrokes("cmd-p 1 enter");
+    cx_a.simulate_keystrokes("cmd-p");
+    cx_a.run_until_parked();
+    cx_a.simulate_keystrokes("1 enter");
     cx_a.run_until_parked();
     workspace_a.update(cx_a, |workspace, cx| {
         let editor = workspace.active_item(cx).unwrap();
@@ -2098,7 +2101,9 @@ async fn test_following_while_deactivated(cx_a: &mut TestAppContext, cx_b: &mut
     share_workspace(&workspace_a, cx_a).await.unwrap();
 
     // a opens 1.txt
-    cx_a.simulate_keystrokes("cmd-p 1 enter");
+    cx_a.simulate_keystrokes("cmd-p");
+    cx_a.run_until_parked();
+    cx_a.simulate_keystrokes("1 enter");
     cx_a.run_until_parked();
     workspace_a.update(cx_a, |workspace, cx| {
         let editor = workspace.active_item(cx).unwrap();
@@ -2118,7 +2123,9 @@ async fn test_following_while_deactivated(cx_a: &mut TestAppContext, cx_b: &mut
     cx_b.simulate_keystrokes("down");
 
     // a opens a different file while not followed
-    cx_a.simulate_keystrokes("cmd-p 2 enter");
+    cx_a.simulate_keystrokes("cmd-p");
+    cx_a.run_until_parked();
+    cx_a.simulate_keystrokes("2 enter");
 
     workspace_b.update(cx_b, |workspace, cx| {
         let editor = workspace.active_item_as::<Editor>(cx).unwrap();
@@ -2128,7 +2135,9 @@ async fn test_following_while_deactivated(cx_a: &mut TestAppContext, cx_b: &mut
     // a opens a file in a new window
     let (_, cx_a2) = client_a.build_test_workspace(&mut cx_a2).await;
     cx_a2.update(|cx| cx.activate_window());
-    cx_a2.simulate_keystrokes("cmd-p 3 enter");
+    cx_a2.simulate_keystrokes("cmd-p");
+    cx_a2.run_until_parked();
+    cx_a2.simulate_keystrokes("3 enter");
     cx_a2.run_until_parked();
 
     // b starts following a again

crates/file_finder/src/file_finder.rs 🔗

@@ -5,6 +5,7 @@ mod file_finder_settings;
 mod new_path_prompt;
 mod open_path_prompt;
 
+use futures::future::join_all;
 pub use open_path_prompt::OpenPathDelegate;
 
 use collections::HashMap;
@@ -59,7 +60,7 @@ impl FileFinder {
     fn register(workspace: &mut Workspace, _: &mut ViewContext<Workspace>) {
         workspace.register_action(|workspace, action: &workspace::ToggleFileFinder, cx| {
             let Some(file_finder) = workspace.active_modal::<Self>(cx) else {
-                Self::open(workspace, action.separate_history, cx);
+                Self::open(workspace, action.separate_history, cx).detach();
                 return;
             };
 
@@ -72,8 +73,13 @@ impl FileFinder {
         });
     }
 
-    fn open(workspace: &mut Workspace, separate_history: bool, cx: &mut ViewContext<Workspace>) {
+    fn open(
+        workspace: &mut Workspace,
+        separate_history: bool,
+        cx: &mut ViewContext<Workspace>,
+    ) -> Task<()> {
         let project = workspace.project().read(cx);
+        let fs = project.fs();
 
         let currently_opened_path = workspace
             .active_item(cx)
@@ -88,28 +94,51 @@ impl FileFinder {
         let history_items = workspace
             .recent_navigation_history(Some(MAX_RECENT_SELECTIONS), cx)
             .into_iter()
-            .filter(|(_, history_abs_path)| match history_abs_path {
-                Some(abs_path) => history_file_exists(abs_path),
-                None => true,
+            .filter_map(|(project_path, abs_path)| {
+                if project.entry_for_path(&project_path, cx).is_some() {
+                    return Some(Task::ready(Some(FoundPath::new(project_path, abs_path))));
+                }
+                let abs_path = abs_path?;
+                if project.is_local() {
+                    let fs = fs.clone();
+                    Some(cx.background_executor().spawn(async move {
+                        if fs.is_file(&abs_path).await {
+                            Some(FoundPath::new(project_path, Some(abs_path)))
+                        } else {
+                            None
+                        }
+                    }))
+                } else {
+                    Some(Task::ready(Some(FoundPath::new(
+                        project_path,
+                        Some(abs_path),
+                    ))))
+                }
             })
-            .map(|(history_path, abs_path)| FoundPath::new(history_path, abs_path))
             .collect::<Vec<_>>();
+        cx.spawn(move |workspace, mut cx| async move {
+            let history_items = join_all(history_items).await.into_iter().flatten();
+
+            workspace
+                .update(&mut cx, |workspace, cx| {
+                    let project = workspace.project().clone();
+                    let weak_workspace = cx.view().downgrade();
+                    workspace.toggle_modal(cx, |cx| {
+                        let delegate = FileFinderDelegate::new(
+                            cx.view().downgrade(),
+                            weak_workspace,
+                            project,
+                            currently_opened_path,
+                            history_items.collect(),
+                            separate_history,
+                            cx,
+                        );
 
-        let project = workspace.project().clone();
-        let weak_workspace = cx.view().downgrade();
-        workspace.toggle_modal(cx, |cx| {
-            let delegate = FileFinderDelegate::new(
-                cx.view().downgrade(),
-                weak_workspace,
-                project,
-                currently_opened_path,
-                history_items,
-                separate_history,
-                cx,
-            );
-
-            FileFinder::new(delegate, cx)
-        });
+                        FileFinder::new(delegate, cx)
+                    });
+                })
+                .ok();
+        })
     }
 
     fn new(delegate: FileFinderDelegate, cx: &mut ViewContext<Self>) -> Self {
@@ -456,16 +485,6 @@ impl FoundPath {
 
 const MAX_RECENT_SELECTIONS: usize = 20;
 
-#[cfg(not(test))]
-fn history_file_exists(abs_path: &PathBuf) -> bool {
-    abs_path.exists()
-}
-
-#[cfg(test)]
-fn history_file_exists(abs_path: &Path) -> bool {
-    !abs_path.ends_with("nonexistent.rs")
-}
-
 pub enum Event {
     Selected(ProjectPath),
     Dismissed,

crates/file_finder/src/file_finder_tests.rs 🔗

@@ -4,7 +4,7 @@ use super::*;
 use editor::Editor;
 use gpui::{Entity, TestAppContext, VisualTestContext};
 use menu::{Confirm, SelectNext, SelectPrev};
-use project::FS_WATCH_LATENCY;
+use project::{RemoveOptions, FS_WATCH_LATENCY};
 use serde_json::json;
 use workspace::{AppState, ToggleFileFinder, Workspace};
 
@@ -1450,6 +1450,15 @@ async fn test_nonexistent_history_items_not_shown(cx: &mut gpui::TestAppContext)
     open_close_queried_buffer("non", 1, "nonexistent.rs", &workspace, cx).await;
     open_close_queried_buffer("thi", 1, "third.rs", &workspace, cx).await;
     open_close_queried_buffer("fir", 1, "first.rs", &workspace, cx).await;
+    app_state
+        .fs
+        .remove_file(
+            Path::new("/src/test/nonexistent.rs"),
+            RemoveOptions::default(),
+        )
+        .await
+        .unwrap();
+    cx.run_until_parked();
 
     let picker = open_file_picker(&workspace, cx);
     cx.simulate_input("rs");

crates/project/src/project.rs 🔗

@@ -1875,11 +1875,7 @@ impl Project {
         })
     }
 
-    pub fn get_open_buffer(
-        &mut self,
-        path: &ProjectPath,
-        cx: &mut ModelContext<Self>,
-    ) -> Option<Model<Buffer>> {
+    pub fn get_open_buffer(&self, path: &ProjectPath, cx: &AppContext) -> Option<Model<Buffer>> {
         self.buffer_store.read(cx).get_by_path(path, cx)
     }
 
@@ -3295,17 +3291,10 @@ impl Project {
     }
 
     pub fn absolute_path(&self, project_path: &ProjectPath, cx: &AppContext) -> Option<PathBuf> {
-        let workspace_root = self
-            .worktree_for_id(project_path.worktree_id, cx)?
+        self.worktree_for_id(project_path.worktree_id, cx)?
             .read(cx)
-            .abs_path();
-        let project_path = project_path.path.as_ref();
-
-        Some(if project_path == Path::new("") {
-            workspace_root.to_path_buf()
-        } else {
-            workspace_root.join(project_path)
-        })
+            .absolutize(&project_path.path)
+            .ok()
     }
 
     /// Attempts to find a `ProjectPath` corresponding to the given path. If the path