Avoid excessive diagnostics refreshes (#21173)

Kirill Bulatov created 1 year ago

Attempts to reduce the diagnostics flicker, when editing very
fundamental parts of the large code base in Rust.


https://github.com/user-attachments/assets/dc3f9c21-8c6e-48db-967b-040649fd00da

Release Notes:

- N/A

Change summary

crates/diagnostics/src/diagnostics.rs       |  6 ++++++
crates/diagnostics/src/diagnostics_tests.rs | 17 ++++++++++++++---
crates/gpui/src/app/test_context.rs         | 11 ++++++++---
3 files changed, 28 insertions(+), 6 deletions(-)

Detailed changes

crates/diagnostics/src/diagnostics.rs

@@ -33,6 +33,7 @@ use std::{
     mem,
     ops::Range,
     sync::Arc,
+    time::Duration,
 };
 use theme::ActiveTheme;
 pub use toolbar_controls::ToolbarControls;
@@ -82,6 +83,8 @@ struct DiagnosticGroupState {
 
 impl EventEmitter<EditorEvent> for ProjectDiagnosticsEditor {}
 
+const DIAGNOSTICS_UPDATE_DEBOUNCE: Duration = Duration::from_millis(50);
+
 impl Render for ProjectDiagnosticsEditor {
     fn render(&mut self, cx: &mut ViewContext<Self>) -> impl IntoElement {
         let child = if self.path_states.is_empty() {
@@ -198,6 +201,9 @@ impl ProjectDiagnosticsEditor {
         }
         let project_handle = self.project.clone();
         self.update_excerpts_task = Some(cx.spawn(|this, mut cx| async move {
+            cx.background_executor()
+                .timer(DIAGNOSTICS_UPDATE_DEBOUNCE)
+                .await;
             loop {
                 let Some((path, language_server_id)) = this.update(&mut cx, |this, _| {
                     let Some((path, language_server_id)) = this.paths_to_update.pop_first() else {

crates/diagnostics/src/diagnostics_tests.rs

@@ -155,7 +155,8 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
     });
     let editor = view.update(cx, |view, _| view.editor.clone());
 
-    view.next_notification(cx).await;
+    view.next_notification(DIAGNOSTICS_UPDATE_DEBOUNCE + Duration::from_millis(10), cx)
+        .await;
     assert_eq!(
         editor_blocks(&editor, cx),
         [
@@ -240,7 +241,8 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
         lsp_store.disk_based_diagnostics_finished(language_server_id, cx);
     });
 
-    view.next_notification(cx).await;
+    view.next_notification(DIAGNOSTICS_UPDATE_DEBOUNCE + Duration::from_millis(10), cx)
+        .await;
     assert_eq!(
         editor_blocks(&editor, cx),
         [
@@ -352,7 +354,8 @@ async fn test_diagnostics(cx: &mut TestAppContext) {
         lsp_store.disk_based_diagnostics_finished(language_server_id, cx);
     });
 
-    view.next_notification(cx).await;
+    view.next_notification(DIAGNOSTICS_UPDATE_DEBOUNCE + Duration::from_millis(10), cx)
+        .await;
     assert_eq!(
         editor_blocks(&editor, cx),
         [
@@ -491,6 +494,8 @@ async fn test_diagnostics_multiple_servers(cx: &mut TestAppContext) {
     });
 
     // Only the first language server's diagnostics are shown.
+    cx.executor()
+        .advance_clock(DIAGNOSTICS_UPDATE_DEBOUNCE + Duration::from_millis(10));
     cx.executor().run_until_parked();
     assert_eq!(
         editor_blocks(&editor, cx),
@@ -537,6 +542,8 @@ async fn test_diagnostics_multiple_servers(cx: &mut TestAppContext) {
     });
 
     // Both language server's diagnostics are shown.
+    cx.executor()
+        .advance_clock(DIAGNOSTICS_UPDATE_DEBOUNCE + Duration::from_millis(10));
     cx.executor().run_until_parked();
     assert_eq!(
         editor_blocks(&editor, cx),
@@ -603,6 +610,8 @@ async fn test_diagnostics_multiple_servers(cx: &mut TestAppContext) {
     });
 
     // Only the first language server's diagnostics are updated.
+    cx.executor()
+        .advance_clock(DIAGNOSTICS_UPDATE_DEBOUNCE + Duration::from_millis(10));
     cx.executor().run_until_parked();
     assert_eq!(
         editor_blocks(&editor, cx),
@@ -659,6 +668,8 @@ async fn test_diagnostics_multiple_servers(cx: &mut TestAppContext) {
     });
 
     // Both language servers' diagnostics are updated.
+    cx.executor()
+        .advance_clock(DIAGNOSTICS_UPDATE_DEBOUNCE + Duration::from_millis(10));
     cx.executor().run_until_parked();
     assert_eq!(
         editor_blocks(&editor, cx),

crates/gpui/src/app/test_context.rs

@@ -538,12 +538,15 @@ impl<T: 'static> Model<T> {
 
 impl<V: 'static> View<V> {
     /// Returns a future that resolves when the view is next updated.
-    pub fn next_notification(&self, cx: &TestAppContext) -> impl Future<Output = ()> {
+    pub fn next_notification(
+        &self,
+        advance_clock_by: Duration,
+        cx: &TestAppContext,
+    ) -> impl Future<Output = ()> {
         use postage::prelude::{Sink as _, Stream as _};
 
         let (mut tx, mut rx) = postage::mpsc::channel(1);
-        let mut cx = cx.app.app.borrow_mut();
-        let subscription = cx.observe(self, move |_, _| {
+        let subscription = cx.app.app.borrow_mut().observe(self, move |_, _| {
             tx.try_send(()).ok();
         });
 
@@ -553,6 +556,8 @@ impl<V: 'static> View<V> {
             Duration::from_secs(1)
         };
 
+        cx.executor().advance_clock(advance_clock_by);
+
         async move {
             let notification = crate::util::timeout(duration, rx.recv())
                 .await