Ensure collaborators cursor colors are the same in channel buffers as in projects

Max Brunsfeld and Mikayla created

Co-authored-by: Mikayla <mikayla@zed.dev>

Change summary

crates/channel/src/channel_buffer.rs            | 13 ++
crates/collab/src/tests/channel_buffer_tests.rs | 59 ++++++++++-
crates/collab_ui/src/channel_view.rs            | 94 +++++++++++++-----
crates/editor/src/editor.rs                     | 10 +
crates/editor/src/element.rs                    | 50 ++++++---
crates/language/src/buffer.rs                   |  8 +
crates/project/src/project.rs                   |  2 
crates/sum_tree/src/tree_map.rs                 | 12 ++
crates/theme/src/theme.rs                       |  1 
styles/src/style_tree/editor.ts                 |  1 
10 files changed, 189 insertions(+), 61 deletions(-)

Detailed changes

crates/channel/src/channel_buffer.rs 🔗

@@ -21,8 +21,12 @@ pub struct ChannelBuffer {
     _subscription: client::Subscription,
 }
 
+pub enum Event {
+    CollaboratorsChanged,
+}
+
 impl Entity for ChannelBuffer {
-    type Event = ();
+    type Event = Event;
 
     fn release(&mut self, _: &mut AppContext) {
         self.client
@@ -54,8 +58,9 @@ impl ChannelBuffer {
 
             let collaborators = response.collaborators;
 
-            let buffer =
-                cx.add_model(|cx| language::Buffer::new(response.replica_id as u16, base_text, cx));
+            let buffer = cx.add_model(|_| {
+                language::Buffer::remote(response.buffer_id, response.replica_id as u16, base_text)
+            });
             buffer.update(&mut cx, |buffer, cx| buffer.apply_ops(operations, cx))?;
 
             let subscription = client.subscribe_to_entity(channel_id)?;
@@ -111,6 +116,7 @@ impl ChannelBuffer {
 
         this.update(&mut cx, |this, cx| {
             this.collaborators.push(collaborator);
+            cx.emit(Event::CollaboratorsChanged);
             cx.notify();
         });
 
@@ -134,6 +140,7 @@ impl ChannelBuffer {
                     true
                 }
             });
+            cx.emit(Event::CollaboratorsChanged);
             cx.notify();
         });
 

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

@@ -63,6 +63,10 @@ async fn test_core_channel_buffers(
 
     // Client B sees the correct text, and then edits it
     let buffer_b = channel_buffer_b.read_with(cx_b, |buffer, _| buffer.buffer());
+    assert_eq!(
+        buffer_b.read_with(cx_b, |buffer, _| buffer.remote_id()),
+        buffer_a.read_with(cx_a, |buffer, _| buffer.remote_id())
+    );
     assert_eq!(buffer_text(&buffer_b, cx_b), "hello, cruel world");
     buffer_b.update(cx_b, |buffer, cx| {
         buffer.edit([(7..12, "beautiful")], None, cx)
@@ -138,6 +142,7 @@ async fn test_channel_buffer_replica_ids(
 
     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);
 
     // Clients A and B join a channel.
     active_call_a
@@ -190,7 +195,7 @@ async fn test_channel_buffer_replica_ids(
 
     // Client C is in a separate project.
     client_c.fs().insert_tree("/dir", json!({})).await;
-    let (project_c, _) = client_c.build_local_project("/dir", cx_c).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.
@@ -211,8 +216,14 @@ async fn test_channel_buffer_replica_ids(
         .add_window(|cx| ChannelView::new(project_a.clone(), channel_buffer_a.clone(), None, cx));
     let channel_window_b = cx_b
         .add_window(|cx| ChannelView::new(project_b.clone(), channel_buffer_b.clone(), None, cx));
-    let channel_window_c = cx_c
-        .add_window(|cx| ChannelView::new(project_c.clone(), channel_buffer_c.clone(), None, cx));
+    let channel_window_c = cx_c.add_window(|cx| {
+        ChannelView::new(
+            separate_project_c.clone(),
+            channel_buffer_c.clone(),
+            None,
+            cx,
+        )
+    });
 
     let channel_view_a = channel_window_a.root(cx_a);
     let channel_view_b = channel_window_b.root(cx_b);
@@ -222,24 +233,54 @@ async fn test_channel_buffer_replica_ids(
     // 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.project_replica_ids_by_channel_buffer_replica_id(cx),
-            [(1, 0), (2, 1)].into_iter().collect::<HashMap<_, _>>()
+            view.editor.read(cx).replica_id_map().unwrap(),
+            &[(1, 0), (2, 1)].into_iter().collect::<HashMap<_, _>>()
         );
     });
     channel_view_b.read_with(cx_b, |view, cx| {
         assert_eq!(
-            view.project_replica_ids_by_channel_buffer_replica_id(cx),
-            [(1, 0), (2, 1)].into_iter().collect::<HashMap<u16, u16>>(),
+            view.editor.read(cx).replica_id_map().unwrap(),
+            &[(1, 0), (2, 1)].into_iter().collect::<HashMap<u16, u16>>(),
         )
     });
 
     // 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.project_replica_ids_by_channel_buffer_replica_id(cx),
-            [(0, 0)].into_iter().collect::<HashMap<u16, u16>>(),
+            view.editor.read(cx).replica_id_map().unwrap(),
+            &[(0, 0)].into_iter().collect::<HashMap<u16, u16>>(),
+        );
+    });
+
+    // 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))
+        .await
+        .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);
+    });
+
+    // 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::<HashMap<_, _>>()
         );
     });
