From 47122a31159753c46afa525cf15acc34dfbb36aa Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 4 Jun 2024 10:31:43 +0300 Subject: [PATCH] Fix excluded file creation (#12620) Fixes https://github.com/zed-industries/zed/issues/10890 * removes `unwrap()` that caused panics for text elements with no text, remaining after edit state is cleared but project entries are not updated, having the fake, "new entry" * improves discoverability of the FS errors during file/directory creation: now those are shown as workspace notifications * stops printing anyhow backtraces in workspace notifications, printing the more readable chain of contexts instead * better indicates when new entries are created as excluded ones Release Notes: - Improve excluded entry creation workflow in the project panel ([10890](https://github.com/zed-industries/zed/issues/10890)) --- Cargo.lock | 2 + crates/collab/Cargo.toml | 1 + crates/collab/src/tests/integration_tests.rs | 9 +- .../src/platform/cosmic_text/text_system.rs | 4 +- crates/project/src/project.rs | 44 ++- crates/project/src/project_tests.rs | 2 + crates/project_panel/Cargo.toml | 1 + crates/project_panel/src/project_panel.rs | 340 ++++++++++++++++-- crates/terminal_view/src/terminal_view.rs | 1 + crates/workspace/src/notifications.rs | 19 +- crates/worktree/src/worktree.rs | 75 +++- crates/worktree/src/worktree_tests.rs | 7 +- 12 files changed, 447 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e7ca312a20..0c948f4da2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2404,6 +2404,7 @@ dependencies = [ "util", "uuid", "workspace", + "worktree", ] [[package]] @@ -7825,6 +7826,7 @@ dependencies = [ "unicase", "util", "workspace", + "worktree", ] [[package]] diff --git a/crates/collab/Cargo.toml b/crates/collab/Cargo.toml index 005d4a27a0..d7a2e458f6 100644 --- a/crates/collab/Cargo.toml +++ b/crates/collab/Cargo.toml @@ -107,4 +107,5 @@ theme.workspace = true unindent.workspace = true util.workspace = true workspace = { workspace = true, features = ["test-support"] } +worktree = { workspace = true, features = ["test-support"] } headless.workspace = true diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 003c0c6ae3..3cf9c302dd 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -3022,7 +3022,6 @@ async fn test_fs_operations( let project_b = client_b.build_dev_server_project(project_id, cx_b).await; let worktree_a = project_a.read_with(cx_a, |project, _| project.worktrees().next().unwrap()); - let worktree_b = project_b.read_with(cx_b, |project, _| project.worktrees().next().unwrap()); let entry = project_b @@ -3031,6 +3030,7 @@ async fn test_fs_operations( }) .await .unwrap() + .to_included() .unwrap(); worktree_a.read_with(cx_a, |worktree, _| { @@ -3059,6 +3059,7 @@ async fn test_fs_operations( }) .await .unwrap() + .to_included() .unwrap(); worktree_a.read_with(cx_a, |worktree, _| { @@ -3087,6 +3088,7 @@ async fn test_fs_operations( }) .await .unwrap() + .to_included() .unwrap(); worktree_a.read_with(cx_a, |worktree, _| { @@ -3115,20 +3117,25 @@ async fn test_fs_operations( }) .await .unwrap() + .to_included() .unwrap(); + project_b .update(cx_b, |project, cx| { project.create_entry((worktree_id, "DIR/SUBDIR"), true, cx) }) .await .unwrap() + .to_included() .unwrap(); + project_b .update(cx_b, |project, cx| { project.create_entry((worktree_id, "DIR/SUBDIR/f.txt"), false, cx) }) .await .unwrap() + .to_included() .unwrap(); worktree_a.read_with(cx_a, |worktree, _| { diff --git a/crates/gpui/src/platform/cosmic_text/text_system.rs b/crates/gpui/src/platform/cosmic_text/text_system.rs index 57032dc646..039c6dd3d5 100644 --- a/crates/gpui/src/platform/cosmic_text/text_system.rs +++ b/crates/gpui/src/platform/cosmic_text/text_system.rs @@ -294,7 +294,7 @@ impl CosmicTextSystemState { .0, ) .clone() - .unwrap(); + .with_context(|| format!("no image for {params:?} in font {font:?}"))?; Ok(Bounds { origin: point(image.placement.left.into(), (-image.placement.top).into()), size: size(image.placement.width.into(), image.placement.height.into()), @@ -328,7 +328,7 @@ impl CosmicTextSystemState { .0, ) .clone() - .unwrap(); + .with_context(|| format!("no image for {params:?} in font {font:?}"))?; if params.is_emoji { // Convert from RGBA to BGRA. diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index fc21b1294e..50e79263d9 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -68,7 +68,7 @@ use project_settings::{LspSettings, ProjectSettings}; use rand::prelude::*; use search_history::SearchHistory; use snippet::Snippet; -use worktree::LocalSnapshot; +use worktree::{CreatedEntry, LocalSnapshot}; use http::{HttpClient, Url}; use rpc::{ErrorCode, ErrorExt as _}; @@ -1414,10 +1414,12 @@ impl Project { project_path: impl Into, is_directory: bool, cx: &mut ModelContext, - ) -> Task>> { + ) -> Task> { let project_path = project_path.into(); let Some(worktree) = self.worktree_for_id(project_path.worktree_id, cx) else { - return Task::ready(Ok(None)); + return Task::ready(Err(anyhow!(format!( + "No worktree for path {project_path:?}" + )))); }; if self.is_local() { worktree.update(cx, |worktree, cx| { @@ -1448,8 +1450,15 @@ impl Project { ) })? .await - .map(Some), - None => Ok(None), + .map(CreatedEntry::Included), + None => { + let abs_path = worktree.update(&mut cx, |worktree, _| { + worktree + .absolutize(&project_path.path) + .with_context(|| format!("absolutizing {project_path:?}")) + })??; + Ok(CreatedEntry::Excluded { abs_path }) + } } }) } @@ -1506,9 +1515,9 @@ impl Project { entry_id: ProjectEntryId, new_path: impl Into>, cx: &mut ModelContext, - ) -> Task>> { + ) -> Task> { let Some(worktree) = self.worktree_for_entry(entry_id, cx) else { - return Task::ready(Ok(None)); + return Task::ready(Err(anyhow!(format!("No worktree for entry {entry_id:?}")))); }; let new_path = new_path.into(); if self.is_local() { @@ -1540,8 +1549,15 @@ impl Project { ) })? .await - .map(Some), - None => Ok(None), + .map(CreatedEntry::Included), + None => { + let abs_path = worktree.update(&mut cx, |worktree, _| { + worktree + .absolutize(&new_path) + .with_context(|| format!("absolutizing {new_path:?}")) + })??; + Ok(CreatedEntry::Excluded { abs_path }) + } } }) } @@ -8617,7 +8633,10 @@ impl Project { })? .await?; Ok(proto::ProjectEntryResponse { - entry: entry.as_ref().map(|e| e.into()), + entry: match &entry { + CreatedEntry::Included(entry) => Some(entry.into()), + CreatedEntry::Excluded { .. } => None, + }, worktree_scan_id: worktree_scan_id as u64, }) } @@ -8644,7 +8663,10 @@ impl Project { })? .await?; Ok(proto::ProjectEntryResponse { - entry: entry.as_ref().map(|e| e.into()), + entry: match &entry { + CreatedEntry::Included(entry) => Some(entry.into()), + CreatedEntry::Excluded { .. } => None, + }, worktree_scan_id: worktree_scan_id as u64, }) } diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index ade29efa97..3562366096 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -3127,6 +3127,7 @@ async fn test_buffer_identity_across_renames(cx: &mut gpui::TestAppContext) { }) .unwrap() .await + .to_included() .unwrap(); cx.executor().run_until_parked(); @@ -4465,6 +4466,7 @@ async fn test_create_entry(cx: &mut gpui::TestAppContext) { }) .unwrap() .await + .to_included() .unwrap(); // Can't create paths outside the project diff --git a/crates/project_panel/Cargo.toml b/crates/project_panel/Cargo.toml index 92912bf007..33b57bbf33 100644 --- a/crates/project_panel/Cargo.toml +++ b/crates/project_panel/Cargo.toml @@ -34,6 +34,7 @@ ui.workspace = true unicase.workspace = true util.workspace = true client.workspace = true +worktree.workspace = true workspace.workspace = true [dev-dependencies] diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index e25fbb5d72..2f225b2881 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -35,9 +35,10 @@ use unicase::UniCase; use util::{maybe, NumericPrefixWithSuffix, ResultExt, TryFutureExt}; use workspace::{ dock::{DockPosition, Panel, PanelEvent}, - notifications::DetachAndPromptErr, + notifications::{DetachAndPromptErr, NotifyTaskExt}, OpenInTerminal, Workspace, }; +use worktree::CreatedEntry; const PROJECT_PANEL_KEY: &str = "ProjectPanel"; const NEW_ENTRY_ID: ProjectEntryId = ProjectEntryId::MAX; @@ -711,7 +712,7 @@ impl ProjectPanel { fn confirm(&mut self, _: &Confirm, cx: &mut ViewContext) { if let Some(task) = self.confirm_edit(cx) { - task.detach_and_log_err(cx); + task.detach_and_notify_err(cx); } } @@ -794,29 +795,66 @@ impl ProjectPanel { edit_state.processing_filename = Some(filename); cx.notify(); - Some(cx.spawn(|this, mut cx| async move { + Some(cx.spawn(|project_panel, mut cx| async move { let new_entry = edit_task.await; - this.update(&mut cx, |this, cx| { - this.edit_state.take(); + project_panel.update(&mut cx, |project_panel, cx| { + project_panel.edit_state.take(); cx.notify(); })?; - 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.marked_entries.clear(); - this.expand_to_selection(cx); + match new_entry { + Err(e) => { + project_panel.update(&mut cx, |project_panel, cx| { + project_panel.marked_entries.clear(); + project_panel.update_visible_entries(None, cx); + }).ok(); + Err(e)?; + } + Ok(CreatedEntry::Included(new_entry)) => { + project_panel.update(&mut cx, |project_panel, cx| { + if let Some(selection) = &mut project_panel.selection { + if selection.entry_id == edited_entry_id { + selection.worktree_id = worktree_id; + selection.entry_id = new_entry.id; + project_panel.marked_entries.clear(); + project_panel.expand_to_selection(cx); + } } + project_panel.update_visible_entries(None, cx); + if is_new_entry && !is_dir { + project_panel.open_entry(new_entry.id, false, true, false, cx); + } + cx.notify(); + })?; + } + Ok(CreatedEntry::Excluded { abs_path }) => { + if let Some(open_task) = project_panel + .update(&mut cx, |project_panel, cx| { + project_panel.marked_entries.clear(); + project_panel.update_visible_entries(None, cx); + + if is_dir { + project_panel.project.update(cx, |_, cx| { + cx.emit(project::Event::Notification(format!( + "Created an excluded directory at {abs_path:?}.\nAlter `file_scan_exclusions` in the settings to show it in the panel" + ))) + }); + None + } else { + project_panel + .workspace + .update(cx, |workspace, cx| { + workspace.open_abs_path(abs_path, true, cx) + }) + .ok() + } + }) + .ok() + .flatten() + { + let _ = open_task.await?; } - this.update_visible_entries(None, cx); - if is_new_entry && !is_dir { - this.open_entry(new_entry.id, false, true, false, cx); - } - cx.notify(); - })?; + } } Ok(()) })) @@ -2369,13 +2407,16 @@ impl ClipboardEntry { mod tests { use super::*; use collections::HashSet; - use gpui::{TestAppContext, View, VisualTestContext, WindowHandle}; + use gpui::{Empty, TestAppContext, View, VisualTestContext, WindowHandle}; use pretty_assertions::assert_eq; use project::{FakeFs, WorktreeSettings}; use serde_json::json; use settings::SettingsStore; use std::path::{Path, PathBuf}; - use workspace::AppState; + use workspace::{ + item::{Item, ProjectItem}, + register_project_item, AppState, + }; #[gpui::test] async fn test_visible_list(cx: &mut gpui::TestAppContext) { @@ -4488,6 +4529,199 @@ mod tests { ); } + #[gpui::test] + async fn test_creating_excluded_entries(cx: &mut gpui::TestAppContext) { + init_test(cx); + cx.update(|cx| { + cx.update_global::(|store, cx| { + store.update_user_settings::(cx, |project_settings| { + project_settings.file_scan_exclusions = + Some(vec!["excluded_dir".to_string(), "**/.git".to_string()]); + }); + }); + }); + + cx.update(|cx| { + register_project_item::(cx); + }); + + let fs = FakeFs::new(cx.executor().clone()); + fs.insert_tree( + "/root1", + json!({ + ".dockerignore": "", + ".git": { + "HEAD": "", + }, + }), + ) + .await; + + let project = Project::test(fs.clone(), ["/root1".as_ref()], cx).await; + let workspace = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let cx = &mut VisualTestContext::from_window(*workspace, cx); + let panel = workspace + .update(cx, |workspace, cx| { + let panel = ProjectPanel::new(workspace, cx); + workspace.add_panel(panel.clone(), cx); + panel + }) + .unwrap(); + + select_path(&panel, "root1", cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..10, cx), + &["v root1 <== selected", " .dockerignore",] + ); + workspace + .update(cx, |workspace, cx| { + assert!( + workspace.active_item(cx).is_none(), + "Should have no active items in the beginning" + ); + }) + .unwrap(); + + let excluded_file_path = ".git/COMMIT_EDITMSG"; + let excluded_dir_path = "excluded_dir"; + + panel.update(cx, |panel, cx| panel.new_file(&NewFile, cx)); + panel.update(cx, |panel, cx| { + assert!(panel.filename_editor.read(cx).is_focused(cx)); + }); + panel + .update(cx, |panel, cx| { + panel + .filename_editor + .update(cx, |editor, cx| editor.set_text(excluded_file_path, cx)); + panel.confirm_edit(cx).unwrap() + }) + .await + .unwrap(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..13, cx), + &["v root1", " .dockerignore"], + "Excluded dir should not be shown after opening a file in it" + ); + panel.update(cx, |panel, cx| { + assert!( + !panel.filename_editor.read(cx).is_focused(cx), + "Should have closed the file name editor" + ); + }); + workspace + .update(cx, |workspace, cx| { + let active_entry_path = workspace + .active_item(cx) + .expect("should have opened and activated the excluded item") + .act_as::(cx) + .expect( + "should have opened the corresponding project item for the excluded item", + ) + .read(cx) + .path + .clone(); + assert_eq!( + active_entry_path.path.as_ref(), + Path::new(excluded_file_path), + "Should open the excluded file" + ); + + assert!( + workspace.notification_ids().is_empty(), + "Should have no notifications after opening an excluded file" + ); + }) + .unwrap(); + assert!( + fs.is_file(Path::new("/root1/.git/COMMIT_EDITMSG")).await, + "Should have created the excluded file" + ); + + select_path(&panel, "root1", cx); + panel.update(cx, |panel, cx| panel.new_directory(&NewDirectory, cx)); + panel.update(cx, |panel, cx| { + assert!(panel.filename_editor.read(cx).is_focused(cx)); + }); + panel + .update(cx, |panel, cx| { + panel + .filename_editor + .update(cx, |editor, cx| editor.set_text(excluded_file_path, cx)); + panel.confirm_edit(cx).unwrap() + }) + .await + .unwrap(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..13, cx), + &["v root1", " .dockerignore"], + "Should not change the project panel after trying to create an excluded directorya directory with the same name as the excluded file" + ); + panel.update(cx, |panel, cx| { + assert!( + !panel.filename_editor.read(cx).is_focused(cx), + "Should have closed the file name editor" + ); + }); + workspace + .update(cx, |workspace, cx| { + let notifications = workspace.notification_ids(); + assert_eq!( + notifications.len(), + 1, + "Should receive one notification with the error message" + ); + workspace.dismiss_notification(notifications.first().unwrap(), cx); + assert!(workspace.notification_ids().is_empty()); + }) + .unwrap(); + + select_path(&panel, "root1", cx); + panel.update(cx, |panel, cx| panel.new_directory(&NewDirectory, cx)); + panel.update(cx, |panel, cx| { + assert!(panel.filename_editor.read(cx).is_focused(cx)); + }); + panel + .update(cx, |panel, cx| { + panel + .filename_editor + .update(cx, |editor, cx| editor.set_text(excluded_dir_path, cx)); + panel.confirm_edit(cx).unwrap() + }) + .await + .unwrap(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..13, cx), + &["v root1", " .dockerignore"], + "Should not change the project panel after trying to create an excluded directory" + ); + panel.update(cx, |panel, cx| { + assert!( + !panel.filename_editor.read(cx).is_focused(cx), + "Should have closed the file name editor" + ); + }); + workspace + .update(cx, |workspace, cx| { + let notifications = workspace.notification_ids(); + assert_eq!( + notifications.len(), + 1, + "Should receive one notification explaining that no directory is actually shown" + ); + workspace.dismiss_notification(notifications.first().unwrap(), cx); + assert!(workspace.notification_ids().is_empty()); + }) + .unwrap(); + assert!( + fs.is_dir(Path::new("/root1/excluded_dir")).await, + "Should have created the excluded directory" + ); + } + fn toggle_expand_dir( panel: &View, path: impl AsRef, @@ -4716,4 +4950,68 @@ mod tests { }) .unwrap(); } + + struct TestProjectItemView { + focus_handle: FocusHandle, + path: ProjectPath, + } + + struct TestProjectItem { + path: ProjectPath, + } + + impl project::Item for TestProjectItem { + fn try_open( + _project: &Model, + path: &ProjectPath, + cx: &mut AppContext, + ) -> Option>>> { + let path = path.clone(); + Some(cx.spawn(|mut cx| async move { cx.new_model(|_| Self { path }) })) + } + + fn entry_id(&self, _: &AppContext) -> Option { + None + } + + fn project_path(&self, _: &AppContext) -> Option { + Some(self.path.clone()) + } + } + + impl ProjectItem for TestProjectItemView { + type Item = TestProjectItem; + + fn for_project_item( + _: Model, + project_item: Model, + cx: &mut ViewContext, + ) -> Self + where + Self: Sized, + { + Self { + path: project_item.update(cx, |project_item, _| project_item.path.clone()), + focus_handle: cx.focus_handle(), + } + } + } + + impl Item for TestProjectItemView { + type Event = (); + } + + impl EventEmitter<()> for TestProjectItemView {} + + impl FocusableView for TestProjectItemView { + fn focus_handle(&self, _: &AppContext) -> FocusHandle { + self.focus_handle.clone() + } + } + + impl Render for TestProjectItemView { + fn render(&mut self, _: &mut ViewContext) -> impl IntoElement { + Empty + } + } } diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 0bc0a609a0..bea068439b 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -1309,6 +1309,7 @@ mod tests { }) .await .unwrap() + .to_included() .unwrap(); (wt, entry) diff --git a/crates/workspace/src/notifications.rs b/crates/workspace/src/notifications.rs index 76924303e3..325b75542f 100644 --- a/crates/workspace/src/notifications.rs +++ b/crates/workspace/src/notifications.rs @@ -122,6 +122,15 @@ impl Workspace { } } + #[cfg(any(test, feature = "test-support"))] + pub fn notification_ids(&self) -> Vec { + self.notifications + .iter() + .map(|(id, _)| id) + .cloned() + .collect() + } + pub fn show_notification( &mut self, id: NotificationId, @@ -144,7 +153,7 @@ impl Workspace { pub fn show_error(&mut self, err: &E, cx: &mut ViewContext) where - E: std::fmt::Debug, + E: std::fmt::Debug + std::fmt::Display, { struct WorkspaceErrorNotification; @@ -153,7 +162,7 @@ impl Workspace { cx, |cx| { cx.new_view(|_cx| { - simple_message_notification::MessageNotification::new(format!("Error: {err:?}")) + simple_message_notification::MessageNotification::new(format!("Error: {err:#}")) }) }, ); @@ -464,7 +473,7 @@ pub trait NotifyResultExt { impl NotifyResultExt for Result where - E: std::fmt::Debug, + E: std::fmt::Debug + std::fmt::Display, { type Ok = T; @@ -483,7 +492,7 @@ where match self { Ok(value) => Some(value), Err(err) => { - log::error!("TODO {err:?}"); + log::error!("{err:?}"); cx.update_root(|view, cx| { if let Ok(workspace) = view.downcast::() { workspace.update(cx, |workspace, cx| workspace.show_error(&err, cx)) @@ -502,7 +511,7 @@ pub trait NotifyTaskExt { impl NotifyTaskExt for Task> where - E: std::fmt::Debug + Sized + 'static, + E: std::fmt::Debug + std::fmt::Display + Sized + 'static, R: 'static, { fn detach_and_notify_err(self, cx: &mut WindowContext) { diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 6dca9aa898..11527dbd99 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -97,6 +97,25 @@ pub enum Worktree { Remote(RemoteWorktree), } +/// An entry, created in the worktree. +#[derive(Debug)] +pub enum CreatedEntry { + /// Got created and indexed by the worktree, receiving a corresponding entry. + Included(Entry), + /// Got created, but not indexed due to falling under exclusion filters. + Excluded { abs_path: PathBuf }, +} + +#[cfg(any(test, feature = "test-support"))] +impl CreatedEntry { + pub fn to_included(self) -> Option { + match self { + CreatedEntry::Included(entry) => Some(entry), + CreatedEntry::Excluded { .. } => None, + } + } +} + pub struct LocalWorktree { snapshot: LocalSnapshot, scan_requests_tx: channel::Sender, @@ -1322,22 +1341,34 @@ 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); + let abs_path = match self.absolutize(&path) { + Ok(path) => path, + Err(e) => return Task::ready(Err(e.context(format!("absolutizing path {path:?}")))), + }; + let path_excluded = self.is_path_excluded(&abs_path); let fs = self.fs.clone(); + let task_abs_path = abs_path.clone(); let write = cx.background_executor().spawn(async move { if is_dir { - fs.create_dir(&abs_path?).await - } else { - fs.save(&abs_path?, &Default::default(), Default::default()) + fs.create_dir(&task_abs_path) .await + .with_context(|| format!("creating directory {task_abs_path:?}")) + } else { + fs.save(&task_abs_path, &Rope::default(), LineEnding::default()) + .await + .with_context(|| format!("creating file {task_abs_path:?}")) } }); + let lowest_ancestor = self.lowest_ancestor(&path); cx.spawn(|this, mut cx| async move { write.await?; + if path_excluded { + return Ok(CreatedEntry::Excluded { abs_path }); + } + let (result, refreshes) = this.update(&mut cx, |this, cx| { let mut refreshes = Vec::new(); let refresh_paths = path.strip_prefix(&lowest_ancestor).unwrap(); @@ -1362,7 +1393,10 @@ impl LocalWorktree { refresh.await.log_err(); } - result.await + Ok(result + .await? + .map(CreatedEntry::Included) + .unwrap_or_else(|| CreatedEntry::Excluded { abs_path })) }) } @@ -1448,19 +1482,22 @@ impl LocalWorktree { entry_id: ProjectEntryId, new_path: impl Into>, cx: &mut ModelContext, - ) -> Task>> { + ) -> Task> { let old_path = match self.entry_for_id(entry_id) { Some(entry) => entry.path.clone(), - None => return Task::ready(Ok(None)), + None => return Task::ready(Err(anyhow!("no entry to rename for id {entry_id:?}"))), }; let new_path = new_path.into(); let abs_old_path = self.absolutize(&old_path); - let abs_new_path = self.absolutize(&new_path); + let Ok(abs_new_path) = self.absolutize(&new_path) else { + return Task::ready(Err(anyhow!("absolutizing path {new_path:?}"))); + }; + let abs_path = abs_new_path.clone(); let fs = self.fs.clone(); let case_sensitive = self.fs_case_sensitive; let rename = cx.background_executor().spawn(async move { let abs_old_path = abs_old_path?; - let abs_new_path = abs_new_path?; + let abs_new_path = abs_new_path; let abs_old_path_lower = abs_old_path.to_str().map(|p| p.to_lowercase()); let abs_new_path_lower = abs_new_path.to_str().map(|p| p.to_lowercase()); @@ -1480,16 +1517,20 @@ impl LocalWorktree { }, ) .await + .with_context(|| format!("Renaming {abs_old_path:?} into {abs_new_path:?}")) }); cx.spawn(|this, mut cx| async move { rename.await?; - this.update(&mut cx, |this, cx| { - this.as_local_mut() - .unwrap() - .refresh_entry(new_path.clone(), Some(old_path), cx) - })? - .await + Ok(this + .update(&mut cx, |this, cx| { + this.as_local_mut() + .unwrap() + .refresh_entry(new_path.clone(), Some(old_path), cx) + })? + .await? + .map(CreatedEntry::Included) + .unwrap_or_else(|| CreatedEntry::Excluded { abs_path })) }) } diff --git a/crates/worktree/src/worktree_tests.rs b/crates/worktree/src/worktree_tests.rs index 9f040da857..46c7fe5865 100644 --- a/crates/worktree/src/worktree_tests.rs +++ b/crates/worktree/src/worktree_tests.rs @@ -1212,6 +1212,7 @@ async fn test_create_directory_during_initial_scan(cx: &mut TestAppContext) { }) .await .unwrap() + .to_included() .unwrap(); assert!(entry.is_dir()); @@ -1268,6 +1269,7 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) { }) .await .unwrap() + .to_included() .unwrap(); assert!(entry.is_file()); @@ -1310,6 +1312,7 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) { }) .await .unwrap() + .to_included() .unwrap(); assert!(entry.is_file()); @@ -1329,6 +1332,7 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) { }) .await .unwrap() + .to_included() .unwrap(); assert!(entry.is_file()); @@ -1346,6 +1350,7 @@ async fn test_create_dir_all_on_create_entry(cx: &mut TestAppContext) { }) .await .unwrap() + .to_included() .unwrap(); assert!(entry.is_file()); @@ -1673,7 +1678,7 @@ fn randomly_mutate_worktree( ); let task = worktree.rename_entry(entry.id, new_path, cx); cx.background_executor().spawn(async move { - task.await?.unwrap(); + task.await?.to_included().unwrap(); Ok(()) }) }