refactor: introduce diagnostics toolbar editor trait

This commit refactors the interaction between the diagnostics editors
(`BufferDiagnosticsEditor` and `ProjectDiagnosticsEditor`) so that,
instead of passing an optional enum to the `ToolbarControls` struct, as
well as the `DiagnosticBlock`, we pass a reference to a type that
implements a new trait, `DiagnosticsToolbarEditor`, which defines the
methods that the toolbar controls needs, namely:

- `include_warnings`
- `toggle_warnings`
- `has_stale_excerpts`
- `is_updating`
- `stop_updating`
- `refresh_diagnostics`
- `get_diagnostics_for_buffer`

This makes it so that both `ToolbarControls` and `DiagnosticBlock`
implementations don't need to be aware of the internal details of each
of the editors, as well as avoiding duplicating the
`DiagnosticsEditorHandle` enum.

crates/diagnostics/src/buffer_diagnostics.rs

@@ -1,6 +1,7 @@
 use crate::{
     DIAGNOSTICS_UPDATE_DELAY, IncludeWarnings, ToggleWarnings, context_range_for_entry,
-    diagnostic_renderer::{DiagnosticBlock, DiagnosticRenderer, DiagnosticsEditorHandle},
+    diagnostic_renderer::{DiagnosticBlock, DiagnosticRenderer},
+    toolbar_controls::DiagnosticsToolbarEditor,
 };
 use anyhow::Result;
 use collections::HashMap;
@@ -11,7 +12,7 @@ use editor::{
 use gpui::{
     AnyElement, App, AppContext, Context, Entity, EntityId, EventEmitter, FocusHandle, Focusable,
     InteractiveElement, IntoElement, ParentElement, Render, SharedString, Styled, Subscription,
-    Task, Window, actions, div,
+    Task, WeakEntity, Window, actions, div,
 };
 use language::{Buffer, DiagnosticEntry, Point};
 use project::{
@@ -246,7 +247,7 @@ impl BufferDiagnosticsEditor {
                     )
                 });
 
-                workspace.add_item_to_active_pane(Box::new(item.clone()), None, true, window, cx);
+                workspace.add_item_to_active_pane(Box::new(item), None, true, window, cx);
             }
         }
     }
@@ -372,9 +373,7 @@ impl BufferDiagnosticsEditor {
                     DiagnosticRenderer::diagnostic_blocks_for_group(
                         group,
                         buffer_snapshot.remote_id(),
-                        Some(DiagnosticsEditorHandle::Buffer(
+                        Some(Arc::new(buffer_diagnostics_editor.clone())),
-                            buffer_diagnostics_editor.clone(),
-                        )),
                         cx,
                     )
                 })?;
@@ -917,3 +916,59 @@ impl Render for BufferDiagnosticsEditor {
             .child(child)
     }
 }
+
+impl DiagnosticsToolbarEditor for WeakEntity<BufferDiagnosticsEditor> {
+    fn include_warnings(&self, cx: &App) -> bool {
+        self.read_with(cx, |buffer_diagnostics_editor, _cx| {
+            buffer_diagnostics_editor.include_warnings
+        })
+        .unwrap_or(false)
+    }
+
+    fn has_stale_excerpts(&self, _cx: &App) -> bool {
+        false
+    }
+
+    fn is_updating(&self, cx: &App) -> bool {
+        self.read_with(cx, |buffer_diagnostics_editor, cx| {
+            buffer_diagnostics_editor.update_excerpts_task.is_some()
+                || buffer_diagnostics_editor
+                    .project
+                    .read(cx)
+                    .language_servers_running_disk_based_diagnostics(cx)
+                    .next()
+                    .is_some()
+        })
+        .unwrap_or(false)
+    }
+
+    fn stop_updating(&self, cx: &mut App) {
+        let _ = self.update(cx, |buffer_diagnostics_editor, cx| {
+            buffer_diagnostics_editor.update_excerpts_task = None;
+            cx.notify();
+        });
+    }
+
+    fn refresh_diagnostics(&self, window: &mut Window, cx: &mut App) {
+        let _ = self.update(cx, |buffer_diagnostics_editor, cx| {
+            buffer_diagnostics_editor.update_all_excerpts(window, cx);
+        });
+    }
+
+    fn toggle_warnings(&self, window: &mut Window, cx: &mut App) {
+        let _ = self.update(cx, |buffer_diagnostics_editor, cx| {
+            buffer_diagnostics_editor.toggle_warnings(&Default::default(), window, cx);
+        });
+    }
+
+    fn get_diagnostics_for_buffer(
+        &self,
+        _buffer_id: text::BufferId,
+        cx: &App,
+    ) -> Vec<language::DiagnosticEntry<text::Anchor>> {
+        self.read_with(cx, |buffer_diagnostics_editor, _cx| {
+            buffer_diagnostics_editor.diagnostics.clone()
+        })
+        .unwrap_or_default()
+    }
+}

