Merge pull request #740 from zed-industries/prompt-only-on-last-dirty-item

Nathan Sobo created

Show unsaved/conflict prompt only when closing the last tab for an item

Change summary

crates/file_finder/src/file_finder.rs |  15 
crates/gpui/src/app.rs                |  89 +++++-
crates/workspace/src/pane.rs          | 391 +++++++++++++++++-----------
crates/workspace/src/workspace.rs     |   7 
crates/zed/src/zed.rs                 |  59 ++-
5 files changed, 346 insertions(+), 215 deletions(-)

Detailed changes

crates/file_finder/src/file_finder.rs 🔗

@@ -446,7 +446,7 @@ mod tests {
             .unwrap();
         cx.read(|cx| workspace.read(cx).worktree_scans_complete(cx))
             .await;
-        cx.dispatch_action(window_id, vec![workspace.id()], Toggle);
+        cx.dispatch_action(window_id, Toggle);
 
         let finder = cx.read(|cx| {
             workspace
@@ -457,19 +457,16 @@ mod tests {
                 .downcast::<FileFinder>()
                 .unwrap()
         });
-        let query_buffer = cx.read(|cx| finder.read(cx).query_editor.clone());
-
-        let chain = vec![finder.id(), query_buffer.id()];
-        cx.dispatch_action(window_id, chain.clone(), Input("b".into()));
-        cx.dispatch_action(window_id, chain.clone(), Input("n".into()));
-        cx.dispatch_action(window_id, chain.clone(), Input("a".into()));
+        cx.dispatch_action(window_id, Input("b".into()));
+        cx.dispatch_action(window_id, Input("n".into()));
+        cx.dispatch_action(window_id, Input("a".into()));
         finder
             .condition(&cx, |finder, _| finder.matches.len() == 2)
             .await;
 
         let active_pane = cx.read(|cx| workspace.read(cx).active_pane().clone());
-        cx.dispatch_action(window_id, vec![workspace.id(), finder.id()], SelectNext);
-        cx.dispatch_action(window_id, vec![workspace.id(), finder.id()], Confirm);
+        cx.dispatch_action(window_id, SelectNext);
+        cx.dispatch_action(window_id, Confirm);
         active_pane
             .condition(&cx, |pane, _| pane.active_item().is_some())
             .await;

crates/gpui/src/app.rs 🔗

@@ -426,15 +426,17 @@ impl TestAppContext {
         cx
     }
 
-    pub fn dispatch_action<A: Action>(
-        &self,
-        window_id: usize,
-        responder_chain: Vec<usize>,
-        action: A,
-    ) {
-        self.cx
-            .borrow_mut()
-            .dispatch_action_any(window_id, &responder_chain, &action);
+    pub fn dispatch_action<A: Action>(&self, window_id: usize, action: A) {
+        let mut cx = self.cx.borrow_mut();
+        let dispatch_path = cx
+            .presenters_and_platform_windows
+            .get(&window_id)
+            .unwrap()
+            .0
+            .borrow()
+            .dispatch_path(cx.as_ref());
+
+        cx.dispatch_action_any(window_id, &dispatch_path, &action);
     }
 
     pub fn dispatch_global_action<A: Action>(&self, action: A) {
@@ -455,9 +457,9 @@ impl TestAppContext {
                 .unwrap()
                 .0
                 .clone();
-            let responder_chain = presenter.borrow().dispatch_path(cx.as_ref());
+            let dispatch_path = presenter.borrow().dispatch_path(cx.as_ref());
 
-            if !cx.dispatch_keystroke(window_id, responder_chain, &keystroke) {
+            if !cx.dispatch_keystroke(window_id, dispatch_path, &keystroke) {
                 presenter.borrow_mut().dispatch_event(
                     Event::KeyDown {
                         keystroke,
@@ -595,6 +597,15 @@ impl TestAppContext {
     pub fn leak_detector(&self) -> Arc<Mutex<LeakDetector>> {
         self.cx.borrow().leak_detector()
     }
+
+    #[cfg(any(test, feature = "test-support"))]
+    pub fn assert_dropped(&self, handle: impl WeakHandle) {
+        self.cx
+            .borrow()
+            .leak_detector()
+            .lock()
+            .assert_dropped(handle.id())
+    }
 }
 
 impl AsyncAppContext {
@@ -1314,10 +1325,10 @@ impl MutableAppContext {
     pub fn dispatch_action<A: Action>(
         &mut self,
         window_id: usize,
-        responder_chain: Vec<usize>,
+        dispatch_path: Vec<usize>,
         action: &A,
     ) {
-        self.dispatch_action_any(window_id, &responder_chain, action);
+        self.dispatch_action_any(window_id, &dispatch_path, action);
     }
 
     pub(crate) fn dispatch_action_any(
@@ -1403,11 +1414,11 @@ impl MutableAppContext {
     pub fn dispatch_keystroke(
         &mut self,
         window_id: usize,
-        responder_chain: Vec<usize>,
+        dispatch_path: Vec<usize>,
         keystroke: &Keystroke,
     ) -> bool {
         let mut context_chain = Vec::new();
-        for view_id in &responder_chain {
+        for view_id in &dispatch_path {
             let view = self
                 .cx
                 .views
@@ -1420,13 +1431,12 @@ impl MutableAppContext {
         for (i, cx) in context_chain.iter().enumerate().rev() {
             match self
                 .keystroke_matcher
-                .push_keystroke(keystroke.clone(), responder_chain[i], cx)
+                .push_keystroke(keystroke.clone(), dispatch_path[i], cx)
             {
                 MatchResult::None => {}
                 MatchResult::Pending => pending = true,
                 MatchResult::Action(action) => {
-                    if self.dispatch_action_any(window_id, &responder_chain[0..=i], action.as_ref())
-                    {
+                    if self.dispatch_action_any(window_id, &dispatch_path[0..=i], action.as_ref()) {
                         self.keystroke_matcher.clear_pending();
                         return true;
                     }
@@ -3301,6 +3311,10 @@ pub trait Handle<T> {
         Self: Sized;
 }
 
+pub trait WeakHandle {
+    fn id(&self) -> usize;
+}
+
 #[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
 pub enum EntityLocation {
     Model(usize),
@@ -3575,6 +3589,12 @@ pub struct WeakModelHandle<T> {
     model_type: PhantomData<T>,
 }
 
+impl<T> WeakHandle for WeakModelHandle<T> {
+    fn id(&self) -> usize {
+        self.model_id
+    }
+}
+
 unsafe impl<T> Send for WeakModelHandle<T> {}
 unsafe impl<T> Sync for WeakModelHandle<T> {}
 
@@ -4144,6 +4164,12 @@ pub struct WeakViewHandle<T> {
     view_type: PhantomData<T>,
 }
 
+impl<T> WeakHandle for WeakViewHandle<T> {
+    fn id(&self) -> usize {
+        self.view_id
+    }
+}
+
 impl<T: View> WeakViewHandle<T> {
     fn new(window_id: usize, view_id: usize) -> Self {
         Self {
@@ -4470,11 +4496,36 @@ impl LeakDetector {
         }
     }
 
+    pub fn assert_dropped(&mut self, entity_id: usize) {
+        if let Some((type_name, backtraces)) = self.handle_backtraces.get_mut(&entity_id) {
+            for trace in backtraces.values_mut() {
+                if let Some(trace) = trace {
+                    trace.resolve();
+                    eprintln!("{:?}", crate::util::CwdBacktrace(trace));
+                }
+            }
+
+            let hint = if *LEAK_BACKTRACE {
+                ""
+            } else {
+                " – set LEAK_BACKTRACE=1 for more information"
+            };
+
+            panic!(
+                "{} handles to {} {} still exist{}",
+                backtraces.len(),
+                type_name.unwrap_or("entity"),
+                entity_id,
+                hint
+            );
+        }
+    }
+
     pub fn detect(&mut self) {
         let mut found_leaks = false;
         for (id, (type_name, backtraces)) in self.handle_backtraces.iter_mut() {
             eprintln!(
-                "leaked {} handles to {:?} {}",
+                "leaked {} handles to {} {}",
                 backtraces.len(),
                 type_name.unwrap_or("entity"),
                 id

crates/workspace/src/pane.rs 🔗

@@ -1,5 +1,6 @@
 use super::{ItemHandle, SplitDirection};
 use crate::{toolbar::Toolbar, Item, Settings, WeakItemHandle, Workspace};
+use anyhow::Result;
 use collections::{HashMap, VecDeque};
 use futures::StreamExt;
 use gpui::{
@@ -8,10 +9,10 @@ use gpui::{
     geometry::{rect::RectF, vector::vec2f},
     keymap::Binding,
     platform::{CursorStyle, NavigationDirection},
-    AppContext, Entity, ModelHandle, MutableAppContext, PromptLevel, Quad, RenderContext, Task,
-    View, ViewContext, ViewHandle, WeakViewHandle,
+    AppContext, Entity, MutableAppContext, PromptLevel, Quad, RenderContext, Task, View,
+    ViewContext, ViewHandle, WeakViewHandle,
 };
-use project::{Project, ProjectEntryId, ProjectPath};
+use project::{ProjectEntryId, ProjectPath};
 use std::{any::Any, cell::RefCell, cmp, mem, path::Path, rc::Rc};
 use util::ResultExt;
 
@@ -21,10 +22,16 @@ action!(ActivatePrevItem);
 action!(ActivateNextItem);
 action!(CloseActiveItem);
 action!(CloseInactiveItems);
-action!(CloseItem, usize);
+action!(CloseItem, CloseItemParams);
 action!(GoBack, Option<WeakViewHandle<Pane>>);
 action!(GoForward, Option<WeakViewHandle<Pane>>);
 
+#[derive(Clone)]
+pub struct CloseItemParams {
+    pub item_id: usize,
+    pub pane: WeakViewHandle<Pane>,
+}
+
 const MAX_NAVIGATION_HISTORY_LEN: usize = 1024;
 
 pub fn init(cx: &mut MutableAppContext) {
@@ -37,14 +44,11 @@ pub fn init(cx: &mut MutableAppContext) {
     cx.add_action(|pane: &mut Pane, _: &ActivateNextItem, cx| {
         pane.activate_next_item(cx);
     });
-    cx.add_action(|pane: &mut Pane, _: &CloseActiveItem, cx| {
-        pane.close_active_item(cx).detach();
-    });
-    cx.add_action(|pane: &mut Pane, _: &CloseInactiveItems, cx| {
-        pane.close_inactive_items(cx).detach();
-    });
-    cx.add_action(|pane: &mut Pane, action: &CloseItem, cx| {
-        pane.close_item(action.0, cx).detach();
+    cx.add_async_action(Pane::close_active_item);
+    cx.add_async_action(Pane::close_inactive_items);
+    cx.add_async_action(|workspace: &mut Workspace, action: &CloseItem, cx| {
+        let pane = action.0.pane.upgrade(cx)?;
+        Some(Pane::close_item(workspace, pane, action.0.item_id, cx))
     });
     cx.add_action(|pane: &mut Pane, action: &Split, cx| {
         pane.split(action.0, cx);
@@ -98,7 +102,6 @@ pub struct Pane {
     active_item_index: usize,
     nav_history: Rc<RefCell<NavHistory>>,
     toolbar: ViewHandle<Toolbar>,
-    project: ModelHandle<Project>,
 }
 
 pub struct ItemNavHistory {
@@ -134,13 +137,12 @@ pub struct NavigationEntry {
 }
 
 impl Pane {
-    pub fn new(project: ModelHandle<Project>, cx: &mut ViewContext<Self>) -> Self {
+    pub fn new(cx: &mut ViewContext<Self>) -> Self {
         Self {
             items: Vec::new(),
             active_item_index: 0,
             nav_history: Default::default(),
             toolbar: cx.add_view(|_| Toolbar::new()),
-            project,
         }
     }
 
@@ -410,162 +412,183 @@ impl Pane {
         self.activate_item(index, true, cx);
     }
 
-    pub fn close_active_item(&mut self, cx: &mut ViewContext<Self>) -> Task<()> {
-        if self.items.is_empty() {
-            Task::ready(())
+    fn close_active_item(
+        workspace: &mut Workspace,
+        _: &CloseActiveItem,
+        cx: &mut ViewContext<Workspace>,
+    ) -> Option<Task<Result<()>>> {
+        let pane_handle = workspace.active_pane().clone();
+        let pane = pane_handle.read(cx);
+        if pane.items.is_empty() {
+            None
         } else {
-            self.close_item(self.items[self.active_item_index].id(), cx)
+            let item_id_to_close = pane.items[pane.active_item_index].id();
+            Some(Self::close_items(
+                workspace,
+                pane_handle,
+                cx,
+                move |item_id| item_id == item_id_to_close,
+            ))
         }
     }
 
-    pub fn close_inactive_items(&mut self, cx: &mut ViewContext<Self>) -> Task<()> {
-        if self.items.is_empty() {
-            Task::ready(())
+    pub fn close_inactive_items(
+        workspace: &mut Workspace,
+        _: &CloseInactiveItems,
+        cx: &mut ViewContext<Workspace>,
+    ) -> Option<Task<Result<()>>> {
+        let pane_handle = workspace.active_pane().clone();
+        let pane = pane_handle.read(cx);
+        if pane.items.is_empty() {
+            None
         } else {
-            let active_item_id = self.items[self.active_item_index].id();
-            self.close_items(cx, move |id| id != active_item_id)
+            let active_item_id = pane.items[pane.active_item_index].id();
+            Some(Self::close_items(workspace, pane_handle, cx, move |id| {
+                id != active_item_id
+            }))
         }
     }
 
-    pub fn close_item(&mut self, view_id_to_close: usize, cx: &mut ViewContext<Self>) -> Task<()> {
-        self.close_items(cx, move |view_id| view_id == view_id_to_close)
+    pub fn close_item(
+        workspace: &mut Workspace,
+        pane: ViewHandle<Pane>,
+        item_id_to_close: usize,
+        cx: &mut ViewContext<Workspace>,
+    ) -> Task<Result<()>> {
+        Self::close_items(workspace, pane, cx, move |view_id| {
+            view_id == item_id_to_close
+        })
     }
 
     pub fn close_items(
-        &mut self,
-        cx: &mut ViewContext<Self>,
+        workspace: &mut Workspace,
+        pane: ViewHandle<Pane>,
+        cx: &mut ViewContext<Workspace>,
         should_close: impl 'static + Fn(usize) -> bool,
-    ) -> Task<()> {
+    ) -> Task<Result<()>> {
         const CONFLICT_MESSAGE: &'static str = "This file has changed on disk since you started editing it. Do you want to overwrite it?";
         const DIRTY_MESSAGE: &'static str =
             "This file contains unsaved edits. Do you want to save it?";
 
-        let project = self.project.clone();
-        cx.spawn(|this, mut cx| async move {
-            while let Some(item_to_close_ix) = this.read_with(&cx, |this, _| {
-                this.items.iter().position(|item| should_close(item.id()))
+        let project = workspace.project().clone();
+        cx.spawn(|workspace, mut cx| async move {
+            while let Some(item_to_close_ix) = pane.read_with(&cx, |pane, _| {
+                pane.items.iter().position(|item| should_close(item.id()))
             }) {
                 let item =
-                    this.read_with(&cx, |this, _| this.items[item_to_close_ix].boxed_clone());
-                if cx.read(|cx| item.is_dirty(cx)) {
-                    if cx.read(|cx| item.can_save(cx)) {
-                        let mut answer = this.update(&mut cx, |this, cx| {
-                            this.activate_item(item_to_close_ix, true, cx);
+                    pane.read_with(&cx, |pane, _| pane.items[item_to_close_ix].boxed_clone());
+
+                let is_last_item_for_entry = workspace.read_with(&cx, |workspace, cx| {
+                    let project_entry_id = item.project_entry_id(cx);
+                    project_entry_id.is_none()
+                        || workspace
+                            .items(cx)
+                            .filter(|item| item.project_entry_id(cx) == project_entry_id)
+                            .count()
+                            == 1
+                });
+
+                if is_last_item_for_entry {
+                    if cx.read(|cx| item.has_conflict(cx) && item.can_save(cx)) {
+                        let mut answer = pane.update(&mut cx, |pane, cx| {
+                            pane.activate_item(item_to_close_ix, true, cx);
                             cx.prompt(
                                 PromptLevel::Warning,
-                                DIRTY_MESSAGE,
-                                &["Save", "Don't Save", "Cancel"],
+                                CONFLICT_MESSAGE,
+                                &["Overwrite", "Discard", "Cancel"],
                             )
                         });
 
                         match answer.next().await {
                             Some(0) => {
-                                if cx
-                                    .update(|cx| item.save(project.clone(), cx))
-                                    .await
-                                    .log_err()
-                                    .is_none()
-                                {
-                                    break;
-                                }
+                                cx.update(|cx| item.save(project.clone(), cx)).await?;
+                            }
+                            Some(1) => {
+                                cx.update(|cx| item.reload(project.clone(), cx)).await?;
                             }
-                            Some(1) => {}
                             _ => break,
                         }
-                    } else if cx.read(|cx| item.can_save_as(cx)) {
-                        let mut answer = this.update(&mut cx, |this, cx| {
-                            this.activate_item(item_to_close_ix, true, cx);
-                            cx.prompt(
-                                PromptLevel::Warning,
-                                DIRTY_MESSAGE,
-                                &["Save", "Don't Save", "Cancel"],
-                            )
-                        });
+                    } else if cx.read(|cx| item.is_dirty(cx)) {
+                        if cx.read(|cx| item.can_save(cx)) {
+                            let mut answer = pane.update(&mut cx, |pane, cx| {
+                                pane.activate_item(item_to_close_ix, true, cx);
+                                cx.prompt(
+                                    PromptLevel::Warning,
+                                    DIRTY_MESSAGE,
+                                    &["Save", "Don't Save", "Cancel"],
+                                )
+                            });
 
-                        match answer.next().await {
-                            Some(0) => {
-                                let start_abs_path = project
-                                    .read_with(&cx, |project, cx| {
-                                        let worktree = project.visible_worktrees(cx).next()?;
-                                        Some(worktree.read(cx).as_local()?.abs_path().to_path_buf())
-                                    })
-                                    .unwrap_or(Path::new("").into());
-
-                                let mut abs_path =
-                                    cx.update(|cx| cx.prompt_for_new_path(&start_abs_path));
-                                if let Some(abs_path) = abs_path.next().await.flatten() {
-                                    if cx
-                                        .update(|cx| item.save_as(project.clone(), abs_path, cx))
-                                        .await
-                                        .log_err()
-                                        .is_none()
-                                    {
+                            match answer.next().await {
+                                Some(0) => {
+                                    cx.update(|cx| item.save(project.clone(), cx)).await?;
+                                }
+                                Some(1) => {}
+                                _ => break,
+                            }
+                        } else if cx.read(|cx| item.can_save_as(cx)) {
+                            let mut answer = pane.update(&mut cx, |pane, cx| {
+                                pane.activate_item(item_to_close_ix, true, cx);
+                                cx.prompt(
+                                    PromptLevel::Warning,
+                                    DIRTY_MESSAGE,
+                                    &["Save", "Don't Save", "Cancel"],
+                                )
+                            });
+
+                            match answer.next().await {
+                                Some(0) => {
+                                    let start_abs_path = project
+                                        .read_with(&cx, |project, cx| {
+                                            let worktree = project.visible_worktrees(cx).next()?;
+                                            Some(
+                                                worktree
+                                                    .read(cx)
+                                                    .as_local()?
+                                                    .abs_path()
+                                                    .to_path_buf(),
+                                            )
+                                        })
+                                        .unwrap_or(Path::new("").into());
+
+                                    let mut abs_path =
+                                        cx.update(|cx| cx.prompt_for_new_path(&start_abs_path));
+                                    if let Some(abs_path) = abs_path.next().await.flatten() {
+                                        cx.update(|cx| item.save_as(project.clone(), abs_path, cx))
+                                            .await?;
+                                    } else {
                                         break;
                                     }
-                                } else {
-                                    break;
                                 }
+                                Some(1) => {}
+                                _ => break,
                             }
-                            Some(1) => {}
-                            _ => break,
                         }
                     }
-                } else if cx.read(|cx| item.has_conflict(cx) && item.can_save(cx)) {
-                    let mut answer = this.update(&mut cx, |this, cx| {
-                        this.activate_item(item_to_close_ix, true, cx);
-                        cx.prompt(
-                            PromptLevel::Warning,
-                            CONFLICT_MESSAGE,
-                            &["Overwrite", "Discard", "Cancel"],
-                        )
-                    });
-
-                    match answer.next().await {
-                        Some(0) => {
-                            if cx
-                                .update(|cx| item.save(project.clone(), cx))
-                                .await
-                                .log_err()
-                                .is_none()
-                            {
-                                break;
-                            }
-                        }
-                        Some(1) => {
-                            if cx
-                                .update(|cx| item.reload(project.clone(), cx))
-                                .await
-                                .log_err()
-                                .is_none()
-                            {
-                                break;
-                            }
-                        }
-                        _ => break,
-                    }
                 }
 
-                this.update(&mut cx, |this, cx| {
-                    if let Some(item_ix) = this.items.iter().position(|i| i.id() == item.id()) {
-                        if item_ix == this.active_item_index {
-                            if item_ix + 1 < this.items.len() {
-                                this.activate_next_item(cx);
+                pane.update(&mut cx, |pane, cx| {
+                    if let Some(item_ix) = pane.items.iter().position(|i| i.id() == item.id()) {
+                        if item_ix == pane.active_item_index {
+                            if item_ix + 1 < pane.items.len() {
+                                pane.activate_next_item(cx);
                             } else if item_ix > 0 {
-                                this.activate_prev_item(cx);
+                                pane.activate_prev_item(cx);
                             }
                         }
 
-                        let item = this.items.remove(item_ix);
-                        if this.items.is_empty() {
+                        let item = pane.items.remove(item_ix);
+                        if pane.items.is_empty() {
                             item.deactivated(cx);
+                            pane.update_toolbar(cx);
                             cx.emit(Event::Remove);
                         }
 
-                        if item_ix < this.active_item_index {
-                            this.active_item_index -= 1;
+                        if item_ix < pane.active_item_index {
+                            pane.active_item_index -= 1;
                         }
 
-                        let mut nav_history = this.nav_history.borrow_mut();
+                        let mut nav_history = pane.nav_history.borrow_mut();
                         if let Some(path) = item.project_path(cx) {
                             nav_history.paths_by_item.insert(item.id(), path);
                         } else {
@@ -575,7 +598,8 @@ impl Pane {
                 });
             }
 
-            this.update(&mut cx, |_, cx| cx.notify());
+            pane.update(&mut cx, |_, cx| cx.notify());
+            Ok(())
         })
     }
 
@@ -607,6 +631,7 @@ impl Pane {
         let theme = cx.global::<Settings>().theme.clone();
 
         enum Tabs {}
+        let pane = cx.handle();
         let tabs = MouseEventHandler::new::<Tabs, _, _>(0, cx, |mouse_state, cx| {
             let mut row = Flex::row();
             for (ix, item) in self.items.iter().enumerate() {
@@ -698,8 +723,14 @@ impl Pane {
                                             )
                                             .with_padding(Padding::uniform(4.))
                                             .with_cursor_style(CursorStyle::PointingHand)
-                                            .on_click(move |cx| {
-                                                cx.dispatch_action(CloseItem(item_id))
+                                            .on_click({
+                                                let pane = pane.clone();
+                                                move |cx| {
+                                                    cx.dispatch_action(CloseItem(CloseItemParams {
+                                                        item_id,
+                                                        pane: pane.clone(),
+                                                    }))
+                                                }
                                             })
                                             .named("close-tab-icon")
                                         } else {
@@ -861,10 +892,11 @@ impl NavHistory {
 
 #[cfg(test)]
 mod tests {
-    use crate::WorkspaceParams;
-
     use super::*;
-    use gpui::TestAppContext;
+    use crate::WorkspaceParams;
+    use gpui::{ModelHandle, TestAppContext, ViewContext};
+    use project::Project;
+    use std::sync::atomic::AtomicUsize;
 
     #[gpui::test]
     async fn test_close_items(cx: &mut TestAppContext) {
@@ -874,7 +906,7 @@ mod tests {
         let (window_id, workspace) = cx.add_window(|cx| Workspace::new(&params, cx));
         let item1 = cx.add_view(window_id, |_| {
             let mut item = TestItem::new();
-            item.has_conflict = true;
+            item.is_dirty = true;
             item
         });
         let item2 = cx.add_view(window_id, |_| {
@@ -885,15 +917,11 @@ mod tests {
         });
         let item3 = cx.add_view(window_id, |_| {
             let mut item = TestItem::new();
+            item.is_dirty = true;
             item.has_conflict = true;
             item
         });
         let item4 = cx.add_view(window_id, |_| {
-            let mut item = TestItem::new();
-            item.is_dirty = true;
-            item
-        });
-        let item5 = cx.add_view(window_id, |_| {
             let mut item = TestItem::new();
             item.is_dirty = true;
             item.can_save = false;
@@ -904,26 +932,26 @@ mod tests {
             workspace.add_item(Box::new(item2.clone()), cx);
             workspace.add_item(Box::new(item3.clone()), cx);
             workspace.add_item(Box::new(item4.clone()), cx);
-            workspace.add_item(Box::new(item5.clone()), cx);
             workspace.active_pane().clone()
         });
 
-        let close_items = pane.update(cx, |pane, cx| {
-            pane.activate_item(1, true, cx);
-            assert_eq!(pane.active_item().unwrap().id(), item2.id());
+        let close_items = workspace.update(cx, |workspace, cx| {
+            pane.update(cx, |pane, cx| {
+                pane.activate_item(1, true, cx);
+                assert_eq!(pane.active_item().unwrap().id(), item2.id());
+            });
 
             let item1_id = item1.id();
             let item3_id = item3.id();
             let item4_id = item4.id();
-            let item5_id = item5.id();
-            pane.close_items(cx, move |id| {
-                [item1_id, item3_id, item4_id, item5_id].contains(&id)
+            Pane::close_items(workspace, pane.clone(), cx, move |id| {
+                [item1_id, item3_id, item4_id].contains(&id)
             })
         });
 
         cx.foreground().run_until_parked();
         pane.read_with(cx, |pane, _| {
-            assert_eq!(pane.items.len(), 5);
+            assert_eq!(pane.items.len(), 4);
             assert_eq!(pane.active_item().unwrap().id(), item1.id());
         });
 
@@ -933,7 +961,7 @@ mod tests {
             assert_eq!(item1.read(cx).save_count, 1);
             assert_eq!(item1.read(cx).save_as_count, 0);
             assert_eq!(item1.read(cx).reload_count, 0);
-            assert_eq!(pane.items.len(), 4);
+            assert_eq!(pane.items.len(), 3);
             assert_eq!(pane.active_item().unwrap().id(), item3.id());
         });
 
@@ -943,33 +971,67 @@ mod tests {
             assert_eq!(item3.read(cx).save_count, 0);
             assert_eq!(item3.read(cx).save_as_count, 0);
             assert_eq!(item3.read(cx).reload_count, 1);
-            assert_eq!(pane.items.len(), 3);
+            assert_eq!(pane.items.len(), 2);
             assert_eq!(pane.active_item().unwrap().id(), item4.id());
         });
 
         cx.simulate_prompt_answer(window_id, 0);
         cx.foreground().run_until_parked();
+        cx.simulate_new_path_selection(|_| Some(Default::default()));
+        close_items.await.unwrap();
         pane.read_with(cx, |pane, cx| {
-            assert_eq!(item4.read(cx).save_count, 1);
-            assert_eq!(item4.read(cx).save_as_count, 0);
+            assert_eq!(item4.read(cx).save_count, 0);
+            assert_eq!(item4.read(cx).save_as_count, 1);
             assert_eq!(item4.read(cx).reload_count, 0);
-            assert_eq!(pane.items.len(), 2);
-            assert_eq!(pane.active_item().unwrap().id(), item5.id());
+            assert_eq!(pane.items.len(), 1);
+            assert_eq!(pane.active_item().unwrap().id(), item2.id());
         });
+    }
 
-        cx.simulate_prompt_answer(window_id, 0);
+    #[gpui::test]
+    async fn test_prompting_only_on_last_item_for_entry(cx: &mut TestAppContext) {
+        cx.foreground().forbid_parking();
+
+        let params = cx.update(WorkspaceParams::test);
+        let (window_id, workspace) = cx.add_window(|cx| Workspace::new(&params, cx));
+        let item = cx.add_view(window_id, |_| {
+            let mut item = TestItem::new();
+            item.is_dirty = true;
+            item.project_entry_id = Some(ProjectEntryId::new(&AtomicUsize::new(1)));
+            item
+        });
+
+        let (left_pane, right_pane) = workspace.update(cx, |workspace, cx| {
+            workspace.add_item(Box::new(item.clone()), cx);
+            let left_pane = workspace.active_pane().clone();
+            let right_pane = workspace.split_pane(left_pane.clone(), SplitDirection::Right, cx);
+            (left_pane, right_pane)
+        });
+
+        workspace
+            .update(cx, |workspace, cx| {
+                let item = right_pane.read(cx).active_item().unwrap();
+                Pane::close_item(workspace, right_pane.clone(), item.id(), cx)
+            })
+            .await
+            .unwrap();
+        workspace.read_with(cx, |workspace, _| {
+            assert_eq!(workspace.panes(), [left_pane.clone()]);
+        });
+
+        let close_item = workspace.update(cx, |workspace, cx| {
+            let item = left_pane.read(cx).active_item().unwrap();
+            Pane::close_item(workspace, left_pane.clone(), item.id(), cx)
+        });
         cx.foreground().run_until_parked();
-        cx.simulate_new_path_selection(|_| Some(Default::default()));
-        close_items.await;
-        pane.read_with(cx, |pane, cx| {
-            assert_eq!(item5.read(cx).save_count, 0);
-            assert_eq!(item5.read(cx).save_as_count, 1);
-            assert_eq!(item5.read(cx).reload_count, 0);
-            assert_eq!(pane.items.len(), 1);
-            assert_eq!(pane.active_item().unwrap().id(), item2.id());
+        cx.simulate_prompt_answer(window_id, 0);
+        close_item.await.unwrap();
+        left_pane.read_with(cx, |pane, _| {
+            assert_eq!(pane.items.len(), 0);
         });
     }
 
+    #[derive(Clone)]
     struct TestItem {
         save_count: usize,
         save_as_count: usize,
@@ -977,6 +1039,7 @@ mod tests {
         is_dirty: bool,
         has_conflict: bool,
         can_save: bool,
+        project_entry_id: Option<ProjectEntryId>,
     }
 
     impl TestItem {
@@ -988,6 +1051,7 @@ mod tests {
                 is_dirty: false,
                 has_conflict: false,
                 can_save: true,
+                project_entry_id: None,
             }
         }
     }
@@ -1016,11 +1080,18 @@ mod tests {
         }
 
         fn project_entry_id(&self, _: &AppContext) -> Option<ProjectEntryId> {
-            None
+            self.project_entry_id
         }
 
         fn set_nav_history(&mut self, _: ItemNavHistory, _: &mut ViewContext<Self>) {}
 
+        fn clone_on_split(&self, _: &mut ViewContext<Self>) -> Option<Self>
+        where
+            Self: Sized,
+        {
+            Some(self.clone())
+        }
+
         fn is_dirty(&self, _: &AppContext) -> bool {
             self.is_dirty
         }

crates/workspace/src/workspace.rs 🔗

@@ -497,8 +497,7 @@ impl<T: Item> ItemHandle for ViewHandle<T> {
             }
 
             if T::should_close_item_on_event(event) {
-                pane.update(cx, |pane, cx| pane.close_item(item.id(), cx))
-                    .detach();
+                Pane::close_item(workspace, pane, item.id(), cx).detach_and_log_err(cx);
                 return;
             }
 
@@ -738,7 +737,7 @@ impl Workspace {
         })
         .detach();
 
-        let pane = cx.add_view(|cx| Pane::new(params.project.clone(), cx));
+        let pane = cx.add_view(|cx| Pane::new(cx));
         let pane_id = pane.id();
         cx.observe(&pane, move |me, _, cx| {
             let active_entry = me.active_project_path(cx);
@@ -1070,7 +1069,7 @@ impl Workspace {
     }
 
     fn add_pane(&mut self, cx: &mut ViewContext<Self>) -> ViewHandle<Pane> {
-        let pane = cx.add_view(|cx| Pane::new(self.project.clone(), cx));
+        let pane = cx.add_view(|cx| Pane::new(cx));
         let pane_id = pane.id();
         cx.observe(&pane, move |me, _, cx| {
             let active_entry = me.active_project_path(cx);

crates/zed/src/zed.rs 🔗

@@ -563,7 +563,7 @@ mod tests {
         let worktree = cx.read(|cx| workspace.read(cx).worktrees(cx).next().unwrap());
 
         // Create a new untitled buffer
-        cx.dispatch_action(window_id, vec![workspace.id()], OpenNew(app_state.clone()));
+        cx.dispatch_action(window_id, OpenNew(app_state.clone()));
         let editor = workspace.read_with(cx, |workspace, cx| {
             workspace
                 .active_item(cx)
@@ -618,7 +618,7 @@ mod tests {
 
         // Open the same newly-created file in another pane item. The new editor should reuse
         // the same buffer.
-        cx.dispatch_action(window_id, vec![workspace.id()], OpenNew(app_state.clone()));
+        cx.dispatch_action(window_id, OpenNew(app_state.clone()));
         workspace
             .update(cx, |workspace, cx| {
                 workspace.split_pane(workspace.active_pane().clone(), SplitDirection::Right, cx);
@@ -655,7 +655,7 @@ mod tests {
         let (window_id, workspace) = cx.add_window(|cx| Workspace::new(&params, cx));
 
         // Create a new untitled buffer
-        cx.dispatch_action(window_id, vec![workspace.id()], OpenNew(app_state.clone()));
+        cx.dispatch_action(window_id, OpenNew(app_state.clone()));
         let editor = workspace.read_with(cx, |workspace, cx| {
             workspace
                 .active_item(cx)
@@ -725,32 +725,47 @@ mod tests {
             .update(cx, |w, cx| w.open_path(file1.clone(), cx))
             .await
             .unwrap();
-        cx.read(|cx| {
-            assert_eq!(
-                pane_1.read(cx).active_item().unwrap().project_path(cx),
-                Some(file1.clone())
-            );
+
+        let (editor_1, buffer) = pane_1.update(cx, |pane_1, cx| {
+            let editor = pane_1.active_item().unwrap().downcast::<Editor>().unwrap();
+            assert_eq!(editor.project_path(cx), Some(file1.clone()));
+            let buffer = editor.update(cx, |editor, cx| {
+                editor.insert("dirt", cx);
+                editor.buffer().downgrade()
+            });
+            (editor.downgrade(), buffer)
         });
 
-        cx.dispatch_action(
-            window_id,
-            vec![pane_1.id()],
-            pane::Split(SplitDirection::Right),
-        );
-        cx.update(|cx| {
+        cx.dispatch_action(window_id, pane::Split(SplitDirection::Right));
+        let editor_2 = cx.update(|cx| {
             let pane_2 = workspace.read(cx).active_pane().clone();
             assert_ne!(pane_1, pane_2);
 
             let pane2_item = pane_2.read(cx).active_item().unwrap();
             assert_eq!(pane2_item.project_path(cx.as_ref()), Some(file1.clone()));
 
-            cx.dispatch_action(window_id, vec![pane_2.id()], &workspace::CloseActiveItem);
+            pane2_item.downcast::<Editor>().unwrap().downgrade()
         });
+        cx.dispatch_action(window_id, workspace::CloseActiveItem);
+
         cx.foreground().run_until_parked();
         workspace.read_with(cx, |workspace, _| {
             assert_eq!(workspace.panes().len(), 1);
             assert_eq!(workspace.active_pane(), &pane_1);
         });
+
+        cx.dispatch_action(window_id, workspace::CloseActiveItem);
+        cx.foreground().run_until_parked();
+        cx.simulate_prompt_answer(window_id, 1);
+        cx.foreground().run_until_parked();
+
+        workspace.read_with(cx, |workspace, cx| {
+            assert!(workspace.active_item(cx).is_none());
+        });
+
+        cx.assert_dropped(editor_1);
+        cx.assert_dropped(editor_2);
+        cx.assert_dropped(buffer);
     }
 
     #[gpui::test]
@@ -878,11 +893,10 @@ mod tests {
             .update(cx, |workspace, cx| {
                 let editor3_id = editor3.id();
                 drop(editor3);
-                workspace
-                    .active_pane()
-                    .update(cx, |pane, cx| pane.close_item(editor3_id, cx))
+                Pane::close_item(workspace, workspace.active_pane().clone(), editor3_id, cx)
             })
-            .await;
+            .await
+            .unwrap();
         workspace
             .update(cx, |w, cx| Pane::go_forward(w, None, cx))
             .await;
@@ -896,11 +910,10 @@ mod tests {
             .update(cx, |workspace, cx| {
                 let editor2_id = editor2.id();
                 drop(editor2);
-                workspace
-                    .active_pane()
-                    .update(cx, |pane, cx| pane.close_item(editor2_id, cx))
+                Pane::close_item(workspace, workspace.active_pane().clone(), editor2_id, cx)
             })
-            .await;
+            .await
+            .unwrap();
         app_state
             .fs
             .as_fake()