From 7da90451d91f0561ae9a4fc37301b3c8125da611 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 18 May 2023 14:52:21 -0700 Subject: [PATCH] =?UTF-8?q?Avoid=20unnecessary=20code=20action=20requests?= =?UTF-8?q?=20when=20applying=20leader=20updates=20t=E2=80=A6=20(#2489)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- crates/editor/src/editor_tests.rs | 7 ++++++ crates/editor/src/multi_buffer.rs | 40 +++++++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 0f749bde4841ebce9dbee4c012e0e8ff2fab8830..c82103e18540ae76e03cbc82619c667743c10548 100644 --- a/crates/editor/src/editor_tests.rs +++ b/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. diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index a2160b47e5d342122b57f964f2070cdc8010684a..057ae9012ccce5adde27d9d514f64970229fc2dd 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/crates/editor/src/multi_buffer.rs @@ -1165,6 +1165,9 @@ impl MultiBuffer { ) { self.sync(cx); let ids = excerpt_ids.into_iter().collect::>(); + 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::(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]