Revert "Revert "workspace: Improve save prompt. (#3025)""

Conrad Irwin created

This reverts commit 5c75450a77b0579649bbf79365c9cef5a2c1110f.

Change summary

crates/search/src/buffer_search.rs |   4 
crates/util/src/util.rs            |  14 +++
crates/workspace/src/pane.rs       | 136 ++++++++++++++++++++++++-------
crates/workspace/src/workspace.rs  |  39 +++++++-
4 files changed, 154 insertions(+), 39 deletions(-)

Detailed changes

crates/search/src/buffer_search.rs 🔗

@@ -541,10 +541,10 @@ impl BufferSearchBar {
 
     pub fn set_replacement(&mut self, replacement: Option<&str>, cx: &mut ViewContext<Self>) {
         if replacement.is_none() {
-            self.replace_is_active = false;
+            self.replace_enabled = false;
             return;
         }
-        self.replace_is_active = true;
+        self.replace_enabled = true;
         self.replacement_editor
             .update(cx, |replacement_editor, cx| {
                 replacement_editor

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<T: From<u8> + AddAssign<T> + Copy>(value: &mut T) -> T {
     let prev = *value;
     *value += T::from(1);

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")]
@@ -849,10 +850,45 @@ impl Pane {
         )
     }
 
+    pub(super) fn file_names_for_prompt(
+        items: &mut dyn Iterator<Item = &Box<dyn ItemHandle>>,
+        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<Pane>,
-        save_intent: SaveIntent,
+        mut save_intent: SaveIntent,
         should_close: impl 'static + Fn(usize) -> bool,
     ) -> Task<Result<()>> {
         // Find the items to close.
@@ -871,6 +907,25 @@ impl Pane {
 
         let workspace = self.workspace.clone();
         cx.spawn(|pane, mut cx| async move {
+            if save_intent == SaveIntent::Close && 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_intent = SaveIntent::Save,
+                    Some(1) => save_intent = SaveIntent::Skip,
+                    _ => {}
+                }
+            }
             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
@@ -1013,7 +1068,6 @@ impl Pane {
     ) -> Result<bool> {
         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_intent == SaveIntent::Skip {
             return Ok(true);
@@ -1068,15 +1122,16 @@ impl Pane {
                 if !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"],
                         )
                     })?;
                     match answer.next().await {
                         Some(0) => {}
-                        Some(1) => return Ok(true), // Don't save this file
+                        Some(1) => return Ok(true), // Don't save his file
                         _ => return Ok(false),      // Cancel
                     }
                 }
@@ -2154,6 +2209,15 @@ impl<V: 'static> Element<V> for PaneBackdrop<V> {
     }
 }
 
+fn dirty_message_for(buffer_path: Option<ProjectPath>) -> 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::*;
@@ -2473,12 +2537,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);
     }
 
@@ -2499,10 +2565,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);
     }
 
@@ -2518,12 +2586,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);
     }
 
@@ -2539,12 +2609,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);
     }
 
@@ -2563,12 +2635,14 @@ 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 { save_intent: None }, cx)
-        })
-        .unwrap()
-        .await
-        .unwrap();
+        let t = pane
+            .update(cx, |pane, cx| {
+                pane.close_all_items(&CloseAllItems { save_intent: None }, cx)
+            })
+            .unwrap();
+        cx.foreground().run_until_parked();
+        window.simulate_prompt_answer(2, cx);
+        t.await.unwrap();
         assert_item_labels(&pane, [], cx);
     }
 

crates/workspace/src/workspace.rs 🔗

@@ -1372,13 +1372,12 @@ impl Workspace {
 
     fn save_all_internal(
         &mut self,
-        save_intent: SaveIntent,
+        mut save_intent: SaveIntent,
         cx: &mut ViewContext<Self>,
     ) -> Task<Result<bool>> {
         if self.project.read(cx).is_read_only() {
             return Task::ready(Ok(true));
         }
-
         let dirty_items = self
             .panes
             .iter()
@@ -1394,7 +1393,27 @@ impl Workspace {
             .collect::<Vec<_>>();
 
         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_intent == SaveIntent::Close && 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_intent = SaveIntent::Save,
+                    Some(1) => save_intent = SaveIntent::Skip,
+                    _ => {}
+                }
+            }
             for (pane, item) in dirty_items {
                 let (singleton, project_entry_ids) =
                     cx.read(|cx| (item.is_singleton(cx), item.project_entry_ids(cx)));
@@ -4361,7 +4380,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());
@@ -4419,13 +4440,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();
@@ -4553,6 +4576,10 @@ mod tests {
         let close = left_pane.update(cx, |pane, cx| {
             pane.close_items(cx, SaveIntent::Close, 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!(