Merge pull request #1962 from zed-industries/scrolling-breaks-follow

Max Brunsfeld created 3 years ago

Avoid breaking follow when syncing leader's scroll position

Change summary

crates/editor/src/editor_tests.rs | 26 ++++++++++++++++++++++++++
crates/editor/src/items.rs        |  5 ++---
crates/editor/src/scroll.rs       |  8 ++++----
3 files changed, 32 insertions(+), 7 deletions(-)

Detailed changes

crates/editor/src/editor_tests.rs

@@ -4983,9 +4983,11 @@ fn test_following(cx: &mut gpui::MutableAppContext) {
         |cx| build_editor(buffer.clone(), cx),
     );
 
+    let is_still_following = Rc::new(RefCell::new(true));
     let pending_update = Rc::new(RefCell::new(None));
     follower.update(cx, {
         let update = pending_update.clone();
+        let is_still_following = is_still_following.clone();
         |_, cx| {
             cx.subscribe(&leader, move |_, leader, event, cx| {
                 leader
@@ -4993,6 +4995,13 @@ fn test_following(cx: &mut gpui::MutableAppContext) {
                     .add_event_to_update_proto(event, &mut *update.borrow_mut(), cx);
             })
             .detach();
+
+            cx.subscribe(&follower, move |_, _, event, cx| {
+                if Editor::should_unfollow_on_event(event, cx) {
+                    *is_still_following.borrow_mut() = false;
+                }
+            })
+            .detach();
         }
     });
 
@@ -5006,6 +5015,7 @@ fn test_following(cx: &mut gpui::MutableAppContext) {
             .unwrap();
     });
     assert_eq!(follower.read(cx).selections.ranges(cx), vec![1..1]);
+    assert_eq!(*is_still_following.borrow(), true);
 
     // Update the scroll position only
     leader.update(cx, |leader, cx| {
@@ -5020,6 +5030,7 @@ fn test_following(cx: &mut gpui::MutableAppContext) {
         follower.update(cx, |follower, cx| follower.scroll_position(cx)),
         vec2f(1.5, 3.5)
     );
+    assert_eq!(*is_still_following.borrow(), true);
 
     // Update the selections and scroll position
     leader.update(cx, |leader, cx| {
@@ -5036,6 +5047,7 @@ fn test_following(cx: &mut gpui::MutableAppContext) {
         assert!(follower.scroll_manager.has_autoscroll_request());
     });
     assert_eq!(follower.read(cx).selections.ranges(cx), vec![0..0]);
+    assert_eq!(*is_still_following.borrow(), true);
 
     // Creating a pending selection that precedes another selection
     leader.update(cx, |leader, cx| {
@@ -5048,6 +5060,7 @@ fn test_following(cx: &mut gpui::MutableAppContext) {
             .unwrap();
     });
     assert_eq!(follower.read(cx).selections.ranges(cx), vec![0..0, 1..1]);
+    assert_eq!(*is_still_following.borrow(), true);
 
     // Extend the pending selection so that it surrounds another selection
     leader.update(cx, |leader, cx| {
@@ -5059,6 +5072,19 @@ fn test_following(cx: &mut gpui::MutableAppContext) {
             .unwrap();
     });
     assert_eq!(follower.read(cx).selections.ranges(cx), vec![0..2]);
+
+    // Scrolling locally breaks the follow
+    follower.update(cx, |follower, cx| {
+        let top_anchor = follower.buffer().read(cx).read(cx).anchor_after(0);
+        follower.set_scroll_anchor(
+            ScrollAnchor {
+                top_anchor,
+                offset: vec2f(0.0, 0.5),
+            },
+            cx,
+        );
+    });
+    assert_eq!(*is_still_following.borrow(), false);
 }
 
 #[test]

crates/editor/src/items.rs

@@ -88,7 +88,7 @@ impl FollowableItem for Editor {
                 }
 
                 if let Some(anchor) = state.scroll_top_anchor {
-                    editor.set_scroll_anchor_internal(
+                    editor.set_scroll_anchor_remote(
                         ScrollAnchor {
                             top_anchor: Anchor {
                                 buffer_id: Some(state.buffer_id as usize),
@@ -98,7 +98,6 @@ impl FollowableItem for Editor {
                             },
                             offset: vec2f(state.scroll_x, state.scroll_y),
                         },
-                        false,
                         cx,
                     );
                 }
@@ -213,7 +212,7 @@ impl FollowableItem for Editor {
                     self.set_selections_from_remote(selections, cx);
                     self.request_autoscroll_remotely(Autoscroll::newest(), cx);
                 } else if let Some(anchor) = message.scroll_top_anchor {
-                    self.set_scroll_anchor(
+                    self.set_scroll_anchor_remote(
                         ScrollAnchor {
                             top_anchor: Anchor {
                                 buffer_id: Some(buffer_id),

crates/editor/src/scroll.rs

@@ -284,17 +284,17 @@ impl Editor {
     }
 
     pub fn set_scroll_anchor(&mut self, scroll_anchor: ScrollAnchor, cx: &mut ViewContext<Self>) {
-        self.set_scroll_anchor_internal(scroll_anchor, true, cx);
+        hide_hover(self, cx);
+        self.scroll_manager.set_anchor(scroll_anchor, true, cx);
     }
 
-    pub(crate) fn set_scroll_anchor_internal(
+    pub(crate) fn set_scroll_anchor_remote(
         &mut self,
         scroll_anchor: ScrollAnchor,
-        local: bool,
         cx: &mut ViewContext<Self>,
     ) {
         hide_hover(self, cx);
-        self.scroll_manager.set_anchor(scroll_anchor, local, cx);
+        self.scroll_manager.set_anchor(scroll_anchor, false, cx);
     }
 
     pub fn scroll_screen(&mut self, amount: &ScrollAmount, cx: &mut ViewContext<Self>) {