Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

remote server: Fix error log about inability to open buffer (#19824)

Browse source 
Turns out that we used client-side `fs` to check whether something is a
directory or not, which obviously doesn't work with SSH projects.

Release Notes:

- N/A

---------

Co-authored-by: Bennet <bennet@zed.dev>
This commit is contained in:
Thorsten Ball 2024-10-28 16:35:37 +01:00 • committed by GitHub
parent 5e89fba681
commit cc81f19c68
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 213 additions and 59 deletions
Download patch file Download diff file Expand all files Collapse all files

45
crates/editor/src/hover_links.rs
View file

@ -706,10 +706,11 @@ pub(crate) async fn find_file(
) -> Option<ResolvedPath> {
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::<HoveredLinkState>()
.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));
}
}

4
crates/file_finder/src/file_finder.rs
View file

@ -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
};

64
crates/project/src/project.rs
View file

@ -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<Buffer>,
@ -3102,47 +3102,56 @@ impl Project {
) -> Task<Option<ResolvedPath>> {
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<Self>) -> Task<bool> {
let resolve_task = self.resolve_abs_file_path(path, cx);
pub fn resolve_abs_file_path(
&self,
path: &str,
cx: &mut ModelContext<Self>,
) -> Task<Option<ResolvedPath>> {
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<Self>,
) -> Task<Option<ResolvedPath>> {
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 {

16
crates/proto/proto/zed.proto
View file

@ -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 {}

14
crates/proto/src/proto.rs
View file

@ -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!(

16
crates/remote_server/src/headless_project.rs
View file

@ -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<Self>,
envelope: TypedEnvelope<proto::CheckFileExists>,
envelope: TypedEnvelope<proto::GetPathMetadata>,
cx: AsyncAppContext,
) -> Result<proto::CheckFileExistsResponse> {
) -> Result<proto::GetPathMetadataResponse> {
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,
})
}

77
crates/remote_server/src/remote_editing_tests.rs
View file

@ -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)]

36
crates/workspace/src/workspace.rs
View file

@ -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<Self>,
) -> Task<anyhow::Result<Box<dyn ItemHandle>>> {
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<Bounds<Pixels>> {
fn open_items(
serialized_workspace: Option<SerializedWorkspace>,
mut project_paths_to_open: Vec<(PathBuf, Option<ProjectPath>)>,
app_state: Arc<AppState>,
cx: &mut ViewContext<Workspace>,
) -> impl 'static + Future<Output = Result<Vec<Option<Result<Box<dyn ItemHandle>>>>>> {
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?;