Start work on updating editors's scroll positions when following

Max Brunsfeld and Antonio Scandurra created

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

Change summary

crates/editor/src/editor.rs       |   9 ++
crates/editor/src/items.rs        |  68 ++++++++++++---------
crates/editor/src/multi_buffer.rs |   8 ++
crates/language/src/proto.rs      |  20 +++--
crates/rpc/proto/zed.proto        |   4 
crates/workspace/src/workspace.rs | 101 ++++++++++++++++++++++----------
6 files changed, 139 insertions(+), 71 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -1033,6 +1033,14 @@ impl Editor {
             self.scroll_top_anchor = Some(anchor);
         }
 
+        cx.emit(Event::ScrollPositionChanged);
+        cx.notify();
+    }
+
+    fn set_scroll_top_anchor(&mut self, anchor: Anchor, cx: &mut ViewContext<Self>) {
+        self.scroll_position = Vector2F::zero();
+        self.scroll_top_anchor = Some(anchor);
+        cx.emit(Event::ScrollPositionChanged);
         cx.notify();
     }
 
@@ -5634,6 +5642,7 @@ pub enum Event {
     Saved,
     TitleChanged,
     SelectionsChanged,
+    ScrollPositionChanged,
     Closed,
 }
 

crates/editor/src/items.rs 🔗

@@ -6,7 +6,7 @@ use gpui::{
 };
 use language::{Bias, Buffer, Diagnostic, File as _};
 use project::{File, Project, ProjectEntryId, ProjectPath};
-use rpc::proto;
+use rpc::proto::{self, update_followers::update_view};
 use std::{fmt::Write, path::PathBuf};
 use text::{Point, Selection};
 use util::ResultExt;
