Refresh diagnostics inside the tab (#3225)

Kirill Bulatov created 2 years ago

r-a now has 2 different types of diagnostics: 
* "disk-based" ones that come from `cargo check` and related, that emit
`project::Event::DiskBasedDiagnosticsStarted` and
`DiskBasedDiagnosticsFinished`
* "flycheck" diagnostics from r-a itself, that it tries to dynamically
apply to every buffer open, that come with `DiagnosticsUpdated` event.

Latter diagnostics update frequently, on every file close and open, but
`diagnostics.rs` logic had never polled for new diagnostics after
registering the `DiagnosticsUpdated` event, so the only way we could
have newer diagnostics was to re-open the whole panel.
The PR fixes that, and also adds more debug logging to the module.
The logic of the fix looks very familiar to previous related fix:
https://github.com/zed-industries/zed/pull/3128

One notable thing after the fix: "flycheck" diagnostics stay forever if
the diagnostics panel is opened: excerpts in that panel do not allow the
buffer to get dropped (hence, closed in terms of r-a) and get the
updated, zero diagnostics.
If the diagnostics panel is opened and closed multiple times, those
errors gradually disappear.

Release Notes:

- Fixed diagnostics panel not refreshing its contents properly

Change summary

Cargo.lock                            |  1 
crates/diagnostics/Cargo.toml         |  1 
crates/diagnostics/src/diagnostics.rs | 43 ++++++++++++++++------------
crates/editor/src/items.rs            |  4 +-
4 files changed, 28 insertions(+), 21 deletions(-)

Detailed changes

Cargo.lock

@@ -2461,6 +2461,7 @@ dependencies = [
  "editor",
  "gpui",
  "language",
+ "log",
  "lsp",
  "postage",
  "project",

crates/diagnostics/Cargo.toml

@@ -20,6 +20,7 @@ theme = { path = "../theme" }
 util = { path = "../util" }
 workspace = { path = "../workspace" }
 
+log.workspace = true
 anyhow.workspace = true
 schemars.workspace = true
 serde.workspace = true

crates/diagnostics/src/diagnostics.rs

@@ -151,6 +151,7 @@ impl ProjectDiagnosticsEditor {
     ) -> Self {
         cx.subscribe(&project_handle, |this, _, event, cx| match event {
             project::Event::DiskBasedDiagnosticsFinished { language_server_id } => {
+                log::debug!("Disk based diagnostics finished for server {language_server_id}");
                 this.update_excerpts(Some(*language_server_id), cx);
                 this.update_title(cx);
             }
@@ -158,8 +159,11 @@ impl ProjectDiagnosticsEditor {
                 language_server_id,
                 path,
             } => {
+                log::debug!("Adding path {path:?} to update for server {language_server_id}");
                 this.paths_to_update
                     .insert((path.clone(), *language_server_id));
+                this.update_excerpts(Some(*language_server_id), cx);
+                this.update_title(cx);
             }
             _ => {}
         })
@@ -229,6 +233,7 @@ impl ProjectDiagnosticsEditor {
         language_server_id: Option<LanguageServerId>,
         cx: &mut ViewContext<Self>,
     ) {
+        log::debug!("Updating excerpts for server {language_server_id:?}");
         let mut paths = Vec::new();
         self.paths_to_update.retain(|(path, server_id)| {
             if language_server_id
@@ -1301,25 +1306,6 @@ mod tests {
                     cx,
                 )
                 .unwrap();
-            project
-                .update_diagnostic_entries(
-                    server_id_2,
-                    PathBuf::from("/test/main.js"),
-                    None,
-                    vec![DiagnosticEntry {
-                        range: Unclipped(PointUtf16::new(1, 0))..Unclipped(PointUtf16::new(1, 1)),
-                        diagnostic: Diagnostic {
-                            message: "warning 1".to_string(),
-                            severity: DiagnosticSeverity::ERROR,
-                            is_primary: true,
-                            is_disk_based: true,
-                            group_id: 2,
-                            ..Default::default()
-                        },
-                    }],
-                    cx,
-                )
-                .unwrap();
         });
 
         // The first language server finishes
@@ -1353,6 +1339,25 @@ mod tests {
 
         // The second language server finishes
         project.update(cx, |project, cx| {
+            project
+                .update_diagnostic_entries(
+                    server_id_2,
+                    PathBuf::from("/test/main.js"),
+                    None,
+                    vec![DiagnosticEntry {
+                        range: Unclipped(PointUtf16::new(1, 0))..Unclipped(PointUtf16::new(1, 1)),
+                        diagnostic: Diagnostic {
+                            message: "warning 1".to_string(),
+                            severity: DiagnosticSeverity::ERROR,
+                            is_primary: true,
+                            is_disk_based: true,
+                            group_id: 2,
+                            ..Default::default()
+                        },
+                    }],
+                    cx,
+                )
+                .unwrap();
             project.disk_based_diagnostics_finished(server_id_2, cx);
         });
 

crates/editor/src/items.rs

@@ -33,9 +33,9 @@ use util::{
     paths::{PathExt, FILE_ROW_COLUMN_DELIMITER},
     ResultExt, TryFutureExt,
 };
-use workspace::item::{BreadcrumbText, FollowableItemHandle};
+use workspace::item::{BreadcrumbText, FollowableItemHandle, ItemHandle};
 use workspace::{
-    item::{FollowableItem, Item, ItemEvent, ItemHandle, ProjectItem},
+    item::{FollowableItem, Item, ItemEvent, ProjectItem},
     searchable::{Direction, SearchEvent, SearchableItem, SearchableItemHandle},
     ItemId, ItemNavHistory, Pane, StatusItemView, ToolbarItemLocation, ViewId, Workspace,
     WorkspaceId,