From 0a491e773b689a74f96b7555070cf5a3bf245543 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Mon, 25 Sep 2023 16:15:29 +0200 Subject: [PATCH] workspace: Improve save prompt. (#3025) Add buffer path to the prompt. Z-2903 Release Notes: - Added a "Save all/Discard all" prompt when closing a pane with multiple edited buffers. --- crates/util/src/util.rs | 14 ++++ crates/workspace/src/pane.rs | 128 +++++++++++++++++++++++------- crates/workspace/src/workspace.rs | 39 +++++++-- 3 files changed, 148 insertions(+), 33 deletions(-) diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 3f83f8e37a..629f950014 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -41,6 +41,8 @@ pub fn truncate(s: &str, max_chars: usize) -> &str { } } +/// Removes characters from the end of the string if it's length is greater than `max_chars` and +/// appends "..." to the string. Returns string unchanged if it's length is smaller than max_chars. pub fn truncate_and_trailoff(s: &str, max_chars: usize) -> String { debug_assert!(max_chars >= 5); @@ -51,6 +53,18 @@ pub fn truncate_and_trailoff(s: &str, max_chars: usize) -> String { } } +/// Removes characters from the front of the string if it's length is greater than `max_chars` and +/// prepends the string with "...". Returns string unchanged if it's length is smaller than max_chars. +pub fn truncate_and_remove_front(s: &str, max_chars: usize) -> String { + debug_assert!(max_chars >= 5); + + let truncation_ix = s.char_indices().map(|(i, _)| i).nth_back(max_chars); + match truncation_ix { + Some(length) => "…".to_string() + &s[length..], + None => s.to_string(), + } +} + pub fn post_inc + AddAssign + Copy>(value: &mut T) -> T { let prev = *value; *value += T::from(1); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index a3e6a547dd..a191adcc05 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -42,6 +42,7 @@ use std::{ }, }; use theme::{Theme, ThemeSettings}; +use util::truncate_and_remove_front; #[derive(PartialEq, Clone, Copy, Deserialize, Debug)] #[serde(rename_all = "camelCase")] @@ -839,10 +840,45 @@ impl Pane { Some(self.close_items(cx, SaveBehavior::PromptOnWrite, |_| true)) } + pub(super) fn file_names_for_prompt( + items: &mut dyn Iterator>, + all_dirty_items: usize, + cx: &AppContext, + ) -> String { + /// Quantity of item paths displayed in prompt prior to cutoff.. + const FILE_NAMES_CUTOFF_POINT: usize = 10; + let mut file_names: Vec<_> = items + .filter_map(|item| { + item.project_path(cx).and_then(|project_path| { + project_path + .path + .file_name() + .and_then(|name| name.to_str().map(ToOwned::to_owned)) + }) + }) + .take(FILE_NAMES_CUTOFF_POINT) + .collect(); + let should_display_followup_text = + all_dirty_items > FILE_NAMES_CUTOFF_POINT || file_names.len() != all_dirty_items; + if should_display_followup_text { + let not_shown_files = all_dirty_items - file_names.len(); + if not_shown_files == 1 { + file_names.push(".. 1 file not shown".into()); + } else { + file_names.push(format!(".. {} files not shown", not_shown_files).into()); + } + } + let file_names = file_names.join("\n"); + format!( + "Do you want to save changes to the following {} files?\n{file_names}", + all_dirty_items + ) + } + pub fn close_items( &mut self, cx: &mut ViewContext, - save_behavior: SaveBehavior, + mut save_behavior: SaveBehavior, should_close: impl 'static + Fn(usize) -> bool, ) -> Task> { // Find the items to close. @@ -861,6 +897,25 @@ impl Pane { let workspace = self.workspace.clone(); cx.spawn(|pane, mut cx| async move { + if save_behavior == SaveBehavior::PromptOnWrite && items_to_close.len() > 1 { + let mut answer = pane.update(&mut cx, |_, cx| { + let prompt = Self::file_names_for_prompt( + &mut items_to_close.iter(), + items_to_close.len(), + cx, + ); + cx.prompt( + PromptLevel::Warning, + &prompt, + &["Save all", "Discard all", "Cancel"], + ) + })?; + match answer.next().await { + Some(0) => save_behavior = SaveBehavior::PromptOnConflict, + Some(1) => save_behavior = SaveBehavior::DontSave, + _ => {} + } + } let mut saved_project_items_ids = HashSet::default(); for item in items_to_close.clone() { // Find the item's current index and its set of project item models. Avoid @@ -1003,7 +1058,6 @@ impl Pane { ) -> Result { const CONFLICT_MESSAGE: &str = "This file has changed on disk since you started editing it. Do you want to overwrite it?"; - const DIRTY_MESSAGE: &str = "This file contains unsaved edits. Do you want to save it?"; if save_behavior == SaveBehavior::DontSave { return Ok(true); @@ -1046,9 +1100,10 @@ impl Pane { let should_save = if save_behavior == SaveBehavior::PromptOnWrite && !will_autosave { let mut answer = pane.update(cx, |pane, cx| { pane.activate_item(item_ix, true, true, cx); + let prompt = dirty_message_for(item.project_path(cx)); cx.prompt( PromptLevel::Warning, - DIRTY_MESSAGE, + &prompt, &["Save", "Don't Save", "Cancel"], ) })?; @@ -2135,6 +2190,15 @@ impl Element for PaneBackdrop { } } +fn dirty_message_for(buffer_path: Option) -> String { + let path = buffer_path + .as_ref() + .and_then(|p| p.path.to_str()) + .unwrap_or(&"Untitled buffer"); + let path = truncate_and_remove_front(path, 80); + format!("{path} contains unsaved edits. Do you want to save it?") +} + #[cfg(test)] mod tests { use super::*; @@ -2479,12 +2543,14 @@ mod tests { set_labeled_items(&pane, ["A", "B", "C*", "D", "E"], cx); - pane.update(cx, |pane, cx| { - pane.close_inactive_items(&CloseInactiveItems, cx) - }) - .unwrap() - .await - .unwrap(); + let task = pane + .update(cx, |pane, cx| { + pane.close_inactive_items(&CloseInactiveItems, cx) + }) + .unwrap(); + cx.foreground().run_until_parked(); + window.simulate_prompt_answer(2, cx); + task.await.unwrap(); assert_item_labels(&pane, ["C*"], cx); } @@ -2505,10 +2571,12 @@ mod tests { add_labeled_item(&pane, "E", false, cx); assert_item_labels(&pane, ["A^", "B", "C^", "D", "E*"], cx); - pane.update(cx, |pane, cx| pane.close_clean_items(&CloseCleanItems, cx)) - .unwrap() - .await + let task = pane + .update(cx, |pane, cx| pane.close_clean_items(&CloseCleanItems, cx)) .unwrap(); + cx.foreground().run_until_parked(); + window.simulate_prompt_answer(2, cx); + task.await.unwrap(); assert_item_labels(&pane, ["A^", "C*^"], cx); } @@ -2524,12 +2592,14 @@ mod tests { set_labeled_items(&pane, ["A", "B", "C*", "D", "E"], cx); - pane.update(cx, |pane, cx| { - pane.close_items_to_the_left(&CloseItemsToTheLeft, cx) - }) - .unwrap() - .await - .unwrap(); + let task = pane + .update(cx, |pane, cx| { + pane.close_items_to_the_left(&CloseItemsToTheLeft, cx) + }) + .unwrap(); + cx.foreground().run_until_parked(); + window.simulate_prompt_answer(2, cx); + task.await.unwrap(); assert_item_labels(&pane, ["C*", "D", "E"], cx); } @@ -2545,12 +2615,14 @@ mod tests { set_labeled_items(&pane, ["A", "B", "C*", "D", "E"], cx); - pane.update(cx, |pane, cx| { - pane.close_items_to_the_right(&CloseItemsToTheRight, cx) - }) - .unwrap() - .await - .unwrap(); + let task = pane + .update(cx, |pane, cx| { + pane.close_items_to_the_right(&CloseItemsToTheRight, cx) + }) + .unwrap(); + cx.foreground().run_until_parked(); + window.simulate_prompt_answer(2, cx); + task.await.unwrap(); assert_item_labels(&pane, ["A", "B", "C*"], cx); } @@ -2569,10 +2641,12 @@ mod tests { add_labeled_item(&pane, "C", false, cx); assert_item_labels(&pane, ["A", "B", "C*"], cx); - pane.update(cx, |pane, cx| pane.close_all_items(&CloseAllItems, cx)) - .unwrap() - .await + let t = pane + .update(cx, |pane, cx| pane.close_all_items(&CloseAllItems, cx)) .unwrap(); + cx.foreground().run_until_parked(); + window.simulate_prompt_answer(2, cx); + t.await.unwrap(); assert_item_labels(&pane, [], cx); } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index feab53d094..263652184a 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1333,13 +1333,12 @@ impl Workspace { fn save_all_internal( &mut self, - save_behaviour: SaveBehavior, + mut save_behaviour: SaveBehavior, cx: &mut ViewContext, ) -> Task> { if self.project.read(cx).is_read_only() { return Task::ready(Ok(true)); } - let dirty_items = self .panes .iter() @@ -1355,7 +1354,27 @@ impl Workspace { .collect::>(); let project = self.project.clone(); - cx.spawn(|_, mut cx| async move { + cx.spawn(|workspace, mut cx| async move { + // Override save mode and display "Save all files" prompt + if save_behaviour == SaveBehavior::PromptOnWrite && dirty_items.len() > 1 { + let mut answer = workspace.update(&mut cx, |_, cx| { + let prompt = Pane::file_names_for_prompt( + &mut dirty_items.iter().map(|(_, handle)| handle), + dirty_items.len(), + cx, + ); + cx.prompt( + PromptLevel::Warning, + &prompt, + &["Save all", "Discard all", "Cancel"], + ) + })?; + match answer.next().await { + Some(0) => save_behaviour = SaveBehavior::PromptOnConflict, + Some(1) => save_behaviour = SaveBehavior::DontSave, + _ => {} + } + } for (pane, item) in dirty_items { let (singleton, project_entry_ids) = cx.read(|cx| (item.is_singleton(cx), item.project_entry_ids(cx))); @@ -4320,7 +4339,9 @@ mod tests { }); let task = workspace.update(cx, |w, cx| w.prepare_to_close(false, cx)); cx.foreground().run_until_parked(); - window.simulate_prompt_answer(2, cx); // cancel + window.simulate_prompt_answer(2, cx); // cancel save all + cx.foreground().run_until_parked(); + window.simulate_prompt_answer(2, cx); // cancel save all cx.foreground().run_until_parked(); assert!(!window.has_pending_prompt(cx)); assert!(!task.await.unwrap()); @@ -4378,13 +4399,15 @@ mod tests { }); cx.foreground().run_until_parked(); + assert!(window.has_pending_prompt(cx)); + // Ignore "Save all" prompt + window.simulate_prompt_answer(2, cx); + cx.foreground().run_until_parked(); // There's a prompt to save item 1. pane.read_with(cx, |pane, _| { assert_eq!(pane.items_len(), 4); assert_eq!(pane.active_item().unwrap().id(), item1.id()); }); - assert!(window.has_pending_prompt(cx)); - // Confirm saving item 1. window.simulate_prompt_answer(0, cx); cx.foreground().run_until_parked(); @@ -4512,6 +4535,10 @@ mod tests { let close = left_pane.update(cx, |pane, cx| { pane.close_items(cx, SaveBehavior::PromptOnWrite, move |_| true) }); + cx.foreground().run_until_parked(); + // Discard "Save all" prompt + window.simulate_prompt_answer(2, cx); + cx.foreground().run_until_parked(); left_pane.read_with(cx, |pane, cx| { assert_eq!(