+    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::<HashMap<_, _>>(),
+        )
+    });
 }
 
 #[track_caller]

crates/collab_ui/src/channel_view.rs 🔗

@@ -1,4 +1,4 @@
-use channel::channel_buffer::ChannelBuffer;
+use channel::channel_buffer::{self, ChannelBuffer};
 use client::proto;
 use clock::ReplicaId;
 use collections::HashMap;
@@ -24,7 +24,7 @@ pub(crate) fn init(cx: &mut AppContext) {
 }
 
 pub struct ChannelView {
-    editor: ViewHandle<Editor>,
+    pub editor: ViewHandle<Editor>,
     project: ModelHandle<Project>,
     channel_buffer: ModelHandle<ChannelBuffer>,
     remote_id: Option<ViewId>,
@@ -43,6 +43,10 @@ impl ChannelView {
         let editor = cx.add_view(|cx| Editor::for_buffer(buffer, None, cx));
         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 {
             editor,
             project,
@@ -50,38 +54,70 @@ impl ChannelView {
             remote_id: None,
             _editor_event_subscription,
         };
-        let mapping = this.project_replica_ids_by_channel_buffer_replica_id(cx);
-        this.editor
-            .update(cx, |editor, cx| editor.set_replica_id_mapping(mapping, cx));
-
+        this.refresh_replica_id_map(cx);
         this
     }
 
-    /// Channel Buffer Replica ID -> Project Replica ID
-    pub fn project_replica_ids_by_channel_buffer_replica_id(
-        &self,
-        cx: &AppContext,
-    ) -> HashMap<ReplicaId, ReplicaId> {
+    fn handle_project_event(
+        &mut self,
+        _: ModelHandle<Project>,
+        event: &project::Event,
+        cx: &mut ViewContext<Self>,
+    ) {
+        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<ChannelBuffer>,
+        _: &channel_buffer::Event,
+        cx: &mut ViewContext<Self>,
+    ) {
+        self.refresh_replica_id_map(cx);
+    }
+
+    /// 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<Self>) {
+        let mut project_replica_ids_by_channel_buffer_replica_id = HashMap::default();
         let project = self.project.read(cx);
-        let mut result = HashMap::default();
-        result.insert(
-            self.channel_buffer.read(cx).replica_id(cx),
-            project.replica_id(),
+        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,
+                                ))
+                        })
+                }),
         );
-        for collaborator in self.channel_buffer.read(cx).collaborators() {
-            let project_replica_id =
-                project
-                    .collaborators()
-                    .values()
-                    .find_map(|project_collaborator| {
-                        (project_collaborator.user_id == collaborator.user_id)
-                            .then_some(project_collaborator.replica_id)
-                    });
-            if let Some(project_replica_id) = project_replica_id {
-                result.insert(collaborator.replica_id as ReplicaId, project_replica_id);
-            }
-        }
-        result
+
+        self.editor.update(cx, |editor, cx| {
+            editor.set_replica_id_map(Some(project_replica_ids_by_channel_buffer_replica_id), cx)
+        });
     }
 }
 

