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))
This commit is contained in:
Kirill Bulatov 2024-06-04 10:31:43 +03:00 committed by GitHub
parent edd613062a
commit 47122a3115
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 447 additions and 58 deletions

View file

@ -34,6 +34,7 @@ ui.workspace = true
unicase.workspace = true
util.workspace = true
client.workspace = true
worktree.workspace = true
workspace.workspace = true
[dev-dependencies]

View file

@ -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<Self>) {
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::<SettingsStore, _>(|store, cx| {
store.update_user_settings::<WorktreeSettings>(cx, |project_settings| {
project_settings.file_scan_exclusions =
Some(vec!["excluded_dir".to_string(), "**/.git".to_string()]);
});
});
});
cx.update(|cx| {
register_project_item::<TestProjectItemView>(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::<TestProjectItemView>(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<ProjectPanel>,
path: impl AsRef<Path>,
@ -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<Project>,
path: &ProjectPath,
cx: &mut AppContext,
) -> Option<Task<gpui::Result<Model<Self>>>> {
let path = path.clone();
Some(cx.spawn(|mut cx| async move { cx.new_model(|_| Self { path }) }))
}
fn entry_id(&self, _: &AppContext) -> Option<ProjectEntryId> {
None
}
fn project_path(&self, _: &AppContext) -> Option<ProjectPath> {
Some(self.path.clone())
}
}
impl ProjectItem for TestProjectItemView {
type Item = TestProjectItem;
fn for_project_item(
_: Model<Project>,
project_item: Model<Self::Item>,
cx: &mut ViewContext<Self>,
) -> 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<Self>) -> impl IntoElement {
Empty
}
}
}