Return back to history-based tabs activation on close (#19150)

Kirill Bulatov created

Closes https://github.com/zed-industries/zed/issues/19036

Alters https://github.com/zed-industries/zed/pull/18168 and moves its
change behind a settings flag, restoring the previous behavior.

Release Notes:

- Fixed tab closing not respecting history. Use `tabs.activate_on_close
= neighbour` settings to activate near tabs instead.

Change summary

assets/settings/default.json |  9 +++
crates/workspace/src/item.rs | 18 ++++--
crates/workspace/src/pane.rs | 93 ++++++++++++++++++++++++++++++++++---
docs/src/configuring-zed.md  | 27 ++++++++++
4 files changed, 129 insertions(+), 18 deletions(-)

Detailed changes

assets/settings/default.json 🔗

@@ -494,7 +494,14 @@
     // Position of the close button on the editor tabs.
     "close_position": "right",
     // Whether to show the file icon for a tab.
-    "file_icons": false
+    "file_icons": false,
+    // What to do after closing the current tab.
+    //
+    // 1. Activate the tab that was open previously (default)
+    //     "History"
+    // 2. Activate the neighbour tab (prefers the right one, if present)
+    //     "Neighbour"
+    "activate_on_close": "history"
   },
   // Settings related to preview tabs.
   "preview_tabs": {

crates/workspace/src/item.rs 🔗

@@ -40,6 +40,7 @@ pub const LEADER_UPDATE_THROTTLE: Duration = Duration::from_millis(200);
 pub struct ItemSettings {
     pub git_status: bool,
     pub close_position: ClosePosition,
+    pub activate_on_close: ActivateOnClose,
     pub file_icons: bool,
 }
 
@@ -58,13 +59,12 @@ pub enum ClosePosition {
     Right,
 }
 
-impl ClosePosition {
-    pub fn right(&self) -> bool {
-        match self {
-            ClosePosition::Left => false,
-            ClosePosition::Right => true,
-        }
-    }
+#[derive(Clone, Default, Serialize, Deserialize, JsonSchema)]
+#[serde(rename_all = "lowercase")]
+pub enum ActivateOnClose {
+    #[default]
+    History,
+    Neighbour,
 }
 
 #[derive(Clone, Default, Serialize, Deserialize, JsonSchema)]
@@ -81,6 +81,10 @@ pub struct ItemSettingsContent {
     ///
     /// Default: false
     file_icons: Option<bool>,
+    /// What to do after closing the current tab.
+    ///
+    /// Default: history
+    pub activate_on_close: Option<ActivateOnClose>,
 }
 
 #[derive(Clone, Default, Serialize, Deserialize, JsonSchema)]

crates/workspace/src/pane.rs 🔗

@@ -1,7 +1,7 @@
 use crate::{
     item::{
-        ClosePosition, Item, ItemHandle, ItemSettings, PreviewTabsSettings, TabContentParams,
-        WeakItemHandle,
+        ActivateOnClose, ClosePosition, Item, ItemHandle, ItemSettings, PreviewTabsSettings,
+        TabContentParams, WeakItemHandle,
     },
     move_item,
     notifications::NotifyResultExt,
@@ -1400,6 +1400,7 @@ impl Pane {
         focus_on_pane_if_closed: Option<View<Pane>>,
         cx: &mut ViewContext<Self>,
     ) {
+        let activate_on_close = &ItemSettings::get_global(cx).activate_on_close;
         self.activation_history
             .retain(|entry| entry.entity_id != self.items[item_index].item_id());
 
@@ -1407,12 +1408,26 @@ impl Pane {
             self.pinned_tab_count -= 1;
         }
         if item_index == self.active_item_index {
-            self.activation_history.pop();
-
-            let index_to_activate = if item_index + 1 < self.items.len() {
-                item_index + 1
-            } else {
-                item_index.saturating_sub(1)
+            let index_to_activate = match activate_on_close {
+                ActivateOnClose::History => self
+                    .activation_history
+                    .pop()
+                    .and_then(|last_activated_item| {
+                        self.items.iter().enumerate().find_map(|(index, item)| {
+                            (item.item_id() == last_activated_item.entity_id).then_some(index)
+                        })
+                    })
+                    // We didn't have a valid activation history entry, so fallback
+                    // to activating the item to the left
+                    .unwrap_or_else(|| item_index.min(self.items.len()).saturating_sub(1)),
+                ActivateOnClose::Neighbour => {
+                    self.activation_history.pop();
+                    if item_index + 1 < self.items.len() {
+                        item_index + 1
+                    } else {
+                        item_index.saturating_sub(1)
+                    }
+                }
             };
 
             let should_activate = activate_pane || self.has_focus(cx);
@@ -3292,7 +3307,7 @@ mod tests {
     }
 
     #[gpui::test]
-    async fn test_remove_item_ordering(cx: &mut TestAppContext) {
+    async fn test_remove_item_ordering_history(cx: &mut TestAppContext) {
         init_test(cx);
         let fs = FakeFs::new(cx.executor());
 
@@ -3310,6 +3325,66 @@ mod tests {
         add_labeled_item(&pane, "1", false, cx);
         assert_item_labels(&pane, ["A", "B", "1*", "C", "D"], cx);
 
+        pane.update(cx, |pane, cx| {
+            pane.close_active_item(&CloseActiveItem { save_intent: None }, cx)
+        })
+        .unwrap()
+        .await
+        .unwrap();
+        assert_item_labels(&pane, ["A", "B*", "C", "D"], cx);
+
+        pane.update(cx, |pane, cx| pane.activate_item(3, false, false, cx));
+        assert_item_labels(&pane, ["A", "B", "C", "D*"], cx);
+
+        pane.update(cx, |pane, cx| {
+            pane.close_active_item(&CloseActiveItem { save_intent: None }, cx)
+        })
+        .unwrap()
+        .await
+        .unwrap();
+        assert_item_labels(&pane, ["A", "B*", "C"], cx);
+
+        pane.update(cx, |pane, cx| {
+            pane.close_active_item(&CloseActiveItem { save_intent: None }, cx)
+        })
+        .unwrap()
+        .await
+        .unwrap();
+        assert_item_labels(&pane, ["A", "C*"], cx);
+
+        pane.update(cx, |pane, cx| {
+            pane.close_active_item(&CloseActiveItem { save_intent: None }, cx)
+        })
+        .unwrap()
+        .await
+        .unwrap();
+        assert_item_labels(&pane, ["A*"], cx);
+    }
+
+    #[gpui::test]
+    async fn test_remove_item_ordering_neighbour(cx: &mut TestAppContext) {
+        init_test(cx);
+        cx.update_global::<SettingsStore, ()>(|s, cx| {
+            s.update_user_settings::<ItemSettings>(cx, |s| {
+                s.activate_on_close = Some(ActivateOnClose::Neighbour);
+            });
+        });
+        let fs = FakeFs::new(cx.executor());
+
+        let project = Project::test(fs, None, cx).await;
+        let (workspace, cx) = cx.add_window_view(|cx| Workspace::test_new(project.clone(), cx));
+        let pane = workspace.update(cx, |workspace, _| workspace.active_pane().clone());
+
+        add_labeled_item(&pane, "A", false, cx);
+        add_labeled_item(&pane, "B", false, cx);
+        add_labeled_item(&pane, "C", false, cx);
+        add_labeled_item(&pane, "D", false, cx);
+        assert_item_labels(&pane, ["A", "B", "C", "D*"], cx);
+
+        pane.update(cx, |pane, cx| pane.activate_item(1, false, false, cx));
+        add_labeled_item(&pane, "1", false, cx);
+        assert_item_labels(&pane, ["A", "B", "1*", "C", "D"], cx);
+
         pane.update(cx, |pane, cx| {
             pane.close_active_item(&CloseActiveItem { save_intent: None }, cx)
         })

docs/src/configuring-zed.md 🔗

@@ -539,7 +539,8 @@ List of `string` values
 "tabs": {
   "close_position": "right",
   "file_icons": false,
-  "git_status": false
+  "git_status": false,
+  "activate_on_close": "history"
 },
 ```
 
@@ -579,6 +580,30 @@ List of `string` values
 - Setting: `git_status`
 - Default: `false`
 
+### Activate on close
+
+- Description: What to do after closing the current tab.
+- Setting: `activate_on_close`
+- Default: `history`
+
+**Options**
+
+1.  Activate the tab that was open previously:
+
+```json
+{
+  "activate_on_close": "history"
+}
+```
+
+2. Activate the neighbour tab (prefers the right one, if present):
+
+```json
+{
+  "activate_on_close": "neighbour"
+}
+```
+
 ## Editor Toolbar
 
 - Description: Whether or not to show various elements in the editor toolbar.