crates/editor/src/editor.rs 🔗

@@ -1606,12 +1606,16 @@ impl Editor {
         self.read_only = read_only;
     }
 
-    pub fn set_replica_id_mapping(
+    pub fn replica_id_map(&self) -> Option<&HashMap<ReplicaId, ReplicaId>> {
+        self.replica_id_mapping.as_ref()
+    }
+
+    pub fn set_replica_id_map(
         &mut self,
-        mapping: HashMap<ReplicaId, ReplicaId>,
+        mapping: Option<HashMap<ReplicaId, ReplicaId>>,
         cx: &mut ViewContext<Self>,
     ) {
-        self.replica_id_mapping = Some(mapping);
+        self.replica_id_mapping = mapping;
         cx.notify();
     }
 

crates/editor/src/element.rs 🔗

@@ -62,6 +62,7 @@ struct SelectionLayout {
     head: DisplayPoint,
     cursor_shape: CursorShape,
     is_newest: bool,
+    is_local: bool,
     range: Range<DisplayPoint>,
     active_rows: Range<u32>,
 }
@@ -73,6 +74,7 @@ impl SelectionLayout {
         cursor_shape: CursorShape,
         map: &DisplaySnapshot,
         is_newest: bool,
+        is_local: bool,
     ) -> Self {
         let point_selection = selection.map(|p| p.to_point(&map.buffer_snapshot));
         let display_selection = point_selection.map(|p| p.to_display_point(map));
@@ -109,6 +111,7 @@ impl SelectionLayout {
             head,
             cursor_shape,
             is_newest,
+            is_local,
             range,
             active_rows,
         }
@@ -763,7 +766,6 @@ impl EditorElement {
         cx: &mut PaintContext<Editor>,
     ) {
         let style = &self.style;
-        let local_replica_id = editor.replica_id(cx);
         let scroll_position = layout.position_map.snapshot.scroll_position();
         let start_row = layout.visible_display_row_range.start;
         let scroll_top = scroll_position.y() * layout.position_map.line_height;
@@ -852,15 +854,13 @@ impl EditorElement {
 
         for (replica_id, selections) in &layout.selections {
             let replica_id = *replica_id;
-            let selection_style = style.replica_selection_style(replica_id);
+            let selection_style = if let Some(replica_id) = replica_id {
+                style.replica_selection_style(replica_id)
+            } else {
+                &style.absent_selection
+            };
 
             for selection in selections {
-                if !selection.range.is_empty()
-                    && (replica_id == local_replica_id
-                        || Some(replica_id) == editor.leader_replica_id)
-                {
-                    invisible_display_ranges.push(selection.range.clone());
-                }
                 self.paint_highlighted_range(
                     scene,
                     selection.range.clone(),
@@ -874,7 +874,10 @@ impl EditorElement {
                     bounds,
                 );
 
-                if editor.show_local_cursors(cx) || replica_id != local_replica_id {
+                if selection.is_local && !selection.range.is_empty() {
+                    invisible_display_ranges.push(selection.range.clone());
+                }
+                if !selection.is_local || editor.show_local_cursors(cx) {
                     let cursor_position = selection.head;
                     if layout
                         .visible_display_row_range
@@ -2124,7 +2127,7 @@ impl Element<Editor> for EditorElement {
                 .anchor_before(DisplayPoint::new(end_row, 0).to_offset(&snapshot, Bias::Right))
         };
 
-        let mut selections: Vec<(ReplicaId, Vec<SelectionLayout>)> = Vec::new();
+        let mut selections: Vec<(Option<ReplicaId>, Vec<SelectionLayout>)> = Vec::new();
         let mut active_rows = BTreeMap::new();
         let mut fold_ranges = Vec::new();
         let is_singleton = editor.is_singleton(cx);
@@ -2155,8 +2158,14 @@ impl Element<Editor> for EditorElement {
             .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 {
+                None
+            };
+
             // The local selections match the leader's selections.
-            if Some(replica_id) == editor.leader_replica_id {
+            if replica_id.is_some() && replica_id == editor.leader_replica_id {
                 continue;
             }
             remote_selections
@@ -2168,6 +2177,7 @@ impl Element<Editor> for EditorElement {
                     cursor_shape,
                     &snapshot.display_snapshot,
                     false,
+                    false,
                 ));
         }
         selections.extend(remote_selections);
@@ -2191,6 +2201,7 @@ impl Element<Editor> for EditorElement {
                     editor.cursor_shape,
                     &snapshot.display_snapshot,
                     is_newest,
+                    true,
                 );
                 if is_newest {
                     newest_selection_head = Some(layout.head);
@@ -2206,11 +2217,18 @@ impl Element<Editor> for EditorElement {
             }
 
             // Render the local selections in the leader's color when following.
-            let local_replica_id = editor
-                .leader_replica_id
-                .unwrap_or_else(|| editor.replica_id(cx));
+            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)
+                } else {
+                    replica_id
+                }
+            };
 
-            selections.push((local_replica_id, layouts));
+            selections.push((Some(local_replica_id), layouts));
         }
 
         let scrollbar_settings = &settings::get::<EditorSettings>(cx).scrollbar;
@@ -2591,7 +2609,7 @@ pub struct LayoutState {
     blocks: Vec<BlockLayout>,
     highlighted_ranges: Vec<(Range<DisplayPoint>, Color)>,
     fold_ranges: Vec<(BufferRow, Range<DisplayPoint>, Color)>,
-    selections: Vec<(ReplicaId, Vec<SelectionLayout>)>,
+    selections: Vec<(Option<ReplicaId>, Vec<SelectionLayout>)>,
     scrollbar_row_range: Range<f32>,
     show_scrollbars: bool,
     is_singleton: bool,

crates/language/src/buffer.rs 🔗

@@ -359,6 +359,14 @@ impl Buffer {
         )
     }
 
+    pub fn remote(remote_id: u64, replica_id: ReplicaId, base_text: String) -> Self {
+        Self::build(
+            TextBuffer::new(replica_id, remote_id, base_text),
+            None,
+            None,
+        )
+    }
+
     pub fn from_proto(
         replica_id: ReplicaId,
         message: proto::BufferState,

crates/project/src/project.rs 🔗

@@ -282,6 +282,7 @@ pub enum Event {
         old_peer_id: proto::PeerId,
         new_peer_id: proto::PeerId,
     },
+    CollaboratorJoined(proto::PeerId),
     CollaboratorLeft(proto::PeerId),
     RefreshInlayHints,
 }
@@ -5931,6 +5932,7 @@ impl Project {
         let collaborator = Collaborator::from_proto(collaborator)?;
         this.update(&mut cx, |this, cx| {
             this.shared_buffers.remove(&collaborator.peer_id);
+            cx.emit(Event::CollaboratorJoined(collaborator.peer_id));
             this.collaborators
                 .insert(collaborator.peer_id, collaborator);
             cx.notify();

crates/sum_tree/src/tree_map.rs 🔗

@@ -2,7 +2,7 @@ use std::{cmp::Ordering, fmt::Debug};
 
 use crate::{Bias, Dimension, Edit, Item, KeyedItem, SeekTarget, SumTree, Summary};
 
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Clone, PartialEq, Eq)]
 pub struct TreeMap<K, V>(SumTree<MapEntry<K, V>>)
 where
     K: Clone + Debug + Default + Ord,
@@ -162,6 +162,16 @@ impl<K: Clone + Debug + Default + Ord, V: Clone + Debug> TreeMap<K, V> {
     }
 }
 
+impl<K: Debug, V: Debug> Debug for TreeMap<K, V>
+where
+    K: Clone + Debug + Default + Ord,
+    V: Clone + Debug,
+{
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_map().entries(self.iter()).finish()
+    }
+}
+
 #[derive(Debug)]
 struct MapSeekTargetAdaptor<'a, T>(&'a T);
 

crates/theme/src/theme.rs 🔗

@@ -756,6 +756,7 @@ pub struct Editor {
     pub line_number: Color,
     pub line_number_active: Color,
     pub guest_selections: Vec<SelectionStyle>,
+    pub absent_selection: SelectionStyle,
     pub syntax: Arc<SyntaxTheme>,
     pub hint: HighlightStyle,
     pub suggestion: HighlightStyle,

styles/src/style_tree/editor.ts 🔗

@@ -184,6 +184,7 @@ export default function editor(): any {
             theme.players[6],
             theme.players[7],
         ],
+        absent_selection: theme.players[7],
         autocomplete: {
             background: background(theme.middle),
             corner_radius: 8,