From 2f741c86864f1d9670e3cb01a05c1cbfe5395581 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Thu, 13 Feb 2025 10:13:43 -0700 Subject: [PATCH] vim: Fix :wq in multibuffer (#24603) Supercedes #24561 Closes #21059 Before this change we would skip saving multibuffers regardless of the save intent. Now we correctly save them. Along the way: * Prompt to save when closing the last singleton copy of an item (even if it's still open in a multibuffer). * Update our file name prompt to pull out dirty project items from multibuffers instead of counting multibuffers as untitled files. * Fix our prompt test helpers to require passing the button name instead of the index. A few tests were passing invalid responses to save prompts. * Refactor the code a bit to hopefully clarify it for the next bug. Release Notes: - Fixed edge-cases when closing multiple items including multibuffers. Previously no prompt was generated when closing an item that was open in a multibuffer, now you will be prompted. - vim: Fix :wq in a multibuffer --- crates/collab/src/tests/following_tests.rs | 2 +- crates/gpui/src/app/test_context.rs | 10 +- crates/gpui/src/platform/test/platform.rs | 52 +++- crates/gpui/src/platform/test/window.rs | 4 +- crates/project_panel/src/project_panel.rs | 2 +- crates/recent_projects/src/recent_projects.rs | 3 +- crates/vim/src/command.rs | 6 +- crates/workspace/src/item.rs | 22 +- crates/workspace/src/pane.rs | 216 ++++++++----- crates/workspace/src/workspace.rs | 283 ++++++------------ crates/zed/src/zed.rs | 8 +- 11 files changed, 318 insertions(+), 290 deletions(-) diff --git a/crates/collab/src/tests/following_tests.rs b/crates/collab/src/tests/following_tests.rs index 58aed662b2..dea74f80a2 100644 --- a/crates/collab/src/tests/following_tests.rs +++ b/crates/collab/src/tests/following_tests.rs @@ -250,7 +250,7 @@ async fn test_basic_following( }); executor.run_until_parked(); // are you sure you want to leave the call? - cx_c.simulate_prompt_answer(0); + cx_c.simulate_prompt_answer("Close window and hang up"); cx_c.cx.update(|_| { drop(workspace_c); }); diff --git a/crates/gpui/src/app/test_context.rs b/crates/gpui/src/app/test_context.rs index 7ad7623d33..c4cb595252 100644 --- a/crates/gpui/src/app/test_context.rs +++ b/crates/gpui/src/app/test_context.rs @@ -286,8 +286,9 @@ impl TestAppContext { } /// Simulates clicking a button in an platform-level alert dialog. - pub fn simulate_prompt_answer(&self, button_ix: usize) { - self.test_platform.simulate_prompt_answer(button_ix); + #[track_caller] + pub fn simulate_prompt_answer(&self, button: &str) { + self.test_platform.simulate_prompt_answer(button); } /// Returns true if there's an alert dialog open. @@ -295,6 +296,11 @@ impl TestAppContext { self.test_platform.has_pending_prompt() } + /// Returns true if there's an alert dialog open. + pub fn pending_prompt(&self) -> Option<(String, String)> { + self.test_platform.pending_prompt() + } + /// All the urls that have been opened with cx.open_url() during this test. pub fn opened_url(&self) -> Option { self.test_platform.opened_url.borrow().clone() diff --git a/crates/gpui/src/platform/test/platform.rs b/crates/gpui/src/platform/test/platform.rs index 50ad24a520..cb29783c84 100644 --- a/crates/gpui/src/platform/test/platform.rs +++ b/crates/gpui/src/platform/test/platform.rs @@ -64,9 +64,16 @@ impl ScreenCaptureSource for TestScreenCaptureSource { impl ScreenCaptureStream for TestScreenCaptureStream {} +struct TestPrompt { + msg: String, + detail: Option, + answers: Vec, + tx: oneshot::Sender, +} + #[derive(Default)] pub(crate) struct TestPrompts { - multiple_choice: VecDeque>, + multiple_choice: VecDeque, new_path: VecDeque<(PathBuf, oneshot::Sender>>)>, } @@ -123,33 +130,64 @@ impl TestPlatform { .new_path .pop_front() .expect("no pending new path prompt"); + self.background_executor().set_waiting_hint(None); tx.send(Ok(select_path(&path))).ok(); } - pub(crate) fn simulate_prompt_answer(&self, response_ix: usize) { - let tx = self + #[track_caller] + pub(crate) fn simulate_prompt_answer(&self, response: &str) { + let prompt = self .prompts .borrow_mut() .multiple_choice .pop_front() .expect("no pending multiple choice prompt"); self.background_executor().set_waiting_hint(None); - tx.send(response_ix).ok(); + let Some(ix) = prompt.answers.iter().position(|a| a == response) else { + panic!( + "PROMPT: {}\n{:?}\n{:?}\nCannot respond with {}", + prompt.msg, prompt.detail, prompt.answers, response + ) + }; + prompt.tx.send(ix).ok(); } pub(crate) fn has_pending_prompt(&self) -> bool { !self.prompts.borrow().multiple_choice.is_empty() } + pub(crate) fn pending_prompt(&self) -> Option<(String, String)> { + let prompts = self.prompts.borrow(); + let prompt = prompts.multiple_choice.front()?; + Some(( + prompt.msg.clone(), + prompt.detail.clone().unwrap_or_default(), + )) + } + pub(crate) fn set_screen_capture_sources(&self, sources: Vec) { *self.screen_capture_sources.borrow_mut() = sources; } - pub(crate) fn prompt(&self, msg: &str, detail: Option<&str>) -> oneshot::Receiver { + pub(crate) fn prompt( + &self, + msg: &str, + detail: Option<&str>, + answers: &[&str], + ) -> oneshot::Receiver { let (tx, rx) = oneshot::channel(); + let answers: Vec = answers.iter().map(|&s| s.to_string()).collect(); self.background_executor() .set_waiting_hint(Some(format!("PROMPT: {:?} {:?}", msg, detail))); - self.prompts.borrow_mut().multiple_choice.push_back(tx); + self.prompts + .borrow_mut() + .multiple_choice + .push_back(TestPrompt { + msg: msg.to_string(), + detail: detail.map(|s| s.to_string()), + answers: answers.clone(), + tx, + }); rx } @@ -292,6 +330,8 @@ impl Platform for TestPlatform { directory: &std::path::Path, ) -> oneshot::Receiver>> { let (tx, rx) = oneshot::channel(); + self.background_executor() + .set_waiting_hint(Some(format!("PROMPT FOR PATH: {:?}", directory))); self.prompts .borrow_mut() .new_path diff --git a/crates/gpui/src/platform/test/window.rs b/crates/gpui/src/platform/test/window.rs index 89aab79c1d..d950919ea7 100644 --- a/crates/gpui/src/platform/test/window.rs +++ b/crates/gpui/src/platform/test/window.rs @@ -159,7 +159,7 @@ impl PlatformWindow for TestWindow { _level: crate::PromptLevel, msg: &str, detail: Option<&str>, - _answers: &[&str], + answers: &[&str], ) -> Option> { Some( self.0 @@ -167,7 +167,7 @@ impl PlatformWindow for TestWindow { .platform .upgrade() .expect("platform dropped") - .prompt(msg, detail), + .prompt(msg, detail, answers), ) } diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index cf83d7b726..6dec701f0b 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -9551,7 +9551,7 @@ mod tests { cx.has_pending_prompt(), "Should have a prompt after the deletion" ); - cx.simulate_prompt_answer(0); + cx.simulate_prompt_answer("Delete"); assert!( !cx.has_pending_prompt(), "Should have no prompts after prompt was replied to" diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index 6dc394b2f2..41b0e3a8e2 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/crates/recent_projects/src/recent_projects.rs @@ -694,8 +694,7 @@ mod tests { cx.has_pending_prompt(), "Dirty workspace should prompt before opening the new recent project" ); - // Cancel - cx.simulate_prompt_answer(0); + cx.simulate_prompt_answer("Cancel"); assert!( !cx.has_pending_prompt(), "Should have no pending prompt after cancelling" diff --git a/crates/vim/src/command.rs b/crates/vim/src/command.rs index 76c7b464e0..5f14890540 100644 --- a/crates/vim/src/command.rs +++ b/crates/vim/src/command.rs @@ -1695,12 +1695,10 @@ mod test { // conflict! cx.simulate_keystrokes("i @ escape"); cx.simulate_keystrokes(": w enter"); - assert!(cx.has_pending_prompt()); - // "Cancel" - cx.simulate_prompt_answer(0); + cx.simulate_prompt_answer("Cancel"); + assert_eq!(fs.load(path).await.unwrap().replace("\r\n", "\n"), "oops\n"); assert!(!cx.has_pending_prompt()); - // force overwrite cx.simulate_keystrokes(": w ! enter"); assert!(!cx.has_pending_prompt()); assert_eq!(fs.load(path).await.unwrap().replace("\r\n", "\n"), "@@\n"); diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index 825a7cae99..ff2c022a96 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -1257,6 +1257,19 @@ pub mod test { is_dirty: false, }) } + + pub fn new_dirty(id: u64, path: &str, cx: &mut App) -> Entity { + let entry_id = Some(ProjectEntryId::from_proto(id)); + let project_path = Some(ProjectPath { + worktree_id: WorktreeId::from_usize(0), + path: Path::new(path).into(), + }); + cx.new(|_| Self { + entry_id, + project_path, + is_dirty: true, + }) + } } impl TestItem { @@ -1460,10 +1473,17 @@ pub mod test { _: bool, _: Entity, _window: &mut Window, - _: &mut Context, + cx: &mut Context, ) -> Task> { self.save_count += 1; self.is_dirty = false; + for item in &self.project_items { + item.update(cx, |item, _| { + if item.is_dirty { + item.is_dirty = false; + } + }) + } Task::ready(Ok(())) } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 7e628accdd..7d5eda26ad 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -1453,6 +1453,40 @@ impl Pane { .for_each(|&index| self._remove_item(index, false, false, None, window, cx)); } + // Usually when you close an item that has unsaved changes, we prompt you to + // save it. That said, if you still have the buffer open in a different pane + // we can close this one without fear of losing data. + pub fn skip_save_on_close(item: &dyn ItemHandle, workspace: &Workspace, cx: &App) -> bool { + let mut dirty_project_item_ids = Vec::new(); + item.for_each_project_item(cx, &mut |project_item_id, project_item| { + if project_item.is_dirty() { + dirty_project_item_ids.push(project_item_id); + } + }); + if dirty_project_item_ids.is_empty() { + if item.is_singleton(cx) && item.is_dirty(cx) { + return false; + } + return true; + } + + for open_item in workspace.items(cx) { + if open_item.item_id() == item.item_id() { + continue; + } + if !open_item.is_singleton(cx) { + continue; + } + let other_project_item_ids = open_item.project_item_model_ids(cx); + dirty_project_item_ids.retain(|id| !other_project_item_ids.contains(id)); + } + if dirty_project_item_ids.is_empty() { + return true; + } + + false + } + pub(super) fn file_names_for_prompt( items: &mut dyn Iterator>, all_dirty_items: usize, @@ -1460,19 +1494,23 @@ impl Pane { ) -> (String, 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 + let mut file_names = Vec::new(); + let mut should_display_followup_text = false; + for (ix, item) in items.enumerate() { + item.for_each_project_item(cx, &mut |_, project_item| { + let filename = project_item.project_path(cx).and_then(|path| { + 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; + }); + file_names.push(filename.unwrap_or("untitled".to_string())); + }); + + if ix == FILE_NAMES_CUTOFF_POINT { + should_display_followup_text = true; + break; + } + } if should_display_followup_text { let not_shown_files = all_dirty_items - file_names.len(); if not_shown_files == 1 { @@ -1499,34 +1537,40 @@ impl Pane { ) -> Task> { // Find the items to close. let mut items_to_close = Vec::new(); - let mut item_ids_to_close = HashSet::default(); - let mut dirty_items = Vec::new(); for item in &self.items { if should_close(item.item_id()) { items_to_close.push(item.boxed_clone()); - item_ids_to_close.insert(item.item_id()); - if item.is_dirty(cx) { - dirty_items.push(item.boxed_clone()); - } } } let active_item_id = self.active_item().map(|item| item.item_id()); items_to_close.sort_by_key(|item| { + let path = item.project_path(cx); // Put the currently active item at the end, because if the currently active item is not closed last // closing the currently active item will cause the focus to switch to another item // This will cause Zed to expand the content of the currently active item - active_item_id.filter(|&id| id == item.item_id()).is_some() - // If a buffer is open both in a singleton editor and in a multibuffer, make sure - // to focus the singleton buffer when prompting to save that buffer, as opposed - // to focusing the multibuffer, because this gives the user a more clear idea - // of what content they would be saving. - || !item.is_singleton(cx) + // + // Beyond that sort in order of project path, with untitled files and multibuffers coming last. + (active_item_id == Some(item.item_id()), path.is_none(), path) }); let workspace = self.workspace.clone(); + let Some(project) = self.project.upgrade() else { + return Task::ready(Ok(())); + }; cx.spawn_in(window, |pane, mut cx| async move { + let dirty_items = workspace.update(&mut cx, |workspace, cx| { + items_to_close + .iter() + .filter(|item| { + item.is_dirty(cx) + && !Self::skip_save_on_close(item.as_ref(), &workspace, cx) + }) + .map(|item| item.boxed_clone()) + .collect::>() + })?; + if save_intent == SaveIntent::Close && dirty_items.len() > 1 { let answer = pane.update_in(&mut cx, |_, window, cx| { let (prompt, detail) = @@ -1542,68 +1586,33 @@ impl Pane { match answer.await { Ok(0) => save_intent = SaveIntent::SaveAll, Ok(1) => save_intent = SaveIntent::Skip, + Ok(2) => return Ok(()), _ => {} } } - let mut saved_project_items_ids = HashSet::default(); + for item_to_close in items_to_close { - // Find the item's current index and its set of dirty project item models. Avoid - // storing these in advance, in case they have changed since this task - // was started. - let mut dirty_project_item_ids = Vec::new(); - let Some(item_ix) = pane.update(&mut cx, |pane, cx| { - item_to_close.for_each_project_item( - cx, - &mut |project_item_id, project_item| { - if project_item.is_dirty() { - dirty_project_item_ids.push(project_item_id); - } - }, - ); - pane.index_for_item(&*item_to_close) - })? - else { - continue; - }; - - // Check if this view has any project items that are not open anywhere else - // in the workspace, AND that the user has not already been prompted to save. - // If there are any such project entries, prompt the user to save this item. - let project = workspace.update(&mut cx, |workspace, cx| { - for open_item in workspace.items(cx) { - let open_item_id = open_item.item_id(); - if !item_ids_to_close.contains(&open_item_id) { - let other_project_item_ids = open_item.project_item_model_ids(cx); - dirty_project_item_ids - .retain(|id| !other_project_item_ids.contains(id)); + let mut should_save = true; + if save_intent == SaveIntent::Close { + workspace.update(&mut cx, |workspace, cx| { + if Self::skip_save_on_close(item_to_close.as_ref(), &workspace, cx) { + should_save = false; } - } - workspace.project().clone() - })?; - let should_save = dirty_project_item_ids - .iter() - .any(|id| saved_project_items_ids.insert(*id)) - // Always propose to save singleton files without any project paths: those cannot be saved via multibuffer, as require a file path selection modal. - || cx - .update(|_window, cx| { - item_to_close.can_save(cx) && item_to_close.is_dirty(cx) - && item_to_close.is_singleton(cx) - && item_to_close.project_path(cx).is_none() - }) - .unwrap_or(false); + })?; + } - if should_save - && !Self::save_item( + if should_save { + if !Self::save_item( project.clone(), &pane, - item_ix, &*item_to_close, save_intent, &mut cx, ) .await? - { - break; + { + break; + } } // Remove the item from the pane. @@ -1777,7 +1786,6 @@ impl Pane { pub async fn save_item( project: Entity, pane: &WeakEntity, - item_ix: usize, item: &dyn ItemHandle, save_intent: SaveIntent, cx: &mut AsyncWindowContext, @@ -1791,6 +1799,13 @@ impl Pane { if save_intent == SaveIntent::Skip { return Ok(true); } + let Some(item_ix) = pane + .update(cx, |pane, _| pane.index_for_item(item)) + .ok() + .flatten() + else { + return Ok(true); + }; let (mut has_conflict, mut is_dirty, mut can_save, is_singleton, has_deleted_file) = cx .update(|_window, cx| { @@ -1939,6 +1954,7 @@ impl Pane { .await?; } else if can_save_as { let abs_path = pane.update_in(cx, |pane, window, cx| { + pane.activate_item(item_ix, true, true, window, cx); pane.workspace.update(cx, |workspace, cx| { workspace.prompt_for_new_path(window, cx) }) @@ -4382,15 +4398,15 @@ mod tests { add_labeled_item(&pane, "A", true, cx).update(cx, |item, cx| { item.project_items - .push(TestProjectItem::new(1, "A.txt", cx)) + .push(TestProjectItem::new_dirty(1, "A.txt", cx)) }); add_labeled_item(&pane, "B", true, cx).update(cx, |item, cx| { item.project_items - .push(TestProjectItem::new(2, "B.txt", cx)) + .push(TestProjectItem::new_dirty(2, "B.txt", cx)) }); add_labeled_item(&pane, "C", true, cx).update(cx, |item, cx| { item.project_items - .push(TestProjectItem::new(3, "C.txt", cx)) + .push(TestProjectItem::new_dirty(3, "C.txt", cx)) }); assert_item_labels(&pane, ["A^", "B^", "C*^"], cx); @@ -4408,7 +4424,7 @@ mod tests { .unwrap(); cx.executor().run_until_parked(); - cx.simulate_prompt_answer(2); + cx.simulate_prompt_answer("Save all"); save.await.unwrap(); assert_item_labels(&pane, [], cx); @@ -4430,11 +4446,55 @@ mod tests { .unwrap(); cx.executor().run_until_parked(); - cx.simulate_prompt_answer(2); + cx.simulate_prompt_answer("Discard all"); save.await.unwrap(); assert_item_labels(&pane, [], cx); } + #[gpui::test] + async fn test_close_with_save_intent(cx: &mut TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + + let project = Project::test(fs, None, cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx)); + let pane = workspace.update(cx, |workspace, _| workspace.active_pane().clone()); + + let a = cx.update(|_, cx| TestProjectItem::new_dirty(1, "A.txt", cx)); + let b = cx.update(|_, cx| TestProjectItem::new_dirty(1, "B.txt", cx)); + let c = cx.update(|_, cx| TestProjectItem::new_dirty(1, "C.txt", cx)); + + add_labeled_item(&pane, "AB", true, cx).update(cx, |item, _| { + item.project_items.push(a.clone()); + item.project_items.push(b.clone()); + }); + add_labeled_item(&pane, "C", true, cx) + .update(cx, |item, _| item.project_items.push(c.clone())); + assert_item_labels(&pane, ["AB^", "C*^"], cx); + + pane.update_in(cx, |pane, window, cx| { + pane.close_all_items( + &CloseAllItems { + save_intent: Some(SaveIntent::Save), + close_pinned: false, + }, + window, + cx, + ) + }) + .unwrap() + .await + .unwrap(); + + assert_item_labels(&pane, [], cx); + cx.update(|_, cx| { + assert!(!a.read(cx).is_dirty); + assert!(!b.read(cx).is_dirty); + assert!(!c.read(cx).is_dirty); + }); + } + #[gpui::test] async fn test_close_all_items_including_pinned(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 53bd58596c..961bd1373f 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2032,6 +2032,7 @@ impl Workspace { match answer.await.log_err() { Some(0) => save_intent = SaveIntent::SaveAll, Some(1) => save_intent = SaveIntent::Skip, + Some(2) => return Ok(false), _ => {} } } @@ -2045,21 +2046,10 @@ impl Workspace { let (singleton, project_entry_ids) = cx.update(|_, cx| (item.is_singleton(cx), item.project_entry_ids(cx)))?; if singleton || !project_entry_ids.is_empty() { - if let Some(ix) = - pane.update(&mut cx, |pane, _| pane.index_for_item(item.as_ref()))? - { - if !Pane::save_item( - project.clone(), - &pane, - ix, - &*item, - save_intent, - &mut cx, - ) + if !Pane::save_item(project.clone(), &pane, &*item, save_intent, &mut cx) .await? - { - return Ok(false); - } + { + return Ok(false); } } } @@ -2328,13 +2318,12 @@ impl Workspace { ) -> Task> { let project = self.project.clone(); let pane = self.active_pane(); - let item_ix = pane.read(cx).active_item_index(); let item = pane.read(cx).active_item(); let pane = pane.downgrade(); window.spawn(cx, |mut cx| async move { if let Some(item) = item { - Pane::save_item(project, &pane, item_ix, item.as_ref(), save_intent, &mut cx) + Pane::save_item(project, &pane, item.as_ref(), save_intent, &mut cx) .await .map(|_| ()) } else { @@ -6958,9 +6947,7 @@ mod tests { w.prepare_to_close(CloseIntent::CloseWindow, window, cx) }); cx.executor().run_until_parked(); - cx.simulate_prompt_answer(2); // cancel save all - cx.executor().run_until_parked(); - cx.simulate_prompt_answer(2); // cancel save all + cx.simulate_prompt_answer("Cancel"); // cancel save all cx.executor().run_until_parked(); assert!(!cx.has_pending_prompt()); assert!(!task.await.unwrap()); @@ -7059,16 +7046,8 @@ mod tests { cx.executor().run_until_parked(); assert!(cx.has_pending_prompt()); - // Ignore "Save all" prompt - cx.simulate_prompt_answer(2); - cx.executor().run_until_parked(); - // There's a prompt to save item 1. - pane.update(cx, |pane, _| { - assert_eq!(pane.items_len(), 4); - assert_eq!(pane.active_item().unwrap().item_id(), item1.item_id()); - }); - // Confirm saving item 1. - cx.simulate_prompt_answer(0); + cx.simulate_prompt_answer("Save all"); + cx.executor().run_until_parked(); // Item 1 is saved. There's a prompt to save item 3. @@ -7082,7 +7061,7 @@ mod tests { assert!(cx.has_pending_prompt()); // Cancel saving item 3. - cx.simulate_prompt_answer(1); + cx.simulate_prompt_answer("Discard"); cx.executor().run_until_parked(); // Item 3 is reloaded. There's a prompt to save item 4. @@ -7093,11 +7072,6 @@ mod tests { assert_eq!(pane.items_len(), 2); assert_eq!(pane.active_item().unwrap().item_id(), item4.item_id()); }); - assert!(cx.has_pending_prompt()); - - // Confirm saving item 4. - cx.simulate_prompt_answer(0); - cx.executor().run_until_parked(); // There's a prompt for a path for item 4. cx.simulate_new_path_selection(|_| Some(Default::default())); @@ -7159,68 +7133,110 @@ mod tests { // Create two panes that contain the following project entries: // left pane: // multi-entry items: (2, 3) - // single-entry items: 0, 1, 2, 3, 4 + // single-entry items: 0, 2, 3, 4 // right pane: - // single-entry items: 1 + // single-entry items: 4, 1 // multi-entry items: (3, 4) - let left_pane = workspace.update_in(cx, |workspace, window, cx| { + let (left_pane, right_pane) = workspace.update_in(cx, |workspace, window, cx| { let left_pane = workspace.active_pane().clone(); workspace.add_item_to_active_pane(Box::new(item_2_3.clone()), None, true, window, cx); - for item in single_entry_items { - workspace.add_item_to_active_pane(Box::new(item), None, true, window, cx); - } - left_pane.update(cx, |pane, cx| { - pane.activate_item(2, true, true, window, cx); - }); + workspace.add_item_to_active_pane( + single_entry_items[0].boxed_clone(), + None, + true, + window, + cx, + ); + workspace.add_item_to_active_pane( + single_entry_items[2].boxed_clone(), + None, + true, + window, + cx, + ); + workspace.add_item_to_active_pane( + single_entry_items[3].boxed_clone(), + None, + true, + window, + cx, + ); + workspace.add_item_to_active_pane( + single_entry_items[4].boxed_clone(), + None, + true, + window, + cx, + ); let right_pane = workspace .split_and_clone(left_pane.clone(), SplitDirection::Right, window, cx) .unwrap(); right_pane.update(cx, |pane, cx| { + pane.add_item( + single_entry_items[1].boxed_clone(), + true, + true, + None, + window, + cx, + ); pane.add_item(Box::new(item_3_4.clone()), true, true, None, window, cx); }); - left_pane + (left_pane, right_pane) }); - cx.focus(&left_pane); + cx.focus(&right_pane); - // When closing all of the items in the left pane, we should be prompted twice: - // once for project entry 0, and once for project entry 2. Project entries 1, - // 3, and 4 are all still open in the other paten. After those two - // prompts, the task should complete. - - let close = left_pane.update_in(cx, |pane, window, cx| { + let mut close = right_pane.update_in(cx, |pane, window, cx| { pane.close_all_items(&CloseAllItems::default(), window, cx) .unwrap() }); cx.executor().run_until_parked(); - // Discard "Save all" prompt - cx.simulate_prompt_answer(2); + let msg = cx.pending_prompt().unwrap().0; + assert!(msg.contains("1.txt")); + assert!(!msg.contains("2.txt")); + assert!(!msg.contains("3.txt")); + assert!(!msg.contains("4.txt")); - cx.executor().run_until_parked(); - left_pane.update(cx, |pane, cx| { - assert_eq!( - pane.active_item().unwrap().project_entry_ids(cx).as_slice(), - &[ProjectEntryId::from_proto(0)] - ); - }); - cx.simulate_prompt_answer(0); + cx.simulate_prompt_answer("Cancel"); + close.await.unwrap(); - cx.executor().run_until_parked(); - left_pane.update(cx, |pane, cx| { - assert_eq!( - pane.active_item().unwrap().project_entry_ids(cx).as_slice(), - &[ProjectEntryId::from_proto(2)] - ); + left_pane + .update_in(cx, |left_pane, window, cx| { + left_pane.close_item_by_id( + single_entry_items[3].entity_id(), + SaveIntent::Skip, + window, + cx, + ) + }) + .await + .unwrap(); + + close = right_pane.update_in(cx, |pane, window, cx| { + pane.close_all_items(&CloseAllItems::default(), window, cx) + .unwrap() }); - cx.simulate_prompt_answer(0); + cx.executor().run_until_parked(); + + let details = cx.pending_prompt().unwrap().1; + assert!(details.contains("1.txt")); + assert!(!details.contains("2.txt")); + assert!(details.contains("3.txt")); + // ideally this assertion could be made, but today we can only + // save whole items not project items, so the orphaned item 3 causes + // 4 to be saved too. + // assert!(!details.contains("4.txt")); + + cx.simulate_prompt_answer("Save all"); cx.executor().run_until_parked(); close.await.unwrap(); - left_pane.update(cx, |pane, _| { + right_pane.update(cx, |pane, _| { assert_eq!(pane.items_len(), 0); }); } @@ -8158,17 +8174,14 @@ mod tests { }) .expect("should have inactive files to close"); cx.background_executor.run_until_parked(); - assert!( - !cx.has_pending_prompt(), - "Multi buffer still has the unsaved buffer inside, so no save prompt should be shown" - ); + assert!(!cx.has_pending_prompt()); close_all_but_multi_buffer_task .await .expect("Closing all buffers but the multi buffer failed"); pane.update(cx, |pane, cx| { - assert_eq!(dirty_regular_buffer.read(cx).save_count, 0); + assert_eq!(dirty_regular_buffer.read(cx).save_count, 1); assert_eq!(dirty_multi_buffer_with_both.read(cx).save_count, 0); - assert_eq!(dirty_regular_buffer_2.read(cx).save_count, 0); + assert_eq!(dirty_regular_buffer_2.read(cx).save_count, 1); assert_eq!(pane.items_len(), 1); assert_eq!( pane.active_item().unwrap().item_id(), @@ -8181,6 +8194,10 @@ mod tests { ); }); + dirty_regular_buffer.update(cx, |buffer, cx| { + buffer.project_items[0].update(cx, |pi, _| pi.is_dirty = true) + }); + let close_multi_buffer_task = pane .update_in(cx, |pane, window, cx| { pane.close_active_item( @@ -8198,7 +8215,7 @@ mod tests { cx.has_pending_prompt(), "Dirty multi buffer should prompt a save dialog" ); - cx.simulate_prompt_answer(0); + cx.simulate_prompt_answer("Save"); cx.background_executor.run_until_parked(); close_multi_buffer_task .await @@ -8219,118 +8236,6 @@ mod tests { }); } - #[gpui::test] - async fn test_no_save_prompt_when_dirty_singleton_buffer_closed_with_a_multi_buffer_containing_it_present_in_the_pane( - cx: &mut TestAppContext, - ) { - init_test(cx); - - let fs = FakeFs::new(cx.background_executor.clone()); - let project = Project::test(fs, [], cx).await; - let (workspace, cx) = - cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx)); - let pane = workspace.update(cx, |workspace, _| workspace.active_pane().clone()); - - let dirty_regular_buffer = cx.new(|cx| { - TestItem::new(cx) - .with_dirty(true) - .with_label("1.txt") - .with_project_items(&[dirty_project_item(1, "1.txt", cx)]) - }); - let dirty_regular_buffer_2 = cx.new(|cx| { - TestItem::new(cx) - .with_dirty(true) - .with_label("2.txt") - .with_project_items(&[dirty_project_item(2, "2.txt", cx)]) - }); - let clear_regular_buffer = cx.new(|cx| { - TestItem::new(cx) - .with_label("3.txt") - .with_project_items(&[TestProjectItem::new(3, "3.txt", cx)]) - }); - - let dirty_multi_buffer_with_both = cx.new(|cx| { - TestItem::new(cx) - .with_dirty(true) - .with_singleton(false) - .with_label("Fake Project Search") - .with_project_items(&[ - dirty_regular_buffer.read(cx).project_items[0].clone(), - dirty_regular_buffer_2.read(cx).project_items[0].clone(), - clear_regular_buffer.read(cx).project_items[0].clone(), - ]) - }); - workspace.update_in(cx, |workspace, window, cx| { - workspace.add_item( - pane.clone(), - Box::new(dirty_regular_buffer.clone()), - None, - false, - false, - window, - cx, - ); - workspace.add_item( - pane.clone(), - Box::new(dirty_multi_buffer_with_both.clone()), - None, - false, - false, - window, - cx, - ); - }); - - pane.update_in(cx, |pane, window, cx| { - pane.activate_item(0, true, true, window, cx); - assert_eq!( - pane.active_item().unwrap().item_id(), - dirty_regular_buffer.item_id(), - "Should select the dirty singleton buffer in the pane" - ); - }); - let close_singleton_buffer_task = pane - .update_in(cx, |pane, window, cx| { - pane.close_active_item( - &CloseActiveItem { - save_intent: None, - close_pinned: false, - }, - window, - cx, - ) - }) - .expect("should have active singleton buffer to close"); - cx.background_executor.run_until_parked(); - assert!( - !cx.has_pending_prompt(), - "Multi buffer is still in the pane and has the unsaved buffer inside, so no save prompt should be shown" - ); - - close_singleton_buffer_task - .await - .expect("Should not fail closing the singleton buffer"); - pane.update(cx, |pane, cx| { - assert_eq!(dirty_regular_buffer.read(cx).save_count, 0); - assert_eq!( - dirty_multi_buffer_with_both.read(cx).save_count, - 0, - "Multi buffer itself should not be saved" - ); - assert_eq!(dirty_regular_buffer_2.read(cx).save_count, 0); - assert_eq!( - pane.items_len(), - 1, - "A dirty multi buffer should be present in the pane" - ); - assert_eq!( - pane.active_item().unwrap().item_id(), - dirty_multi_buffer_with_both.item_id(), - "Should activate the only remaining item in the pane" - ); - }); - } - #[gpui::test] async fn test_save_prompt_when_dirty_multi_buffer_closed_with_some_of_its_dirty_items_not_present_in_the_pane( cx: &mut TestAppContext, diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 9fd7499323..6295947c2c 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -2061,7 +2061,7 @@ mod tests { .unwrap(); executor.run_until_parked(); - cx.simulate_prompt_answer(1); + cx.simulate_prompt_answer("Don't Save"); close.await.unwrap(); assert!(!window_is_edited(window, cx)); @@ -2122,7 +2122,7 @@ mod tests { assert_eq!(cx.update(|cx| cx.windows().len()), 1); // The window is successfully closed after the user dismisses the prompt. - cx.simulate_prompt_answer(1); + cx.simulate_prompt_answer("Don't Save"); executor.run_until_parked(); assert_eq!(cx.update(|cx| cx.windows().len()), 0); } @@ -2857,7 +2857,7 @@ mod tests { }) .unwrap(); cx.background_executor.run_until_parked(); - cx.simulate_prompt_answer(0); + cx.simulate_prompt_answer("Overwrite"); save_task.await.unwrap(); window .update(cx, |_, _, cx| { @@ -3156,7 +3156,7 @@ mod tests { }, ); cx.background_executor.run_until_parked(); - cx.simulate_prompt_answer(1); + cx.simulate_prompt_answer("Don't Save"); cx.background_executor.run_until_parked(); window