crates/diagnostics/src/diagnostic_renderer.rs

@@ -18,42 +18,7 @@ use ui::{
 };
 use util::maybe;
 
-use crate::ProjectDiagnosticsEditor;
+use crate::toolbar_controls::DiagnosticsToolbarEditor;
-use crate::buffer_diagnostics::BufferDiagnosticsEditor;
-
-#[derive(Clone)]
-pub(crate) enum DiagnosticsEditorHandle {
-    Project(WeakEntity<ProjectDiagnosticsEditor>),
-    Buffer(WeakEntity<BufferDiagnosticsEditor>),
-}
-
-impl DiagnosticsEditorHandle {
-    fn get_diagnostics_for_buffer(
-        &self,
-        buffer_id: BufferId,
-        cx: &App,
-    ) -> Vec<DiagnosticEntry<text::Anchor>> {
-        match self {
-            // Not sure if possible, as currently there shouldn't be a way for this
-            // method to be called for a buffer other than the one the
-            // `BufferDiagnosticsEditor` is working with, but we should probably
-            // save the ID of the buffer that it is working with, so that, if it
-            // doesn't match the argument, we can return an empty vector.
-            Self::Buffer(editor) => editor
-                .read_with(cx, |editor, _cx| editor.diagnostics.clone())
-                .unwrap_or(vec![]),
-            Self::Project(editor) => editor
-                .read_with(cx, |editor, _cx| {
-                    editor
-                        .diagnostics
-                        .get(&buffer_id)
-                        .cloned()
-                        .unwrap_or_default()
-                })
-                .unwrap_or(vec![]),
-        }
-    }
-}
 
 pub struct DiagnosticRenderer;
 
