Disable close clean menu item when all are dirty (#31859)

Joseph T. Lyons created 6 months ago

This PR disables the "Close Clean" tab context menu action if all items
are dirty.

<img width="595" alt="SCR-20250601-kaev"
src="https://github.com/user-attachments/assets/add30762-b483-4701-9053-141d2dfe9b05"
/>

<img width="573" alt="SCR-20250601-kahl"
src="https://github.com/user-attachments/assets/24f260e4-01d6-48d6-a6f4-a13ae59c246e"
/>

Also did a bit more general refactoring.

Release Notes:

- N/A

Change summary

crates/workspace/src/pane.rs | 138 +++++++++++++++----------------------
1 file changed, 58 insertions(+), 80 deletions(-)

Detailed changes

crates/workspace/src/pane.rs

@@ -392,6 +392,11 @@ pub struct DraggedTab {
 
 impl EventEmitter<Event> for Pane {}
 
+pub enum Side {
+    Left,
+    Right,
+}
+
 impl Pane {
     pub fn new(
         workspace: WeakEntity<Workspace>,
@@ -1314,63 +1319,31 @@ impl Pane {
         })
     }
 
-    pub fn close_items_to_the_left(
-        &mut self,
-        action: &CloseItemsToTheLeft,
-        window: &mut Window,
-        cx: &mut Context<Self>,
-    ) -> Task<Result<()>> {
-        if self.items.is_empty() {
-            return Task::ready(Ok(()));
-        }
-
-        let active_item_id = self.active_item_id();
-        let pinned_item_ids = self.pinned_item_ids();
-
-        self.close_items_to_the_left_by_id(active_item_id, action, pinned_item_ids, window, cx)
-    }
-
     pub fn close_items_to_the_left_by_id(
         &mut self,
-        item_id: EntityId,
+        item_id: Option<EntityId>,
         action: &CloseItemsToTheLeft,
-        pinned_item_ids: HashSet<EntityId>,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Task<Result<()>> {
-        if self.items.is_empty() {
-            return Task::ready(Ok(()));
-        }
-
-        let to_the_left_item_ids = self.to_the_left_item_ids(item_id);
-
-        self.close_items(window, cx, SaveIntent::Close, move |item_id| {
-            to_the_left_item_ids.contains(&item_id)
-                && (action.close_pinned || !pinned_item_ids.contains(&item_id))
-        })
+        self.close_items_to_the_side_by_id(item_id, Side::Left, action.close_pinned, window, cx)
     }
 
-    pub fn close_items_to_the_right(
+    pub fn close_items_to_the_right_by_id(
         &mut self,
+        item_id: Option<EntityId>,
         action: &CloseItemsToTheRight,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Task<Result<()>> {
-        if self.items.is_empty() {
-            return Task::ready(Ok(()));
-        }
-
-        let active_item_id = self.active_item_id();
-        let pinned_item_ids = self.pinned_item_ids();
-
-        self.close_items_to_the_right_by_id(active_item_id, action, pinned_item_ids, window, cx)
+        self.close_items_to_the_side_by_id(item_id, Side::Right, action.close_pinned, window, cx)
     }
 
-    pub fn close_items_to_the_right_by_id(
+    pub fn close_items_to_the_side_by_id(
         &mut self,
-        item_id: EntityId,
-        action: &CloseItemsToTheRight,
-        pinned_item_ids: HashSet<EntityId>,
+        item_id: Option<EntityId>,
+        side: Side,
+        close_pinned: bool,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Task<Result<()>> {
@@ -1378,11 +1351,13 @@ impl Pane {
             return Task::ready(Ok(()));
         }
 
-        let to_the_right_item_ids = self.to_the_right_item_ids(item_id);
+        let item_id = item_id.unwrap_or_else(|| self.active_item_id());
+        let to_the_side_item_ids = self.to_the_side_item_ids(item_id, side);
+        let pinned_item_ids = self.pinned_item_ids();
 
         self.close_items(window, cx, SaveIntent::Close, move |item_id| {
-            to_the_right_item_ids.contains(&item_id)
-                && (action.close_pinned || !pinned_item_ids.contains(&item_id))
+            to_the_side_item_ids.contains(&item_id)
+                && (close_pinned || !pinned_item_ids.contains(&item_id))
         })
     }
 
@@ -2376,6 +2351,7 @@ impl Pane {
         let total_items = self.items.len();
         let has_items_to_left = ix > 0;
         let has_items_to_right = ix < total_items - 1;
+        let has_clean_items = self.items.iter().any(|item| !item.is_dirty(cx));
         let is_pinned = self.is_tab_pinned(ix);
         let pane = cx.entity().downgrade();
         let menu_context = item.item_focus_handle(cx);
@@ -2436,9 +2412,8 @@ impl Pane {
                                     .disabled(!has_items_to_left)
                                     .handler(window.handler_for(&pane, move |pane, window, cx| {
                                         pane.close_items_to_the_left_by_id(
-                                            item_id,
+                                            Some(item_id),
                                             &close_items_to_the_left_action,
-                                            pane.pinned_item_ids(),
                                             window,
                                             cx,
                                         )
@@ -2451,9 +2426,8 @@ impl Pane {
                                     .disabled(!has_items_to_right)
                                     .handler(window.handler_for(&pane, move |pane, window, cx| {
                                         pane.close_items_to_the_right_by_id(
-                                            item_id,
+                                            Some(item_id),
                                             &close_items_to_the_right_action,
-                                            pane.pinned_item_ids(),
                                             window,
                                             cx,
                                         )
@@ -2461,14 +2435,19 @@ impl Pane {
                                     })),
                             ))
                             .separator()
-                            .entry(
-                                "Close Clean",
-                                Some(Box::new(close_clean_items_action.clone())),
-                                window.handler_for(&pane, move |pane, window, cx| {
-                                    pane.close_clean_items(&close_clean_items_action, window, cx)
+                            .item(ContextMenuItem::Entry(
+                                ContextMenuEntry::new("Close Clean")
+                                    .action(Box::new(close_clean_items_action.clone()))
+                                    .disabled(!has_clean_items)
+                                    .handler(window.handler_for(&pane, move |pane, window, cx| {
+                                        pane.close_clean_items(
+                                            &close_clean_items_action,
+                                            window,
+                                            cx,
+                                        )
                                         .detach_and_log_err(cx)
-                                }),
-                            )
+                                    })),
+                            ))
                             .entry(
                                 "Close All",
                                 Some(Box::new(close_all_items_action.clone())),
@@ -3102,19 +3081,20 @@ impl Pane {
             .collect()
     }
 
-    fn to_the_left_item_ids(&self, item_id: EntityId) -> HashSet<EntityId> {
-        self.items()
-            .take_while(|item| item.item_id() != item_id)
-            .map(|item| item.item_id())
-            .collect()
-    }
-
-    fn to_the_right_item_ids(&self, item_id: EntityId) -> HashSet<EntityId> {
-        self.items()
-            .rev()
-            .take_while(|item| item.item_id() != item_id)
-            .map(|item| item.item_id())
-            .collect()
+    fn to_the_side_item_ids(&self, item_id: EntityId, side: Side) -> HashSet<EntityId> {
+        match side {
+            Side::Left => self
+                .items()
+                .take_while(|item| item.item_id() != item_id)
+                .map(|item| item.item_id())
+                .collect(),
+            Side::Right => self
+                .items()
+                .rev()
+                .take_while(|item| item.item_id() != item_id)
+                .map(|item| item.item_id())
+                .collect(),
+        }
     }
 
     pub fn drag_split_direction(&self) -> Option<SplitDirection> {
@@ -3333,13 +3313,13 @@ impl Render for Pane {
             )
             .on_action(cx.listener(
                 |pane: &mut Self, action: &CloseItemsToTheLeft, window, cx| {
-                    pane.close_items_to_the_left(action, window, cx)
+                    pane.close_items_to_the_left_by_id(None, action, window, cx)
                         .detach_and_log_err(cx)
                 },
             ))
             .on_action(cx.listener(
                 |pane: &mut Self, action: &CloseItemsToTheRight, window, cx| {
-                    pane.close_items_to_the_right(action, window, cx)
+                    pane.close_items_to_the_right_by_id(None, action, window, cx)
                         .detach_and_log_err(cx)
                 },
             ))
@@ -3349,12 +3329,6 @@ impl Render for Pane {
                         .detach_and_log_err(cx)
                 }),
             )
-            .on_action(
-                cx.listener(|pane: &mut Self, action: &CloseActiveItem, window, cx| {
-                    pane.close_active_item(action, window, cx)
-                        .detach_and_log_err(cx)
-                }),
-            )
             .on_action(
                 cx.listener(|pane: &mut Self, action: &RevealInProjectPanel, _, cx| {
                     let entry_id = action
@@ -4436,7 +4410,8 @@ mod tests {
         set_labeled_items(&pane, ["A", "B", "C*", "D", "E"], cx);
 
         pane.update_in(cx, |pane, window, cx| {
-            pane.close_items_to_the_left(
+            pane.close_items_to_the_left_by_id(
+                None,
                 &CloseItemsToTheLeft {
                     close_pinned: false,
                 },
@@ -4462,7 +4437,8 @@ mod tests {
         set_labeled_items(&pane, ["A", "B", "C*", "D", "E"], cx);
 
         pane.update_in(cx, |pane, window, cx| {
-            pane.close_items_to_the_right(
+            pane.close_items_to_the_right_by_id(
+                None,
                 &CloseItemsToTheRight {
                     close_pinned: false,
                 },
@@ -4779,7 +4755,8 @@ mod tests {
         .unwrap();
 
         pane.update_in(cx, |pane, window, cx| {
-            pane.close_items_to_the_right(
+            pane.close_items_to_the_right_by_id(
+                None,
                 &CloseItemsToTheRight {
                     close_pinned: false,
                 },
@@ -4791,7 +4768,8 @@ mod tests {
         .unwrap();
 
         pane.update_in(cx, |pane, window, cx| {
-            pane.close_items_to_the_left(
+            pane.close_items_to_the_left_by_id(
+                None,
                 &CloseItemsToTheLeft {
                     close_pinned: false,
                 },