Merge pull request #938 from l0ng-ai/feat/760-stable-workspace-numbers

feat(workspaces): number workspaces in a stable order (#760)

crates/tty7-core/src/core/session.rs

@@ -462,6 +462,36 @@ impl WindowViews {
             .collect()
     }
 
+    /// The workspaces `SelectWorkspace1`…`9`, the Workspaces menu and the
+    /// switcher number, in the order the numbers keep: the order this client
+    /// first had each one (#760). The list is append-only apart from
+    /// deletions, so a number stays with its workspace however often it is
+    /// switched to, closed or reopened — the MRU order it used to follow
+    /// handed slot 1 to whichever one was used last.
+    ///
+    /// Synced references are left out: a machine's listing mirrored at
+    /// connect time is not a workspace this client has had, and letting it in
+    /// would hand the next nine slots to whatever another client made there.
+    /// Opening one is what [`WindowViews::mark_claimed`] numbers it for.
+    pub fn numbered(&self) -> impl Iterator<Item = &WindowView> {
+        self.views.iter().filter(|w| !w.synced)
+    }
+
+    /// The entry for `id` stops being a synced reference and becomes this
+    /// client's own. A synced entry is moved to the end first, so it takes
+    /// the next free number rather than slotting in among — and shifting —
+    /// the ones already handed out.
+    pub fn mark_claimed(&mut self, id: WorkspaceId) {
+        let Some(at) = self.views.iter().position(|w| w.id == id) else {
+            return;
+        };
+        if self.views[at].synced {
+            let mut view = self.views.remove(at);
+            view.synced = false;
+            self.views.push(view);
+        }
+    }
+
     pub fn open_views(&self) -> impl Iterator<Item = &WindowView> {
         self.views.iter().filter(|w| w.open)
     }
@@ -1076,4 +1106,60 @@ mod tests {
         };
         assert_eq!(all.workspace_to_restore(), Some(open_id));
     }
+
+    fn numbers(views: &WindowViews) -> Vec<WorkspaceId> {
+        views.numbered().map(|w| w.id).collect()
+    }
+
+    #[test]
+    fn workspace_numbers_do_not_follow_use() {
+        let (a, b, c) = (view(), view(), view());
+        let (a_id, b_id, c_id) = (a.id, b.id, c.id);
+        let mut views = WindowViews {
+            views: vec![a, b, c],
+            ..WindowViews::default()
+        };
+        // Using the third one, closing the first: the MRU order the numbers
+        // used to follow would now be c, b, a (#760).
+        views.get_mut(c_id).unwrap().last_active = u64::MAX;
+        views.get_mut(a_id).unwrap().open = false;
+        views.active = Some(c_id);
+        assert_eq!(numbers(&views), vec![a_id, b_id, c_id]);
+    }
+
+    #[test]
+    fn a_synced_reference_takes_no_number_until_it_is_opened() {
+        let a = view();
+        let mut synced = view();
+        synced.synced = true;
+        synced.open = false;
+        let b = view();
+        let (a_id, s_id, b_id) = (a.id, synced.id, b.id);
+        let mut views = WindowViews {
+            views: vec![a, synced, b],
+            ..WindowViews::default()
+        };
+        assert_eq!(numbers(&views), vec![a_id, b_id]);
+
+        // Opened, it takes the next number instead of pushing `b` along.
+        views.mark_claimed(s_id);
+        assert_eq!(numbers(&views), vec![a_id, b_id, s_id]);
+        assert!(!views.get(s_id).unwrap().synced);
+
+        // Claiming one that already has its number leaves it where it is.
+        views.mark_claimed(a_id);
+        assert_eq!(numbers(&views), vec![a_id, b_id, s_id]);
+    }
+
+    #[test]
+    fn deleting_a_workspace_closes_the_gap_behind_it() {
+        let (a, b, c) = (view(), view(), view());
+        let (a_id, b_id, c_id) = (a.id, b.id, c.id);
+        let mut views = WindowViews {
+            views: vec![a, b, c],
+            ..WindowViews::default()
+        };
+        views.views.retain(|w| w.id != b_id);
+        assert_eq!(numbers(&views), vec![a_id, c_id]);
+    }
 }

docs/getting-started/concepts.mdx

@@ -37,6 +37,13 @@ just that machine. <kbd>⌘ ⇧ N</kbd> opens the New Workspace form, where you
 pick the machine (this computer, or any configured SSH host) and optionally a
 name.
 
+The first nine workspaces are also numbered 1 to 9: the number sits beside
+each one in the switcher, and the **Window** menu lists them in that order.
+Numbers go by the order the workspaces were first opened on this computer and
+do not change as you switch between them, so a key bound to **Go to
+Workspace 3** (`SelectWorkspace3`, unbound by default) always reaches the same
+one. Deleting a workspace moves the ones after it up a number.
+
 Workspaces are how tty7 keeps ten repositories from becoming forty
 indistinguishable tabs. They also travel: a workspace on a remote machine is
 still a workspace, opened from the same switcher.

src/core/session.rs

