From 8ef6b1d8a9787091cd967ba74bc39bb369e381a5 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 20 Apr 2022 15:02:38 +0200 Subject: [PATCH] Don't emit event when LSP reports consecutive empty diagnostics This is related to #849: in that pull request we avoided *storing* empty diagnostics, but we'd still report an event when receiving consecutive empty diagnostics. So if the project diagnostics editor was open, it could happen that opening a buffer would cause the language server to report zero diagnostics. We would therefore close the buffer because there were no diagnostics, but doing so would cause the LSP to report another event with zero diagnostics. This would repeat forever, causing Zed to use a lot of CPU and the UI not to refresh properly. With this commit we will simply avoid emitting a `DiagnosticsUpdated` event altogether if no diagnostics were present before *and* the LSP is reporting a `PublishDiagnostics` event with no diagnostics in it. --- crates/project/src/project.rs | 33 +++++++++++++++++++++--- crates/project/src/worktree.rs | 46 ++++++++++++++++++---------------- 2 files changed, 54 insertions(+), 25 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index a74ee6026c..5c1064400e 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -154,7 +154,7 @@ pub struct ProjectPath { pub path: Arc, } -#[derive(Clone, Debug, Default, PartialEq, Serialize)] +#[derive(Copy, Clone, Debug, Default, PartialEq, Serialize)] pub struct DiagnosticSummary { pub error_count: usize, pub warning_count: usize, @@ -1961,13 +1961,15 @@ impl Project { self.update_buffer_diagnostics(&buffer, diagnostics.clone(), version, cx)?; } - worktree.update(cx, |worktree, cx| { + let updated = worktree.update(cx, |worktree, cx| { worktree .as_local_mut() .ok_or_else(|| anyhow!("not a local worktree"))? .update_diagnostics(project_path.path.clone(), diagnostics, cx) })?; - cx.emit(Event::DiagnosticsUpdated(project_path)); + if updated { + cx.emit(Event::DiagnosticsUpdated(project_path)); + } Ok(()) } @@ -4879,7 +4881,7 @@ mod tests { }; use lsp::Url; use serde_json::json; - use std::{cell::RefCell, os::unix, path::PathBuf, rc::Rc}; + use std::{cell::RefCell, os::unix, path::PathBuf, rc::Rc, task::Poll}; use unindent::Unindent as _; use util::{assert_set_eq, test::temp_tree}; use worktree::WorktreeHandle as _; @@ -5564,6 +5566,29 @@ mod tests { }] ) }); + + // Ensure publishing empty diagnostics twice only results in one update event. + fake_server.notify::( + lsp::PublishDiagnosticsParams { + uri: Url::from_file_path("/dir/a.rs").unwrap(), + version: None, + diagnostics: Default::default(), + }, + ); + assert_eq!( + events.next().await.unwrap(), + Event::DiagnosticsUpdated((worktree_id, Path::new("a.rs")).into()) + ); + + fake_server.notify::( + lsp::PublishDiagnosticsParams { + uri: Url::from_file_path("/dir/a.rs").unwrap(), + version: None, + diagnostics: Default::default(), + }, + ); + cx.foreground().run_until_parked(); + assert_eq!(futures::poll!(events.next()), Poll::Pending); } #[gpui::test] diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index f9e36a461e..19b490b0e2 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -564,33 +564,37 @@ impl LocalWorktree { worktree_path: Arc, diagnostics: Vec>, _: &mut ModelContext, - ) -> Result<()> { - let summary = DiagnosticSummary::new(&diagnostics); - if summary.is_empty() { + ) -> Result { + self.diagnostics.remove(&worktree_path); + let old_summary = self + .diagnostic_summaries + .remove(&PathKey(worktree_path.clone())) + .unwrap_or_default(); + let new_summary = DiagnosticSummary::new(&diagnostics); + if !new_summary.is_empty() { self.diagnostic_summaries - .remove(&PathKey(worktree_path.clone())); - self.diagnostics.remove(&worktree_path); - } else { - self.diagnostic_summaries - .insert(PathKey(worktree_path.clone()), summary.clone()); + .insert(PathKey(worktree_path.clone()), new_summary); self.diagnostics.insert(worktree_path.clone(), diagnostics); } - if let Some(share) = self.share.as_ref() { - self.client - .send(proto::UpdateDiagnosticSummary { - project_id: share.project_id, - worktree_id: self.id().to_proto(), - summary: Some(proto::DiagnosticSummary { - path: worktree_path.to_string_lossy().to_string(), - error_count: summary.error_count as u32, - warning_count: summary.warning_count as u32, - }), - }) - .log_err(); + let updated = !old_summary.is_empty() || !new_summary.is_empty(); + if updated { + if let Some(share) = self.share.as_ref() { + self.client + .send(proto::UpdateDiagnosticSummary { + project_id: share.project_id, + worktree_id: self.id().to_proto(), + summary: Some(proto::DiagnosticSummary { + path: worktree_path.to_string_lossy().to_string(), + error_count: new_summary.error_count as u32, + warning_count: new_summary.warning_count as u32, + }), + }) + .log_err(); + } } - Ok(()) + Ok(updated) } pub fn scan_complete(&self) -> impl Future {