diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index 19411ed892b98c613f15d52e19e1365af0738610..76cc8cb9e1793f8b40f184a80b8d9efaaeb6e6d3 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -599,152 +599,6 @@ async fn test_channel_buffers_and_server_restarts( }); } -#[gpui::test(iterations = 10)] -async fn test_following_to_channel_notes_without_a_shared_project( - deterministic: BackgroundExecutor, - mut cx_a: &mut TestAppContext, - mut cx_b: &mut TestAppContext, - mut cx_c: &mut TestAppContext, -) { - let mut server = TestServer::start(deterministic.clone()).await; - let client_a = server.create_client(cx_a, "user_a").await; - 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)], - ) - .await; - let channel_2_id = server - .make_channel( - "channel-2", - None, - (&client_a, cx_a), - &mut [(&client_b, cx_b), (&client_c, cx_c)], - ) - .await; - - // Clients A, B, and C join a channel. - let active_call_a = cx_a.read(ActiveCall::global); - let active_call_b = cx_b.read(ActiveCall::global); - let active_call_c = cx_c.read(ActiveCall::global); - for (call, cx) in [ - (&active_call_a, &mut cx_a), - (&active_call_b, &mut cx_b), - (&active_call_c, &mut cx_c), - ] { - call.update(*cx, |call, cx| call.join_channel(channel_1_id, cx)) - .await - .unwrap(); - } - deterministic.run_until_parked(); - - // Clients A, B, and C all open their own unshared projects. - client_a.fs().insert_tree("/a", json!({})).await; - client_b.fs().insert_tree("/b", json!({})).await; - client_c.fs().insert_tree("/c", json!({})).await; - let (project_a, _) = client_a.build_local_project("/a", cx_a).await; - let (project_b, _) = client_b.build_local_project("/b", cx_b).await; - let (project_c, _) = client_b.build_local_project("/c", cx_c).await; - let (workspace_a, cx_a) = client_a.build_workspace(&project_a, cx_a); - let (workspace_b, cx_b) = client_b.build_workspace(&project_b, cx_b); - let (_workspace_c, _cx_c) = client_c.build_workspace(&project_c, 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.clone(), cx)) - .await - .unwrap(); - channel_view_1_a.update(cx_a, |notes, cx| { - assert_eq!(notes.channel(cx).unwrap().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 - .update(cx_b, |workspace, cx| { - workspace - .start_following(client_a.peer_id().unwrap(), cx) - .unwrap() - }) - .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.update(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.update(cx_b, |notes, cx| { - assert_eq!(notes.channel(cx).unwrap().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. - eprintln!("opening -------------------->"); - - let channel_view_2_a = cx_a - .update(|cx| ChannelView::open(channel_2_id, workspace_a.clone(), cx)) - .await - .unwrap(); - channel_view_2_a.update(cx_a, |notes, cx| { - assert_eq!(notes.channel(cx).unwrap().name, "channel-2"); - }); - - // Client B is taken to the notes for channel 2. - deterministic.run_until_parked(); - - eprintln!("opening <--------------------"); - - let channel_view_2_b = workspace_b.update(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.update(cx_b, |notes, cx| { - assert_eq!(notes.channel(cx).unwrap().name, "channel-2"); - }); -} - #[gpui::test] async fn test_channel_buffer_changes( deterministic: BackgroundExecutor, diff --git a/crates/collab/src/tests/following_tests.rs b/crates/collab/src/tests/following_tests.rs index 6106f8d5f1531302374de4456bf9fd9aff11b871..dc5488ebb335ba0e1ecce9800a7c689d217d5a0e 100644 --- a/crates/collab/src/tests/following_tests.rs +++ b/crates/collab/src/tests/following_tests.rs @@ -1,9 +1,12 @@ use crate::{rpc::RECONNECT_TIMEOUT, tests::TestServer}; use call::{ActiveCall, ParticipantLocation}; -use collab_ui::notifications::project_shared_notification::ProjectSharedNotification; +use collab_ui::{ + channel_view::ChannelView, + notifications::project_shared_notification::ProjectSharedNotification, +}; use editor::{Editor, ExcerptRange, MultiBuffer}; use gpui::{ - point, BackgroundExecutor, Context, SharedString, TestAppContext, View, VisualContext, + point, BackgroundExecutor, Context, Entity, SharedString, TestAppContext, View, VisualContext, VisualTestContext, }; use language::Capability; @@ -1822,3 +1825,167 @@ fn pane_summaries(workspace: &View, cx: &mut VisualTestContext) -> Ve .collect() }) } + +#[gpui::test(iterations = 10)] +async fn test_following_to_channel_notes_without_a_shared_project( + deterministic: BackgroundExecutor, + mut cx_a: &mut TestAppContext, + mut cx_b: &mut TestAppContext, + mut cx_c: &mut TestAppContext, +) { + let mut server = TestServer::start(deterministic.clone()).await; + let client_a = server.create_client(cx_a, "user_a").await; + 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)], + ) + .await; + let channel_2_id = server + .make_channel( + "channel-2", + None, + (&client_a, cx_a), + &mut [(&client_b, cx_b), (&client_c, cx_c)], + ) + .await; + + // Clients A, B, and C join a channel. + let active_call_a = cx_a.read(ActiveCall::global); + let active_call_b = cx_b.read(ActiveCall::global); + let active_call_c = cx_c.read(ActiveCall::global); + for (call, cx) in [ + (&active_call_a, &mut cx_a), + (&active_call_b, &mut cx_b), + (&active_call_c, &mut cx_c), + ] { + call.update(*cx, |call, cx| call.join_channel(channel_1_id, cx)) + .await + .unwrap(); + } + deterministic.run_until_parked(); + + // Clients A, B, and C all open their own unshared projects. + client_a + .fs() + .insert_tree("/a", json!({ "1.txt": "" })) + .await; + client_b.fs().insert_tree("/b", json!({})).await; + client_c.fs().insert_tree("/c", json!({})).await; + let (project_a, worktree_id) = client_a.build_local_project("/a", cx_a).await; + let (project_b, _) = client_b.build_local_project("/b", cx_b).await; + let (project_c, _) = client_b.build_local_project("/c", cx_c).await; + let (workspace_a, cx_a) = client_a.build_workspace(&project_a, cx_a); + let (workspace_b, cx_b) = client_b.build_workspace(&project_b, cx_b); + let (_workspace_c, _cx_c) = client_c.build_workspace(&project_c, 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_notes_1_a = cx_a + .update(|cx| ChannelView::open(channel_1_id, workspace_a.clone(), cx)) + .await + .unwrap(); + channel_notes_1_a.update(cx_a, |notes, cx| { + assert_eq!(notes.channel(cx).unwrap().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 + .update(cx_b, |workspace, cx| { + workspace + .start_following(client_a.peer_id().unwrap(), cx) + .unwrap() + }) + .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_notes_1_b = workspace_b.update(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_notes_1_b.update(cx_b, |notes, cx| { + assert_eq!(notes.channel(cx).unwrap().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_notes_2_a = cx_a + .update(|cx| ChannelView::open(channel_2_id, workspace_a.clone(), cx)) + .await + .unwrap(); + channel_notes_2_a.update(cx_a, |notes, cx| { + assert_eq!(notes.channel(cx).unwrap().name, "channel-2"); + }); + + // Client B is taken to the notes for channel 2. + deterministic.run_until_parked(); + let channel_notes_2_b = workspace_b.update(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_notes_2_b.update(cx_b, |notes, cx| { + assert_eq!(notes.channel(cx).unwrap().name, "channel-2"); + }); + + // Client A opens a local buffer in their unshared project. + let _unshared_editor_a1 = workspace_a + .update(cx_a, |workspace, cx| { + workspace.open_path((worktree_id, "1.txt"), None, true, cx) + }) + .await + .unwrap() + .downcast::() + .unwrap(); + + // This does not send any leader update message to client B. + // If it did, an error would occur on client B, since this buffer + // is not shared with them. + deterministic.run_until_parked(); + workspace_b.update(cx_b, |workspace, cx| { + assert_eq!( + workspace.active_item(cx).expect("no active item").item_id(), + channel_notes_2_b.entity_id() + ); + }); +} diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 78f9b150512e6ee153b3165e88bc162ea015aa14..a8583d48afae6abfee508026a65d109f78aafc95 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -82,7 +82,9 @@ impl FollowableItem for Editor { let pane = pane.downgrade(); Some(cx.spawn(|mut cx| async move { - let mut buffers = futures::future::try_join_all(buffers).await?; + let mut buffers = futures::future::try_join_all(buffers) + .await + .debug_assert_ok("leaders don't share views for unshared buffers")?; let editor = pane.update(&mut cx, |pane, cx| { let mut editors = pane.items_of_type::(); editors.find(|editor| { diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index e32dd88b86fab6f0e8ad3fa9de1ce7ce6e8793a6..a4031da8cde538eac1833a85d08068488086b9a7 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -137,6 +137,8 @@ pub trait ResultExt { type Ok; fn log_err(self) -> Option; + /// Assert that this result should never be an error in development or tests. + fn debug_assert_ok(self, reason: &str) -> Self; fn warn_on_err(self) -> Option; fn inspect_error(self, func: impl FnOnce(&E)) -> Self; } @@ -159,6 +161,14 @@ where } } + #[track_caller] + fn debug_assert_ok(self, reason: &str) -> Self { + if let Err(error) = &self { + debug_panic!("{reason} - {error:?}"); + } + self + } + fn warn_on_err(self) -> Option { match self { Ok(value) => Some(value), diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index db8554712e68b091fa59d20f71c61e1824a5cf05..ca463e76e058c645839f853eadd8e0877ecca6da 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2608,11 +2608,20 @@ impl Workspace { let cx = &cx; move |item| { let item = item.to_followable_item_handle(cx)?; - if (project_id.is_none() || project_id != follower_project_id) - && item.is_project_item(cx) + + // If the item belongs to a particular project, then it should + // only be included if this project is shared, and the follower + // is in thie project. + // + // Some items, like channel notes, do not belong to a particular + // project, so they should be included regardless of whether the + // current project is shared, or what project the follower is in. + if item.is_project_item(cx) + && (project_id.is_none() || project_id != follower_project_id) { return None; } + let id = item.remote_id(client, cx)?.to_proto(); let variant = item.to_state_proto(cx)?; Some(proto::View { @@ -2790,8 +2799,12 @@ impl Workspace { update: proto::update_followers::Variant, cx: &mut WindowContext, ) -> Option<()> { + // If this update only applies to for followers in the current project, + // then skip it unless this project is shared. If it applies to all + // followers, regardless of project, then set `project_id` to none, + // indicating that it goes to all followers. let project_id = if project_only { - self.project.read(cx).remote_id() + Some(self.project.read(cx).remote_id()?) } else { None };