From 24141c2f16261c25ebedfa01414ce4b2a62055c0 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 23 Aug 2023 13:32:16 -0700 Subject: [PATCH] Ensure collaborators cursor colors are the same in channel buffers as in projects Co-authored-by: Mikayla --- crates/channel/src/channel_buffer.rs | 13 ++- .../collab/src/tests/channel_buffer_tests.rs | 59 ++++++++++-- crates/collab_ui/src/channel_view.rs | 96 +++++++++++++------ 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, 190 insertions(+), 62 deletions(-) diff --git a/crates/channel/src/channel_buffer.rs b/crates/channel/src/channel_buffer.rs index 5ee3fd6c84..cad3c4f58f 100644 --- a/crates/channel/src/channel_buffer.rs +++ b/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(); }); diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index 8fb50055f5..6a9ef3fc13 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/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::>() + view.editor.read(cx).replica_id_map().unwrap(), + &[(1, 0), (2, 1)].into_iter().collect::>() ); }); 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::>(), + view.editor.read(cx).replica_id_map().unwrap(), + &[(1, 0), (2, 1)].into_iter().collect::>(), ) }); // 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::>(), + view.editor.read(cx).replica_id_map().unwrap(), + &[(0, 0)].into_iter().collect::>(), ); }); + + // 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::>() + ); + }); + 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::>(), + ) + }); } #[track_caller] diff --git a/crates/collab_ui/src/channel_view.rs b/crates/collab_ui/src/channel_view.rs index c13711b29c..dd3969d351 100644 --- a/crates/collab_ui/src/channel_view.rs +++ b/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, + pub editor: ViewHandle, project: ModelHandle, channel_buffer: ModelHandle, remote_id: Option, @@ -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 { - let project = self.project.read(cx); - let mut result = HashMap::default(); - result.insert( - self.channel_buffer.read(cx).replica_id(cx), - project.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); - } + 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, } - result + self.refresh_replica_id_map(cx); + } + + fn handle_channel_buffer_event( + &mut self, + _: ModelHandle, + _: &channel_buffer::Event, + cx: &mut ViewContext, + ) { + 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) { + 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) + }); } } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index e7197d98c5..775f3c07ec 100644 --- a/crates/editor/src/editor.rs +++ b/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> { + self.replica_id_mapping.as_ref() + } + + pub fn set_replica_id_map( &mut self, - mapping: HashMap, + mapping: Option>, cx: &mut ViewContext, ) { - self.replica_id_mapping = Some(mapping); + self.replica_id_mapping = mapping; cx.notify(); } diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 0f26e5819c..9f74eed790 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -62,6 +62,7 @@ struct SelectionLayout { head: DisplayPoint, cursor_shape: CursorShape, is_newest: bool, + is_local: bool, range: Range, active_rows: Range, } @@ -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, ) { 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 for EditorElement { .anchor_before(DisplayPoint::new(end_row, 0).to_offset(&snapshot, Bias::Right)) }; - let mut selections: Vec<(ReplicaId, Vec)> = Vec::new(); + let mut selections: Vec<(Option, Vec)> = 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 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 for EditorElement { cursor_shape, &snapshot.display_snapshot, false, + false, )); } selections.extend(remote_selections); @@ -2191,6 +2201,7 @@ impl Element 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 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::(cx).scrollbar; @@ -2591,7 +2609,7 @@ pub struct LayoutState { blocks: Vec, highlighted_ranges: Vec<(Range, Color)>, fold_ranges: Vec<(BufferRow, Range, Color)>, - selections: Vec<(ReplicaId, Vec)>, + selections: Vec<(Option, Vec)>, scrollbar_row_range: Range, show_scrollbars: bool, is_singleton: bool, diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index d032e8e025..1b83ca5964 100644 --- a/crates/language/src/buffer.rs +++ b/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, diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index bc4fa587cb..49074268f2 100644 --- a/crates/project/src/project.rs +++ b/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(); diff --git a/crates/sum_tree/src/tree_map.rs b/crates/sum_tree/src/tree_map.rs index 4bb98d2ac8..edb9010e50 100644 --- a/crates/sum_tree/src/tree_map.rs +++ b/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(SumTree>) where K: Clone + Debug + Default + Ord, @@ -162,6 +162,16 @@ impl TreeMap { } } +impl Debug for TreeMap +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); diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index 0f34963708..9005fc9757 100644 --- a/crates/theme/src/theme.rs +++ b/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, + pub absent_selection: SelectionStyle, pub syntax: Arc, pub hint: HighlightStyle, pub suggestion: HighlightStyle, diff --git a/styles/src/style_tree/editor.ts b/styles/src/style_tree/editor.ts index 9ad008f38d..9277a2e7a1 100644 --- a/styles/src/style_tree/editor.ts +++ b/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,