@@ -20,7 +20,7 @@ impl FollowableItem for Editor {
         project: ModelHandle<Project>,
         state: &mut Option<proto::view::Variant>,
         cx: &mut MutableAppContext,
-    ) -> Option<Task<Result<Box<dyn ItemHandle>>>> {
+    ) -> Option<Task<Result<ViewHandle<Self>>>> {
         let state = if matches!(state, Some(proto::view::Variant::Editor(_))) {
             if let Some(proto::view::Variant::Editor(state)) = state.take() {
                 state
@@ -36,7 +36,7 @@ impl FollowableItem for Editor {
         });
         Some(cx.spawn(|mut cx| async move {
             let buffer = buffer.await?;
-            let editor = pane
+            Ok(pane
                 .read_with(&cx, |pane, cx| {
                     pane.items_of_type::<Self>().find(|editor| {
                         editor.read(cx).buffer.read(cx).as_singleton().as_ref() == Some(&buffer)
@@ -46,8 +46,7 @@ impl FollowableItem for Editor {
                     cx.add_view(pane.window_id(), |cx| {
                         Editor::for_buffer(buffer, Some(project), cx)
                     })
-                });
-            Ok(Box::new(editor) as Box<_>)
+                }))
         }))
     }
 
@@ -59,17 +58,12 @@ impl FollowableItem for Editor {
             .unwrap()
             .read(cx)
             .remote_id();
-        let selection = self.newest_anchor_selection();
-        let selection = Selection {
-            id: selection.id,
-            start: selection.start.text_anchor.clone(),
-            end: selection.end.text_anchor.clone(),
-            reversed: selection.reversed,
-            goal: Default::default(),
-        };
         proto::view::Variant::Editor(proto::view::Editor {
             buffer_id,
-            newest_selection: Some(language::proto::serialize_selection(&selection)),
+            scroll_top: self
+                .scroll_top_anchor
+                .as_ref()
+                .map(|anchor| language::proto::serialize_anchor(&anchor.text_anchor)),
         })
     }
 
@@ -77,26 +71,42 @@ impl FollowableItem for Editor {
         &self,
         event: &Self::Event,
         cx: &AppContext,
-    ) -> Option<proto::update_followers::update_view::Variant> {
+    ) -> Option<update_view::Variant> {
         match event {
-            Event::SelectionsChanged => {
-                let selection = self.newest_anchor_selection();
-                let selection = Selection {
-                    id: selection.id,
-                    start: selection.start.text_anchor.clone(),
-                    end: selection.end.text_anchor.clone(),
-                    reversed: selection.reversed,
-                    goal: Default::default(),
-                };
-                Some(proto::update_followers::update_view::Variant::Editor(
-                    proto::update_followers::update_view::Editor {
-                        newest_selection: Some(language::proto::serialize_selection(&selection)),
-                    },
-                ))
+            Event::ScrollPositionChanged => {
+                Some(update_view::Variant::Editor(update_view::Editor {
+                    scroll_top: self
+                        .scroll_top_anchor
+                        .as_ref()
+                        .map(|anchor| language::proto::serialize_anchor(&anchor.text_anchor)),
+                }))
             }
             _ => None,
         }
     }
+
+    fn apply_update_message(
+        &mut self,
+        message: update_view::Variant,
+        cx: &mut ViewContext<Self>,
+    ) -> Result<()> {
+        match message {
+            update_view::Variant::Editor(message) => {
+                if let Some(anchor) = message.scroll_top {
+                    let anchor = language::proto::deserialize_anchor(anchor)
+                        .ok_or_else(|| anyhow!("invalid scroll top"))?;
+                    let anchor = {
+                        let buffer = self.buffer.read(cx);
+                        let buffer = buffer.read(cx);
+                        let (excerpt_id, _, _) = buffer.as_singleton().unwrap();
+                        buffer.anchor_in_excerpt(excerpt_id.clone(), anchor)
+                    };
+                    self.set_scroll_top_anchor(anchor, cx);
+                }
+            }
+        }
+        Ok(())
+    }
 }
 
 impl Item for Editor {

crates/editor/src/multi_buffer.rs 🔗

@@ -821,6 +821,14 @@ impl MultiBuffer {
             .map_or(Vec::new(), |state| state.excerpts.clone())
     }
 
+    pub fn excerpt_ids(&self) -> Vec<ExcerptId> {
+        self.buffers
+            .borrow()
+            .values()
+            .flat_map(|state| state.excerpts.iter().cloned())
+            .collect()
+    }
+
     pub fn excerpt_containing(
         &self,
         position: impl ToOffset,

crates/language/src/proto.rs 🔗

@@ -275,19 +275,21 @@ pub fn deserialize_selections(selections: Vec<proto::Selection>) -> Arc<[Selecti
     Arc::from(
         selections
             .into_iter()
-            .filter_map(|selection| {
-                Some(Selection {
-                    id: selection.id as usize,
-                    start: deserialize_anchor(selection.start?)?,
-                    end: deserialize_anchor(selection.end?)?,
-                    reversed: selection.reversed,
-                    goal: SelectionGoal::None,
-                })
-            })
+            .filter_map(deserialize_selection)
             .collect::<Vec<_>>(),
     )
 }
 
+pub fn deserialize_selection(selection: proto::Selection) -> Option<Selection<Anchor>> {
+    Some(Selection {
+        id: selection.id as usize,
+        start: deserialize_anchor(selection.start?)?,
+        end: deserialize_anchor(selection.end?)?,
+        reversed: selection.reversed,
+        goal: SelectionGoal::None,
+    })
+}
+
 pub fn deserialize_diagnostics(
     diagnostics: Vec<proto::Diagnostic>,
 ) -> Arc<[DiagnosticEntry<Anchor>]> {

crates/rpc/proto/zed.proto 🔗

@@ -568,7 +568,7 @@ message UpdateFollowers {
         }
 
         message Editor {
-            Selection newest_selection = 1;
+            Anchor scroll_top = 1;
         }
     }
 }
@@ -588,7 +588,7 @@ message View {
 
     message Editor {
         uint64 buffer_id = 1;
-        Selection newest_selection = 2;
+        Anchor scroll_top = 2;
     }
 }
 

crates/workspace/src/workspace.rs 🔗

@@ -6,7 +6,7 @@ pub mod settings;
 pub mod sidebar;
 mod status_bar;
 
-use anyhow::{anyhow, Result};
+use anyhow::{anyhow, Context, Result};
 use client::{
     proto, Authenticate, ChannelList, Client, PeerId, Subscription, TypedEnvelope, User, UserStore,
 };
@@ -43,7 +43,7 @@ use std::{
     sync::Arc,
 };
 use theme::{Theme, ThemeRegistry};
-use util::ResultExt;
+use util::{ResultExt, TryFutureExt};
 
 type ProjectItemBuilders = HashMap<
     TypeId,
@@ -55,7 +55,7 @@ type FollowableItemBuilder = fn(
     ModelHandle<Project>,
     &mut Option<proto::view::Variant>,
     &mut MutableAppContext,
-) -> Option<Task<Result<Box<dyn ItemHandle>>>>;
+) -> Option<Task<Result<Box<dyn FollowableItemHandle>>>>;
 type FollowableItemBuilders = HashMap<
     TypeId,
     (
@@ -135,9 +135,15 @@ pub fn register_followable_item<I: FollowableItem>(cx: &mut MutableAppContext) {
     cx.update_default_global(|builders: &mut FollowableItemBuilders, _| {
         builders.insert(
             TypeId::of::<I>(),
-            (I::for_state_message, |this| {
-                Box::new(this.downcast::<I>().unwrap())
-            }),
+            (
+                |pane, project, state, cx| {
+                    I::for_state_message(pane, project, state, cx).map(|task| {
+                        cx.foreground()
+                            .spawn(async move { Ok(Box::new(task.await?) as Box<_>) })
+                    })
+                },
+                |this| Box::new(this.downcast::<I>().unwrap()),
+            ),
         );
     });
 }
@@ -240,32 +246,35 @@ pub trait FollowableItem: Item {
         project: ModelHandle<Project>,
         state: &mut Option<proto::view::Variant>,
         cx: &mut MutableAppContext,
-    ) -> Option<Task<Result<Box<dyn ItemHandle>>>>
-    where
-        Self: Sized;
+    ) -> Option<Task<Result<ViewHandle<Self>>>>;
     fn to_state_message(&self, cx: &AppContext) -> proto::view::Variant;
     fn to_update_message(
         &self,
         event: &Self::Event,
         cx: &AppContext,
     ) -> Option<proto::update_followers::update_view::Variant>;
+    fn apply_update_message(
+        &mut self,
+        message: proto::update_followers::update_view::Variant,
+        cx: &mut ViewContext<Self>,
+    ) -> Result<()>;
 }
 
-pub trait FollowableItemHandle {
-    fn id(&self) -> usize;
+pub trait FollowableItemHandle: ItemHandle {
     fn to_state_message(&self, cx: &AppContext) -> proto::view::Variant;
     fn to_update_message(
         &self,
         event: &dyn Any,
         cx: &AppContext,
     ) -> Option<proto::update_followers::update_view::Variant>;
+    fn apply_update_message(
+        &self,
+        message: proto::update_followers::update_view::Variant,
+        cx: &mut MutableAppContext,
+    ) -> Result<()>;
 }
 
 impl<T: FollowableItem> FollowableItemHandle for ViewHandle<T> {
-    fn id(&self) -> usize {
-        self.id()
-    }
-
     fn to_state_message(&self, cx: &AppContext) -> proto::view::Variant {
         self.read(cx).to_state_message(cx)
     }
@@ -277,6 +286,14 @@ impl<T: FollowableItem> FollowableItemHandle for ViewHandle<T> {
     ) -> Option<proto::update_followers::update_view::Variant> {
         self.read(cx).to_update_message(event.downcast_ref()?, cx)
     }
+
+    fn apply_update_message(
+        &self,
+        message: proto::update_followers::update_view::Variant,
+        cx: &mut MutableAppContext,
+    ) -> Result<()> {
+        self.update(cx, |this, cx| this.apply_update_message(message, cx))
+    }
 }
 
 pub trait ItemHandle: 'static {
@@ -584,14 +601,15 @@ struct LeaderState {
     followers: HashSet<PeerId>,
 }
 
+#[derive(Default)]
 struct FollowerState {
     active_view_id: Option<usize>,
     items_by_leader_view_id: HashMap<usize, FollowerItem>,
 }
 
 enum FollowerItem {
-    Loading(Vec<proto::update_followers::Variant>),
-    Loaded(Box<dyn FollowedItemHandle>),
+    Loading(Vec<proto::update_followers::update_view::Variant>),
+    Loaded(Box<dyn FollowableItemHandle>),
 }
 
 impl Workspace {
@@ -1212,21 +1230,40 @@ impl Workspace {
                         });
                     }
 
-                    let items = futures::future::try_join_all(item_tasks).await?;
-                    let follower_state = FollowerState {
-                        active_view_id: response.active_view_id.map(|id| id as usize),
-                        items_by_leader_view_id: response
-                            .views
-                            .iter()
-                            .map(|v| v.id as usize)
-                            .zip(items)
-                            .collect(),
-                    };
                     this.update(&mut cx, |this, cx| {
                         this.follower_states_by_leader
                             .entry(leader_id)
                             .or_default()
-                            .insert(pane.downgrade(), follower_state);
+                            .insert(
+                                pane.downgrade(),
+                                FollowerState {
+                                    active_view_id: response.active_view_id.map(|id| id as usize),
+                                    items_by_leader_view_id: Default::default(),
+                                },
+                            );
+                    });
+
+                    let items = futures::future::try_join_all(item_tasks).await?;
+                    this.update(&mut cx, |this, cx| {
+                        let follower_state = this
+                            .follower_states_by_leader
+                            .entry(leader_id)
+                            .or_default()
+                            .entry(pane.downgrade())
+                            .or_default();
+                        for (id, item) in response.views.iter().map(|v| v.id as usize).zip(items) {
+                            let prev_state = follower_state.items_by_leader_view_id.remove(&id);
+                            if let Some(FollowerItem::Loading(updates)) = prev_state {
+                                for update in updates {
+                                    item.apply_update_message(update, cx)
+                                        .context("failed to apply view update")
+                                        .log_err();
+                                }
+                            }
+                            follower_state
+                                .items_by_leader_view_id
+                                .insert(id, FollowerItem::Loaded(item));
+                        }
                         this.leader_updated(leader_id, cx);
                     });
                 }
@@ -1535,7 +1572,7 @@ impl Workspace {
                 }
                 proto::update_followers::Variant::UpdateView(update_view) => {
                     for state in follower_states.values_mut() {
-                        state.items_by_leader_view_id.get(k)
+                        // state.items_by_leader_view_id.get(k)
                     }
                 }
                 proto::update_followers::Variant::CreateView(_) => todo!(),
@@ -1550,8 +1587,10 @@ impl Workspace {
         let mut items_to_add = Vec::new();
         for (pane, state) in self.follower_states_by_leader.get(&leader_id)? {
             if let Some((pane, active_view_id)) = pane.upgrade(cx).zip(state.active_view_id) {
-                if let Some(item) = state.items_by_leader_view_id.get(&active_view_id) {
-                    items_to_add.push((pane, item.clone()));
+                if let Some(FollowerItem::Loaded(item)) =
+                    state.items_by_leader_view_id.get(&active_view_id)
+                {
+                    items_to_add.push((pane, item.boxed_clone()));
                 }
             }
         }