Match the leader's last selection when unfollowing

Max Brunsfeld and Antonio Scandurra created

Co-Authored-By: Antonio Scandurra <me@as-cii.com>

Change summary

crates/editor/src/editor.rs       |  8 +++---
crates/editor/src/items.rs        | 39 ++++++++++++++++++++++++++------
crates/language/src/buffer.rs     |  6 -----
crates/server/src/rpc.rs          | 35 +++++++++++++++++++++++++----
crates/workspace/src/workspace.rs | 22 +++++++++++++-----
5 files changed, 81 insertions(+), 29 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -464,7 +464,7 @@ pub struct Editor {
     pending_rename: Option<RenameState>,
     searchable: bool,
     cursor_shape: CursorShape,
-    following: bool,
+    leader_replica_id: Option<u16>,
 }
 
 pub struct EditorSnapshot {
@@ -938,7 +938,7 @@ impl Editor {
             searchable: true,
             override_text_style: None,
             cursor_shape: Default::default(),
-            following: false,
+            leader_replica_id: None,
         };
         this.end_selection(cx);
         this
@@ -5038,7 +5038,7 @@ impl Editor {
 
         self.selections = selections;
         self.pending_selection = pending_selection;
-        if self.focused && !self.following {
+        if self.focused && self.leader_replica_id.is_none() {
             self.buffer.update(cx, |buffer, cx| {
                 buffer.set_active_selections(&self.selections, cx)
             });
@@ -5673,7 +5673,7 @@ impl View for Editor {
             self.blink_cursors(self.blink_epoch, cx);
             self.buffer.update(cx, |buffer, cx| {
                 buffer.finalize_last_transaction(cx);
-                if !self.following {
+                if self.leader_replica_id.is_none() {
                     buffer.set_active_selections(&self.selections, cx);
                 }
             });

crates/editor/src/items.rs 🔗

@@ -1,4 +1,4 @@
-use crate::{Autoscroll, Editor, Event, NavigationData, ToOffset, ToPoint as _};
+use crate::{Anchor, Autoscroll, Editor, Event, NavigationData, ToOffset, ToPoint as _};
 use anyhow::{anyhow, Result};
 use gpui::{
     elements::*, AppContext, Entity, ModelHandle, MutableAppContext, RenderContext, Subscription,
@@ -50,20 +50,43 @@ impl FollowableItem for Editor {
         }))
     }
 
-    fn set_following(&mut self, following: bool, cx: &mut ViewContext<Self>) {
-        self.following = following;
-        if self.following {
+    fn set_leader_replica_id(
+        &mut self,
+        leader_replica_id: Option<u16>,
+        cx: &mut ViewContext<Self>,
+    ) {
+        let prev_leader_replica_id = self.leader_replica_id;
+        self.leader_replica_id = leader_replica_id;
+        if self.leader_replica_id.is_some() {
             self.show_local_selections = false;
             self.buffer.update(cx, |buffer, cx| {
                 buffer.remove_active_selections(cx);
             });
         } else {
             self.show_local_selections = true;
-            if self.focused {
-                self.buffer.update(cx, |buffer, cx| {
-                    buffer.set_active_selections(&self.selections, cx);
-                });
+            if let Some(leader_replica_id) = prev_leader_replica_id {
+                let selections = self
+                    .buffer
+                    .read(cx)
+                    .snapshot(cx)
+                    .remote_selections_in_range(&(Anchor::min()..Anchor::max()))
+                    .filter_map(|(replica_id, selections)| {
+                        if replica_id == leader_replica_id {
+                            Some(selections)
+                        } else {
+                            None
+                        }
+                    })
+                    .collect::<Vec<_>>();
+                if !selections.is_empty() {
+                    self.set_selections(selections.into(), None, cx);
+                }
             }
+            self.buffer.update(cx, |buffer, cx| {
+                if self.focused {
+                    buffer.set_active_selections(&self.selections, cx);
+                }
+            });
         }
         cx.notify();
     }

crates/language/src/buffer.rs 🔗

@@ -1801,12 +1801,6 @@ impl BufferSnapshot {
             .min_by_key(|(open_range, close_range)| close_range.end - open_range.start)
     }
 
-    /*
-    impl BufferSnapshot
-      pub fn remote_selections_in_range(&self, Range<Anchor>) -> impl Iterator<Item = (ReplicaId, impl Iterator<Item = &Selection<Anchor>>)>
-      pub fn remote_selections_in_range(&self, Range<Anchor>) -> impl Iterator<Item = (ReplicaId, i
-    */
-
     pub fn remote_selections_in_range<'a>(
         &'a self,
         range: Range<Anchor>,

crates/server/src/rpc.rs 🔗

@@ -1083,8 +1083,8 @@ mod tests {
     };
     use collections::BTreeMap;
     use editor::{
-        self, ConfirmCodeAction, ConfirmCompletion, ConfirmRename, Editor, Input, Redo, Rename,
-        ToOffset, ToggleCodeActions, Undo,
+        self, Anchor, ConfirmCodeAction, ConfirmCompletion, ConfirmRename, Editor, Input, Redo,
+        Rename, ToOffset, ToggleCodeActions, Undo,
     };
     use gpui::{executor, ModelHandle, TestAppContext, ViewHandle};
     use language::{
@@ -4290,7 +4290,13 @@ mod tests {
         // Client B starts following client A.
         workspace_b
             .update(cx_b, |workspace, cx| {
-                let leader_id = *project_b.read(cx).collaborators().keys().next().unwrap();
+                let leader_id = project_b
+                    .read(cx)
+                    .collaborators()
+                    .values()
+                    .next()
+                    .unwrap()
+                    .peer_id;
                 workspace.toggle_follow(&leader_id.into(), cx).unwrap()
             })
             .await
@@ -4318,16 +4324,35 @@ mod tests {
             })
             .await;
 
+        editor_a1.update(cx_a, |editor, cx| {
+            editor.select_ranges([2..2], None, cx);
+        });
+        editor_b1
+            .condition(cx_b, |editor, cx| {
+                let snapshot = editor.buffer().read(cx).snapshot(cx);
+                let selection = snapshot
+                    .remote_selections_in_range(&(Anchor::min()..Anchor::max()))
+                    .next();
+                selection.map_or(false, |selection| {
+                    selection.1.start.to_offset(&snapshot) == 2
+                })
+            })
+            .await;
+
         // After unfollowing, client B stops receiving updates from client A.
         workspace_b.update(cx_b, |workspace, cx| {
             workspace.unfollow(&workspace.active_pane().clone(), cx)
         });
+        editor_b1.update(cx_b, |editor, cx| {
+            assert_eq!(editor.selected_ranges::<usize>(cx), &[2..2]);
+        });
+
         workspace_a.update(cx_a, |workspace, cx| {
             workspace.activate_item(&editor_a2, cx);
-            editor_a2.update(cx, |editor, cx| editor.set_text("ONE", cx));
+            editor_a2.update(cx, |editor, cx| editor.set_text("TWO", cx));
         });
         editor_b2
-            .condition(cx_b, |editor, cx| editor.text(cx) == "ONE")
+            .condition(cx_b, |editor, cx| editor.text(cx) == "TWO")
             .await;
         assert_eq!(
             workspace_b.read_with(cx_b, |workspace, cx| workspace

crates/workspace/src/workspace.rs 🔗

@@ -258,7 +258,7 @@ pub trait FollowableItem: Item {
         state: &mut Option<proto::view::Variant>,
         cx: &mut MutableAppContext,
     ) -> Option<Task<Result<ViewHandle<Self>>>>;
-    fn set_following(&mut self, following: bool, cx: &mut ViewContext<Self>);
+    fn set_leader_replica_id(&mut self, leader_replica_id: Option<u16>, cx: &mut ViewContext<Self>);
     fn to_state_message(&self, cx: &AppContext) -> Option<proto::view::Variant>;
     fn to_update_message(
         &self,
@@ -273,7 +273,7 @@ pub trait FollowableItem: Item {
 }
 
 pub trait FollowableItemHandle: ItemHandle {
-    fn set_following(&self, following: bool, cx: &mut MutableAppContext);
+    fn set_leader_replica_id(&self, leader_replica_id: Option<u16>, cx: &mut MutableAppContext);
     fn to_state_message(&self, cx: &AppContext) -> Option<proto::view::Variant>;
     fn to_update_message(
         &self,
@@ -288,8 +288,10 @@ pub trait FollowableItemHandle: ItemHandle {
 }
 
 impl<T: FollowableItem> FollowableItemHandle for ViewHandle<T> {
-    fn set_following(&self, following: bool, cx: &mut MutableAppContext) {
-        self.update(cx, |this, cx| this.set_following(following, cx))
+    fn set_leader_replica_id(&self, leader_replica_id: Option<u16>, cx: &mut MutableAppContext) {
+        self.update(cx, |this, cx| {
+            this.set_leader_replica_id(leader_replica_id, cx)
+        })
     }
 
     fn to_state_message(&self, cx: &AppContext) -> Option<proto::view::Variant> {
@@ -1263,7 +1265,7 @@ impl Workspace {
             if let Some(state) = states_by_pane.remove(&pane) {
                 for (_, item) in state.items_by_leader_view_id {
                     if let FollowerItem::Loaded(item) = item {
-                        item.set_following(false, cx);
+                        item.set_leader_replica_id(None, cx);
                     }
                 }
 
@@ -1624,6 +1626,14 @@ impl Workspace {
         cx: &mut AsyncAppContext,
     ) -> Result<()> {
         let project = this.read_with(cx, |this, _| this.project.clone());
+        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 item_builders = cx.update(|cx| {
             cx.default_global::<FollowableItemBuilders>()
@@ -1667,7 +1677,7 @@ impl Workspace {
                     .get_mut(&pane)?;
 
                 for (id, item) in leader_view_ids.into_iter().zip(items) {
-                    item.set_following(true, cx);
+                    item.set_leader_replica_id(Some(replica_id), cx);
                     match state.items_by_leader_view_id.entry(id) {
                         hash_map::Entry::Occupied(e) => {
                             let e = e.into_mut();