Populate the window title whenever worktrees or active path change

Max Brunsfeld created

* Refactor the way the project's active entry is assigned. Assign it
  together with the window title, as opposed to on every notification
  from a pane.
* Emit the ActiveItem event from panes consistently, even when adding
  the first item to an empty pane.

Change summary

crates/workspace/src/pane.rs      |  31 +++-
crates/workspace/src/workspace.rs | 223 +++++++++++++++++++++++++++-----
crates/zed/src/menus.rs           |   4 
3 files changed, 210 insertions(+), 48 deletions(-)

Detailed changes

crates/workspace/src/pane.rs 🔗

@@ -15,7 +15,7 @@ use gpui::{
 use project::{Project, ProjectEntryId, ProjectPath};
 use serde::Deserialize;
 use settings::Settings;
-use std::{any::Any, cell::RefCell, cmp, mem, path::Path, rc::Rc};
+use std::{any::Any, cell::RefCell, mem, path::Path, rc::Rc};
 use util::ResultExt;
 
 actions!(
@@ -109,6 +109,7 @@ pub enum Event {
     ActivateItem { local: bool },
     Remove,
     Split(SplitDirection),
+    ChangeItemTitle,
 }
 
 pub struct Pane {
@@ -334,9 +335,20 @@ impl Pane {
         item.set_nav_history(pane.read(cx).nav_history.clone(), cx);
         item.added_to_pane(workspace, pane.clone(), cx);
         pane.update(cx, |pane, cx| {
-            let item_idx = cmp::min(pane.active_item_index + 1, pane.items.len());
-            pane.items.insert(item_idx, item);
-            pane.activate_item(item_idx, activate_pane, focus_item, cx);
+            // If there is already an active item, then insert the new item
+            // right after it. Otherwise, adjust the `active_item_index` field
+            // before activating the new item, so that in the `activate_item`
+            // method, we can detect that the active item is changing.
+            let item_ix;
+            if pane.active_item_index < pane.items.len() {
+                item_ix = pane.active_item_index + 1
+            } else {
+                item_ix = pane.items.len();
+                pane.active_item_index = usize::MAX;
+            };
+
+            pane.items.insert(item_ix, item);
+            pane.activate_item(item_ix, activate_pane, focus_item, cx);
             cx.notify();
         });
     }
@@ -383,11 +395,12 @@ impl Pane {
         use NavigationMode::{GoingBack, GoingForward};
         if index < self.items.len() {
             let prev_active_item_ix = mem::replace(&mut self.active_item_index, index);
-            if matches!(self.nav_history.borrow().mode, GoingBack | GoingForward)
-                || (prev_active_item_ix != self.active_item_index
-                    && prev_active_item_ix < self.items.len())
+            if prev_active_item_ix != self.active_item_index
+                || matches!(self.nav_history.borrow().mode, GoingBack | GoingForward)
             {
-                self.items[prev_active_item_ix].deactivated(cx);
+                if let Some(prev_item) = self.items.get(prev_active_item_ix) {
+                    prev_item.deactivated(cx);
+                }
                 cx.emit(Event::ActivateItem {
                     local: activate_pane,
                 });
@@ -424,7 +437,7 @@ impl Pane {
         self.activate_item(index, true, true, cx);
     }
 
-    fn close_active_item(
+    pub fn close_active_item(
         workspace: &mut Workspace,
         _: &CloseActiveItem,
         cx: &mut ViewContext<Workspace>,

crates/workspace/src/workspace.rs 🔗

@@ -38,6 +38,7 @@ use status_bar::StatusBar;
 pub use status_bar::StatusItemView;
 use std::{
     any::{Any, TypeId},
+    borrow::Cow,
     cell::RefCell,
     fmt,
     future::Future,
@@ -532,7 +533,10 @@ impl<T: Item> ItemHandle for ViewHandle<T> {
             }
 
             if T::should_update_tab_on_event(event) {
-                pane.update(cx, |_, cx| cx.notify());
+                pane.update(cx, |_, cx| {
+                    cx.emit(pane::Event::ChangeItemTitle);
+                    cx.notify();
+                });
             }
         })
         .detach();
@@ -744,6 +748,9 @@ impl Workspace {
                 project::Event::CollaboratorLeft(peer_id) => {
                     this.collaborator_left(*peer_id, cx);
                 }
+                project::Event::WorktreeRemoved(_) | project::Event::WorktreeAdded => {
+                    this.update_window_title(cx);
+                }
                 _ => {}
             }
             if project.read(cx).is_read_only() {
@@ -755,14 +762,8 @@ impl Workspace {
 
         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);
-            me.project
-                .update(cx, |project, cx| project.set_active_path(active_entry, cx));
-        })
-        .detach();
-        cx.subscribe(&pane, move |me, _, event, cx| {
-            me.handle_pane_event(pane_id, event, cx)
+        cx.subscribe(&pane, move |this, _, event, cx| {
+            this.handle_pane_event(pane_id, event, cx)
         })
         .detach();
         cx.focus(&pane);
@@ -825,6 +826,11 @@ impl Workspace {
             _observe_current_user,
         };
         this.project_remote_id_changed(this.project.read(cx).remote_id(), cx);
+
+        cx.defer(|this, cx| {
+            this.update_window_title(cx);
+        });
+
         this
     }
 
@@ -1238,14 +1244,8 @@ impl Workspace {
     fn add_pane(&mut self, cx: &mut ViewContext<Self>) -> ViewHandle<Pane> {
         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);
-            me.project
-                .update(cx, |project, cx| project.set_active_path(active_entry, cx));
-        })
-        .detach();
-        cx.subscribe(&pane, move |me, _, event, cx| {
-            me.handle_pane_event(pane_id, event, cx)
+        cx.subscribe(&pane, move |this, _, event, cx| {
+            this.handle_pane_event(pane_id, event, cx)
         })
         .detach();
         self.panes.push(pane.clone());
@@ -1385,6 +1385,7 @@ impl Workspace {
             self.status_bar.update(cx, |status_bar, cx| {
                 status_bar.set_active_pane(&self.active_pane, cx);
             });
+            self.active_item_path_changed(cx);
             cx.focus(&self.active_pane);
             cx.notify();
         }
@@ -1419,6 +1420,14 @@ impl Workspace {
                     if *local {
                         self.unfollow(&pane, cx);
                     }
+                    if pane == self.active_pane {
+                        self.active_item_path_changed(cx);
+                    }
+                }
+                pane::Event::ChangeItemTitle => {
+                    if pane == self.active_pane {
+                        self.active_item_path_changed(cx);
+                    }
                 }
             }
         } else {
@@ -1451,6 +1460,8 @@ impl Workspace {
             self.unfollow(&pane, cx);
             self.last_leaders_by_pane.remove(&pane.downgrade());
             cx.notify();
+        } else {
+            self.active_item_path_changed(cx);
         }
     }
 
@@ -1638,15 +1649,7 @@ impl Workspace {
 
     fn render_titlebar(&self, theme: &Theme, cx: &mut RenderContext<Self>) -> ElementBox {
         let mut worktree_root_names = String::new();
-        {
-            let mut worktrees = self.project.read(cx).visible_worktrees(cx).peekable();
-            while let Some(worktree) = worktrees.next() {
-                worktree_root_names.push_str(worktree.read(cx).root_name());
-                if worktrees.peek().is_some() {
-                    worktree_root_names.push_str(", ");
-                }
-            }
-        }
+        self.worktree_root_names(&mut worktree_root_names, cx);
 
         ConstrainedBox::new(
             Container::new(
@@ -1682,6 +1685,50 @@ impl Workspace {
         .named("titlebar")
     }
 
+    fn active_item_path_changed(&mut self, cx: &mut ViewContext<Self>) {
+        let active_entry = self.active_project_path(cx);
+        self.project
+            .update(cx, |project, cx| project.set_active_path(active_entry, cx));
+        self.update_window_title(cx);
+    }
+
+    fn update_window_title(&mut self, cx: &mut ViewContext<Self>) {
+        let mut title = String::new();
+        if let Some(path) = self.active_item(cx).and_then(|item| item.project_path(cx)) {
+            let filename = path
+                .path
+                .file_name()
+                .map(|s| s.to_string_lossy())
+                .or_else(|| {
+                    Some(Cow::Borrowed(
+                        self.project()
+                            .read(cx)
+                            .worktree_for_id(path.worktree_id, cx)?
+                            .read(cx)
+                            .root_name(),
+                    ))
+                });
+            if let Some(filename) = filename {
+                title.push_str(filename.as_ref());
+                title.push_str(" — ");
+            }
+        }
+        self.worktree_root_names(&mut title, cx);
+        if title.is_empty() {
+            title = "empty project".to_string();
+        }
+        cx.set_window_title(&title);
+    }
+
+    fn worktree_root_names(&self, string: &mut String, cx: &mut MutableAppContext) {
+        for (i, worktree) in self.project.read(cx).visible_worktrees(cx).enumerate() {
+            if i != 0 {
+                string.push_str(", ");
+            }
+            string.push_str(worktree.read(cx).root_name());
+        }
+    }
+
     fn render_collaborators(&self, theme: &Theme, cx: &mut RenderContext<Self>) -> Vec<ElementBox> {
         let mut collaborators = self
             .project
@@ -2417,6 +2464,110 @@ mod tests {
     use project::{FakeFs, Project, ProjectEntryId};
     use serde_json::json;
 
+    #[gpui::test]
+    async fn test_tracking_active_path(cx: &mut TestAppContext) {
+        cx.foreground().forbid_parking();
+        Settings::test_async(cx);
+        let fs = FakeFs::new(cx.background());
+        fs.insert_tree(
+            "/root1",
+            json!({
+                "one.txt": "",
+                "two.txt": "",
+            }),
+        )
+        .await;
+        fs.insert_tree(
+            "/root2",
+            json!({
+                "three.txt": "",
+            }),
+        )
+        .await;
+
+        let project = Project::test(fs, ["root1".as_ref()], cx).await;
+        let (window_id, workspace) = cx.add_window(|cx| Workspace::new(project.clone(), cx));
+        let worktree_id = project.read_with(cx, |project, cx| {
+            project.worktrees(cx).next().unwrap().read(cx).id()
+        });
+
+        let item1 = cx.add_view(window_id, |_| {
+            let mut item = TestItem::new();
+            item.project_path = Some((worktree_id, "one.txt").into());
+            item
+        });
+        let item2 = cx.add_view(window_id, |_| {
+            let mut item = TestItem::new();
+            item.project_path = Some((worktree_id, "two.txt").into());
+            item
+        });
+
+        // Add an item to an empty pane
+        workspace.update(cx, |workspace, cx| workspace.add_item(Box::new(item1), cx));
+        project.read_with(cx, |project, cx| {
+            assert_eq!(
+                project.active_entry(),
+                project.entry_for_path(&(worktree_id, "one.txt").into(), cx)
+            );
+        });
+        assert_eq!(
+            cx.current_window_title(window_id).as_deref(),
+            Some("one.txt — root1")
+        );
+
+        // Add a second item to a non-empty pane
+        workspace.update(cx, |workspace, cx| workspace.add_item(Box::new(item2), cx));
+        assert_eq!(
+            cx.current_window_title(window_id).as_deref(),
+            Some("two.txt — root1")
+        );
+        project.read_with(cx, |project, cx| {
+            assert_eq!(
+                project.active_entry(),
+                project.entry_for_path(&(worktree_id, "two.txt").into(), cx)
+            );
+        });
+
+        // Close the active item
+        workspace
+            .update(cx, |workspace, cx| {
+                Pane::close_active_item(workspace, &Default::default(), cx).unwrap()
+            })
+            .await
+            .unwrap();
+        assert_eq!(
+            cx.current_window_title(window_id).as_deref(),
+            Some("one.txt — root1")
+        );
+        project.read_with(cx, |project, cx| {
+            assert_eq!(
+                project.active_entry(),
+                project.entry_for_path(&(worktree_id, "one.txt").into(), cx)
+            );
+        });
+
+        // Add a project folder
+        project
+            .update(cx, |project, cx| {
+                project.find_or_create_local_worktree("/root2", true, cx)
+            })
+            .await
+            .unwrap();
+        assert_eq!(
+            cx.current_window_title(window_id).as_deref(),
+            Some("one.txt — root1, root2")
+        );
+
+        // Remove a project folder
+        project.update(cx, |project, cx| {
+            project.remove_worktree(worktree_id, cx);
+        });
+        assert_eq!(
+            cx.current_window_title(window_id).as_deref(),
+            Some("one.txt — root2")
+        );
+    }
+
     #[gpui::test]
     async fn test_close_window(cx: &mut TestAppContext) {
         cx.foreground().forbid_parking();
@@ -2456,18 +2607,6 @@ mod tests {
         cx.foreground().run_until_parked();
         assert!(!cx.has_pending_prompt(window_id));
         assert_eq!(task.await.unwrap(), false);
-
-        // If there are multiple dirty items representing the same project entry.
-        workspace.update(cx, |w, cx| {
-            w.add_item(Box::new(item2.clone()), cx);
-            w.add_item(Box::new(item3.clone()), cx);
-        });
-        let task = workspace.update(cx, |w, cx| w.prepare_to_close(cx));
-        cx.foreground().run_until_parked();
-        cx.simulate_prompt_answer(window_id, 2 /* cancel */);
-        cx.foreground().run_until_parked();
-        assert!(!cx.has_pending_prompt(window_id));
-        assert_eq!(task.await.unwrap(), false);
     }
 
     #[gpui::test]
@@ -2667,6 +2806,7 @@ mod tests {
         is_dirty: bool,
         has_conflict: bool,
         project_entry_ids: Vec<ProjectEntryId>,
+        project_path: Option<ProjectPath>,
         is_singleton: bool,
     }
 
@@ -2679,6 +2819,7 @@ mod tests {
                 is_dirty: false,
                 has_conflict: false,
                 project_entry_ids: Vec::new(),
+                project_path: None,
                 is_singleton: true,
             }
         }
@@ -2704,7 +2845,7 @@ mod tests {
         }
 
         fn project_path(&self, _: &AppContext) -> Option<ProjectPath> {
-            None
+            self.project_path.clone()
         }
 
         fn project_entry_ids(&self, _: &AppContext) -> SmallVec<[ProjectEntryId; 3]> {
@@ -2763,5 +2904,9 @@ mod tests {
             self.reload_count += 1;
             Task::ready(Ok(()))
         }
+
+        fn should_update_tab_on_event(_: &Self::Event) -> bool {
+            true
+        }
     }
 }

crates/zed/src/menus.rs 🔗

@@ -229,6 +229,10 @@ pub fn menus() -> Vec<Menu<'static>> {
                 },
             ],
         },
+        Menu {
+            name: "Window",
+            items: vec![MenuItem::Separator],
+        },
         Menu {
             name: "Help",
             items: vec![MenuItem::Action {