Avoid unnecessary code action requests when applying leader updates t… (#2489)

Max Brunsfeld created 2 years ago

We noticed a huge amount of code actions requests being issued by
followers when applying leader updates. It was caused by a call to
`MultiBuffer::remove_excerpts` with an empty list of excerpts to remove.
This PR fixes that by avoiding emitting spurious events when multibuffer
excerpt manipulation methods are called with empty lists.

Change summary

crates/editor/src/editor_tests.rs |  7 +++++
crates/editor/src/multi_buffer.rs | 40 +++++++++++++++++++++++++++++---
2 files changed, 43 insertions(+), 4 deletions(-)

Detailed changes

crates/editor/src/editor_tests.rs

@@ -5459,10 +5459,12 @@ async fn test_following(cx: &mut gpui::TestAppContext) {
     });
 
     let is_still_following = Rc::new(RefCell::new(true));
+    let follower_edit_event_count = Rc::new(RefCell::new(0));
     let pending_update = Rc::new(RefCell::new(None));
     follower.update(cx, {
         let update = pending_update.clone();
         let is_still_following = is_still_following.clone();
+        let follower_edit_event_count = follower_edit_event_count.clone();
         |_, cx| {
             cx.subscribe(&leader, move |_, leader, event, cx| {
                 leader
@@ -5475,6 +5477,9 @@ async fn test_following(cx: &mut gpui::TestAppContext) {
                 if Editor::should_unfollow_on_event(event, cx) {
                     *is_still_following.borrow_mut() = false;
                 }
+                if let Event::BufferEdited = event {
+                    *follower_edit_event_count.borrow_mut() += 1;
+                }
             })
             .detach();
         }
@@ -5494,6 +5499,7 @@ async fn test_following(cx: &mut gpui::TestAppContext) {
         assert_eq!(follower.selections.ranges(cx), vec![1..1]);
     });
     assert_eq!(*is_still_following.borrow(), true);
+    assert_eq!(*follower_edit_event_count.borrow(), 0);
 
     // Update the scroll position only
     leader.update(cx, |leader, cx| {
@@ -5510,6 +5516,7 @@ async fn test_following(cx: &mut gpui::TestAppContext) {
         vec2f(1.5, 3.5)
     );
     assert_eq!(*is_still_following.borrow(), true);
+    assert_eq!(*follower_edit_event_count.borrow(), 0);
 
     // Update the selections and scroll position. The follower's scroll position is updated
     // via autoscroll, not via the leader's exact scroll position.

crates/editor/src/multi_buffer.rs

@@ -1165,6 +1165,9 @@ impl MultiBuffer {
     ) {
         self.sync(cx);
         let ids = excerpt_ids.into_iter().collect::<Vec<_>>();
+        if ids.is_empty() {
+            return;
+        }
 
         let mut buffers = self.buffers.borrow_mut();
         let mut snapshot = self.snapshot.borrow_mut();
@@ -4080,19 +4083,25 @@ mod tests {
 
         let leader_multibuffer = cx.add_model(|_| MultiBuffer::new(0));
         let follower_multibuffer = cx.add_model(|_| MultiBuffer::new(0));
+        let follower_edit_event_count = Rc::new(RefCell::new(0));
 
         follower_multibuffer.update(cx, |_, cx| {
-            cx.subscribe(&leader_multibuffer, |follower, _, event, cx| {
-                match event.clone() {
+            let follower_edit_event_count = follower_edit_event_count.clone();
+            cx.subscribe(
+                &leader_multibuffer,
+                move |follower, _, event, cx| match event.clone() {
                     Event::ExcerptsAdded {
                         buffer,
                         predecessor,
                         excerpts,
                     } => follower.insert_excerpts_with_ids_after(predecessor, buffer, excerpts, cx),
                     Event::ExcerptsRemoved { ids } => follower.remove_excerpts(ids, cx),
+                    Event::Edited => {
+                        *follower_edit_event_count.borrow_mut() += 1;
+                    }
                     _ => {}
-                }
-            })
+                },
+            )
             .detach();
         });
 
@@ -4131,6 +4140,7 @@ mod tests {
             leader_multibuffer.read(cx).snapshot(cx).text(),
             follower_multibuffer.read(cx).snapshot(cx).text(),
         );
+        assert_eq!(*follower_edit_event_count.borrow(), 2);
 
         leader_multibuffer.update(cx, |leader, cx| {
             let excerpt_ids = leader.excerpt_ids();
@@ -4140,6 +4150,27 @@ mod tests {
             leader_multibuffer.read(cx).snapshot(cx).text(),
             follower_multibuffer.read(cx).snapshot(cx).text(),
         );
+        assert_eq!(*follower_edit_event_count.borrow(), 3);
+
+        // Removing an empty set of excerpts is a noop.
+        leader_multibuffer.update(cx, |leader, cx| {
+            leader.remove_excerpts([], cx);
+        });
+        assert_eq!(
+            leader_multibuffer.read(cx).snapshot(cx).text(),
+            follower_multibuffer.read(cx).snapshot(cx).text(),
+        );
+        assert_eq!(*follower_edit_event_count.borrow(), 3);
+
+        // Adding an empty set of excerpts is a noop.
+        leader_multibuffer.update(cx, |leader, cx| {
+            leader.push_excerpts::<usize>(buffer_2.clone(), [], cx);
+        });
+        assert_eq!(
+            leader_multibuffer.read(cx).snapshot(cx).text(),
+            follower_multibuffer.read(cx).snapshot(cx).text(),
+        );
+        assert_eq!(*follower_edit_event_count.borrow(), 3);
 
         leader_multibuffer.update(cx, |leader, cx| {
             leader.clear(cx);
@@ -4148,6 +4179,7 @@ mod tests {
             leader_multibuffer.read(cx).snapshot(cx).text(),
             follower_multibuffer.read(cx).snapshot(cx).text(),
         );
+        assert_eq!(*follower_edit_event_count.borrow(), 4);
     }
 
     #[gpui::test]