@@ -61,7 +26,7 @@ impl DiagnosticRenderer {
     pub fn diagnostic_blocks_for_group(
         diagnostic_group: Vec<DiagnosticEntry<Point>>,
         buffer_id: BufferId,
-        diagnostics_editor: Option<DiagnosticsEditorHandle>,
+        diagnostics_editor: Option<Arc<dyn DiagnosticsToolbarEditor>>,
         cx: &mut App,
     ) -> Vec<DiagnosticBlock> {
         let Some(primary_ix) = diagnostic_group
@@ -218,7 +183,7 @@ pub(crate) struct DiagnosticBlock {
     pub(crate) initial_range: Range<Point>,
     pub(crate) severity: DiagnosticSeverity,
     pub(crate) markdown: Entity<Markdown>,
-    pub(crate) diagnostics_editor: Option<DiagnosticsEditorHandle>,
+    pub(crate) diagnostics_editor: Option<Arc<dyn DiagnosticsToolbarEditor>>,
 }
 
 impl DiagnosticBlock {
@@ -269,7 +234,7 @@ impl DiagnosticBlock {
 
     pub fn open_link(
         editor: &mut Editor,
-        diagnostics_editor: &Option<DiagnosticsEditorHandle>,
+        diagnostics_editor: &Option<Arc<dyn DiagnosticsToolbarEditor>>,
         link: SharedString,
         window: &mut Window,
         cx: &mut Context<Editor>,

crates/diagnostics/src/diagnostics.rs

@@ -37,6 +37,7 @@ use std::{
 };
 use text::{BufferId, OffsetRangeExt};
 use theme::ActiveTheme;
+use toolbar_controls::DiagnosticsToolbarEditor;
 pub use toolbar_controls::ToolbarControls;
 use ui::{Icon, IconName, Label, h_flex, prelude::*};
 use util::ResultExt;
@@ -46,8 +47,6 @@ use workspace::{
     searchable::SearchableItemHandle,
 };
 
-use crate::diagnostic_renderer::DiagnosticsEditorHandle;
-
 actions!(
     diagnostics,
     [
@@ -483,7 +482,7 @@ impl ProjectDiagnosticsEditor {
                     crate::diagnostic_renderer::DiagnosticRenderer::diagnostic_blocks_for_group(
                         group,
                         buffer_snapshot.remote_id(),
-                        Some(DiagnosticsEditorHandle::Project(this.clone())),
+                        Some(Arc::new(this.clone())),
                         cx,
                     )
                 })?;
@@ -825,6 +824,68 @@ impl Item for ProjectDiagnosticsEditor {
     }
 }
 
+impl DiagnosticsToolbarEditor for WeakEntity<ProjectDiagnosticsEditor> {
+    fn include_warnings(&self, cx: &App) -> bool {
+        self.read_with(cx, |project_diagnostics_editor, _cx| {
+            project_diagnostics_editor.include_warnings
+        })
+        .unwrap_or(false)
+    }
+
+    fn has_stale_excerpts(&self, cx: &App) -> bool {
+        self.read_with(cx, |project_diagnostics_editor, _cx| {
+            !project_diagnostics_editor.paths_to_update.is_empty()
+        })
+        .unwrap_or(false)
+    }
+
+    fn is_updating(&self, cx: &App) -> bool {
+        self.read_with(cx, |project_diagnostics_editor, cx| {
+            project_diagnostics_editor.update_excerpts_task.is_some()
+                || project_diagnostics_editor
+                    .project
+                    .read(cx)
+                    .language_servers_running_disk_based_diagnostics(cx)
+                    .next()
+                    .is_some()
+        })
+        .unwrap_or(false)
+    }
+
+    fn stop_updating(&self, cx: &mut App) {
+        let _ = self.update(cx, |project_diagnostics_editor, cx| {
+            project_diagnostics_editor.update_excerpts_task = None;
+            cx.notify();
+        });
+    }
+
+    fn refresh_diagnostics(&self, window: &mut Window, cx: &mut App) {
+        let _ = self.update(cx, |project_diagnostics_editor, cx| {
+            project_diagnostics_editor.update_all_excerpts(window, cx);
+        });
+    }
+
+    fn toggle_warnings(&self, window: &mut Window, cx: &mut App) {
+        let _ = self.update(cx, |project_diagnostics_editor, cx| {
+            project_diagnostics_editor.toggle_warnings(&Default::default(), window, cx);
+        });
+    }
+
+    fn get_diagnostics_for_buffer(
+        &self,
+        buffer_id: text::BufferId,
+        cx: &App,
+    ) -> Vec<language::DiagnosticEntry<text::Anchor>> {
+        self.read_with(cx, |project_diagnostics_editor, _cx| {
+            project_diagnostics_editor
+                .diagnostics
+                .get(&buffer_id)
+                .cloned()
+                .unwrap_or_default()
+        })
+        .unwrap_or_default()
+    }
+}
 const DIAGNOSTIC_EXPANSION_ROW_LIMIT: u32 = 32;
 
 async fn context_range_for_entry(

crates/diagnostics/src/toolbar_controls.rs

@@ -1,20 +1,40 @@
 use crate::{BufferDiagnosticsEditor, ProjectDiagnosticsEditor, ToggleDiagnosticsRefresh};
-use gpui::{Context, EventEmitter, ParentElement, Render, WeakEntity, Window};
+use gpui::{Context, EventEmitter, ParentElement, Render, Window};
+use language::DiagnosticEntry;
+use text::{Anchor, BufferId};
 use ui::prelude::*;
 use ui::{IconButton, IconButtonShape, IconName, Tooltip};
 use workspace::{ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView, item::ItemHandle};
 
 pub struct ToolbarControls {
-    editor: Option<DiagnosticsEditorHandle>,
+    editor: Option<Box<dyn DiagnosticsToolbarEditor>>,
 }
 
-enum DiagnosticsEditorHandle {
+pub(crate) trait DiagnosticsToolbarEditor: Send + Sync {
-    Project(WeakEntity<ProjectDiagnosticsEditor>),
+    /// Informs the toolbar whether warnings are included in the diagnostics.
-    Buffer(WeakEntity<BufferDiagnosticsEditor>),
+    fn include_warnings(&self, cx: &App) -> bool;
+    /// Toggles whether warning diagnostics should be displayed by the
+    /// diagnostics editor.
+    fn toggle_warnings(&self, window: &mut Window, cx: &mut App);
+    /// Indicates whether any of the excerpts displayed by the diagnostics
+    /// editor are stale.
+    fn has_stale_excerpts(&self, cx: &App) -> bool;
+    /// Indicates whether the diagnostics editor is currently updating the
+    /// diagnostics.
+    fn is_updating(&self, cx: &App) -> bool;
+    /// Requests that the diagnostics editor stop updating the diagnostics.
+    fn stop_updating(&self, cx: &mut App);
+    /// Requests that the diagnostics editor updates the displayed diagnostics
+    /// with the latest information.
+    fn refresh_diagnostics(&self, window: &mut Window, cx: &mut App);
+    /// Returns a list of diagnostics for the provided buffer id.
+    fn get_diagnostics_for_buffer(
+        &self,
+        buffer_id: BufferId,
+        cx: &App,
+    ) -> Vec<DiagnosticEntry<Anchor>>;
 }
 
-impl DiagnosticsEditorHandle {}
-
 impl Render for ToolbarControls {
     fn render(&mut self, _: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
         let mut has_stale_excerpts = false;
@@ -22,32 +42,10 @@ impl Render for ToolbarControls {
         let mut is_updating = false;
 
         match &self.editor {
-            Some(DiagnosticsEditorHandle::Project(editor)) => {
+            Some(editor) => {
-                if let Some(editor) = editor.upgrade() {
+                include_warnings = editor.include_warnings(cx);
-                    let diagnostics = editor.read(cx);
+                has_stale_excerpts = editor.has_stale_excerpts(cx);
-                    include_warnings = diagnostics.include_warnings;
+                is_updating = editor.is_updating(cx);
-                    has_stale_excerpts = !diagnostics.paths_to_update.is_empty();
-                    is_updating = diagnostics.update_excerpts_task.is_some()
-                        || diagnostics
-                            .project
-                            .read(cx)
-                            .language_servers_running_disk_based_diagnostics(cx)
-                            .next()
-                            .is_some();
-                }
-            }
-            Some(DiagnosticsEditorHandle::Buffer(editor)) => {
-                if let Some(editor) = editor.upgrade() {
-                    let diagnostics = editor.read(cx);
-                    include_warnings = diagnostics.include_warnings;
-                    is_updating = diagnostics.update_excerpts_task.is_some()
-                        || diagnostics
-                            .project
-                            .read(cx)
-                            .language_servers_running_disk_based_diagnostics(cx)
-                            .next()
-                            .is_some();
-                }
             }
             None => {}
         }
@@ -78,29 +76,9 @@ impl Render for ToolbarControls {
                             ))
                             .on_click(cx.listener(move |toolbar_controls, _, _, cx| {
                                 match toolbar_controls.editor() {
-                                    Some(DiagnosticsEditorHandle::Buffer(
+                                    Some(editor) => {
-                                        buffer_diagnostics_editor,
+                                        editor.stop_updating(cx);
-                                    )) => {
-                                        let _ = buffer_diagnostics_editor.update(
-                                            cx,
-                                            |buffer_diagnostics_editor, cx| {
-                                                buffer_diagnostics_editor.update_excerpts_task =
-                                                    None;
                                         cx.notify();
-                                            },
-                                        );
-                                    }
-                                    Some(DiagnosticsEditorHandle::Project(
-                                        project_diagnostics_editor,
-                                    )) => {
-                                        let _ = project_diagnostics_editor.update(
-                                            cx,
-                                            |project_diagnostics_editor, cx| {
-                                                project_diagnostics_editor.update_excerpts_task =
-                                                    None;
-                                                cx.notify();
-                                            },
-                                        );
                                     }
                                     None => {}
                                 }
@@ -120,28 +98,7 @@ impl Render for ToolbarControls {
                                 move |toolbar_controls, _, window, cx| match toolbar_controls
                                     .editor()
                                 {
-                                    Some(DiagnosticsEditorHandle::Buffer(
+                                    Some(editor) => editor.refresh_diagnostics(window, cx),
-                                        buffer_diagnostics_editor,
-                                    )) => {
-                                        let _ = buffer_diagnostics_editor.update(
-                                            cx,
-                                            |buffer_diagnostics_editor, cx| {
-                                                buffer_diagnostics_editor
-                                                    .update_all_excerpts(window, cx);
-                                            },
-                                        );
-                                    }
-                                    Some(DiagnosticsEditorHandle::Project(
-                                        project_diagnostics_editor,
-                                    )) => {
-                                        let _ = project_diagnostics_editor.update(
-                                            cx,
-                                            |project_diagnostics_editor, cx| {
-                                                project_diagnostics_editor
-                                                    .update_all_excerpts(window, cx);
-                                            },
-                                        );
-                                    }
                                     None => {}
                                 }
                             })),
@@ -154,31 +111,8 @@ impl Render for ToolbarControls {
                     .shape(IconButtonShape::Square)
                     .tooltip(Tooltip::text(warning_tooltip))
                     .on_click(cx.listener(|this, _, window, cx| match &this.editor {
-                        Some(DiagnosticsEditorHandle::Project(project_diagnostics_editor)) => {
+                        Some(editor) => editor.toggle_warnings(window, cx),
-                            let _ = project_diagnostics_editor.update(
+                        None => {}
-                                cx,
-                                |project_diagnostics_editor, cx| {
-                                    project_diagnostics_editor.toggle_warnings(
-                                        &Default::default(),
-                                        window,
-                                        cx,
-                                    );
-                                },
-                            );
-                        }
-                        Some(DiagnosticsEditorHandle::Buffer(buffer_diagnostics_editor)) => {
-                            let _ = buffer_diagnostics_editor.update(
-                                cx,
-                                |buffer_diagnostics_editor, cx| {
-                                    buffer_diagnostics_editor.toggle_warnings(
-                                        &Default::default(),
-                                        window,
-                                        cx,
-                                    );
-                                },
-                            );
-                        }
-                        _ => {}
                     })),
             )
     }
@@ -195,10 +129,10 @@ impl ToolbarItemView for ToolbarControls {
     ) -> ToolbarItemLocation {
         if let Some(pane_item) = active_pane_item.as_ref() {
             if let Some(editor) = pane_item.downcast::<ProjectDiagnosticsEditor>() {
-                self.editor = Some(DiagnosticsEditorHandle::Project(editor.downgrade()));
+                self.editor = Some(Box::new(editor.downgrade()));
                 ToolbarItemLocation::PrimaryRight
             } else if let Some(editor) = pane_item.downcast::<BufferDiagnosticsEditor>() {
-                self.editor = Some(DiagnosticsEditorHandle::Buffer(editor.downgrade()));
+                self.editor = Some(Box::new(editor.downgrade()));
                 ToolbarItemLocation::PrimaryRight
             } else {
                 ToolbarItemLocation::Hidden
@@ -220,7 +154,7 @@ impl ToolbarControls {
         ToolbarControls { editor: None }
     }
 
-    fn editor(&self) -> Option<&DiagnosticsEditorHandle> {
+    fn editor(&self) -> Option<&dyn DiagnosticsToolbarEditor> {
-        self.editor.as_ref()
+        self.editor.as_deref()
     }
 }