Allow following into channel notes regardless of project

Max Brunsfeld created

Change summary

crates/collab/src/tests/channel_buffer_tests.rs |  75 +++++++++++-
crates/collab/src/tests/integration_tests.rs    |   4 
crates/collab_ui/src/channel_view.rs            |  32 ++++-
crates/collab_ui/src/chat_panel.rs              |   4 
crates/collab_ui/src/collab_panel.rs            |   2 
crates/collab_ui/src/collab_titlebar_item.rs    | 107 +++++++++---------
crates/editor/src/items.rs                      |   4 
crates/workspace/src/item.rs                    |   6 +
crates/workspace/src/pane_group.rs              |  19 +-
crates/workspace/src/workspace.rs               |  62 ++++++----
10 files changed, 201 insertions(+), 114 deletions(-)

Detailed changes

crates/collab/src/tests/channel_buffer_tests.rs 🔗

@@ -594,9 +594,17 @@ async fn test_following_to_channel_notes_without_a_shared_project(
     let client_b = server.create_client(cx_b, "user_b").await;
     let client_c = server.create_client(cx_c, "user_c").await;
 
+    cx_a.update(editor::init);
+    cx_b.update(editor::init);
+    cx_c.update(editor::init);
+    cx_a.update(collab_ui::channel_view::init);
+    cx_b.update(collab_ui::channel_view::init);
+    cx_c.update(collab_ui::channel_view::init);
+
     let channel_1_id = server
         .make_channel(
             "channel-1",
+            None,
             (&client_a, cx_a),
             &mut [(&client_b, cx_b), (&client_c, cx_c)],
         )
@@ -604,6 +612,7 @@ async fn test_following_to_channel_notes_without_a_shared_project(
     let channel_2_id = server
         .make_channel(
             "channel-2",
+            None,
             (&client_a, cx_a),
             &mut [(&client_b, cx_b), (&client_c, cx_c)],
         )
@@ -633,20 +642,27 @@ async fn test_following_to_channel_notes_without_a_shared_project(
     let (project_c, _) = client_b.build_local_project("/c", cx_c).await;
     let workspace_a = client_a.build_workspace(&project_a, cx_a).root(cx_a);
     let workspace_b = client_b.build_workspace(&project_b, cx_b).root(cx_b);
-    let workspace_c = client_c.build_workspace(&project_c, cx_c).root(cx_c);
+    let _workspace_c = client_c.build_workspace(&project_c, cx_c).root(cx_c);
+
+    active_call_a
+        .update(cx_a, |call, cx| call.set_location(Some(&project_a), cx))
+        .await
+        .unwrap();
 
     // Client A opens the notes for channel 1.
     let channel_view_1_a = cx_a
-        .update(|cx| {
-            ChannelView::open(
-                channel_1_id,
-                workspace_a.read(cx).active_pane().clone(),
-                workspace_a.clone(),
-                cx,
-            )
-        })
+        .update(|cx| ChannelView::open(channel_1_id, workspace_a.clone(), cx))
         .await
         .unwrap();
+    channel_view_1_a.update(cx_a, |notes, cx| {
+        assert_eq!(notes.channel(cx).name, "channel-1");
+        notes.editor.update(cx, |editor, cx| {
+            editor.insert("Hello from A.", cx);
+            editor.change_selections(None, cx, |selections| {
+                selections.select_ranges(vec![3..4]);
+            });
+        });
+    });
 
     // Client B follows client A.
     workspace_b
@@ -658,12 +674,51 @@ async fn test_following_to_channel_notes_without_a_shared_project(
         .await
         .unwrap();
 
+    // Client B is taken to the notes for channel 1, with the same
+    // text selected as client A.
+    deterministic.run_until_parked();
+    let channel_view_1_b = workspace_b.read_with(cx_b, |workspace, cx| {
+        assert_eq!(
+            workspace.leader_for_pane(workspace.active_pane()),
+            Some(client_a.peer_id().unwrap())
+        );
+        workspace
+            .active_item(cx)
+            .expect("no active item")
+            .downcast::<ChannelView>()
+            .expect("active item is not a channel view")
+    });
+    channel_view_1_b.read_with(cx_b, |notes, cx| {
+        assert_eq!(notes.channel(cx).name, "channel-1");
+        let editor = notes.editor.read(cx);
+        assert_eq!(editor.text(cx), "Hello from A.");
+        assert_eq!(editor.selections.ranges::<usize>(cx), &[3..4]);
+    });
+
+    // Client A opens the notes for channel 2.
+    let channel_view_2_a = cx_a
+        .update(|cx| ChannelView::open(channel_2_id, workspace_a.clone(), cx))
+        .await
+        .unwrap();
+    channel_view_2_a.read_with(cx_a, |notes, cx| {
+        assert_eq!(notes.channel(cx).name, "channel-2");
+    });
+
+    // Client B is taken to the notes for channel 2.
     deterministic.run_until_parked();
-    workspace_b.read_with(cx_b, |workspace, _| {
+    let channel_view_2_b = workspace_b.read_with(cx_b, |workspace, cx| {
         assert_eq!(
             workspace.leader_for_pane(workspace.active_pane()),
             Some(client_a.peer_id().unwrap())
         );
+        workspace
+            .active_item(cx)
+            .expect("no active item")
+            .downcast::<ChannelView>()
+            .expect("active item is not a channel view")
+    });
+    channel_view_2_b.read_with(cx_b, |notes, cx| {
+        assert_eq!(notes.channel(cx).name, "channel-2");
     });
 }
 

crates/collab/src/tests/integration_tests.rs 🔗

@@ -6848,9 +6848,9 @@ async fn test_basic_following(
     let shared_screen = workspace_a.read_with(cx_a, |workspace, cx| {
         workspace
             .active_item(cx)
-            .unwrap()
+            .expect("no active item")
             .downcast::<SharedScreen>()
-            .unwrap()
+            .expect("active item isn't a shared screen")
     });
 
     // Client B activates Zed again, which causes the previous editor to become focused again.

crates/collab_ui/src/channel_view.rs 🔗

@@ -1,6 +1,6 @@
 use anyhow::{anyhow, Result};
 use call::ActiveCall;
-use channel::{ChannelBuffer, ChannelBufferEvent, ChannelId};
+use channel::{Channel, ChannelBuffer, ChannelBufferEvent, ChannelId};
 use client::proto;
 use clock::ReplicaId;
 use collections::HashMap;
@@ -13,7 +13,10 @@ use gpui::{
     ViewContext, ViewHandle,
 };
 use project::Project;
-use std::any::{Any, TypeId};
+use std::{
+    any::{Any, TypeId},
+    sync::Arc,
+};
 use util::ResultExt;
 use workspace::{
     item::{FollowableItem, Item, ItemHandle},
@@ -24,7 +27,7 @@ use workspace::{
 
 actions!(channel_view, [Deploy]);
 
-pub(crate) fn init(cx: &mut AppContext) {
+pub fn init(cx: &mut AppContext) {
     register_followable_item::<ChannelView>(cx)
 }
 
@@ -37,9 +40,13 @@ pub struct ChannelView {
 }
 
 impl ChannelView {
-    pub fn deploy(channel_id: ChannelId, workspace: ViewHandle<Workspace>, cx: &mut AppContext) {
+    pub fn open(
+        channel_id: ChannelId,
+        workspace: ViewHandle<Workspace>,
+        cx: &mut AppContext,
+    ) -> Task<Result<ViewHandle<Self>>> {
         let pane = workspace.read(cx).active_pane().clone();
-        let channel_view = Self::open(channel_id, pane.clone(), workspace.clone(), cx);
+        let channel_view = Self::open_in_pane(channel_id, pane.clone(), workspace.clone(), cx);
         cx.spawn(|mut cx| async move {
             let channel_view = channel_view.await?;
             pane.update(&mut cx, |pane, cx| {
@@ -56,12 +63,11 @@ impl ChannelView {
                 );
                 pane.add_item(Box::new(channel_view.clone()), true, true, None, cx);
             });
-            anyhow::Ok(())
+            anyhow::Ok(channel_view)
         })
-        .detach();
     }
 
-    pub fn open(
+    pub fn open_in_pane(
         channel_id: ChannelId,
         pane: ViewHandle<Pane>,
         workspace: ViewHandle<Workspace>,
@@ -121,6 +127,10 @@ impl ChannelView {
         this
     }
 
+    pub fn channel(&self, cx: &AppContext) -> Arc<Channel> {
+        self.channel_buffer.read(cx).channel()
+    }
+
     fn handle_project_event(
         &mut self,
         _: ModelHandle<Project>,
@@ -318,7 +328,7 @@ impl FollowableItem for ChannelView {
             unreachable!()
         };
 
-        let open = ChannelView::open(state.channel_id, pane, workspace, cx);
+        let open = ChannelView::open_in_pane(state.channel_id, pane, workspace, cx);
 
         Some(cx.spawn(|mut cx| async move {
             let this = open.await?;
@@ -391,4 +401,8 @@ impl FollowableItem for ChannelView {
     fn should_unfollow_on_event(event: &Self::Event, cx: &AppContext) -> bool {
         Editor::should_unfollow_on_event(event, cx)
     }
+
+    fn is_project_item(&self, _cx: &AppContext) -> bool {
+        false
+    }
 }

crates/collab_ui/src/chat_panel.rs 🔗

@@ -409,7 +409,7 @@ impl ChatPanel {
                 })
                 .on_click(MouseButton::Left, move |_, _, cx| {
                     if let Some(workspace) = workspace.upgrade(cx) {
-                        ChannelView::deploy(channel_id, workspace, cx);
+                        ChannelView::open(channel_id, workspace, cx).detach();
                     }
                 })
                 .with_tooltip::<OpenChannelNotes>(
@@ -546,7 +546,7 @@ impl ChatPanel {
         if let Some((chat, _)) = &self.active_chat {
             let channel_id = chat.read(cx).channel().id;
             if let Some(workspace) = self.workspace.upgrade(cx) {
-                ChannelView::deploy(channel_id, workspace, cx);
+                ChannelView::open(channel_id, workspace, cx).detach();
             }
         }
     }

crates/collab_ui/src/collab_panel.rs 🔗

@@ -2745,7 +2745,7 @@ impl CollabPanel {
 
     fn open_channel_notes(&mut self, action: &OpenChannelNotes, cx: &mut ViewContext<Self>) {
         if let Some(workspace) = self.workspace.upgrade(cx) {
-            ChannelView::deploy(action.channel_id, workspace, cx);
+            ChannelView::open(action.channel_id, workspace, cx).detach();
         }
     }
 

crates/collab_ui/src/collab_titlebar_item.rs 🔗

@@ -1004,61 +1004,64 @@ impl CollabTitlebarItem {
             .into_any();
 
         if let Some(location) = location {
-            if let Some(replica_id) = replica_id {
-                enum ToggleFollow {}
+            match (replica_id, location) {
+                (None, ParticipantLocation::SharedProject { project_id }) => {
+                    enum JoinProject {}
 
-                content = MouseEventHandler::new::<ToggleFollow, _>(
-                    replica_id.into(),
-                    cx,
-                    move |_, _| content,
-                )
-                .with_cursor_style(CursorStyle::PointingHand)
-                .on_click(MouseButton::Left, move |_, item, cx| {
-                    if let Some(workspace) = item.workspace.upgrade(cx) {
-                        if let Some(task) = workspace
-                            .update(cx, |workspace, cx| workspace.toggle_follow(peer_id, cx))
-                        {
-                            task.detach_and_log_err(cx);
+                    let user_id = user.id;
+                    content = MouseEventHandler::new::<JoinProject, _>(
+                        peer_id.as_u64() as usize,
+                        cx,
+                        move |_, _| content,
+                    )
+                    .with_cursor_style(CursorStyle::PointingHand)
+                    .on_click(MouseButton::Left, move |_, this, cx| {
+                        if let Some(workspace) = this.workspace.upgrade(cx) {
+                            let app_state = workspace.read(cx).app_state().clone();
+                            workspace::join_remote_project(project_id, user_id, app_state, cx)
+                                .detach_and_log_err(cx);
                         }
-                    }
-                })
-                .with_tooltip::<ToggleFollow>(
-                    peer_id.as_u64() as usize,
-                    if is_being_followed {
-                        format!("Unfollow {}", user.github_login)
-                    } else {
-                        format!("Follow {}", user.github_login)
-                    },
-                    Some(Box::new(FollowNextCollaborator)),
-                    theme.tooltip.clone(),
-                    cx,
-                )
-                .into_any();
-            } else if let ParticipantLocation::SharedProject { project_id } = location {
-                enum JoinProject {}
+                    })
+                    .with_tooltip::<JoinProject>(
+                        peer_id.as_u64() as usize,
+                        format!("Follow {} into external project", user.github_login),
+                        Some(Box::new(FollowNextCollaborator)),
+                        theme.tooltip.clone(),
+                        cx,
+                    )
+                    .into_any();
+                }
+                _ => {
+                    enum ToggleFollow {}
 
-                let user_id = user.id;
-                content = MouseEventHandler::new::<JoinProject, _>(
-                    peer_id.as_u64() as usize,
-                    cx,
-                    move |_, _| content,
-                )
-                .with_cursor_style(CursorStyle::PointingHand)
-                .on_click(MouseButton::Left, move |_, this, cx| {
-                    if let Some(workspace) = this.workspace.upgrade(cx) {
-                        let app_state = workspace.read(cx).app_state().clone();
-                        workspace::join_remote_project(project_id, user_id, app_state, cx)
-                            .detach_and_log_err(cx);
-                    }
-                })
-                .with_tooltip::<JoinProject>(
-                    peer_id.as_u64() as usize,
-                    format!("Follow {} into external project", user.github_login),
-                    Some(Box::new(FollowNextCollaborator)),
-                    theme.tooltip.clone(),
-                    cx,
-                )
-                .into_any();
+                    content = MouseEventHandler::new::<ToggleFollow, _>(
+                        user.id as usize,
+                        cx,
+                        move |_, _| content,
+                    )
+                    .with_cursor_style(CursorStyle::PointingHand)
+                    .on_click(MouseButton::Left, move |_, item, cx| {
+                        if let Some(workspace) = item.workspace.upgrade(cx) {
+                            if let Some(task) = workspace
+                                .update(cx, |workspace, cx| workspace.toggle_follow(peer_id, cx))
+                            {
+                                task.detach_and_log_err(cx);
+                            }
+                        }
+                    })
+                    .with_tooltip::<ToggleFollow>(
+                        peer_id.as_u64() as usize,
+                        if is_being_followed {
+                            format!("Unfollow {}", user.github_login)
+                        } else {
+                            format!("Follow {}", user.github_login)
+                        },
+                        Some(Box::new(FollowNextCollaborator)),
+                        theme.tooltip.clone(),
+                        cx,
+                    )
+                    .into_any();
+                }
             }
         }
         content

crates/editor/src/items.rs 🔗

@@ -309,6 +309,10 @@ impl FollowableItem for Editor {
             _ => false,
         }
     }
+
+    fn is_project_item(&self, _cx: &AppContext) -> bool {
+        true
+    }
 }
 
 async fn update_editor_from_message(

crates/workspace/src/item.rs 🔗

@@ -696,6 +696,7 @@ pub trait FollowableItem: Item {
         message: proto::update_view::Variant,
         cx: &mut ViewContext<Self>,
     ) -> Task<Result<()>>;
+    fn is_project_item(&self, cx: &AppContext) -> bool;
 
     fn set_leader_replica_id(&mut self, leader_replica_id: Option<u16>, cx: &mut ViewContext<Self>);
     fn should_unfollow_on_event(event: &Self::Event, cx: &AppContext) -> bool;
@@ -718,6 +719,7 @@ pub trait FollowableItemHandle: ItemHandle {
         cx: &mut WindowContext,
     ) -> Task<Result<()>>;
     fn should_unfollow_on_event(&self, event: &dyn Any, cx: &AppContext) -> bool;
+    fn is_project_item(&self, cx: &AppContext) -> bool;
 }
 
 impl<T: FollowableItem> FollowableItemHandle for ViewHandle<T> {
@@ -769,6 +771,10 @@ impl<T: FollowableItem> FollowableItemHandle for ViewHandle<T> {
             false
         }
     }
+
+    fn is_project_item(&self, cx: &AppContext) -> bool {
+        self.read(cx).is_project_item(cx)
+    }
 }
 
 #[cfg(any(test, feature = "test-support"))]

crates/workspace/src/pane_group.rs 🔗

@@ -210,7 +210,6 @@ impl Member {
                             } else {
                                 let leader_user = leader.user.clone();
                                 let leader_user_id = leader.user.id;
-                                let app_state = Arc::downgrade(app_state);
                                 Some(
                                     MouseEventHandler::new::<FollowIntoExternalProject, _>(
                                         pane.id(),
@@ -234,16 +233,14 @@ impl Member {
                                         },
                                     )
                                     .with_cursor_style(CursorStyle::PointingHand)
-                                    .on_click(MouseButton::Left, move |_, _, cx| {
-                                        if let Some(app_state) = app_state.upgrade() {
-                                            crate::join_remote_project(
-                                                leader_project_id,
-                                                leader_user_id,
-                                                app_state,
-                                                cx,
-                                            )
-                                            .detach_and_log_err(cx);
-                                        }
+                                    .on_click(MouseButton::Left, move |_, this, cx| {
+                                        crate::join_remote_project(
+                                            leader_project_id,
+                                            leader_user_id,
+                                            this.app_state().clone(),
+                                            cx,
+                                        )
+                                        .detach_and_log_err(cx);
                                     })
                                     .aligned()
                                     .bottom()

crates/workspace/src/workspace.rs 🔗

@@ -2832,14 +2832,12 @@ impl Workspace {
             .read_with(cx, |this, _| this.project.clone())
             .ok_or_else(|| anyhow!("window dropped"))?;
 
-        let replica_id = project
-            .read_with(cx, |project, _| {
-                project
-                    .collaborators()
-                    .get(&leader_id)
-                    .map(|c| c.replica_id)
-            })
-            .ok_or_else(|| anyhow!("no such collaborator {}", leader_id))?;
+        let replica_id = project.read_with(cx, |project, _| {
+            project
+                .collaborators()
+                .get(&leader_id)
+                .map(|c| c.replica_id)
+        });
 
         let item_builders = cx.update(|cx| {
             cx.default_global::<FollowableItemBuilders>()
@@ -2884,7 +2882,7 @@ impl Workspace {
                     .get_mut(&pane)?;
 
                 for (id, item) in leader_view_ids.into_iter().zip(items) {
-                    item.set_leader_replica_id(Some(replica_id), cx);
+                    item.set_leader_replica_id(replica_id, cx);
                     state.items_by_leader_view_id.insert(id, item);
                 }
 
@@ -2947,30 +2945,40 @@ impl Workspace {
         let room = call.read(cx).room()?.read(cx);
         let participant = room.remote_participant_for_peer_id(leader_id)?;
         let mut items_to_activate = Vec::new();
+
+        let leader_in_this_app;
+        let leader_in_this_project;
         match participant.location {
             call::ParticipantLocation::SharedProject { project_id } => {
-                if Some(project_id) == self.project.read(cx).remote_id() {
-                    for (pane, state) in self.follower_states_by_leader.get(&leader_id)? {
-                        if let Some(item) = state
-                            .active_view_id
-                            .and_then(|id| state.items_by_leader_view_id.get(&id))
-                        {
-                            items_to_activate.push((pane.clone(), item.boxed_clone()));
-                        } else if let Some(shared_screen) =
-                            self.shared_screen_for_peer(leader_id, pane, cx)
-                        {
-                            items_to_activate.push((pane.clone(), Box::new(shared_screen)));
-                        }
-                    }
-                }
+                leader_in_this_app = true;
+                leader_in_this_project = Some(project_id) == self.project.read(cx).remote_id();
+            }
+            call::ParticipantLocation::UnsharedProject => {
+                leader_in_this_app = true;
+                leader_in_this_project = false;
             }
-            call::ParticipantLocation::UnsharedProject => {}
             call::ParticipantLocation::External => {
-                for (pane, _) in self.follower_states_by_leader.get(&leader_id)? {
-                    if let Some(shared_screen) = self.shared_screen_for_peer(leader_id, pane, cx) {
-                        items_to_activate.push((pane.clone(), Box::new(shared_screen)));
+                leader_in_this_app = false;
+                leader_in_this_project = false;
+            }
+        };
+
+        for (pane, state) in self.follower_states_by_leader.get(&leader_id)? {
+            let item = state
+                .active_view_id
+                .and_then(|id| state.items_by_leader_view_id.get(&id));
+            let shared_screen = self.shared_screen_for_peer(leader_id, pane, cx);
+
+            if leader_in_this_app {
+                if let Some(item) = item {
+                    if leader_in_this_project || !item.is_project_item(cx) {
+                        items_to_activate.push((pane.clone(), item.boxed_clone()));
                     }
+                } else if let Some(shared_screen) = shared_screen {
+                    items_to_activate.push((pane.clone(), Box::new(shared_screen)));
                 }
+            } else if let Some(shared_screen) = shared_screen {
+                items_to_activate.push((pane.clone(), Box::new(shared_screen)));
             }
         }