From f0ca7141b88c2f227b7697edaa3cfd75759b135f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 4 Dec 2023 17:30:46 +0200 Subject: [PATCH] Ignore excluded files on worktree entry refresh --- crates/collab/src/tests/integration_tests.rs | 27 ++-- .../random_project_collaboration_tests.rs | 1 - crates/project/src/project.rs | 121 +++++++++-------- crates/project/src/worktree.rs | 111 +++++++++------ crates/project/src/worktree_tests.rs | 30 +++-- crates/project_panel/src/project_panel.rs | 55 ++++---- crates/rpc/proto/zed.proto | 2 +- crates/terminal_view/src/terminal_view.rs | 1 + crates/util/src/paths.rs | 3 +- crates/workspace/src/workspace.rs | 1 - crates/zed/src/zed.rs | 127 +++++++++++------- 11 files changed, 267 insertions(+), 212 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index c5820b5395..ad4c59e377 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -2981,11 +2981,10 @@ async fn test_fs_operations( let entry = project_b .update(cx_b, |project, cx| { - project - .create_entry((worktree_id, "c.txt"), false, cx) - .unwrap() + project.create_entry((worktree_id, "c.txt"), false, cx) }) .await + .unwrap() .unwrap(); worktree_a.read_with(cx_a, |worktree, _| { assert_eq!( @@ -3010,7 +3009,6 @@ async fn test_fs_operations( .update(cx_b, |project, cx| { project.rename_entry(entry.id, Path::new("d.txt"), cx) }) - .unwrap() .await .unwrap(); worktree_a.read_with(cx_a, |worktree, _| { @@ -3034,11 +3032,10 @@ async fn test_fs_operations( let dir_entry = project_b .update(cx_b, |project, cx| { - project - .create_entry((worktree_id, "DIR"), true, cx) - .unwrap() + project.create_entry((worktree_id, "DIR"), true, cx) }) .await + .unwrap() .unwrap(); worktree_a.read_with(cx_a, |worktree, _| { assert_eq!( @@ -3061,25 +3058,19 @@ async fn test_fs_operations( project_b .update(cx_b, |project, cx| { - project - .create_entry((worktree_id, "DIR/e.txt"), false, cx) - .unwrap() + project.create_entry((worktree_id, "DIR/e.txt"), false, cx) }) .await .unwrap(); project_b .update(cx_b, |project, cx| { - project - .create_entry((worktree_id, "DIR/SUBDIR"), true, cx) - .unwrap() + project.create_entry((worktree_id, "DIR/SUBDIR"), true, cx) }) .await .unwrap(); project_b .update(cx_b, |project, cx| { - project - .create_entry((worktree_id, "DIR/SUBDIR/f.txt"), false, cx) - .unwrap() + project.create_entry((worktree_id, "DIR/SUBDIR/f.txt"), false, cx) }) .await .unwrap(); @@ -3120,9 +3111,7 @@ async fn test_fs_operations( project_b .update(cx_b, |project, cx| { - project - .copy_entry(entry.id, Path::new("f.txt"), cx) - .unwrap() + project.copy_entry(entry.id, Path::new("f.txt"), cx) }) .await .unwrap(); diff --git a/crates/collab/src/tests/random_project_collaboration_tests.rs b/crates/collab/src/tests/random_project_collaboration_tests.rs index 42a2b79275..f839333c95 100644 --- a/crates/collab/src/tests/random_project_collaboration_tests.rs +++ b/crates/collab/src/tests/random_project_collaboration_tests.rs @@ -665,7 +665,6 @@ impl RandomizedTest for ProjectCollaborationTest { ensure_project_shared(&project, client, cx).await; project .update(cx, |p, cx| p.create_entry(project_path, is_dir, cx)) - .unwrap() .await?; } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index e5189f7b47..fde5e71df3 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -1121,20 +1121,22 @@ impl Project { project_path: impl Into, is_directory: bool, cx: &mut ModelContext, - ) -> Option>> { + ) -> Task>> { let project_path = project_path.into(); - let worktree = self.worktree_for_id(project_path.worktree_id, cx)?; + let Some(worktree) = self.worktree_for_id(project_path.worktree_id, cx) else { + return Task::ready(Ok(None)); + }; if self.is_local() { - Some(worktree.update(cx, |worktree, cx| { + worktree.update(cx, |worktree, cx| { worktree .as_local_mut() .unwrap() .create_entry(project_path.path, is_directory, cx) - })) + }) } else { let client = self.client.clone(); let project_id = self.remote_id().unwrap(); - Some(cx.spawn_weak(|_, mut cx| async move { + cx.spawn_weak(|_, mut cx| async move { let response = client .request(proto::CreateProjectEntry { worktree_id: project_path.worktree_id.to_proto(), @@ -1143,19 +1145,20 @@ impl Project { is_directory, }) .await?; - let entry = response - .entry - .ok_or_else(|| anyhow!("missing entry in response"))?; - worktree - .update(&mut cx, |worktree, cx| { - worktree.as_remote_mut().unwrap().insert_entry( - entry, - response.worktree_scan_id as usize, - cx, - ) - }) - .await - })) + match response.entry { + Some(entry) => worktree + .update(&mut cx, |worktree, cx| { + worktree.as_remote_mut().unwrap().insert_entry( + entry, + response.worktree_scan_id as usize, + cx, + ) + }) + .await + .map(Some), + None => Ok(None), + } + }) } } @@ -1164,8 +1167,10 @@ impl Project { entry_id: ProjectEntryId, new_path: impl Into>, cx: &mut ModelContext, - ) -> Option>> { - let worktree = self.worktree_for_entry(entry_id, cx)?; + ) -> Task>> { + let Some(worktree) = self.worktree_for_entry(entry_id, cx) else { + return Task::ready(Ok(None)); + }; let new_path = new_path.into(); if self.is_local() { worktree.update(cx, |worktree, cx| { @@ -1178,7 +1183,7 @@ impl Project { let client = self.client.clone(); let project_id = self.remote_id().unwrap(); - Some(cx.spawn_weak(|_, mut cx| async move { + cx.spawn_weak(|_, mut cx| async move { let response = client .request(proto::CopyProjectEntry { project_id, @@ -1186,19 +1191,20 @@ impl Project { new_path: new_path.to_string_lossy().into(), }) .await?; - let entry = response - .entry - .ok_or_else(|| anyhow!("missing entry in response"))?; - worktree - .update(&mut cx, |worktree, cx| { - worktree.as_remote_mut().unwrap().insert_entry( - entry, - response.worktree_scan_id as usize, - cx, - ) - }) - .await - })) + match response.entry { + Some(entry) => worktree + .update(&mut cx, |worktree, cx| { + worktree.as_remote_mut().unwrap().insert_entry( + entry, + response.worktree_scan_id as usize, + cx, + ) + }) + .await + .map(Some), + None => Ok(None), + } + }) } } @@ -1207,8 +1213,10 @@ impl Project { entry_id: ProjectEntryId, new_path: impl Into>, cx: &mut ModelContext, - ) -> Option>> { - let worktree = self.worktree_for_entry(entry_id, cx)?; + ) -> Task>> { + let Some(worktree) = self.worktree_for_entry(entry_id, cx) else { + return Task::ready(Ok(None)); + }; let new_path = new_path.into(); if self.is_local() { worktree.update(cx, |worktree, cx| { @@ -1221,7 +1229,7 @@ impl Project { let client = self.client.clone(); let project_id = self.remote_id().unwrap(); - Some(cx.spawn_weak(|_, mut cx| async move { + cx.spawn_weak(|_, mut cx| async move { let response = client .request(proto::RenameProjectEntry { project_id, @@ -1229,19 +1237,20 @@ impl Project { new_path: new_path.to_string_lossy().into(), }) .await?; - let entry = response - .entry - .ok_or_else(|| anyhow!("missing entry in response"))?; - worktree - .update(&mut cx, |worktree, cx| { - worktree.as_remote_mut().unwrap().insert_entry( - entry, - response.worktree_scan_id as usize, - cx, - ) - }) - .await - })) + match response.entry { + Some(entry) => worktree + .update(&mut cx, |worktree, cx| { + worktree.as_remote_mut().unwrap().insert_entry( + entry, + response.worktree_scan_id as usize, + cx, + ) + }) + .await + .map(Some), + None => Ok(None), + } + }) } } @@ -6820,7 +6829,7 @@ impl Project { }) .await?; Ok(proto::ProjectEntryResponse { - entry: Some((&entry).into()), + entry: entry.as_ref().map(|e| e.into()), worktree_scan_id: worktree_scan_id as u64, }) } @@ -6844,11 +6853,10 @@ impl Project { .as_local_mut() .unwrap() .rename_entry(entry_id, new_path, cx) - .ok_or_else(|| anyhow!("invalid entry")) - })? + }) .await?; Ok(proto::ProjectEntryResponse { - entry: Some((&entry).into()), + entry: entry.as_ref().map(|e| e.into()), worktree_scan_id: worktree_scan_id as u64, }) } @@ -6872,11 +6880,10 @@ impl Project { .as_local_mut() .unwrap() .copy_entry(entry_id, new_path, cx) - .ok_or_else(|| anyhow!("invalid entry")) - })? + }) .await?; Ok(proto::ProjectEntryResponse { - entry: Some((&entry).into()), + entry: entry.as_ref().map(|e| e.into()), worktree_scan_id: worktree_scan_id as u64, }) } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index b992ae8849..b77b8a3fba 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -955,13 +955,8 @@ impl LocalWorktree { ) -> Task)>> { let path = Arc::from(path); let abs_path = self.absolutize(&path); - let is_excluded = self.is_path_excluded(abs_path.clone()); let fs = self.fs.clone(); - let entry = if is_excluded { - None - } else { - Some(self.refresh_entry(path.clone(), None, cx)) - }; + let entry = self.refresh_entry(path.clone(), None, cx); cx.spawn(|this, cx| async move { let text = fs.load(&abs_path).await?; @@ -984,22 +979,19 @@ impl LocalWorktree { None }; - match entry { - Some(entry) => { - let entry = entry.await?; - Ok(( - File { - entry_id: Some(entry.id), - worktree: this, - path: entry.path, - mtime: entry.mtime, - is_local: true, - is_deleted: false, - }, - text, - diff_base, - )) - } + match entry.await? { + Some(entry) => Ok(( + File { + entry_id: Some(entry.id), + worktree: this, + path: entry.path, + mtime: entry.mtime, + is_local: true, + is_deleted: false, + }, + text, + diff_base, + )), None => { let metadata = fs .metadata(&abs_path) @@ -1044,17 +1036,37 @@ impl LocalWorktree { let text = buffer.as_rope().clone(); let fingerprint = text.fingerprint(); let version = buffer.version(); - let save = self.write_file(path, text, buffer.line_ending(), cx); + let save = self.write_file(path.as_ref(), text, buffer.line_ending(), cx); + let fs = Arc::clone(&self.fs); + let abs_path = self.absolutize(&path); cx.as_mut().spawn(|mut cx| async move { let entry = save.await?; + let (entry_id, mtime, path) = match entry { + Some(entry) => (Some(entry.id), entry.mtime, entry.path), + None => { + let metadata = fs + .metadata(&abs_path) + .await + .with_context(|| { + format!( + "Fetching metadata after saving the excluded buffer {abs_path:?}" + ) + })? + .with_context(|| { + format!("Excluded buffer {path:?} got removed during saving") + })?; + (None, metadata.mtime, path) + } + }; + if has_changed_file { let new_file = Arc::new(File { - entry_id: Some(entry.id), + entry_id, worktree: handle, - path: entry.path, - mtime: entry.mtime, + path, + mtime, is_local: true, is_deleted: false, }); @@ -1080,13 +1092,13 @@ impl LocalWorktree { project_id, buffer_id, version: serialize_version(&version), - mtime: Some(entry.mtime.into()), + mtime: Some(mtime.into()), fingerprint: serialize_fingerprint(fingerprint), })?; } buffer_handle.update(&mut cx, |buffer, cx| { - buffer.did_save(version.clone(), fingerprint, entry.mtime, cx); + buffer.did_save(version.clone(), fingerprint, mtime, cx); }); Ok(()) @@ -1111,7 +1123,7 @@ impl LocalWorktree { path: impl Into>, is_dir: bool, cx: &mut ModelContext, - ) -> Task> { + ) -> Task>> { let path = path.into(); let lowest_ancestor = self.lowest_ancestor(&path); let abs_path = self.absolutize(&path); @@ -1128,7 +1140,7 @@ impl LocalWorktree { cx.spawn(|this, mut cx| async move { write.await?; let (result, refreshes) = this.update(&mut cx, |this, cx| { - let mut refreshes = Vec::>>::new(); + let mut refreshes = Vec::new(); let refresh_paths = path.strip_prefix(&lowest_ancestor).unwrap(); for refresh_path in refresh_paths.ancestors() { if refresh_path == Path::new("") { @@ -1155,14 +1167,14 @@ impl LocalWorktree { }) } - pub fn write_file( + pub(crate) fn write_file( &self, path: impl Into>, text: Rope, line_ending: LineEnding, cx: &mut ModelContext, - ) -> Task> { - let path = path.into(); + ) -> Task>> { + let path: Arc = path.into(); let abs_path = self.absolutize(&path); let fs = self.fs.clone(); let write = cx @@ -1221,8 +1233,11 @@ impl LocalWorktree { entry_id: ProjectEntryId, new_path: impl Into>, cx: &mut ModelContext, - ) -> Option>> { - let old_path = self.entry_for_id(entry_id)?.path.clone(); + ) -> Task>> { + let old_path = match self.entry_for_id(entry_id) { + Some(entry) => entry.path.clone(), + None => return Task::ready(Ok(None)), + }; let new_path = new_path.into(); let abs_old_path = self.absolutize(&old_path); let abs_new_path = self.absolutize(&new_path); @@ -1232,7 +1247,7 @@ impl LocalWorktree { .await }); - Some(cx.spawn(|this, mut cx| async move { + cx.spawn(|this, mut cx| async move { rename.await?; this.update(&mut cx, |this, cx| { this.as_local_mut() @@ -1240,7 +1255,7 @@ impl LocalWorktree { .refresh_entry(new_path.clone(), Some(old_path), cx) }) .await - })) + }) } pub fn copy_entry( @@ -1248,8 +1263,11 @@ impl LocalWorktree { entry_id: ProjectEntryId, new_path: impl Into>, cx: &mut ModelContext, - ) -> Option>> { - let old_path = self.entry_for_id(entry_id)?.path.clone(); + ) -> Task>> { + let old_path = match self.entry_for_id(entry_id) { + Some(entry) => entry.path.clone(), + None => return Task::ready(Ok(None)), + }; let new_path = new_path.into(); let abs_old_path = self.absolutize(&old_path); let abs_new_path = self.absolutize(&new_path); @@ -1264,7 +1282,7 @@ impl LocalWorktree { .await }); - Some(cx.spawn(|this, mut cx| async move { + cx.spawn(|this, mut cx| async move { copy.await?; this.update(&mut cx, |this, cx| { this.as_local_mut() @@ -1272,7 +1290,7 @@ impl LocalWorktree { .refresh_entry(new_path.clone(), None, cx) }) .await - })) + }) } pub fn expand_entry( @@ -1308,7 +1326,10 @@ impl LocalWorktree { path: Arc, old_path: Option>, cx: &mut ModelContext, - ) -> Task> { + ) -> Task>> { + if self.is_path_excluded(self.absolutize(&path)) { + return Task::ready(Ok(None)); + } let paths = if let Some(old_path) = old_path.as_ref() { vec![old_path.clone(), path.clone()] } else { @@ -1317,13 +1338,15 @@ impl LocalWorktree { let mut refresh = self.refresh_entries_for_paths(paths); cx.spawn_weak(move |this, mut cx| async move { refresh.recv().await; - this.upgrade(&cx) + let new_entry = this + .upgrade(&cx) .ok_or_else(|| anyhow!("worktree was dropped"))? .update(&mut cx, |this, _| { this.entry_for_path(path) .cloned() .ok_or_else(|| anyhow!("failed to read path after update")) - }) + })?; + Ok(Some(new_entry)) }) } diff --git a/crates/project/src/worktree_tests.rs b/crates/project/src/worktree_tests.rs index b4cf162d8f..35c1bb3ab1 100644 --- a/crates/project/src/worktree_tests.rs +++ b/crates/project/src/worktree_tests.rs @@ -1174,6 +1174,7 @@ async fn test_create_directory_during_initial_scan(cx: &mut TestAppContext) { .create_entry("a/e".as_ref(), true, cx) }) .await + .unwrap() .unwrap(); assert!(entry.is_dir()); @@ -1222,6 +1223,7 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) { .create_entry("a/b/c/d.txt".as_ref(), false, cx) }) .await + .unwrap() .unwrap(); assert!(entry.is_file()); @@ -1257,6 +1259,7 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) { .create_entry("a/b/c/d.txt".as_ref(), false, cx) }) .await + .unwrap() .unwrap(); assert!(entry.is_file()); @@ -1275,6 +1278,7 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) { .create_entry("a/b/c/e.txt".as_ref(), false, cx) }) .await + .unwrap() .unwrap(); assert!(entry.is_file()); @@ -1291,6 +1295,7 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) { .create_entry("d/e/f/g.txt".as_ref(), false, cx) }) .await + .unwrap() .unwrap(); assert!(entry.is_file()); @@ -1616,14 +1621,14 @@ fn randomly_mutate_worktree( entry.id.0, new_path ); - let task = worktree.rename_entry(entry.id, new_path, cx).unwrap(); + let task = worktree.rename_entry(entry.id, new_path, cx); cx.foreground().spawn(async move { - task.await?; + task.await?.unwrap(); Ok(()) }) } _ => { - let task = if entry.is_dir() { + if entry.is_dir() { let child_path = entry.path.join(random_filename(rng)); let is_dir = rng.gen_bool(0.3); log::info!( @@ -1631,15 +1636,20 @@ fn randomly_mutate_worktree( if is_dir { "dir" } else { "file" }, child_path, ); - worktree.create_entry(child_path, is_dir, cx) + let task = worktree.create_entry(child_path, is_dir, cx); + cx.foreground().spawn(async move { + task.await?; + Ok(()) + }) } else { log::info!("overwriting file {:?} ({})", entry.path, entry.id.0); - worktree.write_file(entry.path.clone(), "".into(), Default::default(), cx) - }; - cx.foreground().spawn(async move { - task.await?; - Ok(()) - }) + let task = + worktree.write_file(entry.path.clone(), "".into(), Default::default(), cx); + cx.foreground().spawn(async move { + task.await?; + Ok(()) + }) + } } } } diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 875d4d4f83..c37d388041 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -621,7 +621,7 @@ impl ProjectPanel { edited_entry_id = NEW_ENTRY_ID; edit_task = self.project.update(cx, |project, cx| { project.create_entry((worktree_id, &new_path), is_dir, cx) - })?; + }); } else { let new_path = if let Some(parent) = entry.path.clone().parent() { parent.join(&filename) @@ -635,7 +635,7 @@ impl ProjectPanel { edited_entry_id = entry.id; edit_task = self.project.update(cx, |project, cx| { project.rename_entry(entry.id, new_path.as_path(), cx) - })?; + }); }; edit_state.processing_filename = Some(filename); @@ -648,21 +648,22 @@ impl ProjectPanel { cx.notify(); })?; - let new_entry = new_entry?; - this.update(&mut cx, |this, cx| { - if let Some(selection) = &mut this.selection { - if selection.entry_id == edited_entry_id { - selection.worktree_id = worktree_id; - selection.entry_id = new_entry.id; - this.expand_to_selection(cx); + if let Some(new_entry) = new_entry? { + this.update(&mut cx, |this, cx| { + if let Some(selection) = &mut this.selection { + if selection.entry_id == edited_entry_id { + selection.worktree_id = worktree_id; + selection.entry_id = new_entry.id; + this.expand_to_selection(cx); + } } - } - this.update_visible_entries(None, cx); - if is_new_entry && !is_dir { - this.open_entry(new_entry.id, true, cx); - } - cx.notify(); - })?; + this.update_visible_entries(None, cx); + if is_new_entry && !is_dir { + this.open_entry(new_entry.id, true, cx); + } + cx.notify(); + })?; + } Ok(()) })) } @@ -935,15 +936,17 @@ impl ProjectPanel { } if clipboard_entry.is_cut() { - if let Some(task) = self.project.update(cx, |project, cx| { - project.rename_entry(clipboard_entry.entry_id(), new_path, cx) - }) { - task.detach_and_log_err(cx) - } - } else if let Some(task) = self.project.update(cx, |project, cx| { - project.copy_entry(clipboard_entry.entry_id(), new_path, cx) - }) { - task.detach_and_log_err(cx) + self.project + .update(cx, |project, cx| { + project.rename_entry(clipboard_entry.entry_id(), new_path, cx) + }) + .detach_and_log_err(cx) + } else { + self.project + .update(cx, |project, cx| { + project.copy_entry(clipboard_entry.entry_id(), new_path, cx) + }) + .detach_and_log_err(cx) } } None @@ -1026,7 +1029,7 @@ impl ProjectPanel { let mut new_path = destination_path.to_path_buf(); new_path.push(entry_path.path.file_name()?); if new_path != entry_path.path.as_ref() { - let task = project.rename_entry(entry_to_move, new_path, cx)?; + let task = project.rename_entry(entry_to_move, new_path, cx); cx.foreground().spawn(task).detach_and_log_err(cx); } diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 5b8fd5b199..611514aacb 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -430,7 +430,7 @@ message ExpandProjectEntryResponse { } message ProjectEntryResponse { - Entry entry = 1; + optional Entry entry = 1; uint64 worktree_scan_id = 2; } diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 5a13efd07a..dda976b2cd 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -1132,6 +1132,7 @@ mod tests { }) }) .await + .unwrap() .unwrap(); (wt, entry) diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index 77f042a7b8..21d6ff74b2 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -218,7 +218,8 @@ impl PathMatcher { }) } - // TODO kb tests + // TODO kb tests for matching + // TODO kb add an integration test on excluded file opening pub fn is_match>(&self, other: P) -> bool { let other_path = other.as_ref(); other_path.starts_with(&self.maybe_path) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 89cc4b7006..455c27a57e 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1549,7 +1549,6 @@ impl Workspace { let abs_path = abs_path.clone(); async move { let (worktree, project_path) = project_path?; - // TODO kb consider excluded files here? if fs.is_file(&abs_path).await { Some( this.update(&mut cx, |this, cx| { diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 65ba2daad4..d0a5267483 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -615,8 +615,8 @@ fn open_local_settings_file( .update(&mut cx, |project, cx| { project.create_entry((tree_id, dir_path), true, cx) }) - .ok_or_else(|| anyhow!("worktree was removed"))? - .await?; + .await + .context("worktree was removed")?; } } @@ -625,8 +625,8 @@ fn open_local_settings_file( .update(&mut cx, |project, cx| { project.create_entry((tree_id, file_path), false, cx) }) - .ok_or_else(|| anyhow!("worktree was removed"))? - .await?; + .await + .context("worktree was removed")?; } let editor = workspace @@ -1309,7 +1309,7 @@ mod tests { } #[gpui::test] - async fn test_opening_ignored_and_excluded_paths(cx: &mut TestAppContext) { + async fn test_opening_excluded_paths(cx: &mut TestAppContext) { let app_state = init_test(cx); cx.update(|cx| { cx.update_global::(|store, cx| { @@ -1319,7 +1319,6 @@ mod tests { }); }); }); - // TODO kb also test external excluded dirs opening app_state .fs .as_fake() @@ -1334,6 +1333,9 @@ mod tests { "file": "regular file contents", }, "ignored_dir": { + "ignored_subdir": { + "file": "ignored subfile contents", + }, "file": "ignored file contents", }, "excluded_dir": { @@ -1347,58 +1349,79 @@ mod tests { let window = cx.add_window(|cx| Workspace::test_new(project, cx)); let workspace = window.root(cx); - let entries = cx.read(|cx| workspace.file_project_paths(cx)); - // dbg!(&entries); - + let initial_entries = cx.read(|cx| workspace.file_project_paths(cx)); + let paths_to_open = [ + Path::new("/root/excluded_dir/file").to_path_buf(), + Path::new("/root/.git/HEAD").to_path_buf(), + Path::new("/root/excluded_dir/ignored_subdir").to_path_buf(), + ]; let (opened_workspace, new_items) = cx - .update(|cx| { - workspace::open_paths( - &[Path::new("/root/excluded_dir/file").to_path_buf()], - &app_state, - None, - cx, - ) - }) + .update(|cx| workspace::open_paths(&paths_to_open, &app_state, None, cx)) .await .unwrap(); - // dbg!( - // &workspace, - // &opened_workspace, - // new_items - // .iter() - // .map(|i| i - // .as_ref() - // .expect("should be present") - // .as_ref() - // .expect("should not error")) - // .map(|i| cx.read(|cx| i.project_path(cx))) - // .collect::>() - // ); + + assert_eq!( + opened_workspace.id(), + workspace.id(), + "Excluded files in subfolders of a workspace root should be opened in the workspace" + ); + let mut opened_paths = cx.read(|cx| { + assert_eq!( + new_items.len(), + paths_to_open.len(), + "Expect to get the same number of opened items as submitted paths to open" + ); + new_items + .iter() + .zip(paths_to_open.iter()) + .map(|(i, path)| { + match i { + Some(Ok(i)) => { + Some(i.project_path(cx).map(|p| p.path.display().to_string())) + } + Some(Err(e)) => panic!("Excluded file {path:?} failed to open: {e:?}"), + None => None, + } + .flatten() + }) + .collect::>() + }); + opened_paths.sort(); + assert_eq!( + opened_paths, + vec![ + None, + Some(".git/HEAD".to_string()), + Some("excluded_dir/file".to_string()), + ], + "Excluded files should get opened, excluded dir should not get opened" + ); let entries = cx.read(|cx| workspace.file_project_paths(cx)); - dbg!(&entries); - // #[rustfmt::skip] - // workspace.update(cx, |w, cx| { - // dbg!(w.open_paths(vec!["/root/regular_dir/file".into()], true, cx)); - // dbg!(w.open_paths(vec!["/root/ignored_dir/file".into()], true, cx)); - // dbg!(w.open_paths(vec!["/root/excluded_dir/file".into()], true, cx)); - // dbg!(w.open_paths(vec!["/root/excluded_dir/file".into()], false, cx)); - // - // }); + assert_eq!( + initial_entries, entries, + "Workspace entries should not change after opening excluded files and directories paths" + ); - // // Open the first entry - // let entry_1 = workspace - // .update(cx, |w, cx| w.open_path(file1.clone(), None, true, cx)) - // .await - // .unwrap(); - // cx.read(|cx| { - // let pane = workspace.read(cx).active_pane().read(cx); - // assert_eq!( - // pane.active_item().unwrap().project_path(cx), - // Some(file1.clone()) - // ); - // assert_eq!(pane.items_len(), 1); - // }); + cx.read(|cx| { + let pane = workspace.read(cx).active_pane().read(cx); + let mut opened_buffer_paths = pane + .items() + .map(|i| { + i.project_path(cx) + .expect("all excluded files that got open should have a path") + .path + .display() + .to_string() + }) + .collect::>(); + opened_buffer_paths.sort(); + assert_eq!( + opened_buffer_paths, + vec![".git/HEAD".to_string(), "excluded_dir/file".to_string()], + "Despite not being present in the worktrees, buffers for excluded files are opened and added to the pane" + ); + }); } #[gpui::test]