From 83455028b0343fe952bcd3edff928885e53b4f2a Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 19 Sep 2023 09:17:57 -0700 Subject: [PATCH 01/24] Procfile: run zed.dev via 'next dev', not 'vercel dev' --- Procfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Procfile b/Procfile index 127fffbed1f571986fd496e867b5b3fa82f97262..f6fde3cd92c91a448e194e87ff6668af89260382 100644 --- a/Procfile +++ b/Procfile @@ -1,4 +1,4 @@ -web: cd ../zed.dev && PORT=3000 npx vercel dev +web: cd ../zed.dev && PORT=3000 npm run dev collab: cd crates/collab && cargo run serve livekit: livekit-server --dev postgrest: postgrest crates/collab/admin_api.conf From c71566e7f5a1a7023214bb5ae91630f6cae42e8d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 19 Sep 2023 15:37:33 -0700 Subject: [PATCH 02/24] Make project id optional when following - server only --- .../20221109000000_test_schema.sql | 2 +- ...142700_allow_following_without_project.sql | 1 + crates/collab/src/db/queries/projects.rs | 95 +++++++++++++++++-- crates/collab/src/db/queries/rooms.rs | 61 +++++++++++- crates/collab/src/db/tables/follower.rs | 2 +- .../collab/src/db/tables/room_participant.rs | 10 ++ crates/collab/src/rpc.rs | 60 ++++++------ crates/rpc/proto/zed.proto | 23 +++-- crates/rpc/src/proto.rs | 3 - crates/rpc/src/rpc.rs | 2 +- 10 files changed, 204 insertions(+), 55 deletions(-) create mode 100644 crates/collab/migrations/20230918142700_allow_following_without_project.sql diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index ab12039b10bef0f02b117720d2d53e9d99080792..d0c4ead5adca03ff6af2bbc3797560a9488bf2b7 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -175,7 +175,7 @@ CREATE TABLE "servers" ( CREATE TABLE "followers" ( "id" INTEGER PRIMARY KEY AUTOINCREMENT, "room_id" INTEGER NOT NULL REFERENCES rooms (id) ON DELETE CASCADE, - "project_id" INTEGER NOT NULL REFERENCES projects (id) ON DELETE CASCADE, + "project_id" INTEGER REFERENCES projects (id) ON DELETE CASCADE, "leader_connection_server_id" INTEGER NOT NULL REFERENCES servers (id) ON DELETE CASCADE, "leader_connection_id" INTEGER NOT NULL, "follower_connection_server_id" INTEGER NOT NULL REFERENCES servers (id) ON DELETE CASCADE, diff --git a/crates/collab/migrations/20230918142700_allow_following_without_project.sql b/crates/collab/migrations/20230918142700_allow_following_without_project.sql new file mode 100644 index 0000000000000000000000000000000000000000..e0cc0141ec07596340e1ca4655324db9ea11f19f --- /dev/null +++ b/crates/collab/migrations/20230918142700_allow_following_without_project.sql @@ -0,0 +1 @@ +ALTER TABLE followers ALTER COLUMN project_id DROP NOT NULL; diff --git a/crates/collab/src/db/queries/projects.rs b/crates/collab/src/db/queries/projects.rs index 31c7cdae3e4837dd46487526fbf5c20a021d9b7d..80e71eb1eb4dd59d819f57d2f9cc04d484b3840a 100644 --- a/crates/collab/src/db/queries/projects.rs +++ b/crates/collab/src/db/queries/projects.rs @@ -738,7 +738,7 @@ impl Database { Condition::any() .add( Condition::all() - .add(follower::Column::ProjectId.eq(project_id)) + .add(follower::Column::ProjectId.eq(Some(project_id))) .add( follower::Column::LeaderConnectionServerId .eq(connection.owner_id), @@ -747,7 +747,7 @@ impl Database { ) .add( Condition::all() - .add(follower::Column::ProjectId.eq(project_id)) + .add(follower::Column::ProjectId.eq(Some(project_id))) .add( follower::Column::FollowerConnectionServerId .eq(connection.owner_id), @@ -862,13 +862,95 @@ impl Database { .await } + pub async fn check_can_follow( + &self, + room_id: RoomId, + project_id: Option, + leader_id: ConnectionId, + follower_id: ConnectionId, + ) -> Result<()> { + let mut found_leader = false; + let mut found_follower = false; + self.transaction(|tx| async move { + if let Some(project_id) = project_id { + let mut rows = project_collaborator::Entity::find() + .filter(project_collaborator::Column::ProjectId.eq(project_id)) + .stream(&*tx) + .await?; + while let Some(row) = rows.next().await { + let row = row?; + let connection = row.connection(); + if connection == leader_id { + found_leader = true; + } else if connection == follower_id { + found_follower = true; + } + } + } else { + let mut rows = room_participant::Entity::find() + .filter(room_participant::Column::RoomId.eq(room_id)) + .stream(&*tx) + .await?; + while let Some(row) = rows.next().await { + let row = row?; + if let Some(connection) = row.answering_connection() { + if connection == leader_id { + found_leader = true; + } else if connection == follower_id { + found_follower = true; + } + } + } + } + + if !found_leader || !found_follower { + Err(anyhow!("not a room participant"))?; + } + + Ok(()) + }) + .await + } + + pub async fn check_can_unfollow( + &self, + room_id: RoomId, + project_id: Option, + leader_id: ConnectionId, + follower_id: ConnectionId, + ) -> Result<()> { + self.transaction(|tx| async move { + follower::Entity::find() + .filter( + Condition::all() + .add(follower::Column::RoomId.eq(room_id)) + .add(follower::Column::ProjectId.eq(project_id)) + .add(follower::Column::LeaderConnectionId.eq(leader_id.id as i32)) + .add(follower::Column::FollowerConnectionId.eq(follower_id.id as i32)) + .add( + follower::Column::LeaderConnectionServerId + .eq(leader_id.owner_id as i32), + ) + .add( + follower::Column::FollowerConnectionServerId + .eq(follower_id.owner_id as i32), + ), + ) + .one(&*tx) + .await? + .ok_or_else(|| anyhow!("not a follower"))?; + Ok(()) + }) + .await + } + pub async fn follow( &self, - project_id: ProjectId, + room_id: RoomId, + project_id: Option, leader_connection: ConnectionId, follower_connection: ConnectionId, ) -> Result> { - let room_id = self.room_id_for_project(project_id).await?; self.room_transaction(room_id, |tx| async move { follower::ActiveModel { room_id: ActiveValue::set(room_id), @@ -894,15 +976,16 @@ impl Database { pub async fn unfollow( &self, - project_id: ProjectId, + room_id: RoomId, + project_id: Option, leader_connection: ConnectionId, follower_connection: ConnectionId, ) -> Result> { - let room_id = self.room_id_for_project(project_id).await?; self.room_transaction(room_id, |tx| async move { follower::Entity::delete_many() .filter( Condition::all() + .add(follower::Column::RoomId.eq(room_id)) .add(follower::Column::ProjectId.eq(project_id)) .add( follower::Column::LeaderConnectionServerId diff --git a/crates/collab/src/db/queries/rooms.rs b/crates/collab/src/db/queries/rooms.rs index fb81fef176ab8bfa38ece1b4beb284088a96e0bb..651d58c265969311115293af78c16ed777904702 100644 --- a/crates/collab/src/db/queries/rooms.rs +++ b/crates/collab/src/db/queries/rooms.rs @@ -960,6 +960,65 @@ impl Database { Ok(room) } + pub async fn room_id_for_connection(&self, connection_id: ConnectionId) -> Result { + #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] + enum QueryRoomId { + RoomId, + } + + self.transaction(|tx| async move { + Ok(room_participant::Entity::find() + .select_only() + .column(room_participant::Column::RoomId) + .filter( + Condition::all() + .add(room_participant::Column::AnsweringConnectionId.eq(connection_id.id)) + .add( + room_participant::Column::AnsweringConnectionServerId + .eq(ServerId(connection_id.owner_id as i32)), + ), + ) + .into_values::<_, QueryRoomId>() + .one(&*tx) + .await? + .ok_or_else(|| anyhow!("no room for connection {:?}", connection_id))?) + }) + .await + } + + pub async fn room_connection_ids( + &self, + room_id: RoomId, + connection_id: ConnectionId, + ) -> Result>> { + self.room_transaction(room_id, |tx| async move { + let mut participants = room_participant::Entity::find() + .filter(room_participant::Column::RoomId.eq(room_id)) + .stream(&*tx) + .await?; + + let mut is_participant = false; + let mut connection_ids = HashSet::default(); + while let Some(participant) = participants.next().await { + let participant = participant?; + if let Some(answering_connection) = participant.answering_connection() { + if answering_connection == connection_id { + is_participant = true; + } else { + connection_ids.insert(answering_connection); + } + } + } + + if !is_participant { + Err(anyhow!("not a room participant"))?; + } + + Ok(connection_ids) + }) + .await + } + async fn get_channel_room( &self, room_id: RoomId, @@ -1064,7 +1123,7 @@ impl Database { followers.push(proto::Follower { leader_id: Some(db_follower.leader_connection().into()), follower_id: Some(db_follower.follower_connection().into()), - project_id: db_follower.project_id.to_proto(), + project_id: db_follower.project_id.map(|id| id.to_proto()), }); } diff --git a/crates/collab/src/db/tables/follower.rs b/crates/collab/src/db/tables/follower.rs index ffd45434e9006d09939a369a3010cc85393f7dde..b5bc163b21d5b04ddaaaafb7ece5d81793a14df1 100644 --- a/crates/collab/src/db/tables/follower.rs +++ b/crates/collab/src/db/tables/follower.rs @@ -8,7 +8,7 @@ pub struct Model { #[sea_orm(primary_key)] pub id: FollowerId, pub room_id: RoomId, - pub project_id: ProjectId, + pub project_id: Option, pub leader_connection_server_id: ServerId, pub leader_connection_id: i32, pub follower_connection_server_id: ServerId, diff --git a/crates/collab/src/db/tables/room_participant.rs b/crates/collab/src/db/tables/room_participant.rs index 537cac9f14fcbe39e19915571e2833086c115888..57d79fa8306b8e42ce62966b64b306efe024d1cc 100644 --- a/crates/collab/src/db/tables/room_participant.rs +++ b/crates/collab/src/db/tables/room_participant.rs @@ -1,4 +1,5 @@ use crate::db::{ProjectId, RoomId, RoomParticipantId, ServerId, UserId}; +use rpc::ConnectionId; use sea_orm::entity::prelude::*; #[derive(Clone, Debug, PartialEq, Eq, DeriveEntityModel)] @@ -19,6 +20,15 @@ pub struct Model { pub calling_connection_server_id: Option, } +impl Model { + pub fn answering_connection(&self) -> Option { + Some(ConnectionId { + owner_id: self.answering_connection_server_id?.0 as u32, + id: self.answering_connection_id? as u32, + }) + } +} + #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] pub enum Relation { #[sea_orm( diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index fa9cc5ef1e97c05871c4ad07c9372ae9d47f4f3f..b3af2d4e989ca2cb0b155f239ac0befff1c55701 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -1883,24 +1883,19 @@ async fn follow( response: Response, session: Session, ) -> Result<()> { - let project_id = ProjectId::from_proto(request.project_id); + let room_id = RoomId::from_proto(request.room_id); + let project_id = request.project_id.map(ProjectId::from_proto); let leader_id = request .leader_id .ok_or_else(|| anyhow!("invalid leader id"))? .into(); let follower_id = session.connection_id; - { - let project_connection_ids = session - .db() - .await - .project_connection_ids(project_id, session.connection_id) - .await?; - - if !project_connection_ids.contains(&leader_id) { - Err(anyhow!("no such peer"))?; - } - } + session + .db() + .await + .check_can_follow(room_id, project_id, leader_id, session.connection_id) + .await?; let mut response_payload = session .peer @@ -1914,7 +1909,7 @@ async fn follow( let room = session .db() .await - .follow(project_id, leader_id, follower_id) + .follow(room_id, project_id, leader_id, follower_id) .await?; room_updated(&room, &session.peer); @@ -1922,22 +1917,19 @@ async fn follow( } async fn unfollow(request: proto::Unfollow, session: Session) -> Result<()> { - let project_id = ProjectId::from_proto(request.project_id); + let room_id = RoomId::from_proto(request.room_id); + let project_id = request.project_id.map(ProjectId::from_proto); let leader_id = request .leader_id .ok_or_else(|| anyhow!("invalid leader id"))? .into(); let follower_id = session.connection_id; - if !session + session .db() .await - .project_connection_ids(project_id, session.connection_id) - .await? - .contains(&leader_id) - { - Err(anyhow!("no such peer"))?; - } + .check_can_unfollow(room_id, project_id, leader_id, session.connection_id) + .await?; session .peer @@ -1946,7 +1938,7 @@ async fn unfollow(request: proto::Unfollow, session: Session) -> Result<()> { let room = session .db() .await - .unfollow(project_id, leader_id, follower_id) + .unfollow(room_id, project_id, leader_id, follower_id) .await?; room_updated(&room, &session.peer); @@ -1954,13 +1946,19 @@ async fn unfollow(request: proto::Unfollow, session: Session) -> Result<()> { } async fn update_followers(request: proto::UpdateFollowers, session: Session) -> Result<()> { - let project_id = ProjectId::from_proto(request.project_id); - let project_connection_ids = session - .db - .lock() - .await - .project_connection_ids(project_id, session.connection_id) - .await?; + let room_id = RoomId::from_proto(request.room_id); + let database = session.db.lock().await; + + let connection_ids = if let Some(project_id) = request.project_id { + let project_id = ProjectId::from_proto(project_id); + database + .project_connection_ids(project_id, session.connection_id) + .await? + } else { + database + .room_connection_ids(room_id, session.connection_id) + .await? + }; let leader_id = request.variant.as_ref().and_then(|variant| match variant { proto::update_followers::Variant::CreateView(payload) => payload.leader_id, @@ -1969,9 +1967,7 @@ async fn update_followers(request: proto::UpdateFollowers, session: Session) -> }); for follower_peer_id in request.follower_ids.iter().copied() { let follower_connection_id = follower_peer_id.into(); - if project_connection_ids.contains(&follower_connection_id) - && Some(follower_peer_id) != leader_id - { + if Some(follower_peer_id) != leader_id && connection_ids.contains(&follower_connection_id) { session.peer.forward_send( session.connection_id, follower_connection_id, diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index c2bb9e9cef1b505d7fbfd7babdc0d3ebb50e910a..9c1ec4e613f45ca154e4dfa38b0048e32ee19117 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -274,7 +274,7 @@ message ParticipantProject { message Follower { PeerId leader_id = 1; PeerId follower_id = 2; - uint64 project_id = 3; + optional uint64 project_id = 3; } message ParticipantLocation { @@ -1213,8 +1213,9 @@ message UpdateDiagnostics { } message Follow { - uint64 project_id = 1; - PeerId leader_id = 2; + uint64 room_id = 1; + optional uint64 project_id = 2; + PeerId leader_id = 3; } message FollowResponse { @@ -1223,18 +1224,20 @@ message FollowResponse { } message UpdateFollowers { - uint64 project_id = 1; - repeated PeerId follower_ids = 2; + uint64 room_id = 1; + optional uint64 project_id = 2; + repeated PeerId follower_ids = 3; oneof variant { - UpdateActiveView update_active_view = 3; - View create_view = 4; - UpdateView update_view = 5; + UpdateActiveView update_active_view = 4; + View create_view = 5; + UpdateView update_view = 6; } } message Unfollow { - uint64 project_id = 1; - PeerId leader_id = 2; + uint64 room_id = 1; + optional uint64 project_id = 2; + PeerId leader_id = 3; } message GetPrivateUserInfo {} diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 44a7df3b749903fb94b9bb830b1b4b31401263b2..48e9eef7101dff922cfc249264e1f1696bc3b280 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -364,7 +364,6 @@ entity_messages!( CreateProjectEntry, DeleteProjectEntry, ExpandProjectEntry, - Follow, FormatBuffers, GetCodeActions, GetCompletions, @@ -392,12 +391,10 @@ entity_messages!( SearchProject, StartLanguageServer, SynchronizeBuffers, - Unfollow, UnshareProject, UpdateBuffer, UpdateBufferFile, UpdateDiagnosticSummary, - UpdateFollowers, UpdateLanguageServer, UpdateProject, UpdateProjectCollaborator, diff --git a/crates/rpc/src/rpc.rs b/crates/rpc/src/rpc.rs index a1393f56e99a2d5bd83858d3a3adc1359ef91596..942672b94bab387a95bfe37bb6b73308e73496dc 100644 --- a/crates/rpc/src/rpc.rs +++ b/crates/rpc/src/rpc.rs @@ -6,4 +6,4 @@ pub use conn::Connection; pub use peer::*; mod macros; -pub const PROTOCOL_VERSION: u32 = 63; +pub const PROTOCOL_VERSION: u32 = 64; From f34c6bd1ced921ebd3f60d5b0066237b678edb1e Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 19 Sep 2023 17:25:42 -0700 Subject: [PATCH 03/24] Start work on allowing following without a shared project --- crates/call/src/call.rs | 194 +++++++++++++++++++++++++++++- crates/call/src/room.rs | 4 +- crates/workspace/src/workspace.rs | 186 +++++++++++----------------- 3 files changed, 263 insertions(+), 121 deletions(-) diff --git a/crates/call/src/call.rs b/crates/call/src/call.rs index 4db298fe98eb5718f1d641dd6539d2005f90ff4b..3c5e18713b5164779a16e0dd841d7e998eb0f44a 100644 --- a/crates/call/src/call.rs +++ b/crates/call/src/call.rs @@ -8,19 +8,23 @@ use anyhow::{anyhow, Result}; use audio::Audio; use call_settings::CallSettings; use channel::ChannelId; -use client::{proto, ClickhouseEvent, Client, TelemetrySettings, TypedEnvelope, User, UserStore}; +use client::{ + proto::{self, PeerId}, + ClickhouseEvent, Client, TelemetrySettings, TypedEnvelope, User, UserStore, +}; use collections::HashSet; use futures::{future::Shared, FutureExt}; use postage::watch; use gpui::{ - AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, Subscription, Task, - WeakModelHandle, + AnyViewHandle, AnyWeakViewHandle, AppContext, AsyncAppContext, Entity, ModelContext, + ModelHandle, Subscription, Task, ViewContext, WeakModelHandle, }; use project::Project; pub use participant::ParticipantLocation; pub use room::Room; +use util::ResultExt; pub fn init(client: Arc, user_store: ModelHandle, cx: &mut AppContext) { settings::register::(cx); @@ -49,9 +53,25 @@ pub struct ActiveCall { ), client: Arc, user_store: ModelHandle, + follow_handlers: Vec, + followers: Vec, _subscriptions: Vec, } +#[derive(PartialEq, Eq, PartialOrd, Ord)] +struct Follower { + project_id: Option, + peer_id: PeerId, +} + +struct FollowHandler { + project_id: Option, + root_view: AnyWeakViewHandle, + get_views: + Box, &mut AppContext) -> Option>, + update_view: Box, +} + impl Entity for ActiveCall { type Event = room::Event; } @@ -68,9 +88,14 @@ impl ActiveCall { location: None, pending_invites: Default::default(), incoming_call: watch::channel(), + follow_handlers: Default::default(), + followers: Default::default(), _subscriptions: vec![ client.add_request_handler(cx.handle(), Self::handle_incoming_call), client.add_message_handler(cx.handle(), Self::handle_call_canceled), + client.add_request_handler(cx.handle(), Self::handle_follow), + client.add_message_handler(cx.handle(), Self::handle_unfollow), + client.add_message_handler(cx.handle(), Self::handle_update_followers), ], client, user_store, @@ -81,6 +106,48 @@ impl ActiveCall { self.room()?.read(cx).channel_id() } + pub fn add_follow_handler( + &mut self, + root_view: gpui::ViewHandle, + project_id: Option, + get_views: GetViews, + update_view: UpdateView, + _cx: &mut ModelContext, + ) where + GetViews: 'static + + Fn(&mut V, Option, &mut gpui::ViewContext) -> Result, + UpdateView: + 'static + Fn(&mut V, PeerId, proto::UpdateFollowers, &mut ViewContext) -> Result<()>, + { + self.follow_handlers + .retain(|h| h.root_view.id() != root_view.id()); + if let Err(ix) = self + .follow_handlers + .binary_search_by_key(&(project_id, root_view.id()), |f| { + (f.project_id, f.root_view.id()) + }) + { + self.follow_handlers.insert( + ix, + FollowHandler { + project_id, + root_view: root_view.into_any().downgrade(), + get_views: Box::new(move |view, project_id, cx| { + let view = view.clone().downcast::().unwrap(); + view.update(cx, |view, cx| get_views(view, project_id, cx).log_err()) + .flatten() + }), + update_view: Box::new(move |view, leader_id, message, cx| { + let view = view.clone().downcast::().unwrap(); + view.update(cx, |view, cx| { + update_view(view, leader_id, message, cx).log_err() + }); + }), + }, + ); + } + } + async fn handle_incoming_call( this: ModelHandle, envelope: TypedEnvelope, @@ -127,6 +194,127 @@ impl ActiveCall { Ok(()) } + async fn handle_follow( + this: ModelHandle, + envelope: TypedEnvelope, + _: Arc, + mut cx: AsyncAppContext, + ) -> Result { + this.update(&mut cx, |this, cx| { + let follower = Follower { + project_id: envelope.payload.project_id, + peer_id: envelope.original_sender_id()?, + }; + let active_project_id = this + .location + .as_ref() + .and_then(|project| project.upgrade(cx)?.read(cx).remote_id()); + + let mut response = proto::FollowResponse::default(); + for handler in &this.follow_handlers { + if follower.project_id != handler.project_id && follower.project_id.is_some() { + continue; + } + + let Some(root_view) = handler.root_view.upgrade(cx) else { + continue; + }; + + let Some(handler_response) = + (handler.get_views)(&root_view, follower.project_id, cx) + else { + continue; + }; + + if response.views.is_empty() { + response.views = handler_response.views; + } else { + response.views.extend_from_slice(&handler_response.views); + } + + if let Some(active_view_id) = handler_response.active_view_id.clone() { + if response.active_view_id.is_none() || handler.project_id == active_project_id + { + response.active_view_id = Some(active_view_id); + } + } + } + + if let Err(ix) = this.followers.binary_search(&follower) { + this.followers.insert(ix, follower); + } + + Ok(response) + }) + } + + async fn handle_unfollow( + this: ModelHandle, + envelope: TypedEnvelope, + _: Arc, + mut cx: AsyncAppContext, + ) -> Result<()> { + this.update(&mut cx, |this, _| { + let follower = Follower { + project_id: envelope.payload.project_id, + peer_id: envelope.original_sender_id()?, + }; + if let Err(ix) = this.followers.binary_search(&follower) { + this.followers.remove(ix); + } + Ok(()) + }) + } + + async fn handle_update_followers( + this: ModelHandle, + envelope: TypedEnvelope, + _: Arc, + mut cx: AsyncAppContext, + ) -> Result<()> { + let leader_id = envelope.original_sender_id()?; + let update = envelope.payload; + this.update(&mut cx, |this, cx| { + for handler in &this.follow_handlers { + if update.project_id != handler.project_id && update.project_id.is_some() { + continue; + } + let Some(root_view) = handler.root_view.upgrade(cx) else { + continue; + }; + (handler.update_view)(&root_view, leader_id, update.clone(), cx); + } + Ok(()) + }) + } + + pub fn update_followers( + &self, + project_id: Option, + update: proto::update_followers::Variant, + cx: &AppContext, + ) -> Option<()> { + let room_id = self.room()?.read(cx).id(); + let follower_ids: Vec<_> = self + .followers + .iter() + .filter_map(|follower| { + (follower.project_id == project_id).then_some(follower.peer_id.into()) + }) + .collect(); + if follower_ids.is_empty() { + return None; + } + self.client + .send(proto::UpdateFollowers { + room_id, + project_id, + follower_ids, + variant: Some(update), + }) + .log_err() + } + pub fn global(cx: &AppContext) -> ModelHandle { cx.global::>().clone() } diff --git a/crates/call/src/room.rs b/crates/call/src/room.rs index e7899ab2d8ebf48da7d4bddbb7eb62d49a7a7750..ffa941bfa1135c3c97dfb27e92df7870f61aa0a4 100644 --- a/crates/call/src/room.rs +++ b/crates/call/src/room.rs @@ -62,7 +62,7 @@ pub struct Room { leave_when_empty: bool, client: Arc, user_store: ModelHandle, - follows_by_leader_id_project_id: HashMap<(PeerId, u64), Vec>, + follows_by_leader_id_project_id: HashMap<(PeerId, Option), Vec>, subscriptions: Vec, pending_room_update: Option>, maintain_connection: Option>>, @@ -584,7 +584,7 @@ impl Room { pub fn followers_for(&self, leader_id: PeerId, project_id: u64) -> &[PeerId] { self.follows_by_leader_id_project_id - .get(&(leader_id, project_id)) + .get(&(leader_id, Some(project_id))) .map_or(&[], |v| v.as_slice()) } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index feab53d0941d1823fc79930d3697c39e54822207..11cb47afc55195ba73e72ac9efc20a10d7f8b9a5 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -15,7 +15,7 @@ use call::ActiveCall; use channel::ChannelStore; use client::{ proto::{self, PeerId}, - Client, TypedEnvelope, UserStore, + Client, UserStore, }; use collections::{hash_map, HashMap, HashSet}; use drag_and_drop::DragAndDrop; @@ -331,11 +331,6 @@ pub fn init(app_state: Arc, cx: &mut AppContext) { }) .detach(); }); - - let client = &app_state.client; - client.add_view_request_handler(Workspace::handle_follow); - client.add_view_message_handler(Workspace::handle_unfollow); - client.add_view_message_handler(Workspace::handle_update_followers); } type ProjectItemBuilders = HashMap< @@ -507,7 +502,6 @@ pub enum Event { pub struct Workspace { weak_self: WeakViewHandle, - remote_entity_subscription: Option, modal: Option, zoomed: Option, zoomed_position: Option, @@ -523,7 +517,6 @@ pub struct Workspace { titlebar_item: Option, notifications: Vec<(TypeId, usize, Box)>, project: ModelHandle, - leader_state: LeaderState, follower_states_by_leader: FollowerStatesByLeader, last_leaders_by_pane: HashMap, PeerId>, window_edited: bool, @@ -549,11 +542,6 @@ pub struct ViewId { pub id: u64, } -#[derive(Default)] -struct LeaderState { - followers: HashSet, -} - type FollowerStatesByLeader = HashMap, FollowerState>>; #[derive(Default)] @@ -737,12 +725,10 @@ impl Workspace { status_bar, titlebar_item: None, notifications: Default::default(), - remote_entity_subscription: None, left_dock, bottom_dock, right_dock, project: project.clone(), - leader_state: Default::default(), follower_states_by_leader: Default::default(), last_leaders_by_pane: Default::default(), window_edited: false, @@ -2419,19 +2405,21 @@ impl Workspace { } fn project_remote_id_changed(&mut self, remote_id: Option, cx: &mut ViewContext) { - if let Some(remote_id) = remote_id { - self.remote_entity_subscription = Some( - self.app_state - .client - .add_view_for_remote_entity(remote_id, cx), - ); - } else { - self.remote_entity_subscription.take(); + let handle = cx.handle(); + if let Some(call) = self.active_call() { + call.update(cx, |call, cx| { + call.add_follow_handler( + handle, + remote_id, + Self::get_views_for_followers, + Self::handle_update_followers, + cx, + ); + }); } } fn collaborator_left(&mut self, peer_id: PeerId, cx: &mut ViewContext) { - self.leader_state.followers.remove(&peer_id); if let Some(states_by_pane) = self.follower_states_by_leader.remove(&peer_id) { for state in states_by_pane.into_values() { for item in state.items_by_leader_view_id.into_values() { @@ -2463,8 +2451,10 @@ impl Workspace { .insert(pane.clone(), Default::default()); cx.notify(); - let project_id = self.project.read(cx).remote_id()?; + let room_id = self.active_call()?.read(cx).room()?.read(cx).id(); + let project_id = self.project.read(cx).remote_id(); let request = self.app_state.client.request(proto::Follow { + room_id, project_id, leader_id: Some(leader_id), }); @@ -2542,15 +2532,16 @@ impl Workspace { if states_by_pane.is_empty() { self.follower_states_by_leader.remove(&leader_id); - if let Some(project_id) = self.project.read(cx).remote_id() { - self.app_state - .client - .send(proto::Unfollow { - project_id, - leader_id: Some(leader_id), - }) - .log_err(); - } + let project_id = self.project.read(cx).remote_id(); + let room_id = self.active_call()?.read(cx).room()?.read(cx).id(); + self.app_state + .client + .send(proto::Unfollow { + room_id, + project_id, + leader_id: Some(leader_id), + }) + .log_err(); } cx.notify(); @@ -2564,10 +2555,6 @@ impl Workspace { self.follower_states_by_leader.contains_key(&peer_id) } - pub fn is_followed_by(&self, peer_id: PeerId) -> bool { - self.leader_state.followers.contains(&peer_id) - } - fn render_titlebar(&self, theme: &Theme, cx: &mut ViewContext) -> AnyElement { // TODO: There should be a better system in place for this // (https://github.com/zed-industries/zed/issues/1290) @@ -2718,80 +2705,56 @@ impl Workspace { // RPC handlers - async fn handle_follow( - this: WeakViewHandle, - envelope: TypedEnvelope, - _: Arc, - mut cx: AsyncAppContext, + fn get_views_for_followers( + &mut self, + _project_id: Option, + cx: &mut ViewContext, ) -> Result { - this.update(&mut cx, |this, cx| { - let client = &this.app_state.client; - this.leader_state - .followers - .insert(envelope.original_sender_id()?); + let client = &self.app_state.client; - let active_view_id = this.active_item(cx).and_then(|i| { - Some( - i.to_followable_item_handle(cx)? - .remote_id(client, cx)? - .to_proto(), - ) - }); + let active_view_id = self.active_item(cx).and_then(|i| { + Some( + i.to_followable_item_handle(cx)? + .remote_id(client, cx)? + .to_proto(), + ) + }); - cx.notify(); + cx.notify(); - Ok(proto::FollowResponse { - active_view_id, - views: this - .panes() - .iter() - .flat_map(|pane| { - let leader_id = this.leader_for_pane(pane); - pane.read(cx).items().filter_map({ - let cx = &cx; - move |item| { - let item = item.to_followable_item_handle(cx)?; - let id = item.remote_id(client, cx)?.to_proto(); - let variant = item.to_state_proto(cx)?; - Some(proto::View { - id: Some(id), - leader_id, - variant: Some(variant), - }) - } - }) + Ok(proto::FollowResponse { + active_view_id, + views: self + .panes() + .iter() + .flat_map(|pane| { + let leader_id = self.leader_for_pane(pane); + pane.read(cx).items().filter_map({ + let cx = &cx; + move |item| { + let item = item.to_followable_item_handle(cx)?; + let id = item.remote_id(client, cx)?.to_proto(); + let variant = item.to_state_proto(cx)?; + Some(proto::View { + id: Some(id), + leader_id, + variant: Some(variant), + }) + } }) - .collect(), - }) - })? - } - - async fn handle_unfollow( - this: WeakViewHandle, - envelope: TypedEnvelope, - _: Arc, - mut cx: AsyncAppContext, - ) -> Result<()> { - this.update(&mut cx, |this, cx| { - this.leader_state - .followers - .remove(&envelope.original_sender_id()?); - cx.notify(); - Ok(()) - })? + }) + .collect(), + }) } - async fn handle_update_followers( - this: WeakViewHandle, - envelope: TypedEnvelope, - _: Arc, - cx: AsyncAppContext, + fn handle_update_followers( + &mut self, + leader_id: PeerId, + message: proto::UpdateFollowers, + _cx: &mut ViewContext, ) -> Result<()> { - let leader_id = envelope.original_sender_id()?; - this.read_with(&cx, |this, _| { - this.leader_updates_tx - .unbounded_send((leader_id, envelope.payload)) - })??; + self.leader_updates_tx + .unbounded_send((leader_id, message))?; Ok(()) } @@ -2960,18 +2923,9 @@ impl Workspace { update: proto::update_followers::Variant, cx: &AppContext, ) -> Option<()> { - let project_id = self.project.read(cx).remote_id()?; - if !self.leader_state.followers.is_empty() { - self.app_state - .client - .send(proto::UpdateFollowers { - project_id, - follower_ids: self.leader_state.followers.iter().copied().collect(), - variant: Some(update), - }) - .log_err(); - } - None + self.active_call()? + .read(cx) + .update_followers(self.project.read(cx).remote_id(), update, cx) } pub fn leader_for_pane(&self, pane: &ViewHandle) -> Option { From ed8b022b51d75c891378525f12fae9412b6bddfb Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 19 Sep 2023 18:03:38 -0700 Subject: [PATCH 04/24] Add initial failing test for following to channel notes in an unshared project --- .../collab/src/tests/channel_buffer_tests.rs | 91 ++++++++++++++++++- crates/collab/src/tests/test_server.rs | 4 +- crates/collab_ui/src/channel_view.rs | 16 ++-- 3 files changed, 102 insertions(+), 9 deletions(-) diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index ba5a70895a693a3cff33f2cced0a5068acfcba93..fc2d118cf87792c1db2124234ae702b219d00c6b 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -274,7 +274,10 @@ async fn test_channel_buffer_replica_ids( } #[gpui::test] -async fn test_reopen_channel_buffer(deterministic: Arc, cx_a: &mut TestAppContext) { +async fn test_multiple_handles_to_channel_buffer( + deterministic: Arc, + cx_a: &mut TestAppContext, +) { deterministic.forbid_parking(); let mut server = TestServer::start(&deterministic).await; let client_a = server.create_client(cx_a, "user_a").await; @@ -578,6 +581,92 @@ async fn test_channel_buffers_and_server_restarts( }); } +#[gpui::test(iterations = 10)] +async fn test_following_to_channel_notes_without_a_shared_project( + deterministic: Arc, + mut cx_a: &mut TestAppContext, + mut cx_b: &mut TestAppContext, + mut cx_c: &mut TestAppContext, +) { + deterministic.forbid_parking(); + let mut server = TestServer::start(&deterministic).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; + + let channel_1_id = server + .make_channel( + "channel-1", + (&client_a, cx_a), + &mut [(&client_b, cx_b), (&client_c, cx_c)], + ) + .await; + let channel_2_id = server + .make_channel( + "channel-2", + (&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 = 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); + + // 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, + ) + }) + .await + .unwrap(); + + // Client B follows client A. + workspace_b + .update(cx_b, |workspace, cx| { + workspace + .toggle_follow(client_a.peer_id().unwrap(), cx) + .unwrap() + }) + .await + .unwrap(); + + deterministic.run_until_parked(); + workspace_b.read_with(cx_b, |workspace, _| { + assert_eq!( + workspace.leader_for_pane(workspace.active_pane()), + Some(client_a.peer_id().unwrap()) + ); + }); +} + #[track_caller] fn assert_collaborators(collaborators: &[proto::Collaborator], ids: &[Option]) { assert_eq!( diff --git a/crates/collab/src/tests/test_server.rs b/crates/collab/src/tests/test_server.rs index 6572722df347ba72e50e04c1a70d58b63b8484da..6a15cac9e9d0b2f5f31fb72ff4c57b2db5058001 100644 --- a/crates/collab/src/tests/test_server.rs +++ b/crates/collab/src/tests/test_server.rs @@ -206,11 +206,13 @@ impl TestServer { let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http, cx)); let channel_store = cx.add_model(|cx| ChannelStore::new(client.clone(), user_store.clone(), cx)); + let mut language_registry = LanguageRegistry::test(); + language_registry.set_executor(cx.background()); let app_state = Arc::new(workspace::AppState { client: client.clone(), user_store: user_store.clone(), channel_store: channel_store.clone(), - languages: Arc::new(LanguageRegistry::test()), + languages: Arc::new(language_registry), fs: fs.clone(), build_window_options: |_, _, _| Default::default(), initialize_workspace: |_, _, _, _| Task::ready(Ok(())), diff --git a/crates/collab_ui/src/channel_view.rs b/crates/collab_ui/src/channel_view.rs index c6f32cecd271fea3e61101c4cc0d8104ad2329d3..4e6a369db6f3f6e9a2732a77530ba67375773741 100644 --- a/crates/collab_ui/src/channel_view.rs +++ b/crates/collab_ui/src/channel_view.rs @@ -14,6 +14,7 @@ use gpui::{ }; use project::Project; use std::any::{Any, TypeId}; +use util::ResultExt; use workspace::{ item::{FollowableItem, Item, ItemHandle}, register_followable_item, @@ -53,7 +54,7 @@ impl ChannelView { &workspace.read(cx).app_state().client, cx, ); - pane.add_item(Box::new(channel_view), true, true, None, cx); + pane.add_item(Box::new(channel_view.clone()), true, true, None, cx); }); anyhow::Ok(()) }) @@ -79,12 +80,13 @@ impl ChannelView { cx.spawn(|mut cx| async move { let channel_buffer = channel_buffer.await?; - let markdown = markdown.await?; - channel_buffer.update(&mut cx, |buffer, cx| { - buffer.buffer().update(cx, |buffer, cx| { - buffer.set_language(Some(markdown), cx); - }) - }); + if let Some(markdown) = markdown.await.log_err() { + channel_buffer.update(&mut cx, |buffer, cx| { + buffer.buffer().update(cx, |buffer, cx| { + buffer.set_language(Some(markdown), cx); + }) + }); + } pane.update(&mut cx, |pane, cx| { pane.items_of_type::() From 4ffa167256dc91335473fb631af625d4a6d6c7d2 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 21 Sep 2023 16:13:12 -0700 Subject: [PATCH 05/24] 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 fc2d118cf87792c1db2124234ae702b219d00c6b..baab675a1c2e3decada7d292d078977bea4ff4ce 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(); + 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(); - 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::() + .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 10d6baec1901b236d1604eec87b21d728ed0c5ed..b17b7b3fc22dac4007d8d6504d048220529e2dd8 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 4e6a369db6f3f6e9a2732a77530ba67375773741..b66d1ab7c7a16ae126f8fcba1ddd66defeb04105 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 082702fedab6affba62dc88b9d5ee591b72b8938..81a421e8d904272fe56de27d20d20a8a8f036f8e 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 6013ea4907dd7d703a7c107b6c726b7cef9ae201..a13e7a09eeca826ceafcc3ba54506539be95ae1e 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 4537bf04e06759edc440165c86fb6dc3baf049ab..eb70da8dbaf1a2655bc3b8e890247ef186a6bee8 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 568ea223cc10e7037414e5ca4b18d15f18779830..7fdbe82a9a2a88f8131d9b46d60dbe70a05aacf1 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 ea747b3a364720c653f91ce7b8bc609750509fe3..de19e82c8b2fa400e1b2a0377b87b9e20231b581 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 bffdce0f3ec52d86b08241d067861eee359844d5..29068ce923020425e67f0182bfb108961603ab19 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 11cb47afc55195ba73e72ac9efc20a10d7f8b9a5..f874f5ee164a8f1f2d829cf729d5a27c7be61fa1 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))); } } From 77115307045ef4ef4c303448efa24b5ea9d6a0c7 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 21 Sep 2023 16:43:59 -0700 Subject: [PATCH 06/24] Simplify titlebar facepile click rendering / mouse handling --- crates/collab_ui/src/collab_titlebar_item.rs | 259 +++++++++---------- 1 file changed, 129 insertions(+), 130 deletions(-) diff --git a/crates/collab_ui/src/collab_titlebar_item.rs b/crates/collab_ui/src/collab_titlebar_item.rs index eb70da8dbaf1a2655bc3b8e890247ef186a6bee8..a676b40b754840e9b55e0c95144b3b6655047727 100644 --- a/crates/collab_ui/src/collab_titlebar_item.rs +++ b/crates/collab_ui/src/collab_titlebar_item.rs @@ -886,10 +886,12 @@ impl CollabTitlebarItem { theme: &Theme, cx: &mut ViewContext, ) -> AnyElement { + let user_id = user.id; let project_id = workspace.read(cx).project().read(cx).remote_id(); - let room = ActiveCall::global(cx).read(cx).room(); + let room = ActiveCall::global(cx).read(cx).room().cloned(); let is_being_followed = workspace.read(cx).is_being_followed(peer_id); let followed_by_self = room + .as_ref() .and_then(|room| { Some( is_being_followed @@ -929,142 +931,139 @@ impl CollabTitlebarItem { } } - let mut content = Stack::new() - .with_children(user.avatar.as_ref().map(|avatar| { - let face_pile = FacePile::new(theme.titlebar.follower_avatar_overlap) - .with_child(Self::render_face( - avatar.clone(), - Self::location_style(workspace, location, leader_style, cx), - background_color, - microphone_state, - )) - .with_children( - (|| { - let project_id = project_id?; - let room = room?.read(cx); - let followers = room.followers_for(peer_id, project_id); - - Some(followers.into_iter().flat_map(|&follower| { - let remote_participant = - room.remote_participant_for_peer_id(follower); - - let avatar = remote_participant - .and_then(|p| p.user.avatar.clone()) - .or_else(|| { - if follower == workspace.read(cx).client().peer_id()? { - workspace - .read(cx) - .user_store() - .read(cx) - .current_user()? - .avatar - .clone() - } else { - None - } - })?; - - Some(Self::render_face( - avatar.clone(), - follower_style, - background_color, - None, - )) - })) - })() - .into_iter() - .flatten(), - ); - - let mut container = face_pile - .contained() - .with_style(theme.titlebar.leader_selection); + enum TitlebarParticipant {} - if let Some(replica_id) = replica_id { - if followed_by_self { - let color = theme.editor.replica_selection_style(replica_id).selection; - container = container.with_background_color(color); - } - } + let content = MouseEventHandler::new::( + peer_id.as_u64() as usize, + cx, + move |_, cx| { + Stack::new() + .with_children(user.avatar.as_ref().map(|avatar| { + let face_pile = FacePile::new(theme.titlebar.follower_avatar_overlap) + .with_child(Self::render_face( + avatar.clone(), + Self::location_style(workspace, location, leader_style, cx), + background_color, + microphone_state, + )) + .with_children( + (|| { + let project_id = project_id?; + let room = room?.read(cx); + let followers = room.followers_for(peer_id, project_id); + + Some(followers.into_iter().flat_map(|&follower| { + let remote_participant = + room.remote_participant_for_peer_id(follower); + + let avatar = remote_participant + .and_then(|p| p.user.avatar.clone()) + .or_else(|| { + if follower + == workspace.read(cx).client().peer_id()? + { + workspace + .read(cx) + .user_store() + .read(cx) + .current_user()? + .avatar + .clone() + } else { + None + } + })?; + + Some(Self::render_face( + avatar.clone(), + follower_style, + background_color, + None, + )) + })) + })() + .into_iter() + .flatten(), + ); + + let mut container = face_pile + .contained() + .with_style(theme.titlebar.leader_selection); - container - })) - .with_children((|| { - let replica_id = replica_id?; - let color = theme.editor.replica_selection_style(replica_id).cursor; - Some( - AvatarRibbon::new(color) - .constrained() - .with_width(theme.titlebar.avatar_ribbon.width) - .with_height(theme.titlebar.avatar_ribbon.height) - .aligned() - .bottom(), - ) - })()) - .into_any(); + if let Some(replica_id) = replica_id { + if followed_by_self { + let color = + theme.editor.replica_selection_style(replica_id).selection; + container = container.with_background_color(color); + } + } - if let Some(location) = location { - match (replica_id, location) { - (None, ParticipantLocation::SharedProject { project_id }) => { - enum JoinProject {} + container + })) + .with_children((|| { + let replica_id = replica_id?; + let color = theme.editor.replica_selection_style(replica_id).cursor; + Some( + AvatarRibbon::new(color) + .constrained() + .with_width(theme.titlebar.avatar_ribbon.width) + .with_height(theme.titlebar.avatar_ribbon.height) + .aligned() + .bottom(), + ) + })()) + }, + ); - 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(); - } - _ => { - enum ToggleFollow {} + match (replica_id, location) { + // If the user's location isn't known, do nothing. + (_, None) => content.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); - } + // If the user is not in this project, but is in another share project, + // join that project. + (None, Some(ParticipantLocation::SharedProject { project_id })) => 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(), + + // Otherwise, follow the user in the current window. + _ => 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(); - } - } + } + }) + .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 } fn location_style( From 545b5e0161127e8de7b0896dbe03514af194867d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 26 Sep 2023 15:19:38 -0600 Subject: [PATCH 07/24] Assign unique color indices to room participants, use those instead of replica_ids Co-authored-by: Conrad Co-authored-by: Antonio --- Cargo.lock | 4 + crates/call/Cargo.toml | 1 + crates/call/src/participant.rs | 2 + crates/call/src/room.rs | 11 + crates/channel/Cargo.toml | 1 + crates/channel/src/channel_buffer.rs | 105 ++---- crates/channel/src/channel_store.rs | 3 +- crates/client/Cargo.toml | 1 + crates/client/src/user.rs | 37 +++ .../20221109000000_test_schema.sql | 3 +- ...0_add_color_index_to_room_participants.sql | 1 + crates/collab/src/db.rs | 2 +- crates/collab/src/db/queries/buffers.rs | 59 ++-- crates/collab/src/db/queries/rooms.rs | 20 ++ .../collab/src/db/tables/room_participant.rs | 1 + crates/collab/src/db/tests/buffer_tests.rs | 4 +- crates/collab/src/rpc.rs | 56 ++-- .../collab/src/tests/channel_buffer_tests.rs | 269 ++++++++------- .../src/tests/random_channel_buffer_tests.rs | 2 +- crates/collab/src/tests/test_server.rs | 22 +- crates/collab_ui/src/channel_view.rs | 105 ++---- crates/collab_ui/src/collab_titlebar_item.rs | 18 +- crates/editor/src/editor.rs | 94 ++++-- crates/editor/src/element.rs | 90 +++-- crates/editor/src/items.rs | 12 +- crates/project/Cargo.toml | 1 + crates/project/src/project.rs | 27 +- crates/rpc/proto/zed.proto | 310 +++++++++--------- crates/rpc/src/proto.rs | 8 +- crates/theme/src/theme.rs | 14 +- crates/vim/src/vim.rs | 2 +- crates/workspace/src/item.rs | 15 +- crates/workspace/src/pane_group.rs | 34 +- crates/workspace/src/workspace.rs | 16 +- selection-color-notes.txt | 14 + 35 files changed, 716 insertions(+), 648 deletions(-) create mode 100644 crates/collab/migrations/20230926102500_add_color_index_to_room_participants.sql create mode 100644 selection-color-notes.txt diff --git a/Cargo.lock b/Cargo.lock index 3cced78c4272e5fc9f571d8023719a1ea56d06d2..46d8deb62b1f3741c0daf4ac286dc92636115b1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1079,6 +1079,7 @@ dependencies = [ "serde_derive", "serde_json", "settings", + "theme", "util", ] @@ -1219,6 +1220,7 @@ dependencies = [ "sum_tree", "tempfile", "text", + "theme", "thiserror", "time", "tiny_http", @@ -1390,6 +1392,7 @@ dependencies = [ "sum_tree", "tempfile", "text", + "theme", "thiserror", "time", "tiny_http", @@ -5510,6 +5513,7 @@ dependencies = [ "tempdir", "terminal", "text", + "theme", "thiserror", "toml 0.5.11", "unindent", diff --git a/crates/call/Cargo.toml b/crates/call/Cargo.toml index b4e94fe56c3b12533d232eacf30c5edd633d5a03..716bc3c27b028c866042131ad7ff9c8c2169ab60 100644 --- a/crates/call/Cargo.toml +++ b/crates/call/Cargo.toml @@ -31,6 +31,7 @@ language = { path = "../language" } media = { path = "../media" } project = { path = "../project" } settings = { path = "../settings" } +theme = { path = "../theme" } util = { path = "../util" } anyhow.workspace = true diff --git a/crates/call/src/participant.rs b/crates/call/src/participant.rs index e7858869ce63906b75f9cd0cb117cf7b54283efd..b0751be91956a6f47e023fb8738bd227b6a4cde9 100644 --- a/crates/call/src/participant.rs +++ b/crates/call/src/participant.rs @@ -6,6 +6,7 @@ pub use live_kit_client::Frame; use live_kit_client::RemoteAudioTrack; use project::Project; use std::{fmt, sync::Arc}; +use theme::ColorIndex; #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub enum ParticipantLocation { @@ -43,6 +44,7 @@ pub struct RemoteParticipant { pub peer_id: proto::PeerId, pub projects: Vec, pub location: ParticipantLocation, + pub color_index: ColorIndex, pub muted: bool, pub speaking: bool, pub video_tracks: HashMap>, diff --git a/crates/call/src/room.rs b/crates/call/src/room.rs index ffa941bfa1135c3c97dfb27e92df7870f61aa0a4..e6759d87ca6a8af4af4fc5e04bfc3b1c26d5971c 100644 --- a/crates/call/src/room.rs +++ b/crates/call/src/room.rs @@ -21,6 +21,7 @@ use live_kit_client::{ use postage::stream::Stream; use project::Project; use std::{future::Future, mem, pin::Pin, sync::Arc, time::Duration}; +use theme::ColorIndex; use util::{post_inc, ResultExt, TryFutureExt}; pub const RECONNECT_TIMEOUT: Duration = Duration::from_secs(30); @@ -714,6 +715,7 @@ impl Room { participant.user_id, RemoteParticipant { user: user.clone(), + color_index: ColorIndex(participant.color_index), peer_id, projects: participant.projects, location, @@ -807,6 +809,15 @@ impl Room { let _ = this.leave(cx); } + this.user_store.update(cx, |user_store, cx| { + let color_indices_by_user_id = this + .remote_participants + .iter() + .map(|(user_id, participant)| (*user_id, participant.color_index)) + .collect(); + user_store.set_color_indices(color_indices_by_user_id, cx); + }); + this.check_invariants(); cx.notify(); }); diff --git a/crates/channel/Cargo.toml b/crates/channel/Cargo.toml index 16a1d418d5c17750db582252118c3ddcc1cad2d3..e3a74ecbe6e1074d3c7df16daa4d0f04d45aa65c 100644 --- a/crates/channel/Cargo.toml +++ b/crates/channel/Cargo.toml @@ -23,6 +23,7 @@ language = { path = "../language" } settings = { path = "../settings" } feature_flags = { path = "../feature_flags" } sum_tree = { path = "../sum_tree" } +theme = { path = "../theme" } anyhow.workspace = true futures.workspace = true diff --git a/crates/channel/src/channel_buffer.rs b/crates/channel/src/channel_buffer.rs index 06f9093fb5f9c628ba3ec2320f3605480f65486c..a03eb1f1b549772e9efe50c0ca160b916453eefc 100644 --- a/crates/channel/src/channel_buffer.rs +++ b/crates/channel/src/channel_buffer.rs @@ -1,22 +1,25 @@ use crate::Channel; use anyhow::Result; -use client::Client; +use client::{Client, Collaborator, UserStore}; +use collections::HashMap; use gpui::{AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle}; -use rpc::{proto, TypedEnvelope}; +use rpc::{ + proto::{self, PeerId}, + TypedEnvelope, +}; use std::sync::Arc; use util::ResultExt; pub(crate) fn init(client: &Arc) { client.add_model_message_handler(ChannelBuffer::handle_update_channel_buffer); - client.add_model_message_handler(ChannelBuffer::handle_add_channel_buffer_collaborator); - client.add_model_message_handler(ChannelBuffer::handle_remove_channel_buffer_collaborator); - client.add_model_message_handler(ChannelBuffer::handle_update_channel_buffer_collaborator); + client.add_model_message_handler(ChannelBuffer::handle_update_channel_buffer_collaborators); } pub struct ChannelBuffer { pub(crate) channel: Arc, connected: bool, - collaborators: Vec, + collaborators: HashMap, + user_store: ModelHandle, buffer: ModelHandle, buffer_epoch: u64, client: Arc, @@ -46,6 +49,7 @@ impl ChannelBuffer { pub(crate) async fn new( channel: Arc, client: Arc, + user_store: ModelHandle, mut cx: AsyncAppContext, ) -> Result> { let response = client @@ -61,8 +65,6 @@ impl ChannelBuffer { .map(language::proto::deserialize_operation) .collect::, _>>()?; - let collaborators = response.collaborators; - let buffer = cx.add_model(|_| { language::Buffer::remote(response.buffer_id, response.replica_id as u16, base_text) }); @@ -73,34 +75,45 @@ impl ChannelBuffer { anyhow::Ok(cx.add_model(|cx| { cx.subscribe(&buffer, Self::on_buffer_update).detach(); - Self { + let mut this = Self { buffer, buffer_epoch: response.epoch, client, connected: true, - collaborators, + collaborators: Default::default(), channel, subscription: Some(subscription.set_model(&cx.handle(), &mut cx.to_async())), - } + user_store, + }; + this.replace_collaborators(response.collaborators, cx); + this })) } + pub fn user_store(&self) -> &ModelHandle { + &self.user_store + } + pub(crate) fn replace_collaborators( &mut self, collaborators: Vec, cx: &mut ModelContext, ) { - for old_collaborator in &self.collaborators { - if collaborators - .iter() - .any(|c| c.replica_id == old_collaborator.replica_id) - { + let mut new_collaborators = HashMap::default(); + for collaborator in collaborators { + if let Ok(collaborator) = Collaborator::from_proto(collaborator) { + new_collaborators.insert(collaborator.peer_id, collaborator); + } + } + + for (_, old_collaborator) in &self.collaborators { + if !new_collaborators.contains_key(&old_collaborator.peer_id) { self.buffer.update(cx, |buffer, cx| { buffer.remove_peer(old_collaborator.replica_id as u16, cx) }); } } - self.collaborators = collaborators; + self.collaborators = new_collaborators; cx.emit(ChannelBufferEvent::CollaboratorsChanged); cx.notify(); } @@ -127,64 +140,14 @@ impl ChannelBuffer { Ok(()) } - async fn handle_add_channel_buffer_collaborator( - this: ModelHandle, - envelope: TypedEnvelope, - _: Arc, - mut cx: AsyncAppContext, - ) -> Result<()> { - let collaborator = envelope.payload.collaborator.ok_or_else(|| { - anyhow::anyhow!( - "Should have gotten a collaborator in the AddChannelBufferCollaborator message" - ) - })?; - - this.update(&mut cx, |this, cx| { - this.collaborators.push(collaborator); - cx.emit(ChannelBufferEvent::CollaboratorsChanged); - cx.notify(); - }); - - Ok(()) - } - - async fn handle_remove_channel_buffer_collaborator( + async fn handle_update_channel_buffer_collaborators( this: ModelHandle, - message: TypedEnvelope, + message: TypedEnvelope, _: Arc, mut cx: AsyncAppContext, ) -> Result<()> { this.update(&mut cx, |this, cx| { - this.collaborators.retain(|collaborator| { - if collaborator.peer_id == message.payload.peer_id { - this.buffer.update(cx, |buffer, cx| { - buffer.remove_peer(collaborator.replica_id as u16, cx) - }); - false - } else { - true - } - }); - cx.emit(ChannelBufferEvent::CollaboratorsChanged); - cx.notify(); - }); - - Ok(()) - } - - async fn handle_update_channel_buffer_collaborator( - this: ModelHandle, - message: TypedEnvelope, - _: Arc, - mut cx: AsyncAppContext, - ) -> Result<()> { - this.update(&mut cx, |this, cx| { - for collaborator in &mut this.collaborators { - if collaborator.peer_id == message.payload.old_peer_id { - collaborator.peer_id = message.payload.new_peer_id; - break; - } - } + this.replace_collaborators(message.payload.collaborators, cx); cx.emit(ChannelBufferEvent::CollaboratorsChanged); cx.notify(); }); @@ -217,7 +180,7 @@ impl ChannelBuffer { self.buffer.clone() } - pub fn collaborators(&self) -> &[proto::Collaborator] { + pub fn collaborators(&self) -> &HashMap { &self.collaborators } diff --git a/crates/channel/src/channel_store.rs b/crates/channel/src/channel_store.rs index a5a0a922468710cd49e0178515052b0157c876b9..a8f6dd67b6b6246e78617f029da19e50fe3ba75a 100644 --- a/crates/channel/src/channel_store.rs +++ b/crates/channel/src/channel_store.rs @@ -198,10 +198,11 @@ impl ChannelStore { cx: &mut ModelContext, ) -> Task>> { let client = self.client.clone(); + let user_store = self.user_store.clone(); self.open_channel_resource( channel_id, |this| &mut this.opened_buffers, - |channel, cx| ChannelBuffer::new(channel, client, cx), + |channel, cx| ChannelBuffer::new(channel, client, user_store, cx), cx, ) } diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index e3038e5bcc49bd41b756062b676e00f4f355867a..2bd03d789f3af1e59c0edaf153cd7d04c419307a 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -21,6 +21,7 @@ text = { path = "../text" } settings = { path = "../settings" } feature_flags = { path = "../feature_flags" } sum_tree = { path = "../sum_tree" } +theme = { path = "../theme" } anyhow.workspace = true async-recursion = "0.3" diff --git a/crates/client/src/user.rs b/crates/client/src/user.rs index 5f13aa40acee9063bfd90c10b43044ff40952db2..052254558723ad5c30941cf8e696a4aca1a064be 100644 --- a/crates/client/src/user.rs +++ b/crates/client/src/user.rs @@ -7,6 +7,8 @@ use gpui::{AsyncAppContext, Entity, ImageData, ModelContext, ModelHandle, Task}; use postage::{sink::Sink, watch}; use rpc::proto::{RequestMessage, UsersResponse}; use std::sync::{Arc, Weak}; +use text::ReplicaId; +use theme::ColorIndex; use util::http::HttpClient; use util::TryFutureExt as _; @@ -19,6 +21,13 @@ pub struct User { pub avatar: Option>, } +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Collaborator { + pub peer_id: proto::PeerId, + pub replica_id: ReplicaId, + pub user_id: UserId, +} + impl PartialOrd for User { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) @@ -56,6 +65,7 @@ pub enum ContactRequestStatus { pub struct UserStore { users: HashMap>, + color_indices: HashMap, update_contacts_tx: mpsc::UnboundedSender, current_user: watch::Receiver>>, contacts: Vec>, @@ -81,6 +91,7 @@ pub enum Event { kind: ContactEventKind, }, ShowContacts, + ColorIndicesChanged, } #[derive(Clone, Copy)] @@ -118,6 +129,7 @@ impl UserStore { current_user: current_user_rx, contacts: Default::default(), incoming_contact_requests: Default::default(), + color_indices: Default::default(), outgoing_contact_requests: Default::default(), invite_info: None, client: Arc::downgrade(&client), @@ -641,6 +653,21 @@ impl UserStore { } }) } + + pub fn set_color_indices( + &mut self, + color_indices: HashMap, + cx: &mut ModelContext, + ) { + if color_indices != self.color_indices { + self.color_indices = color_indices; + cx.emit(Event::ColorIndicesChanged); + } + } + + pub fn color_indices(&self) -> &HashMap { + &self.color_indices + } } impl User { @@ -672,6 +699,16 @@ impl Contact { } } +impl Collaborator { + pub fn from_proto(message: proto::Collaborator) -> Result { + Ok(Self { + peer_id: message.peer_id.ok_or_else(|| anyhow!("invalid peer id"))?, + replica_id: message.replica_id as ReplicaId, + user_id: message.user_id as UserId, + }) + } +} + async fn fetch_avatar(http: &dyn HttpClient, url: &str) -> Result> { let mut response = http .get(url, Default::default(), true) diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index d0c4ead5adca03ff6af2bbc3797560a9488bf2b7..5f5484679fb968ce63d58f952ea028986ee13fd4 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -158,7 +158,8 @@ CREATE TABLE "room_participants" ( "initial_project_id" INTEGER, "calling_user_id" INTEGER NOT NULL REFERENCES users (id), "calling_connection_id" INTEGER NOT NULL, - "calling_connection_server_id" INTEGER REFERENCES servers (id) ON DELETE SET NULL + "calling_connection_server_id" INTEGER REFERENCES servers (id) ON DELETE SET NULL, + "color_index" INTEGER ); CREATE UNIQUE INDEX "index_room_participants_on_user_id" ON "room_participants" ("user_id"); CREATE INDEX "index_room_participants_on_room_id" ON "room_participants" ("room_id"); diff --git a/crates/collab/migrations/20230926102500_add_color_index_to_room_participants.sql b/crates/collab/migrations/20230926102500_add_color_index_to_room_participants.sql new file mode 100644 index 0000000000000000000000000000000000000000..626268bd5c739503a232db7548818782db298dc1 --- /dev/null +++ b/crates/collab/migrations/20230926102500_add_color_index_to_room_participants.sql @@ -0,0 +1 @@ +ALTER TABLE room_participants ADD COLUMN color_index INTEGER; diff --git a/crates/collab/src/db.rs b/crates/collab/src/db.rs index 527c4faaa5ccd5f8e5776e61bf7c6f2c032ad9ea..ab2fbe39451b9a7230719fa8ce6a127e34e4908d 100644 --- a/crates/collab/src/db.rs +++ b/crates/collab/src/db.rs @@ -510,7 +510,7 @@ pub struct RefreshedRoom { pub struct RefreshedChannelBuffer { pub connection_ids: Vec, - pub removed_collaborators: Vec, + pub collaborators: Vec, } pub struct Project { diff --git a/crates/collab/src/db/queries/buffers.rs b/crates/collab/src/db/queries/buffers.rs index 62ead11932af5cdab9049d719eef7b1238b73ffc..4b149faf2abd5f22651b7ae8aad2ed8477fb7ab9 100644 --- a/crates/collab/src/db/queries/buffers.rs +++ b/crates/collab/src/db/queries/buffers.rs @@ -2,6 +2,12 @@ use super::*; use prost::Message; use text::{EditOperation, UndoOperation}; +pub struct LeftChannelBuffer { + pub channel_id: ChannelId, + pub collaborators: Vec, + pub connections: Vec, +} + impl Database { pub async fn join_channel_buffer( &self, @@ -204,23 +210,26 @@ impl Database { server_id: ServerId, ) -> Result { self.transaction(|tx| async move { - let collaborators = channel_buffer_collaborator::Entity::find() + let db_collaborators = channel_buffer_collaborator::Entity::find() .filter(channel_buffer_collaborator::Column::ChannelId.eq(channel_id)) .all(&*tx) .await?; let mut connection_ids = Vec::new(); - let mut removed_collaborators = Vec::new(); + let mut collaborators = Vec::new(); let mut collaborator_ids_to_remove = Vec::new(); - for collaborator in &collaborators { - if !collaborator.connection_lost && collaborator.connection_server_id == server_id { - connection_ids.push(collaborator.connection()); + for db_collaborator in &db_collaborators { + if !db_collaborator.connection_lost + && db_collaborator.connection_server_id == server_id + { + connection_ids.push(db_collaborator.connection()); + collaborators.push(proto::Collaborator { + peer_id: Some(db_collaborator.connection().into()), + replica_id: db_collaborator.replica_id.0 as u32, + user_id: db_collaborator.user_id.to_proto(), + }) } else { - removed_collaborators.push(proto::RemoveChannelBufferCollaborator { - channel_id: channel_id.to_proto(), - peer_id: Some(collaborator.connection().into()), - }); - collaborator_ids_to_remove.push(collaborator.id); + collaborator_ids_to_remove.push(db_collaborator.id); } } @@ -231,7 +240,7 @@ impl Database { Ok(RefreshedChannelBuffer { connection_ids, - removed_collaborators, + collaborators, }) }) .await @@ -241,7 +250,7 @@ impl Database { &self, channel_id: ChannelId, connection: ConnectionId, - ) -> Result> { + ) -> Result { self.transaction(|tx| async move { self.leave_channel_buffer_internal(channel_id, connection, &*tx) .await @@ -275,7 +284,7 @@ impl Database { pub async fn leave_channel_buffers( &self, connection: ConnectionId, - ) -> Result)>> { + ) -> Result> { self.transaction(|tx| async move { #[derive(Debug, Clone, Copy, EnumIter, DeriveColumn)] enum QueryChannelIds { @@ -294,10 +303,10 @@ impl Database { let mut result = Vec::new(); for channel_id in channel_ids { - let collaborators = self + let left_channel_buffer = self .leave_channel_buffer_internal(channel_id, connection, &*tx) .await?; - result.push((channel_id, collaborators)); + result.push(left_channel_buffer); } Ok(result) @@ -310,7 +319,7 @@ impl Database { channel_id: ChannelId, connection: ConnectionId, tx: &DatabaseTransaction, - ) -> Result> { + ) -> Result { let result = channel_buffer_collaborator::Entity::delete_many() .filter( Condition::all() @@ -327,6 +336,7 @@ impl Database { Err(anyhow!("not a collaborator on this project"))?; } + let mut collaborators = Vec::new(); let mut connections = Vec::new(); let mut rows = channel_buffer_collaborator::Entity::find() .filter( @@ -336,19 +346,26 @@ impl Database { .await?; while let Some(row) = rows.next().await { let row = row?; - connections.push(ConnectionId { - id: row.connection_id as u32, - owner_id: row.connection_server_id.0 as u32, + let connection = row.connection(); + connections.push(connection); + collaborators.push(proto::Collaborator { + peer_id: Some(connection.into()), + replica_id: row.replica_id.0 as u32, + user_id: row.user_id.to_proto(), }); } drop(rows); - if connections.is_empty() { + if collaborators.is_empty() { self.snapshot_channel_buffer(channel_id, &tx).await?; } - Ok(connections) + Ok(LeftChannelBuffer { + channel_id, + collaborators, + connections, + }) } pub async fn get_channel_buffer_collaborators( diff --git a/crates/collab/src/db/queries/rooms.rs b/crates/collab/src/db/queries/rooms.rs index 651d58c265969311115293af78c16ed777904702..fca4c67690c3b9fbaec101da55a0a19a9e72be61 100644 --- a/crates/collab/src/db/queries/rooms.rs +++ b/crates/collab/src/db/queries/rooms.rs @@ -152,6 +152,7 @@ impl Database { room_id: ActiveValue::set(room_id), user_id: ActiveValue::set(called_user_id), answering_connection_lost: ActiveValue::set(false), + color_index: ActiveValue::NotSet, calling_user_id: ActiveValue::set(calling_user_id), calling_connection_id: ActiveValue::set(calling_connection.id as i32), calling_connection_server_id: ActiveValue::set(Some(ServerId( @@ -283,6 +284,22 @@ impl Database { .await? .ok_or_else(|| anyhow!("no such room"))?; + #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] + enum QueryColorIndices { + ColorIndex, + } + let existing_color_indices: Vec = room_participant::Entity::find() + .filter(room_participant::Column::RoomId.eq(room_id)) + .select_only() + .column(room_participant::Column::ColorIndex) + .into_values::<_, QueryColorIndices>() + .all(&*tx) + .await?; + let mut color_index = 0; + while existing_color_indices.contains(&color_index) { + color_index += 1; + } + if let Some(channel_id) = channel_id { self.check_user_is_channel_member(channel_id, user_id, &*tx) .await?; @@ -300,6 +317,7 @@ impl Database { calling_connection_server_id: ActiveValue::set(Some(ServerId( connection.owner_id as i32, ))), + color_index: ActiveValue::Set(color_index), ..Default::default() }]) .on_conflict( @@ -322,6 +340,7 @@ impl Database { .add(room_participant::Column::AnsweringConnectionId.is_null()), ) .set(room_participant::ActiveModel { + color_index: ActiveValue::Set(color_index), answering_connection_id: ActiveValue::set(Some(connection.id as i32)), answering_connection_server_id: ActiveValue::set(Some(ServerId( connection.owner_id as i32, @@ -1071,6 +1090,7 @@ impl Database { peer_id: Some(answering_connection.into()), projects: Default::default(), location: Some(proto::ParticipantLocation { variant: location }), + color_index: db_participant.color_index as u32, }, ); } else { diff --git a/crates/collab/src/db/tables/room_participant.rs b/crates/collab/src/db/tables/room_participant.rs index 57d79fa8306b8e42ce62966b64b306efe024d1cc..8072fed69ce8f32c07386498e6e971eb0ccf4077 100644 --- a/crates/collab/src/db/tables/room_participant.rs +++ b/crates/collab/src/db/tables/room_participant.rs @@ -18,6 +18,7 @@ pub struct Model { pub calling_user_id: UserId, pub calling_connection_id: i32, pub calling_connection_server_id: Option, + pub color_index: i32, } impl Model { diff --git a/crates/collab/src/db/tests/buffer_tests.rs b/crates/collab/src/db/tests/buffer_tests.rs index e71748b88b0571c2ccd7840924b8fc325cd368b7..9808a9955be090b012bab28a877e2bfa6b61b001 100644 --- a/crates/collab/src/db/tests/buffer_tests.rs +++ b/crates/collab/src/db/tests/buffer_tests.rs @@ -134,12 +134,12 @@ async fn test_channel_buffers(db: &Arc) { let zed_collaborats = db.get_channel_buffer_collaborators(zed_id).await.unwrap(); assert_eq!(zed_collaborats, &[a_id, b_id]); - let collaborators = db + let left_buffer = db .leave_channel_buffer(zed_id, connection_id_b) .await .unwrap(); - assert_eq!(collaborators, &[connection_id_a],); + assert_eq!(left_buffer.connections, &[connection_id_a],); let cargo_id = db.create_root_channel("cargo", "2", a_id).await.unwrap(); let _ = db diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index b3af2d4e989ca2cb0b155f239ac0befff1c55701..56cecb2e745a1ac3228907826fe2f2dfbe34d8c6 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -38,8 +38,8 @@ use lazy_static::lazy_static; use prometheus::{register_int_gauge, IntGauge}; use rpc::{ proto::{ - self, Ack, AddChannelBufferCollaborator, AnyTypedEnvelope, ChannelEdge, EntityMessage, - EnvelopedMessage, LiveKitConnectionInfo, RequestMessage, + self, Ack, AnyTypedEnvelope, ChannelEdge, EntityMessage, EnvelopedMessage, + LiveKitConnectionInfo, RequestMessage, UpdateChannelBufferCollaborators, }, Connection, ConnectionId, Peer, Receipt, TypedEnvelope, }; @@ -313,9 +313,16 @@ impl Server { .trace_err() { for connection_id in refreshed_channel_buffer.connection_ids { - for message in &refreshed_channel_buffer.removed_collaborators { - peer.send(connection_id, message.clone()).trace_err(); - } + peer.send( + connection_id, + proto::UpdateChannelBufferCollaborators { + channel_id: channel_id.to_proto(), + collaborators: refreshed_channel_buffer + .collaborators + .clone(), + }, + ) + .trace_err(); } } } @@ -2654,18 +2661,12 @@ async fn join_channel_buffer( .join_channel_buffer(channel_id, session.user_id, session.connection_id) .await?; - let replica_id = open_response.replica_id; let collaborators = open_response.collaborators.clone(); - response.send(open_response)?; - let update = AddChannelBufferCollaborator { + let update = UpdateChannelBufferCollaborators { channel_id: channel_id.to_proto(), - collaborator: Some(proto::Collaborator { - user_id: session.user_id.to_proto(), - peer_id: Some(session.connection_id.into()), - replica_id, - }), + collaborators: collaborators.clone(), }; channel_buffer_updated( session.connection_id, @@ -2712,8 +2713,8 @@ async fn rejoin_channel_buffers( .rejoin_channel_buffers(&request.buffers, session.user_id, session.connection_id) .await?; - for buffer in &buffers { - let collaborators_to_notify = buffer + for rejoined_buffer in &buffers { + let collaborators_to_notify = rejoined_buffer .buffer .collaborators .iter() @@ -2721,10 +2722,9 @@ async fn rejoin_channel_buffers( channel_buffer_updated( session.connection_id, collaborators_to_notify, - &proto::UpdateChannelBufferCollaborator { - channel_id: buffer.buffer.channel_id, - old_peer_id: Some(buffer.old_connection_id.into()), - new_peer_id: Some(session.connection_id.into()), + &proto::UpdateChannelBufferCollaborators { + channel_id: rejoined_buffer.buffer.channel_id, + collaborators: rejoined_buffer.buffer.collaborators.clone(), }, &session.peer, ); @@ -2745,7 +2745,7 @@ async fn leave_channel_buffer( let db = session.db().await; let channel_id = ChannelId::from_proto(request.channel_id); - let collaborators_to_notify = db + let left_buffer = db .leave_channel_buffer(channel_id, session.connection_id) .await?; @@ -2753,10 +2753,10 @@ async fn leave_channel_buffer( channel_buffer_updated( session.connection_id, - collaborators_to_notify, - &proto::RemoveChannelBufferCollaborator { + left_buffer.connections, + &proto::UpdateChannelBufferCollaborators { channel_id: channel_id.to_proto(), - peer_id: Some(session.connection_id.into()), + collaborators: left_buffer.collaborators, }, &session.peer, ); @@ -3231,13 +3231,13 @@ async fn leave_channel_buffers_for_session(session: &Session) -> Result<()> { .leave_channel_buffers(session.connection_id) .await?; - for (channel_id, connections) in left_channel_buffers { + for left_buffer in left_channel_buffers { channel_buffer_updated( session.connection_id, - connections, - &proto::RemoveChannelBufferCollaborator { - channel_id: channel_id.to_proto(), - peer_id: Some(session.connection_id.into()), + left_buffer.connections, + &proto::UpdateChannelBufferCollaborators { + channel_id: left_buffer.channel_id.to_proto(), + collaborators: left_buffer.collaborators, }, &session.peer, ); diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index baab675a1c2e3decada7d292d078977bea4ff4ce..e403ed6f94916ade3f6409379c402af407ad727e 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -4,14 +4,16 @@ use crate::{ }; use call::ActiveCall; use channel::Channel; -use client::UserId; +use client::{Collaborator, UserId}; use collab_ui::channel_view::ChannelView; use collections::HashMap; +use editor::{Anchor, Editor, ToOffset}; use futures::future; -use gpui::{executor::Deterministic, ModelHandle, TestAppContext}; -use rpc::{proto, RECEIVE_TIMEOUT}; +use gpui::{executor::Deterministic, ModelHandle, TestAppContext, ViewContext}; +use rpc::{proto::PeerId, RECEIVE_TIMEOUT}; use serde_json::json; -use std::sync::Arc; +use std::{ops::Range, sync::Arc}; +use theme::ColorIndex; #[gpui::test] async fn test_core_channel_buffers( @@ -120,10 +122,10 @@ async fn test_core_channel_buffers( } #[gpui::test] -async fn test_channel_buffer_replica_ids( +async fn test_channel_notes_color_indices( deterministic: Arc, - cx_a: &mut TestAppContext, - cx_b: &mut TestAppContext, + mut cx_a: &mut TestAppContext, + mut cx_b: &mut TestAppContext, cx_c: &mut TestAppContext, ) { deterministic.forbid_parking(); @@ -132,6 +134,13 @@ async fn test_channel_buffer_replica_ids( let client_b = server.create_client(cx_b, "user_b").await; let client_c = server.create_client(cx_c, "user_c").await; + let active_call_a = cx_a.read(ActiveCall::global); + let active_call_b = cx_b.read(ActiveCall::global); + + cx_a.update(editor::init); + cx_b.update(editor::init); + cx_c.update(editor::init); + let channel_id = server .make_channel( "the-channel", @@ -141,138 +150,160 @@ async fn test_channel_buffer_replica_ids( ) .await; - 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); + client_a + .fs() + .insert_tree("/root", json!({"file.txt": "123"})) + .await; + let (project_a, worktree_id_a) = client_a.build_local_project("/root", cx_a).await; + let project_b = client_b.build_empty_local_project(cx_b); + let project_c = client_c.build_empty_local_project(cx_c); + 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); - // Clients A and B join a channel. - active_call_a - .update(cx_a, |call, cx| call.join_channel(channel_id, cx)) + // Clients A, B, and C open the channel notes + let channel_view_a = cx_a + .update(|cx| ChannelView::open(channel_id, workspace_a.clone(), cx)) .await .unwrap(); - active_call_b - .update(cx_b, |call, cx| call.join_channel(channel_id, cx)) + let channel_view_b = cx_b + .update(|cx| ChannelView::open(channel_id, workspace_b.clone(), cx)) .await .unwrap(); - - // Clients A, B, and C join a channel buffer - // C first so that the replica IDs in the project and the channel buffer are different - let channel_buffer_c = client_c - .channel_store() - .update(cx_c, |store, cx| store.open_channel_buffer(channel_id, cx)) + let channel_view_c = cx_c + .update(|cx| ChannelView::open(channel_id, workspace_c.clone(), cx)) .await .unwrap(); - let channel_buffer_b = client_b - .channel_store() - .update(cx_b, |store, cx| store.open_channel_buffer(channel_id, cx)) - .await - .unwrap(); - let channel_buffer_a = client_a - .channel_store() - .update(cx_a, |store, cx| store.open_channel_buffer(channel_id, cx)) - .await - .unwrap(); - - // Client B shares a project - client_b - .fs() - .insert_tree("/dir", json!({ "file.txt": "contents" })) - .await; - let (project_b, _) = client_b.build_local_project("/dir", cx_b).await; - let shared_project_id = active_call_b - .update(cx_b, |call, cx| call.share_project(project_b.clone(), cx)) - .await - .unwrap(); - - // Client A joins the project - let project_a = client_a.build_remote_project(shared_project_id, cx_a).await; - deterministic.run_until_parked(); - // Client C is in a separate project. - client_c.fs().insert_tree("/dir", json!({})).await; - let (separate_project_c, _) = client_c.build_local_project("/dir", cx_c).await; - - // Note that each user has a different replica id in the projects vs the - // channel buffer. - channel_buffer_a.read_with(cx_a, |channel_buffer, cx| { - assert_eq!(project_a.read(cx).replica_id(), 1); - assert_eq!(channel_buffer.buffer().read(cx).replica_id(), 2); + // Clients A, B, and C all insert and select some text + channel_view_a.update(cx_a, |notes, cx| { + notes.editor.update(cx, |editor, cx| { + editor.insert("a", cx); + editor.change_selections(None, cx, |selections| { + selections.select_ranges(vec![0..1]); + }); + }); }); - channel_buffer_b.read_with(cx_b, |channel_buffer, cx| { - assert_eq!(project_b.read(cx).replica_id(), 0); - assert_eq!(channel_buffer.buffer().read(cx).replica_id(), 1); + deterministic.run_until_parked(); + channel_view_b.update(cx_b, |notes, cx| { + notes.editor.update(cx, |editor, cx| { + editor.move_down(&Default::default(), cx); + editor.insert("b", cx); + editor.change_selections(None, cx, |selections| { + selections.select_ranges(vec![1..2]); + }); + }); }); - channel_buffer_c.read_with(cx_c, |channel_buffer, cx| { - // C is not in the project - assert_eq!(channel_buffer.buffer().read(cx).replica_id(), 0); + deterministic.run_until_parked(); + channel_view_c.update(cx_c, |notes, cx| { + notes.editor.update(cx, |editor, cx| { + editor.move_down(&Default::default(), cx); + editor.insert("c", cx); + editor.change_selections(None, cx, |selections| { + selections.select_ranges(vec![2..3]); + }); + }); }); - let channel_window_a = - cx_a.add_window(|cx| ChannelView::new(project_a.clone(), channel_buffer_a.clone(), cx)); - let channel_window_b = - cx_b.add_window(|cx| ChannelView::new(project_b.clone(), channel_buffer_b.clone(), cx)); - let channel_window_c = cx_c.add_window(|cx| { - ChannelView::new(separate_project_c.clone(), channel_buffer_c.clone(), cx) + // Client A sees clients B and C without assigned colors, because they aren't + // in a call together. + deterministic.run_until_parked(); + channel_view_a.update(cx_a, |notes, cx| { + notes.editor.update(cx, |editor, cx| { + assert_remote_selections(editor, &[(None, 1..2), (None, 2..3)], cx); + }); }); - let channel_view_a = channel_window_a.root(cx_a); - let channel_view_b = channel_window_b.root(cx_b); - let channel_view_c = channel_window_c.root(cx_c); + // Clients A and B join the same call. + for (call, cx) in [(&active_call_a, &mut cx_a), (&active_call_b, &mut cx_b)] { + call.update(*cx, |call, cx| call.join_channel(channel_id, cx)) + .await + .unwrap(); + } - // For clients A and B, the replica ids in the channel buffer are mapped - // so that they match the same users' replica ids in their shared project. - channel_view_a.read_with(cx_a, |view, cx| { - assert_eq!( - view.editor.read(cx).replica_id_map().unwrap(), - &[(1, 0), (2, 1)].into_iter().collect::>() - ); + // Clients A and B see each other with two different assigned colors. Client C + // still doesn't have a color. + deterministic.run_until_parked(); + channel_view_a.update(cx_a, |notes, cx| { + notes.editor.update(cx, |editor, cx| { + assert_remote_selections(editor, &[(Some(ColorIndex(1)), 1..2), (None, 2..3)], cx); + }); }); - channel_view_b.read_with(cx_b, |view, cx| { - assert_eq!( - view.editor.read(cx).replica_id_map().unwrap(), - &[(1, 0), (2, 1)].into_iter().collect::>(), - ) + channel_view_b.update(cx_b, |notes, cx| { + notes.editor.update(cx, |editor, cx| { + assert_remote_selections(editor, &[(Some(ColorIndex(0)), 0..1), (None, 2..3)], cx); + }); }); - // Client C only sees themself, as they're not part of any shared project - channel_view_c.read_with(cx_c, |view, cx| { - assert_eq!( - view.editor.read(cx).replica_id_map().unwrap(), - &[(0, 0)].into_iter().collect::>(), - ); - }); + // Client A shares a project, and client B joins. + let project_id = active_call_a + .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx)) + .await + .unwrap(); + let project_b = client_b.build_remote_project(project_id, cx_b).await; + let workspace_b = client_b.build_workspace(&project_b, cx_b).root(cx_b); - // Client C joins the project that clients A and B are in. - active_call_c - .update(cx_c, |call, cx| call.join_channel(channel_id, cx)) + // Clients A and B open the same file. + let editor_a = workspace_a + .update(cx_a, |workspace, cx| { + workspace.open_path((worktree_id_a, "file.txt"), None, true, cx) + }) .await + .unwrap() + .downcast::() .unwrap(); - let project_c = client_c.build_remote_project(shared_project_id, cx_c).await; - deterministic.run_until_parked(); - project_c.read_with(cx_c, |project, _| { - assert_eq!(project.replica_id(), 2); + let editor_b = workspace_b + .update(cx_b, |workspace, cx| { + workspace.open_path((worktree_id_a, "file.txt"), None, true, cx) + }) + .await + .unwrap() + .downcast::() + .unwrap(); + + editor_a.update(cx_a, |editor, cx| { + editor.change_selections(None, cx, |selections| { + selections.select_ranges(vec![0..1]); + }); + }); + editor_b.update(cx_b, |editor, cx| { + editor.change_selections(None, cx, |selections| { + selections.select_ranges(vec![2..3]); + }); }); + deterministic.run_until_parked(); - // For clients A and B, client C's replica id in the channel buffer is - // now mapped to their replica id in the shared project. - channel_view_a.read_with(cx_a, |view, cx| { - assert_eq!( - view.editor.read(cx).replica_id_map().unwrap(), - &[(1, 0), (2, 1), (0, 2)] - .into_iter() - .collect::>() - ); + // Clients A and B see each other with the same colors as in the channel notes. + editor_a.update(cx_a, |editor, cx| { + assert_remote_selections(editor, &[(Some(ColorIndex(1)), 2..3)], cx); }); - channel_view_b.read_with(cx_b, |view, cx| { - assert_eq!( - view.editor.read(cx).replica_id_map().unwrap(), - &[(1, 0), (2, 1), (0, 2)] - .into_iter() - .collect::>(), - ) + editor_b.update(cx_b, |editor, cx| { + assert_remote_selections(editor, &[(Some(ColorIndex(0)), 0..1)], cx); }); } +#[track_caller] +fn assert_remote_selections( + editor: &mut Editor, + expected_selections: &[(Option, Range)], + cx: &mut ViewContext, +) { + let snapshot = editor.snapshot(cx); + let range = Anchor::min()..Anchor::max(); + let remote_selections = snapshot + .remote_selections_in_range(&range, editor.collaboration_hub().unwrap(), cx) + .map(|s| { + let start = s.selection.start.to_offset(&snapshot.buffer_snapshot); + let end = s.selection.end.to_offset(&snapshot.buffer_snapshot); + (s.color_index, start..end) + }) + .collect::>(); + assert_eq!( + remote_selections, expected_selections, + "incorrect remote selections" + ); +} + #[gpui::test] async fn test_multiple_handles_to_channel_buffer( deterministic: Arc, @@ -568,13 +599,9 @@ async fn test_channel_buffers_and_server_restarts( channel_buffer_a.read_with(cx_a, |buffer_a, _| { channel_buffer_b.read_with(cx_b, |buffer_b, _| { - assert_eq!( - buffer_a - .collaborators() - .iter() - .map(|c| c.user_id) - .collect::>(), - vec![client_a.user_id().unwrap(), client_b.user_id().unwrap()] + assert_collaborators( + buffer_a.collaborators(), + &[client_a.user_id(), client_b.user_id()], ); assert_eq!(buffer_a.collaborators(), buffer_b.collaborators()); }); @@ -723,10 +750,10 @@ async fn test_following_to_channel_notes_without_a_shared_project( } #[track_caller] -fn assert_collaborators(collaborators: &[proto::Collaborator], ids: &[Option]) { +fn assert_collaborators(collaborators: &HashMap, ids: &[Option]) { assert_eq!( collaborators - .into_iter() + .values() .map(|collaborator| collaborator.user_id) .collect::>(), ids.into_iter().map(|id| id.unwrap()).collect::>() diff --git a/crates/collab/src/tests/random_channel_buffer_tests.rs b/crates/collab/src/tests/random_channel_buffer_tests.rs index 2950922e7c63fb1c4e8d44da4f38bdbe859a2a94..ad0181602c9ac3bd5ab25d6029ad84d7ba74ce3e 100644 --- a/crates/collab/src/tests/random_channel_buffer_tests.rs +++ b/crates/collab/src/tests/random_channel_buffer_tests.rs @@ -273,7 +273,7 @@ impl RandomizedTest for RandomChannelBufferTest { // channel buffer. let collaborators = channel_buffer.collaborators(); let mut user_ids = - collaborators.iter().map(|c| c.user_id).collect::>(); + collaborators.values().map(|c| c.user_id).collect::>(); user_ids.sort(); assert_eq!( user_ids, diff --git a/crates/collab/src/tests/test_server.rs b/crates/collab/src/tests/test_server.rs index 6a15cac9e9d0b2f5f31fb72ff4c57b2db5058001..71537f069f78188b21bbb943cda28460ffbca0d1 100644 --- a/crates/collab/src/tests/test_server.rs +++ b/crates/collab/src/tests/test_server.rs @@ -538,15 +538,7 @@ impl TestClient { root_path: impl AsRef, cx: &mut TestAppContext, ) -> (ModelHandle, WorktreeId) { - let project = cx.update(|cx| { - Project::local( - self.client().clone(), - self.app_state.user_store.clone(), - self.app_state.languages.clone(), - self.app_state.fs.clone(), - cx, - ) - }); + let project = self.build_empty_local_project(cx); let (worktree, _) = project .update(cx, |p, cx| { p.find_or_create_local_worktree(root_path, true, cx) @@ -559,6 +551,18 @@ impl TestClient { (project, worktree.read_with(cx, |tree, _| tree.id())) } + pub fn build_empty_local_project(&self, cx: &mut TestAppContext) -> ModelHandle { + cx.update(|cx| { + Project::local( + self.client().clone(), + self.app_state.user_store.clone(), + self.app_state.languages.clone(), + self.app_state.fs.clone(), + cx, + ) + }) + } + pub async fn build_remote_project( &self, host_project_id: u64, diff --git a/crates/collab_ui/src/channel_view.rs b/crates/collab_ui/src/channel_view.rs index b66d1ab7c7a16ae126f8fcba1ddd66defeb04105..1d103350def13380628e1ccd75e25a77f963d029 100644 --- a/crates/collab_ui/src/channel_view.rs +++ b/crates/collab_ui/src/channel_view.rs @@ -1,10 +1,12 @@ use anyhow::{anyhow, Result}; use call::ActiveCall; use channel::{Channel, ChannelBuffer, ChannelBufferEvent, ChannelId}; -use client::proto; -use clock::ReplicaId; +use client::{ + proto::{self, PeerId}, + Collaborator, +}; use collections::HashMap; -use editor::Editor; +use editor::{CollaborationHub, Editor}; use gpui::{ actions, elements::{ChildView, Label}, @@ -109,97 +111,44 @@ impl ChannelView { cx: &mut ViewContext, ) -> Self { let buffer = channel_buffer.read(cx).buffer(); - let editor = cx.add_view(|cx| Editor::for_buffer(buffer, None, cx)); + let editor = cx.add_view(|cx| { + let mut editor = Editor::for_buffer(buffer, None, cx); + editor.set_collaboration_hub(Box::new(ChannelBufferCollaborationHub( + channel_buffer.clone(), + ))); + editor + }); let _editor_event_subscription = cx.subscribe(&editor, |_, _, e, cx| cx.emit(e.clone())); - cx.subscribe(&project, Self::handle_project_event).detach(); cx.subscribe(&channel_buffer, Self::handle_channel_buffer_event) .detach(); - let this = Self { + Self { editor, project, channel_buffer, remote_id: None, _editor_event_subscription, - }; - this.refresh_replica_id_map(cx); - this + } } pub fn channel(&self, cx: &AppContext) -> Arc { self.channel_buffer.read(cx).channel() } - fn handle_project_event( - &mut self, - _: ModelHandle, - event: &project::Event, - cx: &mut ViewContext, - ) { - match event { - project::Event::RemoteIdChanged(_) => {} - project::Event::DisconnectedFromHost => {} - project::Event::Closed => {} - project::Event::CollaboratorUpdated { .. } => {} - project::Event::CollaboratorLeft(_) => {} - project::Event::CollaboratorJoined(_) => {} - _ => return, - } - self.refresh_replica_id_map(cx); - } - fn handle_channel_buffer_event( &mut self, _: ModelHandle, event: &ChannelBufferEvent, cx: &mut ViewContext, ) { - match event { - ChannelBufferEvent::CollaboratorsChanged => { - self.refresh_replica_id_map(cx); - } - ChannelBufferEvent::Disconnected => self.editor.update(cx, |editor, cx| { + if let ChannelBufferEvent::Disconnected = event { + self.editor.update(cx, |editor, cx| { editor.set_read_only(true); cx.notify(); - }), + }) } } - - /// Build a mapping of channel buffer replica ids to the corresponding - /// replica ids in the current project. - /// - /// Using this mapping, a given user can be displayed with the same color - /// in the channel buffer as in other files in the project. Users who are - /// in the channel buffer but not the project will not have a color. - fn refresh_replica_id_map(&self, cx: &mut ViewContext) { - let mut project_replica_ids_by_channel_buffer_replica_id = HashMap::default(); - let project = self.project.read(cx); - let channel_buffer = self.channel_buffer.read(cx); - project_replica_ids_by_channel_buffer_replica_id - .insert(channel_buffer.replica_id(cx), project.replica_id()); - project_replica_ids_by_channel_buffer_replica_id.extend( - channel_buffer - .collaborators() - .iter() - .filter_map(|channel_buffer_collaborator| { - project - .collaborators() - .values() - .find_map(|project_collaborator| { - (project_collaborator.user_id == channel_buffer_collaborator.user_id) - .then_some(( - channel_buffer_collaborator.replica_id as ReplicaId, - project_collaborator.replica_id, - )) - }) - }), - ); - - self.editor.update(cx, |editor, cx| { - editor.set_replica_id_map(Some(project_replica_ids_by_channel_buffer_replica_id), cx) - }); - } } impl Entity for ChannelView { @@ -388,13 +337,9 @@ impl FollowableItem for ChannelView { }) } - fn set_leader_replica_id( - &mut self, - leader_replica_id: Option, - cx: &mut ViewContext, - ) { + fn set_leader_peer_id(&mut self, leader_peer_id: Option, cx: &mut ViewContext) { self.editor.update(cx, |editor, cx| { - editor.set_leader_replica_id(leader_replica_id, cx) + editor.set_leader_peer_id(leader_peer_id, cx) }) } @@ -406,3 +351,15 @@ impl FollowableItem for ChannelView { false } } + +struct ChannelBufferCollaborationHub(ModelHandle); + +impl CollaborationHub for ChannelBufferCollaborationHub { + fn collaborators<'a>(&self, cx: &'a AppContext) -> &'a HashMap { + self.0.read(cx).collaborators() + } + + fn user_color_indices<'a>(&self, cx: &'a AppContext) -> &'a HashMap { + self.0.read(cx).user_store().read(cx).color_indices() + } +} diff --git a/crates/collab_ui/src/collab_titlebar_item.rs b/crates/collab_ui/src/collab_titlebar_item.rs index a676b40b754840e9b55e0c95144b3b6655047727..5dafae0cd5d5aaa1e219aa6c9f5f2173f281aac6 100644 --- a/crates/collab_ui/src/collab_titlebar_item.rs +++ b/crates/collab_ui/src/collab_titlebar_item.rs @@ -923,9 +923,15 @@ impl CollabTitlebarItem { .background_color .unwrap_or_default(); - if let Some(replica_id) = replica_id { + let color_index = self + .user_store + .read(cx) + .color_indices() + .get(&user_id) + .copied(); + if let Some(color_index) = color_index { if followed_by_self { - let selection = theme.editor.replica_selection_style(replica_id).selection; + let selection = theme.editor.replica_selection_style(color_index).selection; background_color = Color::blend(selection, background_color); background_color.a = 255; } @@ -990,10 +996,10 @@ impl CollabTitlebarItem { .contained() .with_style(theme.titlebar.leader_selection); - if let Some(replica_id) = replica_id { + if let Some(color_index) = color_index { if followed_by_self { let color = - theme.editor.replica_selection_style(replica_id).selection; + theme.editor.replica_selection_style(color_index).selection; container = container.with_background_color(color); } } @@ -1001,8 +1007,8 @@ impl CollabTitlebarItem { container })) .with_children((|| { - let replica_id = replica_id?; - let color = theme.editor.replica_selection_style(replica_id).cursor; + let color_index = color_index?; + let color = theme.editor.replica_selection_style(color_index).cursor; Some( AvatarRibbon::new(color) .constrained() diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 0827e1326402e39bfeeab389ad7a9df6d8eb5587..7692a54b01f95cfbd5f42cece19ce36e8cea1fee 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -25,7 +25,7 @@ use ::git::diff::DiffHunk; use aho_corasick::AhoCorasick; use anyhow::{anyhow, Context, Result}; use blink_manager::BlinkManager; -use client::{ClickhouseEvent, TelemetrySettings}; +use client::{ClickhouseEvent, Collaborator, TelemetrySettings}; use clock::{Global, ReplicaId}; use collections::{BTreeMap, Bound, HashMap, HashSet, VecDeque}; use convert_case::{Case, Casing}; @@ -79,6 +79,7 @@ pub use multi_buffer::{ use ordered_float::OrderedFloat; use project::{FormatTrigger, Location, Project, ProjectPath, ProjectTransaction}; use rand::{seq::SliceRandom, thread_rng}; +use rpc::proto::PeerId; use scroll::{ autoscroll::Autoscroll, OngoingScroll, ScrollAnchor, ScrollManager, ScrollbarAutoHide, }; @@ -101,7 +102,7 @@ use std::{ pub use sum_tree::Bias; use sum_tree::TreeMap; use text::Rope; -use theme::{DiagnosticStyle, Theme, ThemeSettings}; +use theme::{ColorIndex, DiagnosticStyle, Theme, ThemeSettings}; use util::{post_inc, RangeExt, ResultExt, TryFutureExt}; use workspace::{ItemNavHistory, ViewId, Workspace}; @@ -580,11 +581,11 @@ pub struct Editor { get_field_editor_theme: Option>, override_text_style: Option>, project: Option>, + collaboration_hub: Option>, focused: bool, blink_manager: ModelHandle, pub show_local_selections: bool, mode: EditorMode, - replica_id_mapping: Option>, show_gutter: bool, show_wrap_guides: Option, placeholder_text: Option>, @@ -608,7 +609,7 @@ pub struct Editor { keymap_context_layers: BTreeMap, input_enabled: bool, read_only: bool, - leader_replica_id: Option, + leader_peer_id: Option, remote_id: Option, hover_state: HoverState, gutter_hovered: bool, @@ -630,6 +631,15 @@ pub struct EditorSnapshot { ongoing_scroll: OngoingScroll, } +pub struct RemoteSelection { + pub replica_id: ReplicaId, + pub selection: Selection, + pub cursor_shape: CursorShape, + pub peer_id: PeerId, + pub line_mode: bool, + pub color_index: Option, +} + #[derive(Clone, Debug)] struct SelectionHistoryEntry { selections: Arc<[Selection]>, @@ -1532,12 +1542,12 @@ impl Editor { active_diagnostics: None, soft_wrap_mode_override, get_field_editor_theme, + collaboration_hub: project.clone().map(|project| Box::new(project) as _), project, focused: false, blink_manager: blink_manager.clone(), show_local_selections: true, mode, - replica_id_mapping: None, show_gutter: mode == EditorMode::Full, show_wrap_guides: None, placeholder_text: None, @@ -1564,7 +1574,7 @@ impl Editor { keymap_context_layers: Default::default(), input_enabled: true, read_only: false, - leader_replica_id: None, + leader_peer_id: None, remote_id: None, hover_state: Default::default(), link_go_to_definition_state: Default::default(), @@ -1631,8 +1641,8 @@ impl Editor { self.buffer.read(cx).replica_id() } - pub fn leader_replica_id(&self) -> Option { - self.leader_replica_id + pub fn leader_peer_id(&self) -> Option { + self.leader_peer_id } pub fn buffer(&self) -> &ModelHandle { @@ -1696,6 +1706,14 @@ impl Editor { self.mode } + pub fn collaboration_hub(&self) -> Option<&dyn CollaborationHub> { + self.collaboration_hub.as_deref() + } + + pub fn set_collaboration_hub(&mut self, hub: Box) { + self.collaboration_hub = Some(hub); + } + pub fn set_placeholder_text( &mut self, placeholder_text: impl Into>, @@ -1772,26 +1790,13 @@ impl Editor { cx.notify(); } - pub fn replica_id_map(&self) -> Option<&HashMap> { - self.replica_id_mapping.as_ref() - } - - pub fn set_replica_id_map( - &mut self, - mapping: Option>, - cx: &mut ViewContext, - ) { - self.replica_id_mapping = mapping; - cx.notify(); - } - fn selections_did_change( &mut self, local: bool, old_cursor_position: &Anchor, cx: &mut ViewContext, ) { - if self.focused && self.leader_replica_id.is_none() { + if self.focused && self.leader_peer_id.is_none() { self.buffer.update(cx, |buffer, cx| { buffer.set_active_selections( &self.selections.disjoint_anchors(), @@ -8563,6 +8568,21 @@ impl Editor { } } +pub trait CollaborationHub { + fn collaborators<'a>(&self, cx: &'a AppContext) -> &'a HashMap; + fn user_color_indices<'a>(&self, cx: &'a AppContext) -> &'a HashMap; +} + +impl CollaborationHub for ModelHandle { + fn collaborators<'a>(&self, cx: &'a AppContext) -> &'a HashMap { + self.read(cx).collaborators() + } + + fn user_color_indices<'a>(&self, cx: &'a AppContext) -> &'a HashMap { + self.read(cx).user_store().read(cx).color_indices() + } +} + fn inlay_hint_settings( location: Anchor, snapshot: &MultiBufferSnapshot, @@ -8606,6 +8626,34 @@ fn ending_row(next_selection: &Selection, display_map: &DisplaySnapshot) } impl EditorSnapshot { + pub fn remote_selections_in_range<'a>( + &'a self, + range: &'a Range, + collaboration_hub: &dyn CollaborationHub, + cx: &'a AppContext, + ) -> impl 'a + Iterator { + let color_indices = collaboration_hub.user_color_indices(cx); + let collaborators_by_peer_id = collaboration_hub.collaborators(cx); + let collaborators_by_replica_id = collaborators_by_peer_id + .iter() + .map(|(_, collaborator)| (collaborator.replica_id, collaborator)) + .collect::>(); + self.buffer_snapshot + .remote_selections_in_range(range) + .filter_map(move |(replica_id, line_mode, cursor_shape, selection)| { + let collaborator = collaborators_by_replica_id.get(&replica_id)?; + let color_index = color_indices.get(&collaborator.user_id).copied(); + Some(RemoteSelection { + replica_id, + selection, + cursor_shape, + line_mode, + color_index, + peer_id: collaborator.peer_id, + }) + }) + } + pub fn language_at(&self, position: T) -> Option<&Arc> { self.display_snapshot.buffer_snapshot.language_at(position) } @@ -8719,7 +8767,7 @@ impl View for Editor { self.focused = true; self.buffer.update(cx, |buffer, cx| { buffer.finalize_last_transaction(cx); - if self.leader_replica_id.is_none() { + if self.leader_peer_id.is_none() { buffer.set_active_selections( &self.selections.disjoint_anchors(), self.selections.line_mode, diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 3390b705300db9d89df6b248002f35dc7cb89068..dad5b06626dfdee0e57cd3297a86728f3e83ae6f 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -17,7 +17,6 @@ use crate::{ }, mouse_context_menu, EditorSettings, EditorStyle, GutterHover, UnfoldAt, }; -use clock::ReplicaId; use collections::{BTreeMap, HashMap}; use git::diff::DiffHunkStatus; use gpui::{ @@ -55,6 +54,7 @@ use std::{ sync::Arc, }; use text::Point; +use theme::SelectionStyle; use workspace::item::Item; enum FoldMarkers {} @@ -868,14 +868,7 @@ impl EditorElement { let corner_radius = 0.15 * layout.position_map.line_height; let mut invisible_display_ranges = SmallVec::<[Range; 32]>::new(); - for (replica_id, selections) in &layout.selections { - let replica_id = *replica_id; - let selection_style = if let Some(replica_id) = replica_id { - style.replica_selection_style(replica_id) - } else { - &style.absent_selection - }; - + for (selection_style, selections) in &layout.selections { for selection in selections { self.paint_highlighted_range( selection.range.clone(), @@ -2193,7 +2186,7 @@ impl Element for EditorElement { .anchor_before(DisplayPoint::new(end_row, 0).to_offset(&snapshot, Bias::Right)) }; - let mut selections: Vec<(Option, Vec)> = Vec::new(); + let mut selections: Vec<(SelectionStyle, Vec)> = Vec::new(); let mut active_rows = BTreeMap::new(); let mut fold_ranges = Vec::new(); let is_singleton = editor.is_singleton(cx); @@ -2219,35 +2212,6 @@ impl Element for EditorElement { }), ); - let mut remote_selections = HashMap::default(); - for (replica_id, line_mode, cursor_shape, selection) in snapshot - .buffer_snapshot - .remote_selections_in_range(&(start_anchor..end_anchor)) - { - let replica_id = if let Some(mapping) = &editor.replica_id_mapping { - mapping.get(&replica_id).copied() - } else { - Some(replica_id) - }; - - // The local selections match the leader's selections. - if replica_id.is_some() && replica_id == editor.leader_replica_id { - continue; - } - remote_selections - .entry(replica_id) - .or_insert(Vec::new()) - .push(SelectionLayout::new( - selection, - line_mode, - cursor_shape, - &snapshot.display_snapshot, - false, - false, - )); - } - selections.extend(remote_selections); - let mut newest_selection_head = None; if editor.show_local_selections { @@ -2282,19 +2246,45 @@ impl Element for EditorElement { layouts.push(layout); } - // Render the local selections in the leader's color when following. - let local_replica_id = if let Some(leader_replica_id) = editor.leader_replica_id { - leader_replica_id - } else { - let replica_id = editor.replica_id(cx); - if let Some(mapping) = &editor.replica_id_mapping { - mapping.get(&replica_id).copied().unwrap_or(replica_id) + selections.push((style.selection, layouts)); + } + + if let Some(collaboration_hub) = &editor.collaboration_hub { + let mut remote_selections = HashMap::default(); + for selection in snapshot.remote_selections_in_range( + &(start_anchor..end_anchor), + collaboration_hub.as_ref(), + cx, + ) { + let selection_style = if let Some(color_index) = selection.color_index { + style.replica_selection_style(color_index) } else { - replica_id + style.absent_selection + }; + + // The local selections match the leader's selections. + if Some(selection.peer_id) == editor.leader_peer_id { + if let Some((local_selection_style, _)) = selections.first_mut() { + *local_selection_style = selection_style; + } + continue; } - }; - selections.push((Some(local_replica_id), layouts)); + remote_selections + .entry(selection.replica_id) + .or_insert((selection_style, Vec::new())) + .1 + .push(SelectionLayout::new( + selection.selection, + selection.line_mode, + selection.cursor_shape, + &snapshot.display_snapshot, + false, + false, + )); + } + + selections.extend(remote_selections.into_values()); } let scrollbar_settings = &settings::get::(cx).scrollbar; @@ -2686,7 +2676,7 @@ pub struct LayoutState { blocks: Vec, highlighted_ranges: Vec<(Range, Color)>, fold_ranges: Vec<(BufferRow, Range, Color)>, - selections: Vec<(Option, Vec)>, + selections: Vec<(SelectionStyle, Vec)>, scrollbar_row_range: Range, show_scrollbars: bool, is_singleton: bool, diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 7fdbe82a9a2a88f8131d9b46d60dbe70a05aacf1..1fee3091816f9a18d8928401cbefdfc623634160 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -17,7 +17,7 @@ use language::{ SelectionGoal, }; use project::{search::SearchQuery, FormatTrigger, Item as _, Project, ProjectPath}; -use rpc::proto::{self, update_view}; +use rpc::proto::{self, update_view, PeerId}; use smallvec::SmallVec; use std::{ borrow::Cow, @@ -156,13 +156,9 @@ impl FollowableItem for Editor { })) } - fn set_leader_replica_id( - &mut self, - leader_replica_id: Option, - cx: &mut ViewContext, - ) { - self.leader_replica_id = leader_replica_id; - if self.leader_replica_id.is_some() { + fn set_leader_peer_id(&mut self, leader_peer_id: Option, cx: &mut ViewContext) { + self.leader_peer_id = leader_peer_id; + if self.leader_peer_id.is_some() { self.buffer.update(cx, |buffer, cx| { buffer.remove_active_selections(cx); }); diff --git a/crates/project/Cargo.toml b/crates/project/Cargo.toml index ffea6646e9439a24cd7e09fd9666ee9eabfcbf49..07f1bfa43e79d6c3d254b38a0f8a04c2a8031b71 100644 --- a/crates/project/Cargo.toml +++ b/crates/project/Cargo.toml @@ -35,6 +35,7 @@ rpc = { path = "../rpc" } settings = { path = "../settings" } sum_tree = { path = "../sum_tree" } terminal = { path = "../terminal" } +theme = { path = "../theme" } util = { path = "../util" } aho-corasick = "1.1" diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index e4858587ad46fb5a7eef8baa52cdb18c2692ce4c..62df8425c401ff5a571fed7498cac8a8724ddfcd 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -11,7 +11,7 @@ mod project_tests; mod worktree_tests; use anyhow::{anyhow, Context, Result}; -use client::{proto, Client, TypedEnvelope, UserId, UserStore}; +use client::{proto, Client, Collaborator, TypedEnvelope, UserStore}; use clock::ReplicaId; use collections::{hash_map, BTreeMap, HashMap, HashSet}; use copilot::Copilot; @@ -76,6 +76,7 @@ use std::{ }; use terminals::Terminals; use text::Anchor; +use theme::ColorIndex; use util::{ debug_panic, defer, http::HttpClient, merge_json_value_into, paths::LOCAL_SETTINGS_RELATIVE_PATH, post_inc, ResultExt, TryFutureExt as _, @@ -119,6 +120,7 @@ pub struct Project { join_project_response_message_id: u32, next_diagnostic_group_id: usize, user_store: ModelHandle, + user_color_indices: HashMap, fs: Arc, client_state: Option, collaborators: HashMap, @@ -253,13 +255,6 @@ enum ProjectClientState { }, } -#[derive(Clone, Debug)] -pub struct Collaborator { - pub peer_id: proto::PeerId, - pub replica_id: ReplicaId, - pub user_id: UserId, -} - #[derive(Clone, Debug, PartialEq)] pub enum Event { LanguageServerAdded(LanguageServerId), @@ -649,6 +644,7 @@ impl Project { languages, client, user_store, + user_color_indices: Default::default(), fs, next_entry_id: Default::default(), next_diagnostic_group_id: Default::default(), @@ -721,6 +717,7 @@ impl Project { _maintain_workspace_config: Self::maintain_workspace_config(cx), languages, user_store: user_store.clone(), + user_color_indices: Default::default(), fs, next_entry_id: Default::default(), next_diagnostic_group_id: Default::default(), @@ -925,6 +922,10 @@ impl Project { self.user_store.clone() } + pub fn user_color_indices(&self) -> &HashMap { + &self.user_color_indices + } + pub fn opened_buffers(&self, cx: &AppContext) -> Vec> { self.opened_buffers .values() @@ -8211,16 +8212,6 @@ impl Entity for Project { } } -impl Collaborator { - fn from_proto(message: proto::Collaborator) -> Result { - Ok(Self { - peer_id: message.peer_id.ok_or_else(|| anyhow!("invalid peer id"))?, - replica_id: message.replica_id as ReplicaId, - user_id: message.user_id as UserId, - }) - } -} - impl> From<(WorktreeId, P)> for ProjectPath { fn from((worktree_id, path): (WorktreeId, P)) -> Self { Self { diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 9c1ec4e613f45ca154e4dfa38b0048e32ee19117..da97cd35c76f50708055dc7fce9dad82f1a8e797 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -23,154 +23,152 @@ message Envelope { CreateRoomResponse create_room_response = 10; JoinRoom join_room = 11; JoinRoomResponse join_room_response = 12; - RejoinRoom rejoin_room = 108; - RejoinRoomResponse rejoin_room_response = 109; - LeaveRoom leave_room = 13; - Call call = 14; - IncomingCall incoming_call = 15; - CallCanceled call_canceled = 16; - CancelCall cancel_call = 17; - DeclineCall decline_call = 18; - UpdateParticipantLocation update_participant_location = 19; - RoomUpdated room_updated = 20; - - ShareProject share_project = 21; - ShareProjectResponse share_project_response = 22; - UnshareProject unshare_project = 23; - JoinProject join_project = 24; - JoinProjectResponse join_project_response = 25; - LeaveProject leave_project = 26; - AddProjectCollaborator add_project_collaborator = 27; - UpdateProjectCollaborator update_project_collaborator = 110; - RemoveProjectCollaborator remove_project_collaborator = 28; - - GetDefinition get_definition = 29; - GetDefinitionResponse get_definition_response = 30; - GetTypeDefinition get_type_definition = 31; - GetTypeDefinitionResponse get_type_definition_response = 32; - GetReferences get_references = 33; - GetReferencesResponse get_references_response = 34; - GetDocumentHighlights get_document_highlights = 35; - GetDocumentHighlightsResponse get_document_highlights_response = 36; - GetProjectSymbols get_project_symbols = 37; - GetProjectSymbolsResponse get_project_symbols_response = 38; - OpenBufferForSymbol open_buffer_for_symbol = 39; - OpenBufferForSymbolResponse open_buffer_for_symbol_response = 40; - - UpdateProject update_project = 41; - UpdateWorktree update_worktree = 43; - - CreateProjectEntry create_project_entry = 45; - RenameProjectEntry rename_project_entry = 46; - CopyProjectEntry copy_project_entry = 47; - DeleteProjectEntry delete_project_entry = 48; - ProjectEntryResponse project_entry_response = 49; - ExpandProjectEntry expand_project_entry = 114; - ExpandProjectEntryResponse expand_project_entry_response = 115; - - UpdateDiagnosticSummary update_diagnostic_summary = 50; - StartLanguageServer start_language_server = 51; - UpdateLanguageServer update_language_server = 52; - - OpenBufferById open_buffer_by_id = 53; - OpenBufferByPath open_buffer_by_path = 54; - OpenBufferResponse open_buffer_response = 55; - CreateBufferForPeer create_buffer_for_peer = 56; - UpdateBuffer update_buffer = 57; - UpdateBufferFile update_buffer_file = 58; - SaveBuffer save_buffer = 59; - BufferSaved buffer_saved = 60; - BufferReloaded buffer_reloaded = 61; - ReloadBuffers reload_buffers = 62; - ReloadBuffersResponse reload_buffers_response = 63; - SynchronizeBuffers synchronize_buffers = 200; - SynchronizeBuffersResponse synchronize_buffers_response = 201; - FormatBuffers format_buffers = 64; - FormatBuffersResponse format_buffers_response = 65; - GetCompletions get_completions = 66; - GetCompletionsResponse get_completions_response = 67; - ApplyCompletionAdditionalEdits apply_completion_additional_edits = 68; - ApplyCompletionAdditionalEditsResponse apply_completion_additional_edits_response = 69; - GetCodeActions get_code_actions = 70; - GetCodeActionsResponse get_code_actions_response = 71; - GetHover get_hover = 72; - GetHoverResponse get_hover_response = 73; - ApplyCodeAction apply_code_action = 74; - ApplyCodeActionResponse apply_code_action_response = 75; - PrepareRename prepare_rename = 76; - PrepareRenameResponse prepare_rename_response = 77; - PerformRename perform_rename = 78; - PerformRenameResponse perform_rename_response = 79; - SearchProject search_project = 80; - SearchProjectResponse search_project_response = 81; - - UpdateContacts update_contacts = 92; - UpdateInviteInfo update_invite_info = 93; - ShowContacts show_contacts = 94; - - GetUsers get_users = 95; - FuzzySearchUsers fuzzy_search_users = 96; - UsersResponse users_response = 97; - RequestContact request_contact = 98; - RespondToContactRequest respond_to_contact_request = 99; - RemoveContact remove_contact = 100; - - Follow follow = 101; - FollowResponse follow_response = 102; - UpdateFollowers update_followers = 103; - Unfollow unfollow = 104; - GetPrivateUserInfo get_private_user_info = 105; - GetPrivateUserInfoResponse get_private_user_info_response = 106; - UpdateDiffBase update_diff_base = 107; - - OnTypeFormatting on_type_formatting = 111; - OnTypeFormattingResponse on_type_formatting_response = 112; - - UpdateWorktreeSettings update_worktree_settings = 113; - - InlayHints inlay_hints = 116; - InlayHintsResponse inlay_hints_response = 117; - ResolveInlayHint resolve_inlay_hint = 137; - ResolveInlayHintResponse resolve_inlay_hint_response = 138; - RefreshInlayHints refresh_inlay_hints = 118; - - CreateChannel create_channel = 119; - CreateChannelResponse create_channel_response = 120; - InviteChannelMember invite_channel_member = 121; - RemoveChannelMember remove_channel_member = 122; - RespondToChannelInvite respond_to_channel_invite = 123; - UpdateChannels update_channels = 124; - JoinChannel join_channel = 125; - DeleteChannel delete_channel = 126; - GetChannelMembers get_channel_members = 127; - GetChannelMembersResponse get_channel_members_response = 128; - SetChannelMemberAdmin set_channel_member_admin = 129; - RenameChannel rename_channel = 130; - RenameChannelResponse rename_channel_response = 154; - - JoinChannelBuffer join_channel_buffer = 131; - JoinChannelBufferResponse join_channel_buffer_response = 132; - UpdateChannelBuffer update_channel_buffer = 133; - LeaveChannelBuffer leave_channel_buffer = 134; - AddChannelBufferCollaborator add_channel_buffer_collaborator = 135; - RemoveChannelBufferCollaborator remove_channel_buffer_collaborator = 136; - UpdateChannelBufferCollaborator update_channel_buffer_collaborator = 139; - RejoinChannelBuffers rejoin_channel_buffers = 140; - RejoinChannelBuffersResponse rejoin_channel_buffers_response = 141; - - JoinChannelChat join_channel_chat = 142; - JoinChannelChatResponse join_channel_chat_response = 143; - LeaveChannelChat leave_channel_chat = 144; - SendChannelMessage send_channel_message = 145; - SendChannelMessageResponse send_channel_message_response = 146; - ChannelMessageSent channel_message_sent = 147; - GetChannelMessages get_channel_messages = 148; - GetChannelMessagesResponse get_channel_messages_response = 149; - RemoveChannelMessage remove_channel_message = 150; - - LinkChannel link_channel = 151; - UnlinkChannel unlink_channel = 152; - MoveChannel move_channel = 153; // Current max: 154 + RejoinRoom rejoin_room = 13; + RejoinRoomResponse rejoin_room_response = 14; + LeaveRoom leave_room = 15; + Call call = 16; + IncomingCall incoming_call = 17; + CallCanceled call_canceled = 18; + CancelCall cancel_call = 19; + DeclineCall decline_call = 20; + UpdateParticipantLocation update_participant_location = 21; + RoomUpdated room_updated = 22; + + ShareProject share_project = 23; + ShareProjectResponse share_project_response = 24; + UnshareProject unshare_project = 25; + JoinProject join_project = 26; + JoinProjectResponse join_project_response = 27; + LeaveProject leave_project = 28; + AddProjectCollaborator add_project_collaborator = 29; + UpdateProjectCollaborator update_project_collaborator = 30; + RemoveProjectCollaborator remove_project_collaborator = 31; + + GetDefinition get_definition = 32; + GetDefinitionResponse get_definition_response = 33; + GetTypeDefinition get_type_definition = 34; + GetTypeDefinitionResponse get_type_definition_response = 35; + GetReferences get_references = 36; + GetReferencesResponse get_references_response = 37; + GetDocumentHighlights get_document_highlights = 38; + GetDocumentHighlightsResponse get_document_highlights_response = 39; + GetProjectSymbols get_project_symbols = 40; + GetProjectSymbolsResponse get_project_symbols_response = 41; + OpenBufferForSymbol open_buffer_for_symbol = 42; + OpenBufferForSymbolResponse open_buffer_for_symbol_response = 43; + + UpdateProject update_project = 44; + UpdateWorktree update_worktree = 45; + + CreateProjectEntry create_project_entry = 46; + RenameProjectEntry rename_project_entry = 47; + CopyProjectEntry copy_project_entry = 48; + DeleteProjectEntry delete_project_entry = 49; + ProjectEntryResponse project_entry_response = 50; + ExpandProjectEntry expand_project_entry = 51; + ExpandProjectEntryResponse expand_project_entry_response = 52; + + UpdateDiagnosticSummary update_diagnostic_summary = 53; + StartLanguageServer start_language_server = 54; + UpdateLanguageServer update_language_server = 55; + + OpenBufferById open_buffer_by_id = 56; + OpenBufferByPath open_buffer_by_path = 57; + OpenBufferResponse open_buffer_response = 58; + CreateBufferForPeer create_buffer_for_peer = 59; + UpdateBuffer update_buffer = 60; + UpdateBufferFile update_buffer_file = 61; + SaveBuffer save_buffer = 62; + BufferSaved buffer_saved = 63; + BufferReloaded buffer_reloaded = 64; + ReloadBuffers reload_buffers = 65; + ReloadBuffersResponse reload_buffers_response = 66; + SynchronizeBuffers synchronize_buffers = 67; + SynchronizeBuffersResponse synchronize_buffers_response = 68; + FormatBuffers format_buffers = 69; + FormatBuffersResponse format_buffers_response = 70; + GetCompletions get_completions = 71; + GetCompletionsResponse get_completions_response = 72; + ApplyCompletionAdditionalEdits apply_completion_additional_edits = 73; + ApplyCompletionAdditionalEditsResponse apply_completion_additional_edits_response = 74; + GetCodeActions get_code_actions = 75; + GetCodeActionsResponse get_code_actions_response = 76; + GetHover get_hover = 77; + GetHoverResponse get_hover_response = 78; + ApplyCodeAction apply_code_action = 79; + ApplyCodeActionResponse apply_code_action_response = 80; + PrepareRename prepare_rename = 81; + PrepareRenameResponse prepare_rename_response = 82; + PerformRename perform_rename = 83; + PerformRenameResponse perform_rename_response = 84; + SearchProject search_project = 85; + SearchProjectResponse search_project_response = 86; + + UpdateContacts update_contacts = 87; + UpdateInviteInfo update_invite_info = 88; + ShowContacts show_contacts = 89; + + GetUsers get_users = 90; + FuzzySearchUsers fuzzy_search_users = 91; + UsersResponse users_response = 92; + RequestContact request_contact = 93; + RespondToContactRequest respond_to_contact_request = 94; + RemoveContact remove_contact = 95; + + Follow follow = 96; + FollowResponse follow_response = 97; + UpdateFollowers update_followers = 98; + Unfollow unfollow = 99; + GetPrivateUserInfo get_private_user_info = 100; + GetPrivateUserInfoResponse get_private_user_info_response = 101; + UpdateDiffBase update_diff_base = 102; + + OnTypeFormatting on_type_formatting = 103; + OnTypeFormattingResponse on_type_formatting_response = 104; + + UpdateWorktreeSettings update_worktree_settings = 105; + + InlayHints inlay_hints = 106; + InlayHintsResponse inlay_hints_response = 107; + ResolveInlayHint resolve_inlay_hint = 108; + ResolveInlayHintResponse resolve_inlay_hint_response = 109; + RefreshInlayHints refresh_inlay_hints = 110; + + CreateChannel create_channel = 111; + CreateChannelResponse create_channel_response = 112; + InviteChannelMember invite_channel_member = 113; + RemoveChannelMember remove_channel_member = 114; + RespondToChannelInvite respond_to_channel_invite = 115; + UpdateChannels update_channels = 116; + JoinChannel join_channel = 117; + DeleteChannel delete_channel = 118; + GetChannelMembers get_channel_members = 119; + GetChannelMembersResponse get_channel_members_response = 120; + SetChannelMemberAdmin set_channel_member_admin = 121; + RenameChannel rename_channel = 122; + RenameChannelResponse rename_channel_response = 123; + + JoinChannelBuffer join_channel_buffer = 124; + JoinChannelBufferResponse join_channel_buffer_response = 125; + UpdateChannelBuffer update_channel_buffer = 126; + LeaveChannelBuffer leave_channel_buffer = 127; + UpdateChannelBufferCollaborators update_channel_buffer_collaborators = 128; + RejoinChannelBuffers rejoin_channel_buffers = 129; + RejoinChannelBuffersResponse rejoin_channel_buffers_response = 130; + + JoinChannelChat join_channel_chat = 131; + JoinChannelChatResponse join_channel_chat_response = 132; + LeaveChannelChat leave_channel_chat = 133; + SendChannelMessage send_channel_message = 134; + SendChannelMessageResponse send_channel_message_response = 135; + ChannelMessageSent channel_message_sent = 136; + GetChannelMessages get_channel_messages = 137; + GetChannelMessagesResponse get_channel_messages_response = 138; + RemoveChannelMessage remove_channel_message = 139; + + LinkChannel link_channel = 140; + UnlinkChannel unlink_channel = 141; + MoveChannel move_channel = 142; } } @@ -258,6 +256,7 @@ message Participant { PeerId peer_id = 2; repeated ParticipantProject projects = 3; ParticipantLocation location = 4; + uint32 color_index = 5; } message PendingParticipant { @@ -440,20 +439,9 @@ message RemoveProjectCollaborator { PeerId peer_id = 2; } -message AddChannelBufferCollaborator { +message UpdateChannelBufferCollaborators { uint64 channel_id = 1; - Collaborator collaborator = 2; -} - -message RemoveChannelBufferCollaborator { - uint64 channel_id = 1; - PeerId peer_id = 2; -} - -message UpdateChannelBufferCollaborator { - uint64 channel_id = 1; - PeerId old_peer_id = 2; - PeerId new_peer_id = 3; + repeated Collaborator collaborators = 2; } message GetDefinition { diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 48e9eef7101dff922cfc249264e1f1696bc3b280..6d0a0f85d12447e8b57c90dfc02d8ce1110b7a31 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -270,9 +270,7 @@ messages!( (JoinChannelBufferResponse, Foreground), (LeaveChannelBuffer, Background), (UpdateChannelBuffer, Foreground), - (RemoveChannelBufferCollaborator, Foreground), - (AddChannelBufferCollaborator, Foreground), - (UpdateChannelBufferCollaborator, Foreground), + (UpdateChannelBufferCollaborators, Foreground), ); request_messages!( @@ -407,10 +405,8 @@ entity_messages!( channel_id, ChannelMessageSent, UpdateChannelBuffer, - RemoveChannelBufferCollaborator, RemoveChannelMessage, - AddChannelBufferCollaborator, - UpdateChannelBufferCollaborator + UpdateChannelBufferCollaborators ); const KIB: usize = 1024; diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index 5ea5ce877829e2c3dae5f622b08ec35e0c283e34..a96a3d9c7ca71e775267bcf10f27285b7b4ca493 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -1064,14 +1064,16 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ColorIndex(pub u32); + impl Editor { - pub fn replica_selection_style(&self, replica_id: u16) -> &SelectionStyle { - let style_ix = replica_id as usize % (self.guest_selections.len() + 1); - if style_ix == 0 { - &self.selection - } else { - &self.guest_selections[style_ix - 1] + pub fn replica_selection_style(&self, color_index: ColorIndex) -> SelectionStyle { + if self.guest_selections.is_empty() { + return SelectionStyle::default(); } + let style_ix = color_index.0 as usize % self.guest_selections.len(); + self.guest_selections[style_ix] } } diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index e2fa6e989ab8ca674e322cb1e5fcaebef7a0471b..c223726422692b5e7e01f64d1e638040d3a16e99 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -168,7 +168,7 @@ impl Vim { self.editor_subscription = Some(cx.subscribe(&editor, |editor, event, cx| match event { Event::SelectionsChanged { local: true } => { let editor = editor.read(cx); - if editor.leader_replica_id().is_none() { + if editor.leader_peer_id().is_none() { let newest = editor.selections.newest::(cx); local_selections_changed(newest, cx); } diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index de19e82c8b2fa400e1b2a0377b87b9e20231b581..a489dfd9a43b9277617ea09f7062c63a21593997 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -4,7 +4,10 @@ use crate::{ }; use crate::{AutosaveSetting, DelayedDebouncedEditAction, WorkspaceSettings}; use anyhow::Result; -use client::{proto, Client}; +use client::{ + proto::{self, PeerId}, + Client, +}; use gpui::geometry::vector::Vector2F; use gpui::AnyWindowHandle; use gpui::{ @@ -698,13 +701,13 @@ pub trait FollowableItem: Item { ) -> Task>; fn is_project_item(&self, cx: &AppContext) -> bool; - fn set_leader_replica_id(&mut self, leader_replica_id: Option, cx: &mut ViewContext); + fn set_leader_peer_id(&mut self, leader_peer_id: Option, cx: &mut ViewContext); fn should_unfollow_on_event(event: &Self::Event, cx: &AppContext) -> bool; } pub trait FollowableItemHandle: ItemHandle { fn remote_id(&self, client: &Arc, cx: &AppContext) -> Option; - fn set_leader_replica_id(&self, leader_replica_id: Option, cx: &mut WindowContext); + fn set_leader_peer_id(&self, leader_peer_id: Option, cx: &mut WindowContext); fn to_state_proto(&self, cx: &AppContext) -> Option; fn add_event_to_update_proto( &self, @@ -732,10 +735,8 @@ impl FollowableItemHandle for ViewHandle { }) } - fn set_leader_replica_id(&self, leader_replica_id: Option, cx: &mut WindowContext) { - self.update(cx, |this, cx| { - this.set_leader_replica_id(leader_replica_id, cx) - }) + fn set_leader_peer_id(&self, leader_peer_id: Option, cx: &mut WindowContext) { + self.update(cx, |this, cx| this.set_leader_peer_id(leader_peer_id, cx)) } fn to_state_proto(&self, cx: &AppContext) -> Option { diff --git a/crates/workspace/src/pane_group.rs b/crates/workspace/src/pane_group.rs index 29068ce923020425e67f0182bfb108961603ab19..425fd00b5abb2c607fb22d2212de27a5db7eb909 100644 --- a/crates/workspace/src/pane_group.rs +++ b/crates/workspace/src/pane_group.rs @@ -183,25 +183,23 @@ impl Member { }) .and_then(|leader_id| { let room = active_call?.read(cx).room()?.read(cx); - let collaborator = project.read(cx).collaborators().get(leader_id)?; - let participant = room.remote_participant_for_peer_id(*leader_id)?; - Some((collaborator.replica_id, participant)) + room.remote_participant_for_peer_id(*leader_id) }); - let border = if let Some((replica_id, _)) = leader.as_ref() { - let leader_color = theme.editor.replica_selection_style(*replica_id).cursor; - let mut border = Border::all(theme.workspace.leader_border_width, leader_color); - border + let mut leader_border = Border::default(); + let mut leader_status_box = None; + if let Some(leader) = &leader { + let leader_color = theme + .editor + .replica_selection_style(leader.color_index) + .cursor; + leader_border = Border::all(theme.workspace.leader_border_width, leader_color); + leader_border .color .fade_out(1. - theme.workspace.leader_border_opacity); - border.overlay = true; - border - } else { - Border::default() - }; + leader_border.overlay = true; - let leader_status_box = if let Some((_, leader)) = leader { - match leader.location { + leader_status_box = match leader.location { ParticipantLocation::SharedProject { project_id: leader_project_id, } => { @@ -279,13 +277,11 @@ impl Member { .right() .into_any(), ), - } - } else { - None - }; + }; + } Stack::new() - .with_child(pane_element.contained().with_border(border)) + .with_child(pane_element.contained().with_border(leader_border)) .with_children(leader_status_box) .into_any() } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index f874f5ee164a8f1f2d829cf729d5a27c7be61fa1..9d0d276e4936aa745b4813bdb593c2aa6677de7f 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2423,7 +2423,7 @@ impl Workspace { if let Some(states_by_pane) = self.follower_states_by_leader.remove(&peer_id) { for state in states_by_pane.into_values() { for item in state.items_by_leader_view_id.into_values() { - item.set_leader_replica_id(None, cx); + item.set_leader_peer_id(None, cx); } } } @@ -2527,7 +2527,7 @@ impl Workspace { let leader_id = *leader_id; if let Some(state) = states_by_pane.remove(pane) { for (_, item) in state.items_by_leader_view_id { - item.set_leader_replica_id(None, cx); + item.set_leader_peer_id(None, cx); } if states_by_pane.is_empty() { @@ -2828,16 +2828,6 @@ impl Workspace { let this = this .upgrade(cx) .ok_or_else(|| anyhow!("workspace dropped"))?; - let project = this - .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) - }); let item_builders = cx.update(|cx| { cx.default_global::() @@ -2882,7 +2872,7 @@ impl Workspace { .get_mut(&pane)?; for (id, item) in leader_view_ids.into_iter().zip(items) { - item.set_leader_replica_id(replica_id, cx); + item.set_leader_peer_id(Some(leader_id), cx); state.items_by_leader_view_id.insert(id, item); } diff --git a/selection-color-notes.txt b/selection-color-notes.txt new file mode 100644 index 0000000000000000000000000000000000000000..6186adcac1aaba2bac7e397e3f71bcbba961d275 --- /dev/null +++ b/selection-color-notes.txt @@ -0,0 +1,14 @@ +Assign selection colors to users. goals: + * current user is always main color + * every other user has the same color in every context + * users don't need to be in a shared project to have a color. they can either be in the call, or in a channel notes. + +Places colors are used: + * editor element, driven by the buffer's `remote_selections` + * pane border (access to more state) + * collab titlebar (access to more state) + +Currently, editor holds an optional "replica id map". + +Most challenging part is in the editor, because the editor should be fairly self-contained, not depend on e.g. the user store. + From 0f39b638019d0826e8f7502a31eafcbf0e3955ac Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Sep 2023 11:18:29 -0700 Subject: [PATCH 08/24] Rename color_index to participant_index Co-authored-by: Conrad --- crates/call/src/participant.rs | 4 +-- crates/call/src/room.rs | 13 +++++----- crates/client/src/user.rs | 24 +++++++++-------- .../20221109000000_test_schema.sql | 2 +- ...0_add_color_index_to_room_participants.sql | 1 - ...participant_index_to_room_participants.sql | 1 + crates/collab/src/db/queries/rooms.rs | 24 ++++++++--------- .../collab/src/db/tables/room_participant.rs | 2 +- .../collab/src/tests/channel_buffer_tests.rs | 24 +++++++++++------ crates/collab_ui/src/channel_view.rs | 9 ++++--- crates/collab_ui/src/collab_titlebar_item.rs | 26 ++++++++++++------- crates/editor/src/editor.rs | 24 ++++++++++------- crates/editor/src/element.rs | 4 +-- crates/project/src/project.rs | 8 ------ crates/rpc/proto/zed.proto | 2 +- crates/theme/src/theme.rs | 7 ++--- crates/workspace/src/pane_group.rs | 2 +- 17 files changed, 97 insertions(+), 80 deletions(-) delete mode 100644 crates/collab/migrations/20230926102500_add_color_index_to_room_participants.sql create mode 100644 crates/collab/migrations/20230926102500_add_participant_index_to_room_participants.sql diff --git a/crates/call/src/participant.rs b/crates/call/src/participant.rs index b0751be91956a6f47e023fb8738bd227b6a4cde9..ab796e56b08d3a8b1a00b541135f90edada3a645 100644 --- a/crates/call/src/participant.rs +++ b/crates/call/src/participant.rs @@ -1,4 +1,5 @@ use anyhow::{anyhow, Result}; +use client::ParticipantIndex; use client::{proto, User}; use collections::HashMap; use gpui::WeakModelHandle; @@ -6,7 +7,6 @@ pub use live_kit_client::Frame; use live_kit_client::RemoteAudioTrack; use project::Project; use std::{fmt, sync::Arc}; -use theme::ColorIndex; #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub enum ParticipantLocation { @@ -44,7 +44,7 @@ pub struct RemoteParticipant { pub peer_id: proto::PeerId, pub projects: Vec, pub location: ParticipantLocation, - pub color_index: ColorIndex, + pub participant_index: ParticipantIndex, pub muted: bool, pub speaking: bool, pub video_tracks: HashMap>, diff --git a/crates/call/src/room.rs b/crates/call/src/room.rs index e6759d87ca6a8af4af4fc5e04bfc3b1c26d5971c..bf30e31a98ed9bd2245cb9491e04005d39f8be45 100644 --- a/crates/call/src/room.rs +++ b/crates/call/src/room.rs @@ -7,7 +7,7 @@ use anyhow::{anyhow, Result}; use audio::{Audio, Sound}; use client::{ proto::{self, PeerId}, - Client, TypedEnvelope, User, UserStore, + Client, ParticipantIndex, TypedEnvelope, User, UserStore, }; use collections::{BTreeMap, HashMap, HashSet}; use fs::Fs; @@ -21,7 +21,6 @@ use live_kit_client::{ use postage::stream::Stream; use project::Project; use std::{future::Future, mem, pin::Pin, sync::Arc, time::Duration}; -use theme::ColorIndex; use util::{post_inc, ResultExt, TryFutureExt}; pub const RECONNECT_TIMEOUT: Duration = Duration::from_secs(30); @@ -715,7 +714,9 @@ impl Room { participant.user_id, RemoteParticipant { user: user.clone(), - color_index: ColorIndex(participant.color_index), + participant_index: ParticipantIndex( + participant.participant_index, + ), peer_id, projects: participant.projects, location, @@ -810,12 +811,12 @@ impl Room { } this.user_store.update(cx, |user_store, cx| { - let color_indices_by_user_id = this + let participant_indices_by_user_id = this .remote_participants .iter() - .map(|(user_id, participant)| (*user_id, participant.color_index)) + .map(|(user_id, participant)| (*user_id, participant.participant_index)) .collect(); - user_store.set_color_indices(color_indices_by_user_id, cx); + user_store.set_participant_indices(participant_indices_by_user_id, cx); }); this.check_invariants(); diff --git a/crates/client/src/user.rs b/crates/client/src/user.rs index 052254558723ad5c30941cf8e696a4aca1a064be..b8cc8fb1b890fce59754f1a1bc93b6e2fa61ce2a 100644 --- a/crates/client/src/user.rs +++ b/crates/client/src/user.rs @@ -8,12 +8,14 @@ use postage::{sink::Sink, watch}; use rpc::proto::{RequestMessage, UsersResponse}; use std::sync::{Arc, Weak}; use text::ReplicaId; -use theme::ColorIndex; use util::http::HttpClient; use util::TryFutureExt as _; pub type UserId = u64; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ParticipantIndex(pub u32); + #[derive(Default, Debug)] pub struct User { pub id: UserId, @@ -65,7 +67,7 @@ pub enum ContactRequestStatus { pub struct UserStore { users: HashMap>, - color_indices: HashMap, + participant_indices: HashMap, update_contacts_tx: mpsc::UnboundedSender, current_user: watch::Receiver>>, contacts: Vec>, @@ -91,7 +93,7 @@ pub enum Event { kind: ContactEventKind, }, ShowContacts, - ColorIndicesChanged, + ParticipantIndicesChanged, } #[derive(Clone, Copy)] @@ -129,7 +131,7 @@ impl UserStore { current_user: current_user_rx, contacts: Default::default(), incoming_contact_requests: Default::default(), - color_indices: Default::default(), + participant_indices: Default::default(), outgoing_contact_requests: Default::default(), invite_info: None, client: Arc::downgrade(&client), @@ -654,19 +656,19 @@ impl UserStore { }) } - pub fn set_color_indices( + pub fn set_participant_indices( &mut self, - color_indices: HashMap, + participant_indices: HashMap, cx: &mut ModelContext, ) { - if color_indices != self.color_indices { - self.color_indices = color_indices; - cx.emit(Event::ColorIndicesChanged); + if participant_indices != self.participant_indices { + self.participant_indices = participant_indices; + cx.emit(Event::ParticipantIndicesChanged); } } - pub fn color_indices(&self) -> &HashMap { - &self.color_indices + pub fn participant_indices(&self) -> &HashMap { + &self.participant_indices } } diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 5f5484679fb968ce63d58f952ea028986ee13fd4..8a153949c271478502956d9a3a5faaa3e9ca0ec2 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -159,7 +159,7 @@ CREATE TABLE "room_participants" ( "calling_user_id" INTEGER NOT NULL REFERENCES users (id), "calling_connection_id" INTEGER NOT NULL, "calling_connection_server_id" INTEGER REFERENCES servers (id) ON DELETE SET NULL, - "color_index" INTEGER + "participant_index" INTEGER ); CREATE UNIQUE INDEX "index_room_participants_on_user_id" ON "room_participants" ("user_id"); CREATE INDEX "index_room_participants_on_room_id" ON "room_participants" ("room_id"); diff --git a/crates/collab/migrations/20230926102500_add_color_index_to_room_participants.sql b/crates/collab/migrations/20230926102500_add_color_index_to_room_participants.sql deleted file mode 100644 index 626268bd5c739503a232db7548818782db298dc1..0000000000000000000000000000000000000000 --- a/crates/collab/migrations/20230926102500_add_color_index_to_room_participants.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE room_participants ADD COLUMN color_index INTEGER; diff --git a/crates/collab/migrations/20230926102500_add_participant_index_to_room_participants.sql b/crates/collab/migrations/20230926102500_add_participant_index_to_room_participants.sql new file mode 100644 index 0000000000000000000000000000000000000000..1493119e2a97ac42f5d69ebc82ac3d3d0dc4dd63 --- /dev/null +++ b/crates/collab/migrations/20230926102500_add_participant_index_to_room_participants.sql @@ -0,0 +1 @@ +ALTER TABLE room_participants ADD COLUMN participant_index INTEGER; diff --git a/crates/collab/src/db/queries/rooms.rs b/crates/collab/src/db/queries/rooms.rs index fca4c67690c3b9fbaec101da55a0a19a9e72be61..70e39c91b342c4dcefbb1be6dc1094980209333d 100644 --- a/crates/collab/src/db/queries/rooms.rs +++ b/crates/collab/src/db/queries/rooms.rs @@ -152,7 +152,7 @@ impl Database { room_id: ActiveValue::set(room_id), user_id: ActiveValue::set(called_user_id), answering_connection_lost: ActiveValue::set(false), - color_index: ActiveValue::NotSet, + participant_index: ActiveValue::NotSet, calling_user_id: ActiveValue::set(calling_user_id), calling_connection_id: ActiveValue::set(calling_connection.id as i32), calling_connection_server_id: ActiveValue::set(Some(ServerId( @@ -285,19 +285,19 @@ impl Database { .ok_or_else(|| anyhow!("no such room"))?; #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] - enum QueryColorIndices { - ColorIndex, + enum QueryParticipantIndices { + ParticipantIndex, } - let existing_color_indices: Vec = room_participant::Entity::find() + let existing_participant_indices: Vec = room_participant::Entity::find() .filter(room_participant::Column::RoomId.eq(room_id)) .select_only() - .column(room_participant::Column::ColorIndex) - .into_values::<_, QueryColorIndices>() + .column(room_participant::Column::ParticipantIndex) + .into_values::<_, QueryParticipantIndices>() .all(&*tx) .await?; - let mut color_index = 0; - while existing_color_indices.contains(&color_index) { - color_index += 1; + let mut participant_index = 0; + while existing_participant_indices.contains(&participant_index) { + participant_index += 1; } if let Some(channel_id) = channel_id { @@ -317,7 +317,7 @@ impl Database { calling_connection_server_id: ActiveValue::set(Some(ServerId( connection.owner_id as i32, ))), - color_index: ActiveValue::Set(color_index), + participant_index: ActiveValue::Set(participant_index), ..Default::default() }]) .on_conflict( @@ -340,7 +340,7 @@ impl Database { .add(room_participant::Column::AnsweringConnectionId.is_null()), ) .set(room_participant::ActiveModel { - color_index: ActiveValue::Set(color_index), + participant_index: ActiveValue::Set(participant_index), answering_connection_id: ActiveValue::set(Some(connection.id as i32)), answering_connection_server_id: ActiveValue::set(Some(ServerId( connection.owner_id as i32, @@ -1090,7 +1090,7 @@ impl Database { peer_id: Some(answering_connection.into()), projects: Default::default(), location: Some(proto::ParticipantLocation { variant: location }), - color_index: db_participant.color_index as u32, + participant_index: db_participant.participant_index as u32, }, ); } else { diff --git a/crates/collab/src/db/tables/room_participant.rs b/crates/collab/src/db/tables/room_participant.rs index 8072fed69ce8f32c07386498e6e971eb0ccf4077..d93667b3fc8b66e018e7c2449f4830e9a769b11f 100644 --- a/crates/collab/src/db/tables/room_participant.rs +++ b/crates/collab/src/db/tables/room_participant.rs @@ -18,7 +18,7 @@ pub struct Model { pub calling_user_id: UserId, pub calling_connection_id: i32, pub calling_connection_server_id: Option, - pub color_index: i32, + pub participant_index: i32, } impl Model { diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index e403ed6f94916ade3f6409379c402af407ad727e..7033c71e8c60161f5678ca164241fae9d1152b1c 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -4,6 +4,7 @@ use crate::{ }; use call::ActiveCall; use channel::Channel; +use client::ParticipantIndex; use client::{Collaborator, UserId}; use collab_ui::channel_view::ChannelView; use collections::HashMap; @@ -13,7 +14,6 @@ use gpui::{executor::Deterministic, ModelHandle, TestAppContext, ViewContext}; use rpc::{proto::PeerId, RECEIVE_TIMEOUT}; use serde_json::json; use std::{ops::Range, sync::Arc}; -use theme::ColorIndex; #[gpui::test] async fn test_core_channel_buffers( @@ -122,7 +122,7 @@ async fn test_core_channel_buffers( } #[gpui::test] -async fn test_channel_notes_color_indices( +async fn test_channel_notes_participant_indices( deterministic: Arc, mut cx_a: &mut TestAppContext, mut cx_b: &mut TestAppContext, @@ -226,12 +226,20 @@ async fn test_channel_notes_color_indices( deterministic.run_until_parked(); channel_view_a.update(cx_a, |notes, cx| { notes.editor.update(cx, |editor, cx| { - assert_remote_selections(editor, &[(Some(ColorIndex(1)), 1..2), (None, 2..3)], cx); + assert_remote_selections( + editor, + &[(Some(ParticipantIndex(1)), 1..2), (None, 2..3)], + cx, + ); }); }); channel_view_b.update(cx_b, |notes, cx| { notes.editor.update(cx, |editor, cx| { - assert_remote_selections(editor, &[(Some(ColorIndex(0)), 0..1), (None, 2..3)], cx); + assert_remote_selections( + editor, + &[(Some(ParticipantIndex(0)), 0..1), (None, 2..3)], + cx, + ); }); }); @@ -275,17 +283,17 @@ async fn test_channel_notes_color_indices( // Clients A and B see each other with the same colors as in the channel notes. editor_a.update(cx_a, |editor, cx| { - assert_remote_selections(editor, &[(Some(ColorIndex(1)), 2..3)], cx); + assert_remote_selections(editor, &[(Some(ParticipantIndex(1)), 2..3)], cx); }); editor_b.update(cx_b, |editor, cx| { - assert_remote_selections(editor, &[(Some(ColorIndex(0)), 0..1)], cx); + assert_remote_selections(editor, &[(Some(ParticipantIndex(0)), 0..1)], cx); }); } #[track_caller] fn assert_remote_selections( editor: &mut Editor, - expected_selections: &[(Option, Range)], + expected_selections: &[(Option, Range)], cx: &mut ViewContext, ) { let snapshot = editor.snapshot(cx); @@ -295,7 +303,7 @@ fn assert_remote_selections( .map(|s| { let start = s.selection.start.to_offset(&snapshot.buffer_snapshot); let end = s.selection.end.to_offset(&snapshot.buffer_snapshot); - (s.color_index, start..end) + (s.participant_index, start..end) }) .collect::>(); assert_eq!( diff --git a/crates/collab_ui/src/channel_view.rs b/crates/collab_ui/src/channel_view.rs index 1d103350def13380628e1ccd75e25a77f963d029..22e7d6637ce1612fb2959018a874021d404eeea3 100644 --- a/crates/collab_ui/src/channel_view.rs +++ b/crates/collab_ui/src/channel_view.rs @@ -3,7 +3,7 @@ use call::ActiveCall; use channel::{Channel, ChannelBuffer, ChannelBufferEvent, ChannelId}; use client::{ proto::{self, PeerId}, - Collaborator, + Collaborator, ParticipantIndex, }; use collections::HashMap; use editor::{CollaborationHub, Editor}; @@ -359,7 +359,10 @@ impl CollaborationHub for ChannelBufferCollaborationHub { self.0.read(cx).collaborators() } - fn user_color_indices<'a>(&self, cx: &'a AppContext) -> &'a HashMap { - self.0.read(cx).user_store().read(cx).color_indices() + fn user_participant_indices<'a>( + &self, + cx: &'a AppContext, + ) -> &'a HashMap { + self.0.read(cx).user_store().read(cx).participant_indices() } } diff --git a/crates/collab_ui/src/collab_titlebar_item.rs b/crates/collab_ui/src/collab_titlebar_item.rs index 5dafae0cd5d5aaa1e219aa6c9f5f2173f281aac6..c1b3689aadabcbebab5ad28686005ee22a049884 100644 --- a/crates/collab_ui/src/collab_titlebar_item.rs +++ b/crates/collab_ui/src/collab_titlebar_item.rs @@ -923,15 +923,18 @@ impl CollabTitlebarItem { .background_color .unwrap_or_default(); - let color_index = self + let participant_index = self .user_store .read(cx) - .color_indices() + .participant_indices() .get(&user_id) .copied(); - if let Some(color_index) = color_index { + if let Some(participant_index) = participant_index { if followed_by_self { - let selection = theme.editor.replica_selection_style(color_index).selection; + let selection = theme + .editor + .selection_style_for_room_participant(participant_index.0) + .selection; background_color = Color::blend(selection, background_color); background_color.a = 255; } @@ -996,10 +999,12 @@ impl CollabTitlebarItem { .contained() .with_style(theme.titlebar.leader_selection); - if let Some(color_index) = color_index { + if let Some(participant_index) = participant_index { if followed_by_self { - let color = - theme.editor.replica_selection_style(color_index).selection; + let color = theme + .editor + .selection_style_for_room_participant(participant_index.0) + .selection; container = container.with_background_color(color); } } @@ -1007,8 +1012,11 @@ impl CollabTitlebarItem { container })) .with_children((|| { - let color_index = color_index?; - let color = theme.editor.replica_selection_style(color_index).cursor; + let participant_index = participant_index?; + let color = theme + .editor + .selection_style_for_room_participant(participant_index.0) + .cursor; Some( AvatarRibbon::new(color) .constrained() diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 7692a54b01f95cfbd5f42cece19ce36e8cea1fee..2e15dd3a929a532924835b23e1a8e676ef147576 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -25,7 +25,7 @@ use ::git::diff::DiffHunk; use aho_corasick::AhoCorasick; use anyhow::{anyhow, Context, Result}; use blink_manager::BlinkManager; -use client::{ClickhouseEvent, Collaborator, TelemetrySettings}; +use client::{ClickhouseEvent, Collaborator, ParticipantIndex, TelemetrySettings}; use clock::{Global, ReplicaId}; use collections::{BTreeMap, Bound, HashMap, HashSet, VecDeque}; use convert_case::{Case, Casing}; @@ -102,7 +102,7 @@ use std::{ pub use sum_tree::Bias; use sum_tree::TreeMap; use text::Rope; -use theme::{ColorIndex, DiagnosticStyle, Theme, ThemeSettings}; +use theme::{DiagnosticStyle, Theme, ThemeSettings}; use util::{post_inc, RangeExt, ResultExt, TryFutureExt}; use workspace::{ItemNavHistory, ViewId, Workspace}; @@ -637,7 +637,7 @@ pub struct RemoteSelection { pub cursor_shape: CursorShape, pub peer_id: PeerId, pub line_mode: bool, - pub color_index: Option, + pub participant_index: Option, } #[derive(Clone, Debug)] @@ -8570,7 +8570,10 @@ impl Editor { pub trait CollaborationHub { fn collaborators<'a>(&self, cx: &'a AppContext) -> &'a HashMap; - fn user_color_indices<'a>(&self, cx: &'a AppContext) -> &'a HashMap; + fn user_participant_indices<'a>( + &self, + cx: &'a AppContext, + ) -> &'a HashMap; } impl CollaborationHub for ModelHandle { @@ -8578,8 +8581,11 @@ impl CollaborationHub for ModelHandle { self.read(cx).collaborators() } - fn user_color_indices<'a>(&self, cx: &'a AppContext) -> &'a HashMap { - self.read(cx).user_store().read(cx).color_indices() + fn user_participant_indices<'a>( + &self, + cx: &'a AppContext, + ) -> &'a HashMap { + self.read(cx).user_store().read(cx).participant_indices() } } @@ -8632,7 +8638,7 @@ impl EditorSnapshot { collaboration_hub: &dyn CollaborationHub, cx: &'a AppContext, ) -> impl 'a + Iterator { - let color_indices = collaboration_hub.user_color_indices(cx); + let participant_indices = collaboration_hub.user_participant_indices(cx); let collaborators_by_peer_id = collaboration_hub.collaborators(cx); let collaborators_by_replica_id = collaborators_by_peer_id .iter() @@ -8642,13 +8648,13 @@ impl EditorSnapshot { .remote_selections_in_range(range) .filter_map(move |(replica_id, line_mode, cursor_shape, selection)| { let collaborator = collaborators_by_replica_id.get(&replica_id)?; - let color_index = color_indices.get(&collaborator.user_id).copied(); + let participant_index = participant_indices.get(&collaborator.user_id).copied(); Some(RemoteSelection { replica_id, selection, cursor_shape, line_mode, - color_index, + participant_index, peer_id: collaborator.peer_id, }) }) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index dad5b06626dfdee0e57cd3297a86728f3e83ae6f..0be395729fa78fc7e94d792b40ff293a145c4e19 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -2256,8 +2256,8 @@ impl Element for EditorElement { collaboration_hub.as_ref(), cx, ) { - let selection_style = if let Some(color_index) = selection.color_index { - style.replica_selection_style(color_index) + let selection_style = if let Some(participant_index) = selection.participant_index { + style.selection_style_for_room_participant(participant_index.0) } else { style.absent_selection }; diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 62df8425c401ff5a571fed7498cac8a8724ddfcd..86303555dd58715e6b2e3f1dd2fd403dd12be4d6 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -76,7 +76,6 @@ use std::{ }; use terminals::Terminals; use text::Anchor; -use theme::ColorIndex; use util::{ debug_panic, defer, http::HttpClient, merge_json_value_into, paths::LOCAL_SETTINGS_RELATIVE_PATH, post_inc, ResultExt, TryFutureExt as _, @@ -120,7 +119,6 @@ pub struct Project { join_project_response_message_id: u32, next_diagnostic_group_id: usize, user_store: ModelHandle, - user_color_indices: HashMap, fs: Arc, client_state: Option, collaborators: HashMap, @@ -644,7 +642,6 @@ impl Project { languages, client, user_store, - user_color_indices: Default::default(), fs, next_entry_id: Default::default(), next_diagnostic_group_id: Default::default(), @@ -717,7 +714,6 @@ impl Project { _maintain_workspace_config: Self::maintain_workspace_config(cx), languages, user_store: user_store.clone(), - user_color_indices: Default::default(), fs, next_entry_id: Default::default(), next_diagnostic_group_id: Default::default(), @@ -922,10 +918,6 @@ impl Project { self.user_store.clone() } - pub fn user_color_indices(&self) -> &HashMap { - &self.user_color_indices - } - pub fn opened_buffers(&self, cx: &AppContext) -> Vec> { self.opened_buffers .values() diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index da97cd35c76f50708055dc7fce9dad82f1a8e797..523ef1d76356a473979a51ea830dc78e0ded6009 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -256,7 +256,7 @@ message Participant { PeerId peer_id = 2; repeated ParticipantProject projects = 3; ParticipantLocation location = 4; - uint32 color_index = 5; + uint32 participant_index = 5; } message PendingParticipant { diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index a96a3d9c7ca71e775267bcf10f27285b7b4ca493..1ca2d839c09b08117a9024065b970f13f837eae8 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -1064,15 +1064,12 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive { } } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct ColorIndex(pub u32); - impl Editor { - pub fn replica_selection_style(&self, color_index: ColorIndex) -> SelectionStyle { + pub fn selection_style_for_room_participant(&self, participant_index: u32) -> SelectionStyle { if self.guest_selections.is_empty() { return SelectionStyle::default(); } - let style_ix = color_index.0 as usize % self.guest_selections.len(); + let style_ix = participant_index as usize % self.guest_selections.len(); self.guest_selections[style_ix] } } diff --git a/crates/workspace/src/pane_group.rs b/crates/workspace/src/pane_group.rs index 425fd00b5abb2c607fb22d2212de27a5db7eb909..7c7a7db7ea3d3539931ecc3876252324ca3ea5eb 100644 --- a/crates/workspace/src/pane_group.rs +++ b/crates/workspace/src/pane_group.rs @@ -191,7 +191,7 @@ impl Member { if let Some(leader) = &leader { let leader_color = theme .editor - .replica_selection_style(leader.color_index) + .selection_style_for_room_participant(leader.participant_index.0) .cursor; leader_border = Border::all(theme.workspace.leader_border_width, leader_color); leader_border From 0c95e5a6ca01e7c1d4d2e7d772dd8b4aa5e40fa3 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Sep 2023 11:37:44 -0700 Subject: [PATCH 09/24] Fix coloring of local selections when following Co-authored-by: Conrad --- crates/editor/src/element.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 0be395729fa78fc7e94d792b40ff293a145c4e19..924d66c21c5532fa79a2abb0af5ac1116cde463e 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -2250,6 +2250,21 @@ impl Element for EditorElement { } if let Some(collaboration_hub) = &editor.collaboration_hub { + // When following someone, render the local selections in their color. + if let Some(leader_id) = editor.leader_peer_id { + if let Some(collaborator) = collaboration_hub.collaborators(cx).get(&leader_id) { + if let Some(participant_index) = collaboration_hub + .user_participant_indices(cx) + .get(&collaborator.user_id) + { + if let Some((local_selection_style, _)) = selections.first_mut() { + *local_selection_style = + style.selection_style_for_room_participant(participant_index.0); + } + } + } + } + let mut remote_selections = HashMap::default(); for selection in snapshot.remote_selections_in_range( &(start_anchor..end_anchor), @@ -2262,11 +2277,9 @@ impl Element for EditorElement { style.absent_selection }; - // The local selections match the leader's selections. + // Don't re-render the leader's selections, since the local selections + // match theirs. if Some(selection.peer_id) == editor.leader_peer_id { - if let Some((local_selection_style, _)) = selections.first_mut() { - *local_selection_style = selection_style; - } continue; } From ce940da8e92361692cf42bcad307e97edf487994 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Sep 2023 12:03:53 -0700 Subject: [PATCH 10/24] Fix errors from assuming all room_participant rows had a non-null participant_index Rows representing pending participants have a null participant_index. Co-authored-by: Conrad --- crates/collab/src/db/queries/rooms.rs | 27 +++++++++++++------ .../collab/src/db/tables/room_participant.rs | 2 +- .../collab/src/tests/channel_buffer_tests.rs | 12 +++++---- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/crates/collab/src/db/queries/rooms.rs b/crates/collab/src/db/queries/rooms.rs index 70e39c91b342c4dcefbb1be6dc1094980209333d..de1e8a1ce7ba9b5321ba81ecd6ce7111cf5afbf0 100644 --- a/crates/collab/src/db/queries/rooms.rs +++ b/crates/collab/src/db/queries/rooms.rs @@ -128,6 +128,7 @@ impl Database { calling_connection_server_id: ActiveValue::set(Some(ServerId( connection.owner_id as i32, ))), + participant_index: ActiveValue::set(Some(0)), ..Default::default() } .insert(&*tx) @@ -289,7 +290,11 @@ impl Database { ParticipantIndex, } let existing_participant_indices: Vec = room_participant::Entity::find() - .filter(room_participant::Column::RoomId.eq(room_id)) + .filter( + room_participant::Column::RoomId + .eq(room_id) + .and(room_participant::Column::ParticipantIndex.is_not_null()), + ) .select_only() .column(room_participant::Column::ParticipantIndex) .into_values::<_, QueryParticipantIndices>() @@ -317,7 +322,7 @@ impl Database { calling_connection_server_id: ActiveValue::set(Some(ServerId( connection.owner_id as i32, ))), - participant_index: ActiveValue::Set(participant_index), + participant_index: ActiveValue::Set(Some(participant_index)), ..Default::default() }]) .on_conflict( @@ -326,6 +331,7 @@ impl Database { room_participant::Column::AnsweringConnectionId, room_participant::Column::AnsweringConnectionServerId, room_participant::Column::AnsweringConnectionLost, + room_participant::Column::ParticipantIndex, ]) .to_owned(), ) @@ -340,7 +346,7 @@ impl Database { .add(room_participant::Column::AnsweringConnectionId.is_null()), ) .set(room_participant::ActiveModel { - participant_index: ActiveValue::Set(participant_index), + participant_index: ActiveValue::Set(Some(participant_index)), answering_connection_id: ActiveValue::set(Some(connection.id as i32)), answering_connection_server_id: ActiveValue::set(Some(ServerId( connection.owner_id as i32, @@ -1056,10 +1062,15 @@ impl Database { let mut pending_participants = Vec::new(); while let Some(db_participant) = db_participants.next().await { let db_participant = db_participant?; - if let Some((answering_connection_id, answering_connection_server_id)) = db_participant - .answering_connection_id - .zip(db_participant.answering_connection_server_id) - { + if let ( + Some(answering_connection_id), + Some(answering_connection_server_id), + Some(participant_index), + ) = ( + db_participant.answering_connection_id, + db_participant.answering_connection_server_id, + db_participant.participant_index, + ) { let location = match ( db_participant.location_kind, db_participant.location_project_id, @@ -1090,7 +1101,7 @@ impl Database { peer_id: Some(answering_connection.into()), projects: Default::default(), location: Some(proto::ParticipantLocation { variant: location }), - participant_index: db_participant.participant_index as u32, + participant_index: participant_index as u32, }, ); } else { diff --git a/crates/collab/src/db/tables/room_participant.rs b/crates/collab/src/db/tables/room_participant.rs index d93667b3fc8b66e018e7c2449f4830e9a769b11f..4c5b8cc11c7a23532de3e7d0ea61f55fe3a4077f 100644 --- a/crates/collab/src/db/tables/room_participant.rs +++ b/crates/collab/src/db/tables/room_participant.rs @@ -18,7 +18,7 @@ pub struct Model { pub calling_user_id: UserId, pub calling_connection_id: i32, pub calling_connection_server_id: Option, - pub participant_index: i32, + pub participant_index: Option, } impl Model { diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index 7033c71e8c60161f5678ca164241fae9d1152b1c..05abda5af3c7e77530860dc4cc3bcde490cd7e9e 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -102,7 +102,7 @@ async fn test_core_channel_buffers( channel_buffer_b.read_with(cx_b, |buffer, _| { assert_collaborators( &buffer.collaborators(), - &[client_b.user_id(), client_a.user_id()], + &[client_a.user_id(), client_b.user_id()], ); }); @@ -759,11 +759,13 @@ async fn test_following_to_channel_notes_without_a_shared_project( #[track_caller] fn assert_collaborators(collaborators: &HashMap, ids: &[Option]) { + let mut user_ids = collaborators + .values() + .map(|collaborator| collaborator.user_id) + .collect::>(); + user_ids.sort(); assert_eq!( - collaborators - .values() - .map(|collaborator| collaborator.user_id) - .collect::>(), + user_ids, ids.into_iter().map(|id| id.unwrap()).collect::>() ); } From e9c1ad6acd9b1ae6750ef958be237e6d05b1f6d7 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Sep 2023 14:21:44 -0700 Subject: [PATCH 11/24] Undo making project optional on stored follower states Following works without a project, but following in unshared projects does not need to be replicated to other participants. --- crates/call/src/call.rs | 2 +- crates/call/src/room.rs | 4 +- .../20221109000000_test_schema.sql | 2 +- ...142700_allow_following_without_project.sql | 1 - crates/collab/src/db/queries/projects.rs | 91 +++++-------------- crates/collab/src/db/queries/rooms.rs | 2 +- crates/collab/src/db/tables/follower.rs | 2 +- crates/collab/src/rpc.rs | 32 ++++--- crates/rpc/proto/zed.proto | 2 +- 9 files changed, 46 insertions(+), 92 deletions(-) delete mode 100644 crates/collab/migrations/20230918142700_allow_following_without_project.sql diff --git a/crates/call/src/call.rs b/crates/call/src/call.rs index 8c570c71650e4d35b6ed36a9ce79b3f5cbd6e16b..6756c2aa533eb8a9e46f68c4310b5436a606991a 100644 --- a/crates/call/src/call.rs +++ b/crates/call/src/call.rs @@ -58,7 +58,7 @@ pub struct ActiveCall { _subscriptions: Vec, } -#[derive(PartialEq, Eq, PartialOrd, Ord)] +#[derive(PartialEq, Eq, PartialOrd, Ord, Debug)] struct Follower { project_id: Option, peer_id: PeerId, diff --git a/crates/call/src/room.rs b/crates/call/src/room.rs index bf30e31a98ed9bd2245cb9491e04005d39f8be45..26a531cc31d47721271689a6407afebf27237429 100644 --- a/crates/call/src/room.rs +++ b/crates/call/src/room.rs @@ -62,7 +62,7 @@ pub struct Room { leave_when_empty: bool, client: Arc, user_store: ModelHandle, - follows_by_leader_id_project_id: HashMap<(PeerId, Option), Vec>, + follows_by_leader_id_project_id: HashMap<(PeerId, u64), Vec>, subscriptions: Vec, pending_room_update: Option>, maintain_connection: Option>>, @@ -584,7 +584,7 @@ impl Room { pub fn followers_for(&self, leader_id: PeerId, project_id: u64) -> &[PeerId] { self.follows_by_leader_id_project_id - .get(&(leader_id, Some(project_id))) + .get(&(leader_id, project_id)) .map_or(&[], |v| v.as_slice()) } diff --git a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql index 8a153949c271478502956d9a3a5faaa3e9ca0ec2..d8325755f80e0baa90903d39326754ae713c7180 100644 --- a/crates/collab/migrations.sqlite/20221109000000_test_schema.sql +++ b/crates/collab/migrations.sqlite/20221109000000_test_schema.sql @@ -176,7 +176,7 @@ CREATE TABLE "servers" ( CREATE TABLE "followers" ( "id" INTEGER PRIMARY KEY AUTOINCREMENT, "room_id" INTEGER NOT NULL REFERENCES rooms (id) ON DELETE CASCADE, - "project_id" INTEGER REFERENCES projects (id) ON DELETE CASCADE, + "project_id" INTEGER NOT NULL REFERENCES projects (id) ON DELETE CASCADE, "leader_connection_server_id" INTEGER NOT NULL REFERENCES servers (id) ON DELETE CASCADE, "leader_connection_id" INTEGER NOT NULL, "follower_connection_server_id" INTEGER NOT NULL REFERENCES servers (id) ON DELETE CASCADE, diff --git a/crates/collab/migrations/20230918142700_allow_following_without_project.sql b/crates/collab/migrations/20230918142700_allow_following_without_project.sql deleted file mode 100644 index e0cc0141ec07596340e1ca4655324db9ea11f19f..0000000000000000000000000000000000000000 --- a/crates/collab/migrations/20230918142700_allow_following_without_project.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE followers ALTER COLUMN project_id DROP NOT NULL; diff --git a/crates/collab/src/db/queries/projects.rs b/crates/collab/src/db/queries/projects.rs index 80e71eb1eb4dd59d819f57d2f9cc04d484b3840a..3e2c00337823a91badeedf183a5598f94f8f2c2a 100644 --- a/crates/collab/src/db/queries/projects.rs +++ b/crates/collab/src/db/queries/projects.rs @@ -862,83 +862,34 @@ impl Database { .await } - pub async fn check_can_follow( + pub async fn check_room_participants( &self, room_id: RoomId, - project_id: Option, leader_id: ConnectionId, follower_id: ConnectionId, ) -> Result<()> { - let mut found_leader = false; - let mut found_follower = false; self.transaction(|tx| async move { - if let Some(project_id) = project_id { - let mut rows = project_collaborator::Entity::find() - .filter(project_collaborator::Column::ProjectId.eq(project_id)) - .stream(&*tx) - .await?; - while let Some(row) = rows.next().await { - let row = row?; - let connection = row.connection(); - if connection == leader_id { - found_leader = true; - } else if connection == follower_id { - found_follower = true; - } - } - } else { - let mut rows = room_participant::Entity::find() - .filter(room_participant::Column::RoomId.eq(room_id)) - .stream(&*tx) - .await?; - while let Some(row) = rows.next().await { - let row = row?; - if let Some(connection) = row.answering_connection() { - if connection == leader_id { - found_leader = true; - } else if connection == follower_id { - found_follower = true; - } - } - } - } + use room_participant::Column; - if !found_leader || !found_follower { - Err(anyhow!("not a room participant"))?; - } - - Ok(()) - }) - .await - } - - pub async fn check_can_unfollow( - &self, - room_id: RoomId, - project_id: Option, - leader_id: ConnectionId, - follower_id: ConnectionId, - ) -> Result<()> { - self.transaction(|tx| async move { - follower::Entity::find() + let count = room_participant::Entity::find() .filter( - Condition::all() - .add(follower::Column::RoomId.eq(room_id)) - .add(follower::Column::ProjectId.eq(project_id)) - .add(follower::Column::LeaderConnectionId.eq(leader_id.id as i32)) - .add(follower::Column::FollowerConnectionId.eq(follower_id.id as i32)) - .add( - follower::Column::LeaderConnectionServerId - .eq(leader_id.owner_id as i32), - ) - .add( - follower::Column::FollowerConnectionServerId - .eq(follower_id.owner_id as i32), - ), + Condition::all().add(Column::RoomId.eq(room_id)).add( + Condition::any() + .add(Column::AnsweringConnectionId.eq(leader_id.id as i32).and( + Column::AnsweringConnectionServerId.eq(leader_id.owner_id as i32), + )) + .add(Column::AnsweringConnectionId.eq(follower_id.id as i32).and( + Column::AnsweringConnectionServerId.eq(follower_id.owner_id as i32), + )), + ), ) - .one(&*tx) - .await? - .ok_or_else(|| anyhow!("not a follower"))?; + .count(&*tx) + .await?; + + if count < 2 { + Err(anyhow!("not room participants"))?; + } + Ok(()) }) .await @@ -947,7 +898,7 @@ impl Database { pub async fn follow( &self, room_id: RoomId, - project_id: Option, + project_id: ProjectId, leader_connection: ConnectionId, follower_connection: ConnectionId, ) -> Result> { @@ -977,7 +928,7 @@ impl Database { pub async fn unfollow( &self, room_id: RoomId, - project_id: Option, + project_id: ProjectId, leader_connection: ConnectionId, follower_connection: ConnectionId, ) -> Result> { diff --git a/crates/collab/src/db/queries/rooms.rs b/crates/collab/src/db/queries/rooms.rs index de1e8a1ce7ba9b5321ba81ecd6ce7111cf5afbf0..41f9755872de9c6a58848d13934fda247f1862c9 100644 --- a/crates/collab/src/db/queries/rooms.rs +++ b/crates/collab/src/db/queries/rooms.rs @@ -1154,7 +1154,7 @@ impl Database { followers.push(proto::Follower { leader_id: Some(db_follower.leader_connection().into()), follower_id: Some(db_follower.follower_connection().into()), - project_id: db_follower.project_id.map(|id| id.to_proto()), + project_id: db_follower.project_id.to_proto(), }); } diff --git a/crates/collab/src/db/tables/follower.rs b/crates/collab/src/db/tables/follower.rs index b5bc163b21d5b04ddaaaafb7ece5d81793a14df1..ffd45434e9006d09939a369a3010cc85393f7dde 100644 --- a/crates/collab/src/db/tables/follower.rs +++ b/crates/collab/src/db/tables/follower.rs @@ -8,7 +8,7 @@ pub struct Model { #[sea_orm(primary_key)] pub id: FollowerId, pub room_id: RoomId, - pub project_id: Option, + pub project_id: ProjectId, pub leader_connection_server_id: ServerId, pub leader_connection_id: i32, pub follower_connection_server_id: ServerId, diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 56cecb2e745a1ac3228907826fe2f2dfbe34d8c6..9e14c4847397465758cd75a570a6ea53a1db44d7 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -1901,7 +1901,7 @@ async fn follow( session .db() .await - .check_can_follow(room_id, project_id, leader_id, session.connection_id) + .check_room_participants(room_id, leader_id, session.connection_id) .await?; let mut response_payload = session @@ -1913,12 +1913,14 @@ async fn follow( .retain(|view| view.leader_id != Some(follower_id.into())); response.send(response_payload)?; - let room = session - .db() - .await - .follow(room_id, project_id, leader_id, follower_id) - .await?; - room_updated(&room, &session.peer); + if let Some(project_id) = project_id { + let room = session + .db() + .await + .follow(room_id, project_id, leader_id, follower_id) + .await?; + room_updated(&room, &session.peer); + } Ok(()) } @@ -1935,19 +1937,21 @@ async fn unfollow(request: proto::Unfollow, session: Session) -> Result<()> { session .db() .await - .check_can_unfollow(room_id, project_id, leader_id, session.connection_id) + .check_room_participants(room_id, leader_id, session.connection_id) .await?; session .peer .forward_send(session.connection_id, leader_id, request)?; - let room = session - .db() - .await - .unfollow(room_id, project_id, leader_id, follower_id) - .await?; - room_updated(&room, &session.peer); + if let Some(project_id) = project_id { + let room = session + .db() + .await + .unfollow(room_id, project_id, leader_id, follower_id) + .await?; + room_updated(&room, &session.peer); + } Ok(()) } diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 523ef1d76356a473979a51ea830dc78e0ded6009..a62a9f06c3f39939b69013dbc9f85d2945ff3ee4 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -273,7 +273,7 @@ message ParticipantProject { message Follower { PeerId leader_id = 1; PeerId follower_id = 2; - optional uint64 project_id = 3; + uint64 project_id = 3; } message ParticipantLocation { From 38a9e6fde1a971b2bf41fe7770fba5691e7f764d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Sep 2023 16:46:43 -0700 Subject: [PATCH 12/24] Fix removal of followers on Unfollow --- crates/call/src/call.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/call/src/call.rs b/crates/call/src/call.rs index 6756c2aa533eb8a9e46f68c4310b5436a606991a..f5bc05e37ab482e7929f713ed24318ed6724d662 100644 --- a/crates/call/src/call.rs +++ b/crates/call/src/call.rs @@ -95,7 +95,7 @@ impl ActiveCall { client.add_message_handler(cx.handle(), Self::handle_call_canceled), client.add_request_handler(cx.handle(), Self::handle_follow), client.add_message_handler(cx.handle(), Self::handle_unfollow), - client.add_message_handler(cx.handle(), Self::handle_update_followers), + client.add_message_handler(cx.handle(), Self::handle_update_from_leader), ], client, user_store, @@ -259,14 +259,14 @@ impl ActiveCall { project_id: envelope.payload.project_id, peer_id: envelope.original_sender_id()?, }; - if let Err(ix) = this.followers.binary_search(&follower) { + if let Ok(ix) = this.followers.binary_search(&follower) { this.followers.remove(ix); } Ok(()) }) } - async fn handle_update_followers( + async fn handle_update_from_leader( this: ModelHandle, envelope: TypedEnvelope, _: Arc, From e34ebbc665ff2b10bfb686c9b79db7d9b9bb11e4 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Sep 2023 16:56:39 -0700 Subject: [PATCH 13/24] Remove unused dependencies on theme --- Cargo.lock | 4 ---- crates/call/Cargo.toml | 1 - crates/channel/Cargo.toml | 1 - crates/client/Cargo.toml | 1 - crates/project/Cargo.toml | 1 - 5 files changed, 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b675614376646c87bb7a943611ab1fd508a97d29..983f6946ab8004477b4210c1ca2c7c9897a48d2a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1106,7 +1106,6 @@ dependencies = [ "serde_derive", "serde_json", "settings", - "theme", "util", ] @@ -1247,7 +1246,6 @@ dependencies = [ "sum_tree", "tempfile", "text", - "theme", "thiserror", "time", "tiny_http", @@ -1419,7 +1417,6 @@ dependencies = [ "sum_tree", "tempfile", "text", - "theme", "thiserror", "time", "tiny_http", @@ -5552,7 +5549,6 @@ dependencies = [ "tempdir", "terminal", "text", - "theme", "thiserror", "toml 0.5.11", "unindent", diff --git a/crates/call/Cargo.toml b/crates/call/Cargo.toml index 716bc3c27b028c866042131ad7ff9c8c2169ab60..b4e94fe56c3b12533d232eacf30c5edd633d5a03 100644 --- a/crates/call/Cargo.toml +++ b/crates/call/Cargo.toml @@ -31,7 +31,6 @@ language = { path = "../language" } media = { path = "../media" } project = { path = "../project" } settings = { path = "../settings" } -theme = { path = "../theme" } util = { path = "../util" } anyhow.workspace = true diff --git a/crates/channel/Cargo.toml b/crates/channel/Cargo.toml index e3a74ecbe6e1074d3c7df16daa4d0f04d45aa65c..16a1d418d5c17750db582252118c3ddcc1cad2d3 100644 --- a/crates/channel/Cargo.toml +++ b/crates/channel/Cargo.toml @@ -23,7 +23,6 @@ language = { path = "../language" } settings = { path = "../settings" } feature_flags = { path = "../feature_flags" } sum_tree = { path = "../sum_tree" } -theme = { path = "../theme" } anyhow.workspace = true futures.workspace = true diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index 2bd03d789f3af1e59c0edaf153cd7d04c419307a..e3038e5bcc49bd41b756062b676e00f4f355867a 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -21,7 +21,6 @@ text = { path = "../text" } settings = { path = "../settings" } feature_flags = { path = "../feature_flags" } sum_tree = { path = "../sum_tree" } -theme = { path = "../theme" } anyhow.workspace = true async-recursion = "0.3" diff --git a/crates/project/Cargo.toml b/crates/project/Cargo.toml index 07f1bfa43e79d6c3d254b38a0f8a04c2a8031b71..ffea6646e9439a24cd7e09fd9666ee9eabfcbf49 100644 --- a/crates/project/Cargo.toml +++ b/crates/project/Cargo.toml @@ -35,7 +35,6 @@ rpc = { path = "../rpc" } settings = { path = "../settings" } sum_tree = { path = "../sum_tree" } terminal = { path = "../terminal" } -theme = { path = "../theme" } util = { path = "../util" } aho-corasick = "1.1" From 00587027499f3d5a2bb8b70027be34a0eecd2bde Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Sep 2023 16:56:51 -0700 Subject: [PATCH 14/24] Remove unused db query method --- crates/collab/src/db/queries/rooms.rs | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/crates/collab/src/db/queries/rooms.rs b/crates/collab/src/db/queries/rooms.rs index 41f9755872de9c6a58848d13934fda247f1862c9..b103ae1c737cfdd977418d528585c6fdd9ebb4b7 100644 --- a/crates/collab/src/db/queries/rooms.rs +++ b/crates/collab/src/db/queries/rooms.rs @@ -985,32 +985,6 @@ impl Database { Ok(room) } - pub async fn room_id_for_connection(&self, connection_id: ConnectionId) -> Result { - #[derive(Copy, Clone, Debug, EnumIter, DeriveColumn)] - enum QueryRoomId { - RoomId, - } - - self.transaction(|tx| async move { - Ok(room_participant::Entity::find() - .select_only() - .column(room_participant::Column::RoomId) - .filter( - Condition::all() - .add(room_participant::Column::AnsweringConnectionId.eq(connection_id.id)) - .add( - room_participant::Column::AnsweringConnectionServerId - .eq(ServerId(connection_id.owner_id as i32)), - ), - ) - .into_values::<_, QueryRoomId>() - .one(&*tx) - .await? - .ok_or_else(|| anyhow!("no room for connection {:?}", connection_id))?) - }) - .await - } - pub async fn room_connection_ids( &self, room_id: RoomId, From 5a15692589139484cac9703001743ab2c571a98d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Sep 2023 17:04:05 -0700 Subject: [PATCH 15/24] :art: Workspace::leader_updated --- crates/workspace/src/workspace.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 404d974c52c7f3a7772c9ff197eab5ed98d1aab7..89dff882c368bb9567ff28eff80cfe0fa28a2488 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -3042,20 +3042,18 @@ impl Workspace { }; 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 { + let item = state + .active_view_id + .and_then(|id| state.items_by_leader_view_id.get(&id)); 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))); + continue; } - } else if let Some(shared_screen) = shared_screen { + } + if let Some(shared_screen) = self.shared_screen_for_peer(leader_id, pane, cx) { items_to_activate.push((pane.clone(), Box::new(shared_screen))); } } From 837ec5a27c69ebbb4bd3cf0feb956558ad5a2ce7 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Sep 2023 17:13:39 -0700 Subject: [PATCH 16/24] Remove stray file --- selection-color-notes.txt | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 selection-color-notes.txt diff --git a/selection-color-notes.txt b/selection-color-notes.txt deleted file mode 100644 index 6186adcac1aaba2bac7e397e3f71bcbba961d275..0000000000000000000000000000000000000000 --- a/selection-color-notes.txt +++ /dev/null @@ -1,14 +0,0 @@ -Assign selection colors to users. goals: - * current user is always main color - * every other user has the same color in every context - * users don't need to be in a shared project to have a color. they can either be in the call, or in a channel notes. - -Places colors are used: - * editor element, driven by the buffer's `remote_selections` - * pane border (access to more state) - * collab titlebar (access to more state) - -Currently, editor holds an optional "replica id map". - -Most challenging part is in the editor, because the editor should be fairly self-contained, not depend on e.g. the user store. - From ca0a4bdf8e24d64039ba2ae1bfad2b2fbbf48b07 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 28 Sep 2023 18:58:52 -0700 Subject: [PATCH 17/24] Introduce a WorkspaceStore for handling following --- crates/call/src/call.rs | 205 ++---------------------- crates/collab/src/tests/test_server.rs | 4 +- crates/workspace/src/workspace.rs | 212 +++++++++++++++++++++---- crates/zed/src/main.rs | 4 +- 4 files changed, 201 insertions(+), 224 deletions(-) diff --git a/crates/call/src/call.rs b/crates/call/src/call.rs index f5bc05e37ab482e7929f713ed24318ed6724d662..bdc402ee77b8e8a2af7dc7748b40468086dd04f8 100644 --- a/crates/call/src/call.rs +++ b/crates/call/src/call.rs @@ -2,29 +2,23 @@ pub mod call_settings; pub mod participant; pub mod room; -use std::sync::Arc; - use anyhow::{anyhow, Result}; use audio::Audio; use call_settings::CallSettings; use channel::ChannelId; -use client::{ - proto::{self, PeerId}, - ClickhouseEvent, Client, TelemetrySettings, TypedEnvelope, User, UserStore, -}; +use client::{proto, ClickhouseEvent, Client, TelemetrySettings, TypedEnvelope, User, UserStore}; use collections::HashSet; use futures::{future::Shared, FutureExt}; -use postage::watch; - use gpui::{ - AnyViewHandle, AnyWeakViewHandle, AppContext, AsyncAppContext, Entity, ModelContext, - ModelHandle, Subscription, Task, ViewContext, WeakModelHandle, + AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, Subscription, Task, + WeakModelHandle, }; +use postage::watch; use project::Project; +use std::sync::Arc; pub use participant::ParticipantLocation; pub use room::Room; -use util::ResultExt; pub fn init(client: Arc, user_store: ModelHandle, cx: &mut AppContext) { settings::register::(cx); @@ -53,25 +47,9 @@ pub struct ActiveCall { ), client: Arc, user_store: ModelHandle, - follow_handlers: Vec, - followers: Vec, _subscriptions: Vec, } -#[derive(PartialEq, Eq, PartialOrd, Ord, Debug)] -struct Follower { - project_id: Option, - peer_id: PeerId, -} - -struct FollowHandler { - project_id: Option, - root_view: AnyWeakViewHandle, - get_views: - Box, &mut AppContext) -> Option>, - update_view: Box, -} - impl Entity for ActiveCall { type Event = room::Event; } @@ -88,14 +66,10 @@ impl ActiveCall { location: None, pending_invites: Default::default(), incoming_call: watch::channel(), - follow_handlers: Default::default(), - followers: Default::default(), + _subscriptions: vec![ client.add_request_handler(cx.handle(), Self::handle_incoming_call), client.add_message_handler(cx.handle(), Self::handle_call_canceled), - client.add_request_handler(cx.handle(), Self::handle_follow), - client.add_message_handler(cx.handle(), Self::handle_unfollow), - client.add_message_handler(cx.handle(), Self::handle_update_from_leader), ], client, user_store, @@ -106,48 +80,6 @@ impl ActiveCall { self.room()?.read(cx).channel_id() } - pub fn add_follow_handler( - &mut self, - root_view: gpui::ViewHandle, - project_id: Option, - get_views: GetViews, - update_view: UpdateView, - _cx: &mut ModelContext, - ) where - GetViews: 'static - + Fn(&mut V, Option, &mut gpui::ViewContext) -> Result, - UpdateView: - 'static + Fn(&mut V, PeerId, proto::UpdateFollowers, &mut ViewContext) -> Result<()>, - { - self.follow_handlers - .retain(|h| h.root_view.id() != root_view.id()); - if let Err(ix) = self - .follow_handlers - .binary_search_by_key(&(project_id, root_view.id()), |f| { - (f.project_id, f.root_view.id()) - }) - { - self.follow_handlers.insert( - ix, - FollowHandler { - project_id, - root_view: root_view.into_any().downgrade(), - get_views: Box::new(move |view, project_id, cx| { - let view = view.clone().downcast::().unwrap(); - view.update(cx, |view, cx| get_views(view, project_id, cx).log_err()) - .flatten() - }), - update_view: Box::new(move |view, leader_id, message, cx| { - let view = view.clone().downcast::().unwrap(); - view.update(cx, |view, cx| { - update_view(view, leader_id, message, cx).log_err() - }); - }), - }, - ); - } - } - async fn handle_incoming_call( this: ModelHandle, envelope: TypedEnvelope, @@ -194,127 +126,6 @@ impl ActiveCall { Ok(()) } - async fn handle_follow( - this: ModelHandle, - envelope: TypedEnvelope, - _: Arc, - mut cx: AsyncAppContext, - ) -> Result { - this.update(&mut cx, |this, cx| { - let follower = Follower { - project_id: envelope.payload.project_id, - peer_id: envelope.original_sender_id()?, - }; - let active_project_id = this - .location - .as_ref() - .and_then(|project| project.upgrade(cx)?.read(cx).remote_id()); - - let mut response = proto::FollowResponse::default(); - for handler in &this.follow_handlers { - if follower.project_id != handler.project_id && follower.project_id.is_some() { - continue; - } - - let Some(root_view) = handler.root_view.upgrade(cx) else { - continue; - }; - - let Some(handler_response) = - (handler.get_views)(&root_view, follower.project_id, cx) - else { - continue; - }; - - if response.views.is_empty() { - response.views = handler_response.views; - } else { - response.views.extend_from_slice(&handler_response.views); - } - - if let Some(active_view_id) = handler_response.active_view_id.clone() { - if response.active_view_id.is_none() || handler.project_id == active_project_id - { - response.active_view_id = Some(active_view_id); - } - } - } - - if let Err(ix) = this.followers.binary_search(&follower) { - this.followers.insert(ix, follower); - } - - Ok(response) - }) - } - - async fn handle_unfollow( - this: ModelHandle, - envelope: TypedEnvelope, - _: Arc, - mut cx: AsyncAppContext, - ) -> Result<()> { - this.update(&mut cx, |this, _| { - let follower = Follower { - project_id: envelope.payload.project_id, - peer_id: envelope.original_sender_id()?, - }; - if let Ok(ix) = this.followers.binary_search(&follower) { - this.followers.remove(ix); - } - Ok(()) - }) - } - - async fn handle_update_from_leader( - this: ModelHandle, - envelope: TypedEnvelope, - _: Arc, - mut cx: AsyncAppContext, - ) -> Result<()> { - let leader_id = envelope.original_sender_id()?; - let update = envelope.payload; - this.update(&mut cx, |this, cx| { - for handler in &this.follow_handlers { - if update.project_id != handler.project_id && update.project_id.is_some() { - continue; - } - let Some(root_view) = handler.root_view.upgrade(cx) else { - continue; - }; - (handler.update_view)(&root_view, leader_id, update.clone(), cx); - } - Ok(()) - }) - } - - pub fn update_followers( - &self, - project_id: Option, - update: proto::update_followers::Variant, - cx: &AppContext, - ) -> Option<()> { - let room_id = self.room()?.read(cx).id(); - let follower_ids: Vec<_> = self - .followers - .iter() - .filter_map(|follower| { - (follower.project_id == project_id).then_some(follower.peer_id.into()) - }) - .collect(); - if follower_ids.is_empty() { - return None; - } - self.client - .send(proto::UpdateFollowers { - room_id, - project_id, - follower_ids, - variant: Some(update), - }) - .log_err() - } - pub fn global(cx: &AppContext) -> ModelHandle { cx.global::>().clone() } @@ -536,6 +347,10 @@ impl ActiveCall { } } + pub fn location(&self) -> Option<&WeakModelHandle> { + self.location.as_ref() + } + pub fn set_location( &mut self, project: Option<&ModelHandle>, diff --git a/crates/collab/src/tests/test_server.rs b/crates/collab/src/tests/test_server.rs index 71537f069f78188b21bbb943cda28460ffbca0d1..a56df311bd7d4c79184c788b1eb4e3bca941cdcd 100644 --- a/crates/collab/src/tests/test_server.rs +++ b/crates/collab/src/tests/test_server.rs @@ -29,7 +29,7 @@ use std::{ }, }; use util::http::FakeHttpClient; -use workspace::Workspace; +use workspace::{Workspace, WorkspaceStore}; pub struct TestServer { pub app_state: Arc, @@ -204,6 +204,7 @@ impl TestServer { let fs = FakeFs::new(cx.background()); let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http, cx)); + let workspace_store = cx.add_model(|cx| WorkspaceStore::new(client.clone(), cx)); let channel_store = cx.add_model(|cx| ChannelStore::new(client.clone(), user_store.clone(), cx)); let mut language_registry = LanguageRegistry::test(); @@ -211,6 +212,7 @@ impl TestServer { let app_state = Arc::new(workspace::AppState { client: client.clone(), user_store: user_store.clone(), + workspace_store, channel_store: channel_store.clone(), languages: Arc::new(language_registry), fs: fs.clone(), diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 89dff882c368bb9567ff28eff80cfe0fa28a2488..256ecfd3e9091012a74cd7c31bdc0281cef24aab 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -15,7 +15,7 @@ use call::ActiveCall; use channel::ChannelStore; use client::{ proto::{self, PeerId}, - Client, UserStore, + Client, TypedEnvelope, UserStore, }; use collections::{hash_map, HashMap, HashSet}; use drag_and_drop::DragAndDrop; @@ -451,6 +451,7 @@ pub struct AppState { pub client: Arc, pub user_store: ModelHandle, pub channel_store: ModelHandle, + pub workspace_store: ModelHandle, pub fs: Arc, pub build_window_options: fn(Option, Option, &dyn Platform) -> WindowOptions<'static>, @@ -459,6 +460,19 @@ pub struct AppState { pub background_actions: BackgroundActions, } +pub struct WorkspaceStore { + workspaces: HashSet>, + followers: Vec, + client: Arc, + _subscriptions: Vec, +} + +#[derive(PartialEq, Eq, PartialOrd, Ord, Debug)] +struct Follower { + project_id: Option, + peer_id: PeerId, +} + impl AppState { #[cfg(any(test, feature = "test-support"))] pub fn test(cx: &mut AppContext) -> Arc { @@ -475,6 +489,7 @@ impl AppState { let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx)); let channel_store = cx.add_model(|cx| ChannelStore::new(client.clone(), user_store.clone(), cx)); + let workspace_store = cx.add_model(|cx| WorkspaceStore::new(client.clone(), cx)); theme::init((), cx); client::init(&client, cx); @@ -486,6 +501,7 @@ impl AppState { languages, user_store, channel_store, + workspace_store, initialize_workspace: |_, _, _, _| Task::ready(Ok(())), build_window_options: |_, _, _| Default::default(), background_actions: || &[], @@ -663,6 +679,10 @@ impl Workspace { cx.focus(¢er_pane); cx.emit(Event::PaneAdded(center_pane.clone())); + app_state.workspace_store.update(cx, |store, _| { + store.workspaces.insert(weak_handle.clone()); + }); + let mut current_user = app_state.user_store.read(cx).watch_current_user(); let mut connection_status = app_state.client.status(); let _observe_current_user = cx.spawn(|this, mut cx| async move { @@ -2492,19 +2512,8 @@ impl Workspace { &self.active_pane } - fn project_remote_id_changed(&mut self, remote_id: Option, cx: &mut ViewContext) { - let handle = cx.handle(); - if let Some(call) = self.active_call() { - call.update(cx, |call, cx| { - call.add_follow_handler( - handle, - remote_id, - Self::get_views_for_followers, - Self::handle_update_followers, - cx, - ); - }); - } + fn project_remote_id_changed(&mut self, _project_id: Option, _cx: &mut ViewContext) { + // TODO } fn collaborator_left(&mut self, peer_id: PeerId, cx: &mut ViewContext) { @@ -2793,11 +2802,7 @@ impl Workspace { // RPC handlers - fn get_views_for_followers( - &mut self, - _project_id: Option, - cx: &mut ViewContext, - ) -> Result { + fn handle_follow(&mut self, cx: &mut ViewContext) -> proto::FollowResponse { let client = &self.app_state.client; let active_view_id = self.active_item(cx).and_then(|i| { @@ -2810,7 +2815,7 @@ impl Workspace { cx.notify(); - Ok(proto::FollowResponse { + proto::FollowResponse { active_view_id, views: self .panes() @@ -2832,7 +2837,7 @@ impl Workspace { }) }) .collect(), - }) + } } fn handle_update_followers( @@ -2840,10 +2845,10 @@ impl Workspace { leader_id: PeerId, message: proto::UpdateFollowers, _cx: &mut ViewContext, - ) -> Result<()> { + ) { self.leader_updates_tx - .unbounded_send((leader_id, message))?; - Ok(()) + .unbounded_send((leader_id, message)) + .ok(); } async fn process_leader_update( @@ -2999,9 +3004,9 @@ impl Workspace { update: proto::update_followers::Variant, cx: &AppContext, ) -> Option<()> { - self.active_call()? - .read(cx) - .update_followers(self.project.read(cx).remote_id(), update, cx) + self.app_state().workspace_store.read_with(cx, |store, cx| { + store.update_followers(self.project.read(cx).remote_id(), update, cx) + }) } pub fn leader_for_pane(&self, pane: &ViewHandle) -> Option { @@ -3472,8 +3477,10 @@ impl Workspace { let channel_store = cx.add_model(|cx| ChannelStore::new(client.clone(), user_store.clone(), cx)); + let workspace_store = cx.add_model(|cx| WorkspaceStore::new(client.clone(), cx)); let app_state = Arc::new(AppState { languages: project.read(cx).languages().clone(), + workspace_store, client, user_store, channel_store, @@ -3717,6 +3724,12 @@ fn notify_if_database_failed(workspace: &WeakViewHandle, cx: &mut Asy impl Entity for Workspace { type Event = Event; + + fn release(&mut self, cx: &mut AppContext) { + self.app_state.workspace_store.update(cx, |store, _| { + store.workspaces.remove(&self.weak_self); + }) + } } impl View for Workspace { @@ -3859,6 +3872,151 @@ impl View for Workspace { } } +impl WorkspaceStore { + pub fn new(client: Arc, cx: &mut ModelContext) -> Self { + Self { + workspaces: Default::default(), + followers: Default::default(), + _subscriptions: vec![ + client.add_request_handler(cx.handle(), Self::handle_follow), + client.add_message_handler(cx.handle(), Self::handle_unfollow), + client.add_message_handler(cx.handle(), Self::handle_update_from_leader), + ], + client, + } + } + + pub fn update_followers( + &self, + project_id: Option, + update: proto::update_followers::Variant, + cx: &AppContext, + ) -> Option<()> { + if !cx.has_global::>() { + return None; + } + + let room_id = ActiveCall::global(cx).read(cx).room()?.read(cx).id(); + let follower_ids: Vec<_> = self + .followers + .iter() + .filter_map(|follower| { + (follower.project_id == project_id).then_some(follower.peer_id.into()) + }) + .collect(); + if follower_ids.is_empty() { + return None; + } + self.client + .send(proto::UpdateFollowers { + room_id, + project_id, + follower_ids, + variant: Some(update), + }) + .log_err() + } + + async fn handle_follow( + this: ModelHandle, + envelope: TypedEnvelope, + _: Arc, + mut cx: AsyncAppContext, + ) -> Result { + this.update(&mut cx, |this, cx| { + let follower = Follower { + project_id: envelope.payload.project_id, + peer_id: envelope.original_sender_id()?, + }; + let active_project_id = ActiveCall::global(cx) + .read(cx) + .location() + .as_ref() + .and_then(|project| project.upgrade(cx)?.read(cx).remote_id()); + + let mut response = proto::FollowResponse::default(); + for workspace in &this.workspaces { + let Some(workspace) = workspace.upgrade(cx) else { + continue; + }; + + workspace.update(cx.as_mut(), |workspace, cx| { + let project_id = workspace.project.read(cx).remote_id(); + if follower.project_id != project_id && follower.project_id.is_some() { + return; + } + + let handler_response = workspace.handle_follow(cx); + if response.views.is_empty() { + response.views = handler_response.views; + } else { + response.views.extend_from_slice(&handler_response.views); + } + + if let Some(active_view_id) = handler_response.active_view_id.clone() { + if response.active_view_id.is_none() || project_id == active_project_id { + response.active_view_id = Some(active_view_id); + } + } + }); + } + + if let Err(ix) = this.followers.binary_search(&follower) { + this.followers.insert(ix, follower); + } + + Ok(response) + }) + } + + async fn handle_unfollow( + this: ModelHandle, + envelope: TypedEnvelope, + _: Arc, + mut cx: AsyncAppContext, + ) -> Result<()> { + this.update(&mut cx, |this, _| { + let follower = Follower { + project_id: envelope.payload.project_id, + peer_id: envelope.original_sender_id()?, + }; + if let Ok(ix) = this.followers.binary_search(&follower) { + this.followers.remove(ix); + } + Ok(()) + }) + } + + async fn handle_update_from_leader( + this: ModelHandle, + envelope: TypedEnvelope, + _: Arc, + mut cx: AsyncAppContext, + ) -> Result<()> { + let leader_id = envelope.original_sender_id()?; + let update = envelope.payload; + this.update(&mut cx, |this, cx| { + for workspace in &this.workspaces { + let Some(workspace) = workspace.upgrade(cx) else { + continue; + }; + workspace.update(cx.as_mut(), |workspace, cx| { + let project_id = workspace.project.read(cx).remote_id(); + if update.project_id != project_id && update.project_id.is_some() { + return; + } + workspace.handle_update_followers(leader_id, update.clone(), cx); + }); + } + Ok(()) + }) + } +} + +impl Entity for WorkspaceStore { + type Event = (); +} + impl ViewId { pub(crate) fn from_proto(message: proto::ViewId) -> Result { Ok(Self { diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index bb44f67841eda0af2007521c275a7ff255bdc677..7991cabde23aee4d5affb81975b42fb64005c358 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -54,7 +54,7 @@ use welcome::{show_welcome_experience, FIRST_OPEN}; use fs::RealFs; use util::{channel::RELEASE_CHANNEL, paths, ResultExt, TryFutureExt}; -use workspace::AppState; +use workspace::{AppState, WorkspaceStore}; use zed::{ assets::Assets, build_window_options, handle_keymap_file_changes, initialize_workspace, languages, menus, @@ -139,6 +139,7 @@ fn main() { let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http.clone(), cx)); let channel_store = cx.add_model(|cx| ChannelStore::new(client.clone(), user_store.clone(), cx)); + let workspace_store = cx.add_model(|cx| WorkspaceStore::new(client.clone(), cx)); cx.set_global(client.clone()); @@ -187,6 +188,7 @@ fn main() { build_window_options, initialize_workspace, background_actions, + workspace_store, }); cx.set_global(Arc::downgrade(&app_state)); From 026b3a1d0f87d97206d54228e7b8ae58b4080eab Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 29 Sep 2023 08:54:23 -0700 Subject: [PATCH 18/24] Remove uneeded Workspace::project_remote_id_changed method --- crates/workspace/src/workspace.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 256ecfd3e9091012a74cd7c31bdc0281cef24aab..9834f473705f54ecd21cd0dc3b3f6734856dbe87 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -622,9 +622,8 @@ impl Workspace { cx.observe(&project, |_, _, cx| cx.notify()).detach(); cx.subscribe(&project, move |this, _, event, cx| { match event { - project::Event::RemoteIdChanged(remote_id) => { + project::Event::RemoteIdChanged(_) => { this.update_window_title(cx); - this.project_remote_id_changed(*remote_id, cx); } project::Event::CollaboratorLeft(peer_id) => { @@ -776,7 +775,8 @@ impl Workspace { }), ]; - let mut this = Workspace { + cx.defer(|this, cx| this.update_window_title(cx)); + Workspace { weak_self: weak_handle.clone(), modal: None, zoomed: None, @@ -805,10 +805,7 @@ impl Workspace { leader_updates_tx, subscriptions, pane_history_timestamp, - }; - this.project_remote_id_changed(project.read(cx).remote_id(), cx); - cx.defer(|this, cx| this.update_window_title(cx)); - this + } } fn new_local( @@ -2512,10 +2509,6 @@ impl Workspace { &self.active_pane } - fn project_remote_id_changed(&mut self, _project_id: Option, _cx: &mut ViewContext) { - // TODO - } - fn collaborator_left(&mut self, peer_id: PeerId, cx: &mut ViewContext) { if let Some(states_by_pane) = self.follower_states_by_leader.remove(&peer_id) { for state in states_by_pane.into_values() { From 555c9847d44854f0c43cf140fcf5b493a03b162c Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 29 Sep 2023 13:43:43 -0700 Subject: [PATCH 19/24] Add ZED_ALWAYS_ACTIVE env var, use it in local collaboration script This makes zed always behave as if the app is active, even if no window is focused. It prevents the 'viewing a window outside of zed' state during collaboration. --- crates/call/src/call.rs | 16 ++++++++++------ crates/client/src/client.rs | 2 ++ script/start-local-collaboration | 1 + 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/crates/call/src/call.rs b/crates/call/src/call.rs index bdc402ee77b8e8a2af7dc7748b40468086dd04f8..ca0d06beb6a44e53b2f7593a74349f30968bd945 100644 --- a/crates/call/src/call.rs +++ b/crates/call/src/call.rs @@ -6,7 +6,10 @@ use anyhow::{anyhow, Result}; use audio::Audio; use call_settings::CallSettings; use channel::ChannelId; -use client::{proto, ClickhouseEvent, Client, TelemetrySettings, TypedEnvelope, User, UserStore}; +use client::{ + proto, ClickhouseEvent, Client, TelemetrySettings, TypedEnvelope, User, UserStore, + ZED_ALWAYS_ACTIVE, +}; use collections::HashSet; use futures::{future::Shared, FutureExt}; use gpui::{ @@ -356,12 +359,13 @@ impl ActiveCall { project: Option<&ModelHandle>, cx: &mut ModelContext, ) -> Task> { - self.location = project.map(|project| project.downgrade()); - if let Some((room, _)) = self.room.as_ref() { - room.update(cx, |room, cx| room.set_location(project, cx)) - } else { - Task::ready(Ok(())) + if project.is_some() || !*ZED_ALWAYS_ACTIVE { + self.location = project.map(|project| project.downgrade()); + if let Some((room, _)) = self.room.as_ref() { + return room.update(cx, |room, cx| room.set_location(project, cx)); + } } + Task::ready(Ok(())) } fn set_room( diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index d28c1ab1a9bc27eac98d1c912e7031b36fd079de..5eae700404c316cdf55d44cccb24470ded092c93 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -62,6 +62,8 @@ lazy_static! { .and_then(|v| v.parse().ok()); pub static ref ZED_APP_PATH: Option = std::env::var("ZED_APP_PATH").ok().map(PathBuf::from); + pub static ref ZED_ALWAYS_ACTIVE: bool = + std::env::var("ZED_ALWAYS_ACTIVE").map_or(false, |e| e.len() > 0); } pub const ZED_SECRET_CLIENT_TOKEN: &str = "618033988749894"; diff --git a/script/start-local-collaboration b/script/start-local-collaboration index a5836ff776aacf98a09cdd11252119862ea10190..0c4e60f9c3d16af4ba545d2cb6cfdfe9b5062148 100755 --- a/script/start-local-collaboration +++ b/script/start-local-collaboration @@ -44,6 +44,7 @@ position_2=${half_width},${y} # Authenticate using the collab server's admin secret. export ZED_STATELESS=1 +export ZED_ALWAYS_ACTIVE=1 export ZED_ADMIN_API_TOKEN=secret export ZED_SERVER_URL=http://localhost:8080 export ZED_WINDOW_SIZE=${half_width},${height} From 973f03e73eb095ec7c34c1c2118aebf305ef5bb4 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 29 Sep 2023 14:09:14 -0700 Subject: [PATCH 20/24] Fix bug in follower updates for non-project items --- crates/workspace/src/item.rs | 3 ++ crates/workspace/src/workspace.rs | 62 ++++++++++++++++++++----------- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index 277dde55bdc11b83cf594bb387bb846ee932d745..f96c19c9ac6b5d2ba038df30388c19723c08e1ad 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -404,6 +404,7 @@ impl ItemHandle for ViewHandle { if let Some(followed_item) = self.to_followable_item_handle(cx) { if let Some(message) = followed_item.to_state_proto(cx) { workspace.update_followers( + followed_item.is_project_item(cx), proto::update_followers::Variant::CreateView(proto::View { id: followed_item .remote_id(&workspace.app_state.client, cx) @@ -439,6 +440,7 @@ impl ItemHandle for ViewHandle { }; if let Some(item) = item.to_followable_item_handle(cx) { + let is_project_item = item.is_project_item(cx); let leader_id = workspace.leader_for_pane(&pane); if leader_id.is_some() && item.should_unfollow_on_event(event, cx) { @@ -458,6 +460,7 @@ impl ItemHandle for ViewHandle { move |this, cx| { pending_update_scheduled.store(false, Ordering::SeqCst); this.update_followers( + is_project_item, proto::update_followers::Variant::UpdateView( proto::UpdateView { id: item diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 9834f473705f54ecd21cd0dc3b3f6734856dbe87..f7cee6a7dbaef3286ab15080cabe53fab2f02dbc 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2795,8 +2795,13 @@ impl Workspace { // RPC handlers - fn handle_follow(&mut self, cx: &mut ViewContext) -> proto::FollowResponse { + fn handle_follow( + &mut self, + follower_project_id: Option, + cx: &mut ViewContext, + ) -> proto::FollowResponse { let client = &self.app_state.client; + let project_id = self.project.read(cx).remote_id(); let active_view_id = self.active_item(cx).and_then(|i| { Some( @@ -2819,6 +2824,12 @@ impl Workspace { let cx = &cx; move |item| { let item = item.to_followable_item_handle(cx)?; + if project_id.is_some() + && project_id != follower_project_id + && item.is_project_item(cx) + { + return None; + } let id = item.remote_id(client, cx)?.to_proto(); let variant = item.to_state_proto(cx)?; Some(proto::View { @@ -2969,20 +2980,23 @@ impl Workspace { } fn update_active_view_for_followers(&self, cx: &AppContext) { - if self.active_pane.read(cx).has_focus() { + let item = self + .active_item(cx) + .and_then(|item| item.to_followable_item_handle(cx)); + if let Some(item) = item { self.update_followers( + item.is_project_item(cx), proto::update_followers::Variant::UpdateActiveView(proto::UpdateActiveView { - id: self.active_item(cx).and_then(|item| { - item.to_followable_item_handle(cx)? - .remote_id(&self.app_state.client, cx) - .map(|id| id.to_proto()) - }), + id: item + .remote_id(&self.app_state.client, cx) + .map(|id| id.to_proto()), leader_id: self.leader_for_pane(&self.active_pane), }), cx, ); } else { self.update_followers( + true, proto::update_followers::Variant::UpdateActiveView(proto::UpdateActiveView { id: None, leader_id: None, @@ -2994,11 +3008,17 @@ impl Workspace { fn update_followers( &self, + project_only: bool, update: proto::update_followers::Variant, cx: &AppContext, ) -> Option<()> { + let project_id = if project_only { + self.project.read(cx).remote_id() + } else { + None + }; self.app_state().workspace_store.read_with(cx, |store, cx| { - store.update_followers(self.project.read(cx).remote_id(), update, cx) + store.update_followers(project_id, update, cx) }) } @@ -3873,7 +3893,7 @@ impl WorkspaceStore { _subscriptions: vec![ client.add_request_handler(cx.handle(), Self::handle_follow), client.add_message_handler(cx.handle(), Self::handle_unfollow), - client.add_message_handler(cx.handle(), Self::handle_update_from_leader), + client.add_message_handler(cx.handle(), Self::handle_update_followers), ], client, } @@ -3894,7 +3914,11 @@ impl WorkspaceStore { .followers .iter() .filter_map(|follower| { - (follower.project_id == project_id).then_some(follower.peer_id.into()) + if follower.project_id == project_id || project_id.is_none() { + Some(follower.peer_id.into()) + } else { + None + } }) .collect(); if follower_ids.is_empty() { @@ -3921,11 +3945,10 @@ impl WorkspaceStore { project_id: envelope.payload.project_id, peer_id: envelope.original_sender_id()?, }; - let active_project_id = ActiveCall::global(cx) + let active_project = ActiveCall::global(cx) .read(cx) .location() - .as_ref() - .and_then(|project| project.upgrade(cx)?.read(cx).remote_id()); + .map(|project| project.id()); let mut response = proto::FollowResponse::default(); for workspace in &this.workspaces { @@ -3934,12 +3957,7 @@ impl WorkspaceStore { }; workspace.update(cx.as_mut(), |workspace, cx| { - let project_id = workspace.project.read(cx).remote_id(); - if follower.project_id != project_id && follower.project_id.is_some() { - return; - } - - let handler_response = workspace.handle_follow(cx); + let handler_response = workspace.handle_follow(follower.project_id, cx); if response.views.is_empty() { response.views = handler_response.views; } else { @@ -3947,7 +3965,9 @@ impl WorkspaceStore { } if let Some(active_view_id) = handler_response.active_view_id.clone() { - if response.active_view_id.is_none() || project_id == active_project_id { + if response.active_view_id.is_none() + || Some(workspace.project.id()) == active_project + { response.active_view_id = Some(active_view_id); } } @@ -3980,7 +4000,7 @@ impl WorkspaceStore { }) } - async fn handle_update_from_leader( + async fn handle_update_followers( this: ModelHandle, envelope: TypedEnvelope, _: Arc, From afd293ee879fb456c6f44802fcbe97a6ae15620e Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 29 Sep 2023 14:12:51 -0700 Subject: [PATCH 21/24] Update active view when activating a window --- crates/workspace/src/workspace.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index f7cee6a7dbaef3286ab15080cabe53fab2f02dbc..3b50157774d45df8e267d03626e74d2b928cf0c3 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -3117,6 +3117,7 @@ impl Workspace { pub fn on_window_activation_changed(&mut self, active: bool, cx: &mut ViewContext) { if active { + self.update_active_view_for_followers(cx); cx.background() .spawn(persistence::DB.update_timestamp(self.database_id())) .detach(); From 55da5bc25d7d675c7a28bbe74e99f99636d4c87f Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 29 Sep 2023 14:16:38 -0700 Subject: [PATCH 22/24] Switch .leader_replica_id -> .leader_peer_id --- crates/vim/src/vim.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 68999b96b254c35cc913623bb897befa6cb5ca8e..d27be2c54b80da39cf44e2beff67d606d1bd92bd 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -195,9 +195,8 @@ impl Vim { if editor_mode == EditorMode::Full && !newest_selection_empty && self.state().mode == Mode::Normal - // if leader_replica_id is set, then you're following someone else's cursor - // don't switch vim mode. - && editor.leader_replica_id().is_none() + // When following someone, don't switch vim mode. + && editor.leader_peer_id().is_none() { self.switch_mode(Mode::Visual, true, cx); } From 948871969fd9603c69c85412c63a8f746566b6b3 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 29 Sep 2023 14:35:21 -0700 Subject: [PATCH 23/24] Fix active view update when center pane is not focused --- crates/workspace/src/workspace.rs | 36 +++++++++++++++---------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 3b50157774d45df8e267d03626e74d2b928cf0c3..44a70f9a08fd21641459884bbd81c5feb155c2aa 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2980,30 +2980,28 @@ impl Workspace { } fn update_active_view_for_followers(&self, cx: &AppContext) { - let item = self - .active_item(cx) - .and_then(|item| item.to_followable_item_handle(cx)); - if let Some(item) = item { - self.update_followers( - item.is_project_item(cx), - proto::update_followers::Variant::UpdateActiveView(proto::UpdateActiveView { + let mut is_project_item = true; + let mut update = proto::UpdateActiveView::default(); + if self.active_pane.read(cx).has_focus() { + let item = self + .active_item(cx) + .and_then(|item| item.to_followable_item_handle(cx)); + if let Some(item) = item { + is_project_item = item.is_project_item(cx); + update = proto::UpdateActiveView { id: item .remote_id(&self.app_state.client, cx) .map(|id| id.to_proto()), leader_id: self.leader_for_pane(&self.active_pane), - }), - cx, - ); - } else { - self.update_followers( - true, - proto::update_followers::Variant::UpdateActiveView(proto::UpdateActiveView { - id: None, - leader_id: None, - }), - cx, - ); + }; + } } + + self.update_followers( + is_project_item, + proto::update_followers::Variant::UpdateActiveView(update), + cx, + ); } fn update_followers( From 7adaa2046d84bf6460a8550cb03c5b0f92d23a2a Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 29 Sep 2023 15:08:25 -0700 Subject: [PATCH 24/24] Show current user as follower when following in unshared projects --- crates/collab_ui/src/collab_titlebar_item.rs | 76 +++++++++----------- 1 file changed, 33 insertions(+), 43 deletions(-) diff --git a/crates/collab_ui/src/collab_titlebar_item.rs b/crates/collab_ui/src/collab_titlebar_item.rs index c1b3689aadabcbebab5ad28686005ee22a049884..9f3b7d2f30ffe360578d66d45a28b91bd149e51a 100644 --- a/crates/collab_ui/src/collab_titlebar_item.rs +++ b/crates/collab_ui/src/collab_titlebar_item.rs @@ -889,22 +889,17 @@ impl CollabTitlebarItem { let user_id = user.id; let project_id = workspace.read(cx).project().read(cx).remote_id(); let room = ActiveCall::global(cx).read(cx).room().cloned(); - let is_being_followed = workspace.read(cx).is_being_followed(peer_id); - let followed_by_self = room - .as_ref() - .and_then(|room| { - Some( - is_being_followed - && room - .read(cx) - .followers_for(peer_id, project_id?) - .iter() - .any(|&follower| { - Some(follower) == workspace.read(cx).client().peer_id() - }), - ) - }) - .unwrap_or(false); + let self_peer_id = workspace.read(cx).client().peer_id(); + let self_following = workspace.read(cx).is_being_followed(peer_id); + let self_following_initialized = self_following + && room.as_ref().map_or(false, |room| match project_id { + None => true, + Some(project_id) => room + .read(cx) + .followers_for(peer_id, project_id) + .iter() + .any(|&follower| Some(follower) == self_peer_id), + }); let leader_style = theme.titlebar.leader_avatar; let follower_style = theme.titlebar.follower_avatar; @@ -930,7 +925,7 @@ impl CollabTitlebarItem { .get(&user_id) .copied(); if let Some(participant_index) = participant_index { - if followed_by_self { + if self_following_initialized { let selection = theme .editor .selection_style_for_room_participant(participant_index.0) @@ -960,31 +955,14 @@ impl CollabTitlebarItem { let project_id = project_id?; let room = room?.read(cx); let followers = room.followers_for(peer_id, project_id); - - Some(followers.into_iter().flat_map(|&follower| { - let remote_participant = - room.remote_participant_for_peer_id(follower); - - let avatar = remote_participant - .and_then(|p| p.user.avatar.clone()) - .or_else(|| { - if follower - == workspace.read(cx).client().peer_id()? - { - workspace - .read(cx) - .user_store() - .read(cx) - .current_user()? - .avatar - .clone() - } else { - None - } - })?; - + Some(followers.into_iter().filter_map(|&follower| { + if Some(follower) == self_peer_id { + return None; + } + let participant = + room.remote_participant_for_peer_id(follower)?; Some(Self::render_face( - avatar.clone(), + participant.user.avatar.clone()?, follower_style, background_color, None, @@ -993,6 +971,18 @@ impl CollabTitlebarItem { })() .into_iter() .flatten(), + ) + .with_children( + self_following_initialized + .then(|| self.user_store.read(cx).current_user()) + .and_then(|user| { + Some(Self::render_face( + user?.avatar.clone()?, + follower_style, + background_color, + None, + )) + }), ); let mut container = face_pile @@ -1000,7 +990,7 @@ impl CollabTitlebarItem { .with_style(theme.titlebar.leader_selection); if let Some(participant_index) = participant_index { - if followed_by_self { + if self_following_initialized { let color = theme .editor .selection_style_for_room_participant(participant_index.0) @@ -1067,7 +1057,7 @@ impl CollabTitlebarItem { }) .with_tooltip::( peer_id.as_u64() as usize, - if is_being_followed { + if self_following { format!("Unfollow {}", user.github_login) } else { format!("Follow {}", user.github_login)