From 4ffa167256dc91335473fb631af625d4a6d6c7d2 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 21 Sep 2023 16:13:12 -0700 Subject: [PATCH] Allow following into channel notes regardless of project --- .../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(-) diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index fc2d118cf8..baab675a1c 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/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(); - workspace_b.read_with(cx_b, |workspace, _| { + 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::() + .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::(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(); + 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::() + .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"); }); } diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 10d6baec19..b17b7b3fc2 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/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::() - .unwrap() + .expect("active item isn't a shared screen") }); // Client B activates Zed again, which causes the previous editor to become focused again. diff --git a/crates/collab_ui/src/channel_view.rs b/crates/collab_ui/src/channel_view.rs index 4e6a369db6..b66d1ab7c7 100644 --- a/crates/collab_ui/src/channel_view.rs +++ b/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::(cx) } @@ -37,9 +40,13 @@ pub struct ChannelView { } impl ChannelView { - pub fn deploy(channel_id: ChannelId, workspace: ViewHandle, cx: &mut AppContext) { + pub fn open( + channel_id: ChannelId, + workspace: ViewHandle, + cx: &mut AppContext, + ) -> Task>> { 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, workspace: ViewHandle, @@ -121,6 +127,10 @@ impl ChannelView { this } + pub fn channel(&self, cx: &AppContext) -> Arc { + self.channel_buffer.read(cx).channel() + } + fn handle_project_event( &mut self, _: ModelHandle, @@ -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 + } } diff --git a/crates/collab_ui/src/chat_panel.rs b/crates/collab_ui/src/chat_panel.rs index 082702feda..81a421e8d9 100644 --- a/crates/collab_ui/src/chat_panel.rs +++ b/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::( @@ -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(); } } } diff --git a/crates/collab_ui/src/collab_panel.rs b/crates/collab_ui/src/collab_panel.rs index 6013ea4907..a13e7a09ee 100644 --- a/crates/collab_ui/src/collab_panel.rs +++ b/crates/collab_ui/src/collab_panel.rs @@ -2745,7 +2745,7 @@ impl CollabPanel { fn open_channel_notes(&mut self, action: &OpenChannelNotes, cx: &mut ViewContext) { if let Some(workspace) = self.workspace.upgrade(cx) { - ChannelView::deploy(action.channel_id, workspace, cx); + ChannelView::open(action.channel_id, workspace, cx).detach(); } } diff --git a/crates/collab_ui/src/collab_titlebar_item.rs b/crates/collab_ui/src/collab_titlebar_item.rs index 4537bf04e0..eb70da8dba 100644 --- a/crates/collab_ui/src/collab_titlebar_item.rs +++ b/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::( - 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::( + 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::( - 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::( + 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::( - 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::( - 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::( + 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::( + 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 diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 568ea223cc..7fdbe82a9a 100644 --- a/crates/editor/src/items.rs +++ b/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( diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index ea747b3a36..de19e82c8b 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -696,6 +696,7 @@ pub trait FollowableItem: Item { message: proto::update_view::Variant, cx: &mut ViewContext, ) -> Task>; + fn is_project_item(&self, cx: &AppContext) -> bool; fn set_leader_replica_id(&mut self, leader_replica_id: Option, cx: &mut ViewContext); fn should_unfollow_on_event(event: &Self::Event, cx: &AppContext) -> bool; @@ -718,6 +719,7 @@ pub trait FollowableItemHandle: ItemHandle { cx: &mut WindowContext, ) -> Task>; fn should_unfollow_on_event(&self, event: &dyn Any, cx: &AppContext) -> bool; + fn is_project_item(&self, cx: &AppContext) -> bool; } impl FollowableItemHandle for ViewHandle { @@ -769,6 +771,10 @@ impl FollowableItemHandle for ViewHandle { false } } + + fn is_project_item(&self, cx: &AppContext) -> bool { + self.read(cx).is_project_item(cx) + } } #[cfg(any(test, feature = "test-support"))] diff --git a/crates/workspace/src/pane_group.rs b/crates/workspace/src/pane_group.rs index bffdce0f3e..29068ce923 100644 --- a/crates/workspace/src/pane_group.rs +++ b/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::( 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() diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 11cb47afc5..f874f5ee16 100644 --- a/crates/workspace/src/workspace.rs +++ b/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::() @@ -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))); } }