Skip to content

Commit

Permalink
Avoid excessive diagnostics refreshes (#21173)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
SomeoneToIgnore authored Nov 25, 2024
1 parent 28142be commit bd02b35
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 6 deletions.
6 changes: 6 additions & 0 deletions crates/diagnostics/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use std::{
mem,
ops::Range,
sync::Arc,
time::Duration,
};
use theme::ActiveTheme;
pub use toolbar_controls::ToolbarControls;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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 {
Expand Down
17 changes: 14 additions & 3 deletions crates/diagnostics/src/diagnostics_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
[
Expand Down Expand Up @@ -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),
[
Expand Down Expand Up @@ -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),
[
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
11 changes: 8 additions & 3 deletions crates/gpui/src/app/test_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

Expand All @@ -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
Expand Down

0 comments on commit bd02b35

Please sign in to comment.