diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 9f79115d282..a40580547ff 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -662,6 +662,17 @@ enum RemoveEntryTask { Delete(Task>), } +struct RemovalPrompt { + message: String, + detail: Option<&'static str>, + confirmation_label: &'static str, +} + +enum RemovalKind { + Trash, + Delete, +} + impl ProjectPanel { fn new( workspace: &mut Workspace, @@ -2564,6 +2575,74 @@ impl ProjectPanel { }); } + /// Builds the confirmation prompt shared by direct removal and undo/redo. + /// + /// The `names` slice must contain every path being removed, and this + /// function will apply markdown formatting and display truncation. The + /// `dirty_buffers` parameter should be the number of those paths that have + /// unsaved changes. + fn build_removal_prompt( + kind: RemovalKind, + names: &[S], + dirty_buffers: usize, + ) -> RemovalPrompt + where + S: AsRef, + { + let (message_start, confirmation_label, detail) = match kind { + RemovalKind::Trash => ("Do you want to trash", "Trash", None), + RemovalKind::Delete => ( + "Are you sure you want to permanently delete", + "Delete", + Some("This cannot be undone."), + ), + }; + + let mut message = match names { + [name] => format!("{message_start} {}?", MarkdownInlineCode(name.as_ref())), + _ => { + const CUTOFF_POINT: usize = 10; + let mut listed_names = names + .iter() + .take(CUTOFF_POINT) + .map(|name| MarkdownInlineCode(name.as_ref()).to_string()) + .collect::>(); + let omitted_count = names.len().saturating_sub(CUTOFF_POINT); + if omitted_count == 1 { + listed_names.push(".. 1 file not shown".into()); + } else if omitted_count > 1 { + listed_names.push(format!(".. {omitted_count} files not shown")); + } + + format!( + "{message_start} the following {} files?\n{}", + names.len(), + listed_names.join("\n") + ) + } + }; + match dirty_buffers { + 0 => {} + 1 if names.len() == 1 => { + message.push_str("\n\nIt has unsaved changes, which will be lost."); + } + 1 => { + message.push_str("\n\n1 of these has unsaved changes, which will be lost."); + } + dirty_buffers => { + message.push_str(&format!( + "\n\n{dirty_buffers} of these have unsaved changes, which will be lost." + )); + } + } + + RemovalPrompt { + message, + detail, + confirmation_label, + } + } + // TODO(yara|dino): trashing and deleting are conceptually distinct, even // more so with the fact that trashing can now be undone, whereas deleting // cannot. @@ -2602,70 +2681,24 @@ impl ProjectPanel { return None; } let answer = if !skip_prompt { - let operation = if trash { "Trash" } else { "Delete" }; - let message_start = if trash { - "Do you want to trash" + let names = file_paths + .iter() + .map(|(_, _, name)| name.as_str()) + .collect::>(); + + let removal_kind = if trash { + RemovalKind::Trash } else { - "Are you sure you want to permanently delete" + RemovalKind::Delete }; - let prompt = match file_paths.first() { - Some((_, _, path)) if file_paths.len() == 1 => { - let unsaved_warning = if dirty_buffers > 0 { - "\n\nIt has unsaved changes, which will be lost." - } else { - "" - }; - format!( - "{message_start} {}?{unsaved_warning}", - MarkdownInlineCode(path) - ) - } - _ => { - const CUTOFF_POINT: usize = 10; - let names = if file_paths.len() > CUTOFF_POINT { - let truncated_path_counts = file_paths.len() - CUTOFF_POINT; - let mut paths = file_paths - .iter() - .map(|(_, _, path)| MarkdownInlineCode(path).to_string()) - .take(CUTOFF_POINT) - .collect::>(); - paths.truncate(CUTOFF_POINT); - if truncated_path_counts == 1 { - paths.push(".. 1 file not shown".into()); - } else { - paths.push(format!(".. {} files not shown", truncated_path_counts)); - } - paths - } else { - file_paths - .iter() - .map(|(_, _, path)| MarkdownInlineCode(path).to_string()) - .collect() - }; - let unsaved_warning = if dirty_buffers == 0 { - String::new() - } else if dirty_buffers == 1 { - "\n\n1 of these has unsaved changes, which will be lost.".to_string() - } else { - format!( - "\n\n{dirty_buffers} of these have unsaved changes, which will be lost." - ) - }; + let prompt = Self::build_removal_prompt(removal_kind, &names, dirty_buffers); - format!( - "{message_start} the following {} files?\n{}{unsaved_warning}", - file_paths.len(), - names.join("\n") - ) - } - }; - let detail = (!trash).then_some("This cannot be undone."); Some(window.prompt( PromptLevel::Info, - &prompt, - detail, - &[operation, "Cancel"], + &prompt.message, + prompt.detail, + &[prompt.confirmation_label, "Cancel"], cx, )) } else { diff --git a/crates/project_panel/src/tests/undo.rs b/crates/project_panel/src/tests/undo.rs index 070b31de6a5..b3341e5988c 100644 --- a/crates/project_panel/src/tests/undo.rs +++ b/crates/project_panel/src/tests/undo.rs @@ -2,17 +2,18 @@ use client::proto; use collections::HashSet; +use editor::Editor; use fs::{FakeFs, Fs}; -use gpui::{App, BorrowAppContext, Entity, VisualTestContext}; +use gpui::{App, BorrowAppContext, Context, Entity, VisualTestContext, Window}; use project::Project; use serde_json::{Value, json}; use settings::SettingsStore; use std::path::Path; use std::sync::Arc; -use workspace::{MultiWorkspace, register_project_item}; +use workspace::{Item, MultiWorkspace, register_project_item}; use crate::project_panel_tests::{self, TestProjectItemView, find_project_entry, select_path}; -use crate::{NewDirectory, NewFile, ProjectPanel, Redo, Rename, Trash, Undo}; +use crate::{NewDirectory, NewFile, Open, ProjectPanel, Redo, Rename, Trash, Undo}; struct TestContext { panel: Entity, @@ -50,6 +51,7 @@ impl TestContext { }); self.cx.run_until_parked(); } + async fn redo(&mut self) { self.panel.update_in(&mut self.cx, |panel, window, cx| { panel.redo(&Redo, window, cx); @@ -57,7 +59,40 @@ impl TestContext { self.cx.run_until_parked(); } + fn open_path(&mut self, path: &str) { + select_path(&self.panel, &format!("workspace/{path}"), &mut self.cx); + self.panel.update_in(&mut self.cx, |panel, window, cx| { + panel.open(&Open, window, cx) + }); + self.cx.run_until_parked(); + } + + #[track_caller] + fn assert_prompt(&self, expected: &str) { + let (prompt, _) = self + .cx + .cx + .pending_prompt() + .expect("should have pending prompt"); + assert!( + prompt.contains(expected), + "expected prompt to contain {expected:?}, got {prompt:?}" + ); + } + + #[track_caller] + fn answer(&self, answer: &str) { + assert!( + self.cx.cx.has_pending_prompt(), + "should have pending prompt" + ); + + self.cx.cx.simulate_prompt_answer(answer); + self.cx.run_until_parked(); + } + /// Note this only works when every file has an extension + #[track_caller] fn assert_fs_state_is(&mut self, state: &[&str]) { let state: HashSet<_> = state .into_iter() @@ -94,6 +129,7 @@ impl TestContext { ); } + #[track_caller] fn assert_not_exists(&mut self, file: &str) { assert_eq!( find_project_entry(&self.panel, &format!("workspace/{file}"), &mut self.cx), @@ -231,6 +267,7 @@ impl TestContext { async fn new_with_tree(cx: &mut gpui::TestAppContext, tree: Value) -> TestContext { project_panel_tests::init_test(cx); + cx.update(register_project_item::); let fs = FakeFs::new(cx.executor()); fs.insert_tree("/workspace", tree).await; @@ -256,6 +293,23 @@ impl TestContext { fn update_app(&mut self, update: impl FnOnce(&mut App) -> R) -> R { self.cx.cx.update(update) } + + #[track_caller] + fn update_active_editor( + &mut self, + update: impl FnOnce(&mut Editor, &mut Window, &mut Context<'_, Editor>) -> R, + ) -> R { + let editor = self + .panel + .read_with(&self.cx, |panel, _| panel.workspace.upgrade()) + .expect("workspace should still exist") + .read_with(&self.cx, |workspace, cx| { + workspace.active_item_as::(cx) + }) + .expect("editor should be open"); + + editor.update_in(&mut self.cx, update) + } } #[gpui::test] @@ -287,6 +341,7 @@ async fn create_undo_redo(cx: &mut gpui::TestAppContext) { cx.fs.write(Path::new(&path), b"Hello!").await.unwrap(); cx.undo().await; + cx.answer("Trash"); cx.assert_not_exists("c.txt"); cx.redo().await; @@ -294,6 +349,45 @@ async fn create_undo_redo(cx: &mut gpui::TestAppContext) { assert_eq!(cx.fs.load(Path::new(&path)).await.unwrap(), "Hello!"); } +#[gpui::test] +async fn undo_create_cancel_trash(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + + cx.create_file("c.txt").await; + + cx.undo().await; + cx.answer("Cancel"); + cx.assert_exists("c.txt"); + + cx.undo().await; + cx.answer("Trash"); + cx.assert_not_exists("c.txt"); +} + +#[gpui::test] +async fn undo_create_dirty_file(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + + cx.create_file("c.txt").await; + + // After `c.txt` is created, it should now be open in the editor, so we'll + // simulate inserting some content into the file, without saving, in order + // to make it dirty. + cx.update_active_editor(|editor, window, cx| { + editor.handle_input("unsaved changes", window, cx); + assert!(editor.is_dirty(cx)); + }); + + cx.undo().await; + cx.answer("Cancel"); + cx.assert_exists("c.txt"); + cx.update_active_editor(|editor, _window, cx| assert!(editor.is_dirty(cx))); + + cx.undo().await; + cx.answer("Don't Save"); + cx.assert_not_exists("c.txt"); +} + #[gpui::test] async fn create_dir_undo(cx: &mut gpui::TestAppContext) { let mut cx = TestContext::new(cx).await; @@ -301,6 +395,7 @@ async fn create_dir_undo(cx: &mut gpui::TestAppContext) { cx.create_directory("new_dir").await; cx.assert_exists("new_dir"); cx.undo().await; + cx.answer("Trash"); cx.assert_not_exists("new_dir"); } @@ -365,6 +460,7 @@ async fn two_sequential_undos(cx: &mut gpui::TestAppContext) { cx.assert_fs_state_is(&["b.txt", "x.txt", "y.txt"]); cx.undo().await; + cx.answer("Trash"); cx.assert_fs_state_is(&["b.txt", "x.txt"]); cx.undo().await; @@ -392,6 +488,7 @@ async fn trash_undo_redo(cx: &mut gpui::TestAppContext) { cx.assert_fs_state_is(&["a.txt", "b.txt"]); cx.redo().await; + cx.answer("Trash"); cx.assert_fs_state_is(&[]); } @@ -423,6 +520,7 @@ async fn trash_directory_undo_redo(cx: &mut gpui::TestAppContext) { ); cx.redo().await; + cx.answer("Trash"); cx.assert_fs_state_is(&["a.txt"]); } @@ -543,3 +641,41 @@ async fn excluded_create_is_not_recorded(cx: &mut gpui::TestAppContext) { cx.undo().await; cx.assert_fs_state_is(&["a.txt", "token.secret", "banana.secret"]); } + +#[gpui::test] +async fn cancel_partial_trash_batch(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + + cx.trash(&["a.txt", "b.txt"]).await; + cx.undo().await; + cx.assert_fs_state_is(&["a.txt", "b.txt"]); + + cx.redo().await; + cx.answer("Cancel"); + cx.assert_fs_state_is(&["a.txt", "b.txt"]); + + cx.redo().await; + cx.answer("Trash"); + cx.assert_fs_state_is(&[]); +} + +#[gpui::test] +async fn batch_trash_warns_about_unsaved_changes(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + + cx.trash(&["a.txt", "b.txt"]).await; + cx.undo().await; + cx.assert_fs_state_is(&["a.txt", "b.txt"]); + + cx.open_path("a.txt"); + cx.update_active_editor(|editor, window, cx| { + editor.handle_input("unsaved changes", window, cx); + }); + + cx.redo().await; + cx.assert_prompt("1 of these has unsaved changes, which will be lost."); + + cx.answer("Cancel"); + cx.assert_fs_state_is(&["a.txt", "b.txt"]); + cx.update_active_editor(|editor, _window, cx| assert!(editor.is_dirty(cx))); +} diff --git a/crates/project_panel/src/undo.rs b/crates/project_panel/src/undo.rs index 65c365c2ef4..3bc552b7cf1 100644 --- a/crates/project_panel/src/undo.rs +++ b/crates/project_panel/src/undo.rs @@ -130,11 +130,13 @@ //! List of "tainted files" that the user may not operate on -use crate::ProjectPanel; +use crate::{ProjectPanel, RemovalKind}; use anyhow::{Context, Result, anyhow}; use fs::{TrashId, TrashRestoreError}; use futures::channel::mpsc; -use gpui::{AppContext, AsyncApp, IntoElement, SharedString, Styled, Task, WeakEntity}; +use gpui::{ + AppContext, AsyncApp, IntoElement, PromptLevel, SharedString, Styled, Task, WeakEntity, +}; use markdown::{Markdown, MarkdownElement}; use project::Project; use project::{ProjectPath, WorktreeId}; @@ -143,7 +145,7 @@ use std::{collections::VecDeque, sync::Arc}; use ui::{App, TextSize}; use util::{paths::PathStyle, rel_path::RelPath}; use workspace::{ - Workspace, + Pane, SaveIntent, Workspace, notifications::{ NotificationId, markdown_style, simple_message_notification::MessageNotification, }, @@ -158,10 +160,63 @@ enum Operation { } impl Operation { - async fn execute(self, undo_manager: &Inner, cx: &mut AsyncApp) -> Result { - Ok(match self { + /// Attempts to execute the operation, returning an `Err` if the operation + /// failed to execute or `Ok(None)` if the operation was cancelled by the + /// user, for example, cancelling trashing a file with unsaved edits. + async fn execute(self, undo_manager: &Inner, cx: &mut AsyncApp) -> Result> { + let change = match self { Operation::Trash(project_path) => { - let trash_id = undo_manager.trash(&project_path, cx).await?; + let Some(trash_id) = undo_manager.trash(&project_path, cx).await? else { + return Ok(None); + }; + Change::Trashed(project_path.worktree_id, trash_id) + } + Operation::Batch(operations) => { + let mut trash_paths = Vec::new(); + + for operation in &operations { + operation.trash_paths(&mut trash_paths); + } + + if !trash_paths.is_empty() + && !undo_manager.confirm_batch_trash(trash_paths, cx).await? + { + return Ok(None); + } + + let mut changes = Vec::new(); + + for operation in operations { + changes.push(Box::pin(operation.execute_confirmed(undo_manager, cx)).await?); + } + + Change::Batched(changes) + } + operation => { + return operation + .execute_confirmed(undo_manager, cx) + .await + .map(Some); + } + }; + + Ok(Some(change)) + } + + /// Same as [`Self::execute`], but assumes the operation has already been + /// confirmed in order to avoid showing confirmation modals to the user. + /// + /// Useful in scenarios where undoing a batch of operations would lead to + /// multiple files being trashed, in which case we only ask the user once + /// whether they'd like to trash the files, and then execute each trash + /// operation without confirming each file one by one. + async fn execute_confirmed(self, undo_manager: &Inner, cx: &mut AsyncApp) -> Result { + let change = match self { + Operation::Trash(project_path) => { + let trash_id = undo_manager + .trash_without_confirmation(&project_path, cx) + .await?; + Change::Trashed(project_path.worktree_id, trash_id) } Operation::Rename(from, to) => { @@ -173,13 +228,29 @@ impl Operation { Change::Restored(project_path) } Operation::Batch(operations) => { - let mut res = Vec::new(); - for op in operations { - res.push(Box::pin(op.execute(undo_manager, cx)).await?); + let mut changes = Vec::new(); + + for operation in operations { + changes.push(Box::pin(operation.execute_confirmed(undo_manager, cx)).await?); } - Change::Batched(res) + + Change::Batched(changes) } - }) + }; + + Ok(change) + } + + fn trash_paths<'a>(&'a self, paths: &mut Vec<&'a ProjectPath>) { + match self { + Operation::Trash(path) => paths.push(path), + Operation::Batch(operations) => { + for operation in operations { + operation.trash_paths(paths); + } + } + Operation::Rename(..) | Operation::Restore(..) => {} + } } } @@ -449,14 +520,17 @@ impl Inner { // manual intervention would likely be needed in order to undo. As such, // we remove the change from the `history` even before attempting to // execute its inversion. - let undo_change = self - .history - .remove(before_cursor) - .expect("we can undo") - .to_inverse() - .execute(self, cx) - .await?; - self.history.insert(before_cursor, undo_change); + let change = self.history.remove(before_cursor).expect("we can undo"); + let operation = change.clone().to_inverse(); + + match operation.execute(self, cx).await? { + Some(undo_change) => self.history.insert(before_cursor, undo_change), + None => { + self.history.insert(before_cursor, change); + self.cursor += 1; + } + }; + Ok(()) } @@ -469,15 +543,15 @@ impl Inner { // manual intervention would likely be needed in order to redo. As such, // we remove the change from the `history` even before attempting to // execute its inversion. - let redo_change = self - .history - .remove(self.cursor) - .expect("we can redo") - .to_inverse() - .execute(self, cx) - .await?; - self.history.insert(self.cursor, redo_change); - self.cursor += 1; + let change = self.history.remove(self.cursor).expect("we can redo"); + let operation = change.clone().to_inverse(); + match operation.execute(self, cx).await? { + Some(redo_change) => { + self.history.insert(self.cursor, redo_change); + self.cursor += 1; + } + None => self.history.insert(self.cursor, change), + } Ok(()) } @@ -569,7 +643,43 @@ impl Inner { }) } - async fn trash(&self, project_path: &ProjectPath, cx: &mut AsyncApp) -> Result { + /// Trashes the specified `project_path` after prompting the user for + /// confirmation. If the path has unsaved changes, the prompt also lets the + /// user save or discard them. + /// + /// Returns `Ok(None)` if the user cancels the operation. + async fn trash( + &self, + project_path: &ProjectPath, + cx: &mut AsyncApp, + ) -> Result> { + let Some(workspace) = self.workspace.upgrade() else { + return Err(anyhow!("Failed to obtain workspace.")); + }; + + let name = workspace.update(cx, |workspace, cx| { + let project = workspace.project().read(cx); + let path_style = project.path_style(cx); + + project_path_display(project, project_path, path_style, cx) + }); + + if !self.confirm_trash(project_path, &name, cx).await? { + return Ok(None); + } + + self.trash_without_confirmation(project_path, cx) + .await + .map(Some) + } + + /// Same as [`Self::trash`] but proceeds without confirming if the user + /// wishes to trash the file. + async fn trash_without_confirmation( + &self, + project_path: &ProjectPath, + cx: &mut AsyncApp, + ) -> Result { let Some(workspace) = self.workspace.upgrade() else { return Err(anyhow!("Failed to obtain workspace.")); }; @@ -683,4 +793,111 @@ impl Inner { }) .ok(); } + + /// Prompts the user to confirm whether they really want to trash the file. + /// In the case the file has unsaved changes, the prompt will ask whether + /// the user wants to save or discard these changes. + /// + /// Returns `true` if the user confirmed they want to trash the file, + /// `false` otherwise. + async fn confirm_trash( + &self, + project_path: &ProjectPath, + name: &str, + cx: &mut AsyncApp, + ) -> Result { + let open_item = self.workspace.update(cx, |workspace, cx| { + workspace.panes().iter().find_map(|pane| { + pane.read(cx).items().find_map(|item| { + (item.is_dirty(cx) + && item + .project_path(cx) + .iter() + .any(|item_path| item_path == project_path)) + .then(|| (pane.clone(), item.boxed_clone())) + }) + }) + })?; + + let Some((pane, item)) = open_item else { + return self.trash_prompt(&[name], 0, cx).await; + }; + + let mut async_window_cx = self + .panel + .update_in(cx, |_panel, window, cx| window.to_async(cx))?; + let project = self + .panel + .read_with(cx, |panel, _cx| panel.project.clone())?; + + Pane::save_item( + project, + pane, + item.as_ref(), + SaveIntent::Close, + &mut async_window_cx, + ) + .await + } + + /// Prompts the user to confirm whether they really want to trash all of the + /// provided `trash_paths`. + /// + /// Meant to be used when executing a batch of operations that will lead to + /// one or more paths being trashed. + /// + /// Returns `true` if the user confirmed they want to trash the files, + /// `false` otherwise. + async fn confirm_batch_trash( + &self, + trash_paths: Vec<&ProjectPath>, + cx: &mut AsyncApp, + ) -> Result { + let (names, dirty_buffers) = self.workspace.update(cx, |workspace, cx| { + let project = workspace.project().read(cx); + let path_style = project.path_style(cx); + let dirty_buffers = project + .dirty_buffers(cx) + .filter(|dirty_path| trash_paths.contains(&dirty_path)) + .count(); + let names = trash_paths + .iter() + .map(|project_path| project_path_display(project, project_path, path_style, cx)) + .collect::>(); + + (names, dirty_buffers) + })?; + + self.trash_prompt(&names, dirty_buffers, cx).await + } + + /// Prompts the user to confirm whether they actually want to trash the + /// file. + /// + /// Returns `true` if the user confirms, `false` otherwise. + async fn trash_prompt( + &self, + names: &[S], + dirty_buffers: usize, + cx: &mut AsyncApp, + ) -> Result + where + S: AsRef, + { + let prompt = ProjectPanel::build_removal_prompt(RemovalKind::Trash, names, dirty_buffers); + let answer = self + .panel + .update_in(cx, |_panel, window, cx| { + window.prompt( + PromptLevel::Info, + &prompt.message, + prompt.detail, + &[prompt.confirmation_label, "Cancel"], + cx, + ) + })? + .await?; + + Ok(answer == 0) + } }