diff --git a/crates/editor/src/hover_links.rs b/crates/editor/src/hover_links.rs index 4a636f673a..31be9e93a9 100644 --- a/crates/editor/src/hover_links.rs +++ b/crates/editor/src/hover_links.rs @@ -706,10 +706,11 @@ pub(crate) async fn find_file( ) -> Option { project .update(cx, |project, cx| { - project.resolve_existing_file_path(&candidate_file_path, buffer, cx) + project.resolve_path_in_buffer(&candidate_file_path, buffer, cx) }) .ok()? .await + .filter(|s| s.is_file()) } if let Some(existing_path) = check_path(&candidate_file_path, &project, buffer, cx).await { @@ -1612,4 +1613,46 @@ mod tests { assert_eq!(file_path.to_str().unwrap(), "/root/dir/file2.rs"); }); } + + #[gpui::test] + async fn test_hover_directories(cx: &mut gpui::TestAppContext) { + init_test(cx, |_| {}); + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + ..Default::default() + }, + cx, + ) + .await; + + // Insert a new file + let fs = cx.update_workspace(|workspace, cx| workspace.project().read(cx).fs().clone()); + fs.as_fake() + .insert_file("/root/dir/file2.rs", "This is file2.rs".as_bytes().to_vec()) + .await; + + cx.set_state(indoc! {" + You can't open ../diˇr because it's a directory. + "}); + + // File does not exist + let screen_coord = cx.pixel_position(indoc! {" + You can't open ../diˇr because it's a directory. + "}); + cx.simulate_mouse_move(screen_coord, None, Modifiers::secondary_key()); + + // No highlight + cx.update_editor(|editor, cx| { + assert!(editor + .snapshot(cx) + .text_highlight_ranges::() + .unwrap_or_default() + .1 + .is_empty()); + }); + + // Does not open the directory + cx.simulate_click(screen_coord, Modifiers::secondary_key()); + cx.update_workspace(|workspace, cx| assert_eq!(workspace.items(cx).count(), 1)); + } } diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 299b129d82..ce0e385057 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -790,9 +790,9 @@ impl FileFinderDelegate { let mut path_matches = Vec::new(); let abs_file_exists = if let Ok(task) = project.update(&mut cx, |this, cx| { - this.abs_file_path_exists(query.path_query(), cx) + this.resolve_abs_file_path(query.path_query(), cx) }) { - task.await + task.await.is_some() } else { false }; diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 7a57e048c8..04ae203b4d 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -3094,7 +3094,7 @@ impl Project { } /// Returns the resolved version of `path`, that was found in `buffer`, if it exists. - pub fn resolve_existing_file_path( + pub fn resolve_path_in_buffer( &self, path: &str, buffer: &Model, @@ -3102,47 +3102,56 @@ impl Project { ) -> Task> { let path_buf = PathBuf::from(path); if path_buf.is_absolute() || path.starts_with("~") { - self.resolve_abs_file_path(path, cx) + self.resolve_abs_path(path, cx) } else { self.resolve_path_in_worktrees(path_buf, buffer, cx) } } - pub fn abs_file_path_exists(&self, path: &str, cx: &mut ModelContext) -> Task { - let resolve_task = self.resolve_abs_file_path(path, cx); + pub fn resolve_abs_file_path( + &self, + path: &str, + cx: &mut ModelContext, + ) -> Task> { + let resolve_task = self.resolve_abs_path(path, cx); cx.background_executor().spawn(async move { let resolved_path = resolve_task.await; - resolved_path.is_some() + resolved_path.filter(|path| path.is_file()) }) } - fn resolve_abs_file_path( + pub fn resolve_abs_path( &self, path: &str, cx: &mut ModelContext, ) -> Task> { if self.is_local() { let expanded = PathBuf::from(shellexpand::tilde(&path).into_owned()); - let fs = self.fs.clone(); cx.background_executor().spawn(async move { let path = expanded.as_path(); - let exists = fs.is_file(path).await; + let metadata = fs.metadata(path).await.ok().flatten(); - exists.then(|| ResolvedPath::AbsPath(expanded)) + metadata.map(|metadata| ResolvedPath::AbsPath { + path: expanded, + is_dir: metadata.is_dir, + }) }) } else if let Some(ssh_client) = self.ssh_client.as_ref() { let request = ssh_client .read(cx) .proto_client() - .request(proto::CheckFileExists { + .request(proto::GetPathMetadata { project_id: SSH_PROJECT_ID, path: path.to_string(), }); cx.background_executor().spawn(async move { let response = request.await.log_err()?; if response.exists { - Some(ResolvedPath::AbsPath(PathBuf::from(response.path))) + Some(ResolvedPath::AbsPath { + path: PathBuf::from(response.path), + is_dir: response.is_dir, + }) } else { None } @@ -3181,10 +3190,14 @@ impl Project { resolved.strip_prefix(root_entry_path).unwrap_or(&resolved); worktree.entry_for_path(stripped).map(|entry| { - ResolvedPath::ProjectPath(ProjectPath { + let project_path = ProjectPath { worktree_id: worktree.id(), path: entry.path.clone(), - }) + }; + ResolvedPath::ProjectPath { + project_path, + is_dir: entry.is_dir(), + } }) }) .ok()?; @@ -4149,24 +4162,41 @@ fn resolve_path(base: &Path, path: &Path) -> PathBuf { /// or an AbsPath and that *exists*. #[derive(Debug, Clone)] pub enum ResolvedPath { - ProjectPath(ProjectPath), - AbsPath(PathBuf), + ProjectPath { + project_path: ProjectPath, + is_dir: bool, + }, + AbsPath { + path: PathBuf, + is_dir: bool, + }, } impl ResolvedPath { pub fn abs_path(&self) -> Option<&Path> { match self { - Self::AbsPath(path) => Some(path.as_path()), + Self::AbsPath { path, .. } => Some(path.as_path()), _ => None, } } pub fn project_path(&self) -> Option<&ProjectPath> { match self { - Self::ProjectPath(path) => Some(&path), + Self::ProjectPath { project_path, .. } => Some(&project_path), _ => None, } } + + pub fn is_file(&self) -> bool { + !self.is_dir() + } + + pub fn is_dir(&self) -> bool { + match self { + Self::ProjectPath { is_dir, .. } => *is_dir, + Self::AbsPath { is_dir, .. } => *is_dir, + } + } } impl Item for Buffer { diff --git a/crates/proto/proto/zed.proto b/crates/proto/proto/zed.proto index 95a54c3d5c..e9e42dac18 100644 --- a/crates/proto/proto/zed.proto +++ b/crates/proto/proto/zed.proto @@ -259,9 +259,6 @@ message Envelope { CloseBuffer close_buffer = 245; UpdateUserSettings update_user_settings = 246; - CheckFileExists check_file_exists = 255; - CheckFileExistsResponse check_file_exists_response = 256; - ShutdownRemoteServer shutdown_remote_server = 257; RemoveWorktree remove_worktree = 258; @@ -284,13 +281,16 @@ message Envelope { GitBranchesResponse git_branches_response = 271; UpdateGitBranch update_git_branch = 272; + ListToolchains list_toolchains = 273; ListToolchainsResponse list_toolchains_response = 274; ActivateToolchain activate_toolchain = 275; ActiveToolchain active_toolchain = 276; - ActiveToolchainResponse active_toolchain_response = 277; // current max - } + ActiveToolchainResponse active_toolchain_response = 277; + GetPathMetadata get_path_metadata = 278; + GetPathMetadataResponse get_path_metadata_response = 279; // current max + } reserved 87 to 88; reserved 158 to 161; @@ -305,6 +305,7 @@ message Envelope { reserved 221; reserved 224 to 229; reserved 247 to 254; + reserved 255 to 256; } // Messages @@ -2357,14 +2358,15 @@ message UpdateUserSettings { } } -message CheckFileExists { +message GetPathMetadata { uint64 project_id = 1; string path = 2; } -message CheckFileExistsResponse { +message GetPathMetadataResponse { bool exists = 1; string path = 2; + bool is_dir = 3; } message ShutdownRemoteServer {} diff --git a/crates/proto/src/proto.rs b/crates/proto/src/proto.rs index 7fcebf0513..4bae2d9931 100644 --- a/crates/proto/src/proto.rs +++ b/crates/proto/src/proto.rs @@ -343,8 +343,6 @@ messages!( (FindSearchCandidatesResponse, Background), (CloseBuffer, Foreground), (UpdateUserSettings, Foreground), - (CheckFileExists, Background), - (CheckFileExistsResponse, Background), (ShutdownRemoteServer, Foreground), (RemoveWorktree, Foreground), (LanguageServerLog, Foreground), @@ -363,7 +361,9 @@ messages!( (ListToolchainsResponse, Foreground), (ActivateToolchain, Foreground), (ActiveToolchain, Foreground), - (ActiveToolchainResponse, Foreground) + (ActiveToolchainResponse, Foreground), + (GetPathMetadata, Background), + (GetPathMetadataResponse, Background) ); request_messages!( @@ -472,7 +472,6 @@ request_messages!( (SynchronizeContexts, SynchronizeContextsResponse), (LspExtSwitchSourceHeader, LspExtSwitchSourceHeaderResponse), (AddWorktree, AddWorktreeResponse), - (CheckFileExists, CheckFileExistsResponse), (ShutdownRemoteServer, Ack), (RemoveWorktree, Ack), (OpenServerSettings, OpenBufferResponse), @@ -483,7 +482,8 @@ request_messages!( (UpdateGitBranch, Ack), (ListToolchains, ListToolchainsResponse), (ActivateToolchain, Ack), - (ActiveToolchain, ActiveToolchainResponse) + (ActiveToolchain, ActiveToolchainResponse), + (GetPathMetadata, GetPathMetadataResponse) ); entity_messages!( @@ -555,7 +555,6 @@ entity_messages!( SynchronizeContexts, LspExtSwitchSourceHeader, UpdateUserSettings, - CheckFileExists, LanguageServerLog, Toast, HideToast, @@ -566,7 +565,8 @@ entity_messages!( UpdateGitBranch, ListToolchains, ActivateToolchain, - ActiveToolchain + ActiveToolchain, + GetPathMetadata ); entity_messages!( diff --git a/crates/remote_server/src/headless_project.rs b/crates/remote_server/src/headless_project.rs index ce34af247f..155b141af6 100644 --- a/crates/remote_server/src/headless_project.rs +++ b/crates/remote_server/src/headless_project.rs @@ -150,7 +150,7 @@ impl HeadlessProject { session.subscribe_to_entity(SSH_PROJECT_ID, &settings_observer); client.add_request_handler(cx.weak_model(), Self::handle_list_remote_directory); - client.add_request_handler(cx.weak_model(), Self::handle_check_file_exists); + client.add_request_handler(cx.weak_model(), Self::handle_get_path_metadata); client.add_request_handler(cx.weak_model(), Self::handle_shutdown_remote_server); client.add_request_handler(cx.weak_model(), Self::handle_ping); @@ -525,18 +525,20 @@ impl HeadlessProject { Ok(proto::ListRemoteDirectoryResponse { entries }) } - pub async fn handle_check_file_exists( + pub async fn handle_get_path_metadata( this: Model, - envelope: TypedEnvelope, + envelope: TypedEnvelope, cx: AsyncAppContext, - ) -> Result { + ) -> Result { let fs = cx.read_model(&this, |this, _| this.fs.clone())?; let expanded = shellexpand::tilde(&envelope.payload.path).to_string(); - let exists = fs.is_file(&PathBuf::from(expanded.clone())).await; + let metadata = fs.metadata(&PathBuf::from(expanded.clone())).await?; + let is_dir = metadata.map(|metadata| metadata.is_dir).unwrap_or(false); - Ok(proto::CheckFileExistsResponse { - exists, + Ok(proto::GetPathMetadataResponse { + exists: metadata.is_some(), + is_dir, path: expanded, }) } diff --git a/crates/remote_server/src/remote_editing_tests.rs b/crates/remote_server/src/remote_editing_tests.rs index 82e3824eb0..c7d3a3c97f 100644 --- a/crates/remote_server/src/remote_editing_tests.rs +++ b/crates/remote_server/src/remote_editing_tests.rs @@ -604,7 +604,10 @@ async fn test_remote_reload(cx: &mut TestAppContext, server_cx: &mut TestAppCont } #[gpui::test] -async fn test_remote_resolve_file_path(cx: &mut TestAppContext, server_cx: &mut TestAppContext) { +async fn test_remote_resolve_path_in_buffer( + cx: &mut TestAppContext, + server_cx: &mut TestAppContext, +) { let fs = FakeFs::new(server_cx.executor()); fs.insert_tree( "/code", @@ -639,10 +642,11 @@ async fn test_remote_resolve_file_path(cx: &mut TestAppContext, server_cx: &mut let path = project .update(cx, |project, cx| { - project.resolve_existing_file_path("/code/project1/README.md", &buffer, cx) + project.resolve_path_in_buffer("/code/project1/README.md", &buffer, cx) }) .await .unwrap(); + assert!(path.is_file()); assert_eq!( path.abs_path().unwrap().to_string_lossy(), "/code/project1/README.md" @@ -650,15 +654,80 @@ async fn test_remote_resolve_file_path(cx: &mut TestAppContext, server_cx: &mut let path = project .update(cx, |project, cx| { - project.resolve_existing_file_path("../README.md", &buffer, cx) + project.resolve_path_in_buffer("../README.md", &buffer, cx) }) .await .unwrap(); - + assert!(path.is_file()); assert_eq!( path.project_path().unwrap().clone(), ProjectPath::from((worktree_id, "README.md")) ); + + let path = project + .update(cx, |project, cx| { + project.resolve_path_in_buffer("../src", &buffer, cx) + }) + .await + .unwrap(); + assert_eq!( + path.project_path().unwrap().clone(), + ProjectPath::from((worktree_id, "src")) + ); + assert!(path.is_dir()); +} + +#[gpui::test] +async fn test_remote_resolve_abs_path(cx: &mut TestAppContext, server_cx: &mut TestAppContext) { + let fs = FakeFs::new(server_cx.executor()); + fs.insert_tree( + "/code", + json!({ + "project1": { + ".git": {}, + "README.md": "# project 1", + "src": { + "lib.rs": "fn one() -> usize { 1 }" + } + }, + }), + ) + .await; + + let (project, _headless) = init_test(&fs, cx, server_cx).await; + + let path = project + .update(cx, |project, cx| { + project.resolve_abs_path("/code/project1/README.md", cx) + }) + .await + .unwrap(); + + assert!(path.is_file()); + assert_eq!( + path.abs_path().unwrap().to_string_lossy(), + "/code/project1/README.md" + ); + + let path = project + .update(cx, |project, cx| { + project.resolve_abs_path("/code/project1/src", cx) + }) + .await + .unwrap(); + + assert!(path.is_dir()); + assert_eq!( + path.abs_path().unwrap().to_string_lossy(), + "/code/project1/src" + ); + + let path = project + .update(cx, |project, cx| { + project.resolve_abs_path("/code/project1/DOESNOTEXIST", cx) + }) + .await; + assert!(path.is_none()); } #[gpui::test(iterations = 10)] diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index de2c985f34..f0786aa479 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1218,7 +1218,7 @@ impl Workspace { notify_if_database_failed(window, &mut cx); let opened_items = window .update(&mut cx, |_workspace, cx| { - open_items(serialized_workspace, project_paths, app_state, cx) + open_items(serialized_workspace, project_paths, cx) })? .await .unwrap_or_default(); @@ -2058,8 +2058,10 @@ impl Workspace { cx: &mut ViewContext, ) -> Task>> { match path { - ResolvedPath::ProjectPath(project_path) => self.open_path(project_path, None, true, cx), - ResolvedPath::AbsPath(path) => self.open_abs_path(path, false, cx), + ResolvedPath::ProjectPath { project_path, .. } => { + self.open_path(project_path, None, true, cx) + } + ResolvedPath::AbsPath { path, .. } => self.open_abs_path(path, false, cx), } } @@ -4563,7 +4565,6 @@ fn window_bounds_env_override() -> Option> { fn open_items( serialized_workspace: Option, mut project_paths_to_open: Vec<(PathBuf, Option)>, - app_state: Arc, cx: &mut ViewContext, ) -> impl 'static + Future>>>>> { let restored_items = serialized_workspace.map(|serialized_workspace| { @@ -4619,14 +4620,20 @@ fn open_items( .enumerate() .map(|(ix, (abs_path, project_path))| { let workspace = workspace.clone(); - cx.spawn(|mut cx| { - let fs = app_state.fs.clone(); - async move { - let file_project_path = project_path?; - if fs.is_dir(&abs_path).await { - None - } else { - Some(( + cx.spawn(|mut cx| async move { + let file_project_path = project_path?; + let abs_path_task = workspace.update(&mut cx, |workspace, cx| { + workspace.project().update(cx, |project, cx| { + project.resolve_abs_path(abs_path.to_string_lossy().as_ref(), cx) + }) + }); + + // We only want to open file paths here. If one of the items + // here is a directory, it was already opened further above + // with a `find_or_create_worktree`. + if let Ok(task) = abs_path_task { + if task.await.map_or(true, |p| p.is_file()) { + return Some(( ix, workspace .update(&mut cx, |workspace, cx| { @@ -4634,9 +4641,10 @@ fn open_items( }) .log_err()? .await, - )) + )); } } + None }) }); @@ -5580,7 +5588,7 @@ pub fn open_ssh_project( .update(&mut cx, |_, cx| { cx.activate_window(); - open_items(serialized_workspace, project_paths_to_open, app_state, cx) + open_items(serialized_workspace, project_paths_to_open, cx) })? .await?;