@@ -37,6 +37,11 @@ impl WorkspaceStore {
         let Some(store) = Self::try_store(cx) else {
             return WorkspaceId::new();
         };
+        // A synced reference being opened takes the next workspace number
+        // rather than the slot it was mirrored into (#760).
+        if let Some(id) = id {
+            store.views.mark_claimed(id);
+        }
         let view = match id {
             // A named workspace keeps its name even when this client has never
             // opened it: the CLI and other clients make workspaces too, and the
@@ -195,9 +200,6 @@ impl WorkspaceStore {
                 if let (Some(h), Some(via)) = (view.host.as_mut(), host.via.clone()) {
                     h.via = Some(via);
                 }
-                // Claimed is opened: the reference stops being a mirror of
-                // someone else's listing and becomes this client's own.
-                view.synced = false;
                 view.id
             }
             None => {
@@ -207,6 +209,10 @@ impl WorkspaceStore {
                 id
             }
         };
+        // Claimed is opened: the reference stops being a mirror of someone
+        // else's listing and becomes this client's own — numbered after the
+        // ones it already had (#760).
+        store.views.mark_claimed(id);
         store.views.save();
         id
     }

src/ui/switcher.rs

@@ -144,6 +144,10 @@ struct Row {
     preempted: bool,
     adopt: Option<Box<RemoteWorkspaceRow>>,
     remote_id: Option<WorkspaceId>,
+    /// Which `SelectWorkspaceN` reaches this workspace, zero-based — the
+    /// stable number the Workspaces menu shows too (#760). The list itself
+    /// stays most-recently-used first; the number is what does not move.
+    slot: Option<usize>,
     tabs: Vec<TabRow>,
 }
 
@@ -683,6 +687,8 @@ impl Tty7App {
 
         let mut groups: Vec<Group> = Vec::new();
         let mut index: HashMap<String, usize> = HashMap::new();
+        let slots = crate::ui::windows::menu_order(cx);
+        let slot_of = |id: WorkspaceId| slots.iter().position(|(slot, _)| *slot == id);
         {
             let app: &App = cx;
             let store = WorkspaceStore::all(app);
@@ -746,6 +752,7 @@ impl Tty7App {
                     preempted: false,
                     adopt: None,
                     remote_id: w.host.as_ref().map(|r| r.workspace),
+                    slot: slot_of(w.id),
                     tabs: self.tab_rows_for(w.id, app),
                 });
             }
@@ -824,6 +831,7 @@ impl Tty7App {
                     preempted: false,
                     adopt: None,
                     remote_id: None,
+                    slot: slot_of(current),
                     tabs: self.tab_rows_for(current, app),
                 },
             );
@@ -872,6 +880,7 @@ impl Tty7App {
                     preempted: false,
                     adopt: None,
                     remote_id: None,
+                    slot: None,
                     tabs: self.tab_rows_for(ws.id, app),
                 })
                 .collect();
@@ -2429,6 +2438,16 @@ impl Tty7App {
                             }),
                     ),
             )
+            // The workspace's number — what Go to Workspace N and the
+            // Workspaces menu call it — so the slot a shortcut reaches can be
+            // read off the list that does not sort by it (#760).
+            .children(row.slot.map(|slot| {
+                div()
+                    .flex_shrink_0()
+                    .text_xs()
+                    .text_color(muted)
+                    .child((slot + 1).to_string())
+            }))
             // A word, not a chip: a filled pill reads as a button, and these
             // are states. Only "taken over" keeps a colour — it is the one
             // that warns.
@@ -3052,6 +3071,7 @@ impl Group {
                 preempted: false,
                 adopt: Some(Box::new(r.clone())),
                 remote_id: Some(r.id),
+                slot: None,
                 // A workspace this client has never adopted has no local id to
                 // hang a machine-tree lookup on. The tab column says so.
                 tabs: Vec::new(),
@@ -3490,6 +3510,7 @@ mod tests {
             preempted: false,
             adopt: None,
             remote_id: None,
+            slot: None,
             tabs,
         }
     }

src/ui/theme.rs

@@ -144,18 +144,14 @@ fn window_menu_items(cx: &App) -> Vec<MenuItem> {
         MenuItem::separator(),
     ];
     let workspace_start = items.len();
-    let mut separated = false;
+    // Slot order, open and closed interleaved: the item's position is its
+    // number and its shortcut, so it cannot move when a window closes (#760).
+    // A closed one says so with its age instead.
     for (i, (id, open)) in order.iter().enumerate() {
         let Some(workspace) = store.get(*id) else {
             continue;
         };
         let Some(action) = slot_action(i) else { break };
-        if !open && !separated {
-            separated = true;
-            if items.len() > workspace_start {
-                items.push(MenuItem::Separator);
-            }
-        }
         let name = crate::ui::machine_mirror::display_name(cx, workspace)
             .unwrap_or_else(|| t(L10nKey::WindowUntitled).to_string());
         let label = if *open {

src/ui/windows.rs

@@ -510,16 +510,16 @@ pub fn refresh_menu(cx: &mut App) {
 
 pub const MENU_SLOTS: usize = 9;
 
+/// The workspaces behind `SelectWorkspace1`…`9` and the Workspaces menu, in
+/// slot order, each with whether a window has it open. A stable order —
+/// [`WindowViews::numbered`](crate::core::session::WindowViews::numbered) —
+/// not the most recently used one: slot 3 is the same workspace however
+/// often the others are switched to (#760).
 pub fn menu_order(cx: &App) -> Vec<(WorkspaceId, bool)> {
-    let all = WorkspaceStore::all(cx);
-    let mut open: Vec<_> = all.views.iter().filter(|w| w.open).collect();
-    let mut closed: Vec<_> = all.views.iter().filter(|w| !w.open).collect();
-    open.sort_by(|a, b| b.last_active.cmp(&a.last_active));
-    closed.sort_by(|a, b| b.last_active.cmp(&a.last_active));
-    open.into_iter()
-        .map(|w| (w.id, true))
-        .chain(closed.into_iter().map(|w| (w.id, false)))
+    WorkspaceStore::all(cx)
+        .numbered()
         .take(MENU_SLOTS)
+        .map(|w| (w.id, w.open))
         .collect()
 }