working diagnostic popover. Also renamed GoToNextDiagnostic to GoToDiagnostic and adjusted it's action to jump to the popover's diagnostic if it is visible

K Simmons created

Change summary

assets/keymaps/default.json          |   2 
crates/diagnostics/src/items.rs      |   8 
crates/editor/src/editor.rs          |  30 ++-
crates/editor/src/element.rs         | 155 +++++++++++++-------
crates/editor/src/hover_popover.rs   | 226 ++++++++++++++++++++---------
crates/text/src/selection.rs         |  14 
crates/theme/src/theme.rs            |   3 
crates/zed/src/menus.rs              |   2 
styles/src/styleTree/hoverPopover.ts |  50 +++++-
styles/src/themes/common/base16.ts   |  23 ++
styles/src/themes/common/theme.ts    |   3 
11 files changed, 353 insertions(+), 163 deletions(-)

Detailed changes

assets/keymaps/default.json 🔗

@@ -190,7 +190,7 @@
             "alt-down": "editor::SelectSmallerSyntaxNode",
             "cmd-u": "editor::UndoSelection",
             "cmd-shift-U": "editor::RedoSelection",
-            "f8": "editor::GoToNextDiagnostic",
+            "f8": "editor::GoToDiagnostic",
             "shift-f8": "editor::GoToPrevDiagnostic",
             "f2": "editor::Rename",
             "f12": "editor::GoToDefinition",

crates/diagnostics/src/items.rs 🔗

@@ -1,5 +1,5 @@
 use collections::HashSet;
-use editor::{Editor, GoToNextDiagnostic};
+use editor::{Editor, GoToDiagnostic};
 use gpui::{
     elements::*, platform::CursorStyle, serde_json, Entity, ModelHandle, MouseButton,
     MutableAppContext, RenderContext, Subscription, View, ViewContext, ViewHandle, WeakViewHandle,
@@ -48,10 +48,10 @@ impl DiagnosticIndicator {
         }
     }
 
-    fn go_to_next_diagnostic(&mut self, _: &GoToNextDiagnostic, cx: &mut ViewContext<Self>) {
+    fn go_to_next_diagnostic(&mut self, _: &GoToDiagnostic, cx: &mut ViewContext<Self>) {
         if let Some(editor) = self.active_editor.as_ref().and_then(|e| e.upgrade(cx)) {
             editor.update(cx, |editor, cx| {
-                editor.go_to_diagnostic(editor::Direction::Next, cx);
+                editor.go_to_diagnostic_impl(editor::Direction::Next, cx);
             })
         }
     }
@@ -202,7 +202,7 @@ impl View for DiagnosticIndicator {
                 })
                 .with_cursor_style(CursorStyle::PointingHand)
                 .on_click(MouseButton::Left, |_, cx| {
-                    cx.dispatch_action(GoToNextDiagnostic)
+                    cx.dispatch_action(GoToDiagnostic)
                 })
                 .boxed(),
             );

crates/editor/src/editor.rs 🔗

@@ -82,9 +82,6 @@ pub struct SelectNext {
     pub replace_newest: bool,
 }
 
-#[derive(Clone, PartialEq)]
-pub struct GoToDiagnostic(pub Direction);
-
 #[derive(Clone, PartialEq)]
 pub struct Scroll(pub Vector2F);
 
@@ -138,7 +135,7 @@ actions!(
         Backspace,
         Delete,
         Newline,
-        GoToNextDiagnostic,
+        GoToDiagnostic,
         GoToPrevDiagnostic,
         Indent,
         Outdent,
@@ -300,7 +297,7 @@ pub fn init(cx: &mut MutableAppContext) {
     cx.add_action(Editor::move_to_enclosing_bracket);
     cx.add_action(Editor::undo_selection);
     cx.add_action(Editor::redo_selection);
-    cx.add_action(Editor::go_to_next_diagnostic);
+    cx.add_action(Editor::go_to_diagnostic);
     cx.add_action(Editor::go_to_prev_diagnostic);
     cx.add_action(Editor::go_to_definition);
     cx.add_action(Editor::page_up);
@@ -4564,17 +4561,32 @@ impl Editor {
         self.selection_history.mode = SelectionHistoryMode::Normal;
     }
 
-    fn go_to_next_diagnostic(&mut self, _: &GoToNextDiagnostic, cx: &mut ViewContext<Self>) {
-        self.go_to_diagnostic(Direction::Next, cx)
+    fn go_to_diagnostic(&mut self, _: &GoToDiagnostic, cx: &mut ViewContext<Self>) {
+        self.go_to_diagnostic_impl(Direction::Next, cx)
     }
 
     fn go_to_prev_diagnostic(&mut self, _: &GoToPrevDiagnostic, cx: &mut ViewContext<Self>) {
-        self.go_to_diagnostic(Direction::Prev, cx)
+        self.go_to_diagnostic_impl(Direction::Prev, cx)
     }
 
-    pub fn go_to_diagnostic(&mut self, direction: Direction, cx: &mut ViewContext<Self>) {
+    pub fn go_to_diagnostic_impl(&mut self, direction: Direction, cx: &mut ViewContext<Self>) {
         let buffer = self.buffer.read(cx).snapshot(cx);
         let selection = self.selections.newest::<usize>(cx);
+
+        // If there is an active Diagnostic Popover. Jump to it's diagnostic instead.
+        if direction == Direction::Next {
+            if let Some(popover) = self.hover_state.diagnostic_popover.as_ref() {
+                let (group_id, jump_to) = popover.activation_info();
+                self.activate_diagnostics(group_id, cx);
+                self.change_selections(Some(Autoscroll::Center), cx, |s| {
+                    let mut new_selection = s.newest_anchor().clone();
+                    new_selection.collapse_to(jump_to, SelectionGoal::None);
+                    s.select_anchors(vec![new_selection.clone()]);
+                });
+                return;
+            }
+        }
+
         let mut active_primary_range = self.active_diagnostics.as_ref().map(|active_diagnostics| {
             active_diagnostics
                 .primary_range

crates/editor/src/element.rs 🔗

@@ -41,6 +41,10 @@ use std::{
     ops::Range,
 };
 
+const MIN_POPOVER_CHARACTER_WIDTH: f32 = 20.;
+const MIN_POPOVER_LINE_HEIGHT: f32 = 4.;
+const HOVER_POPOVER_GAP: f32 = 10.;
+
 struct SelectionLayout {
     head: DisplayPoint,
     range: Range<DisplayPoint>,
@@ -268,8 +272,9 @@ impl EditorElement {
         }
 
         if paint
-            .hover_bounds
-            .map_or(false, |hover_bounds| hover_bounds.contains_point(position))
+            .hover_popover_bounds
+            .iter()
+            .any(|hover_bounds| hover_bounds.contains_point(position))
         {
             return false;
         }
@@ -600,34 +605,77 @@ impl EditorElement {
             cx.scene.pop_stacking_context();
         }
 
-        if let Some((position, hover_popover)) = layout.hover.as_mut() {
+        if let Some((position, hover_popovers)) = layout.hover_popovers.as_mut() {
             cx.scene.push_stacking_context(None);
 
             // This is safe because we check on layout whether the required row is available
             let hovered_row_layout = &layout.line_layouts[(position.row() - start_row) as usize];
-            let size = hover_popover.size();
+
+            // Minimum required size: Take the first popover, and add 1.5 times the minimum popover
+            // height. This is the size we will use to decide whether to render popovers above or below
+            // the hovered line.
+            let first_size = hover_popovers[0].size();
+            let height_to_reserve =
+                first_size.y() + 1.5 * MIN_POPOVER_LINE_HEIGHT as f32 * layout.line_height;
+
+            // Compute Hovered Point
             let x = hovered_row_layout.x_for_index(position.column() as usize) - scroll_left;
-            let y = position.row() as f32 * layout.line_height - scroll_top - size.y();
-            let mut popover_origin = content_origin + vec2f(x, y);
+            let y = position.row() as f32 * layout.line_height - scroll_top;
+            let hovered_point = content_origin + vec2f(x, y);
 
-            if popover_origin.y() < 0.0 {
-                popover_origin.set_y(popover_origin.y() + layout.line_height + size.y());
-            }
+            paint.hover_popover_bounds.clear();
 
-            let x_out_of_bounds = bounds.max_x() - (popover_origin.x() + size.x());
-            if x_out_of_bounds < 0.0 {
-                popover_origin.set_x(popover_origin.x() + x_out_of_bounds);
-            }
+            if hovered_point.y() - height_to_reserve > 0.0 {
+                // There is enough space above. Render popovers above the hovered point
+                let mut current_y = hovered_point.y();
+                for hover_popover in hover_popovers {
+                    let size = hover_popover.size();
+                    let mut popover_origin = vec2f(hovered_point.x(), current_y - size.y());
 
-            hover_popover.paint(
-                popover_origin,
-                RectF::from_points(Vector2F::zero(), vec2f(f32::MAX, f32::MAX)), // Let content bleed outside of editor
-                cx,
-            );
+                    let x_out_of_bounds = bounds.max_x() - (popover_origin.x() + size.x());
+                    if x_out_of_bounds < 0.0 {
+                        popover_origin.set_x(popover_origin.x() + x_out_of_bounds);
+                    }
 
-            paint.hover_bounds = Some(
-                RectF::new(popover_origin, hover_popover.size()).dilate(Vector2F::new(0., 5.)),
-            );
+                    hover_popover.paint(
+                        popover_origin,
+                        RectF::from_points(Vector2F::zero(), vec2f(f32::MAX, f32::MAX)), // Let content bleed outside of editor
+                        cx,
+                    );
+
+                    paint.hover_popover_bounds.push(
+                        RectF::new(popover_origin, hover_popover.size())
+                            .dilate(Vector2F::new(0., 5.)),
+                    );
+
+                    current_y = popover_origin.y() - HOVER_POPOVER_GAP;
+                }
+            } else {
+                // There is not enough space above. Render popovers below the hovered point
+                let mut current_y = hovered_point.y() + layout.line_height;
+                for hover_popover in hover_popovers {
+                    let size = hover_popover.size();
+                    let mut popover_origin = vec2f(hovered_point.x(), current_y);
+
+                    let x_out_of_bounds = bounds.max_x() - (popover_origin.x() + size.x());
+                    if x_out_of_bounds < 0.0 {
+                        popover_origin.set_x(popover_origin.x() + x_out_of_bounds);
+                    }
+
+                    hover_popover.paint(
+                        popover_origin,
+                        RectF::from_points(Vector2F::zero(), vec2f(f32::MAX, f32::MAX)), // Let content bleed outside of editor
+                        cx,
+                    );
+
+                    paint.hover_popover_bounds.push(
+                        RectF::new(popover_origin, hover_popover.size())
+                            .dilate(Vector2F::new(0., 5.)),
+                    );
+
+                    current_y = popover_origin.y() + size.y() + HOVER_POPOVER_GAP;
+                }
+            }
 
             cx.scene.pop_stacking_context();
         }
@@ -1162,6 +1210,8 @@ impl Element for EditorElement {
         });
 
         let scroll_position = snapshot.scroll_position();
+        // The scroll position is a fractional point, the whole number of which represents
+        // the top of the window in terms of display rows.
         let start_row = scroll_position.y() as u32;
         let scroll_top = scroll_position.y() * line_height;
 
@@ -1335,19 +1385,8 @@ impl Element for EditorElement {
                     .map(|indicator| (newest_selection_head.row(), indicator));
             }
 
-            hover = view.hover_state.info_popover.clone().and_then(|hover| {
-                let (point, rendered) = hover.render(&snapshot, style.clone(), cx);
-                // The scroll position is a fractional point, the whole number of which represents
-                // the top of the window in terms of display rows.
-                // Ensure the hover point is above the scroll position
-                if point.row() >= snapshot.scroll_position().y() as u32 {
-                    if line_layouts.len() > (point.row() - start_row) as usize {
-                        return Some((point, rendered));
-                    }
-                }
-
-                None
-            });
+            let visible_rows = start_row..start_row + line_layouts.len() as u32;
+            hover = view.hover_state.render(&snapshot, &style, visible_rows, cx);
         });
 
         if let Some((_, context_menu)) = context_menu.as_mut() {
@@ -1370,21 +1409,23 @@ impl Element for EditorElement {
             );
         }
 
-        if let Some((_, hover)) = hover.as_mut() {
-            hover.layout(
-                SizeConstraint {
-                    min: Vector2F::zero(),
-                    max: vec2f(
-                        (120. * em_width) // Default size
-                            .min(size.x() / 2.) // Shrink to half of the editor width
-                            .max(20. * em_width), // Apply minimum width of 20 characters
-                        (16. * line_height) // Default size
-                            .min(size.y() / 2.) // Shrink to half of the editor height
-                            .max(4. * line_height), // Apply minimum height of 4 lines
-                    ),
-                },
-                cx,
-            );
+        if let Some((_, hover_popovers)) = hover.as_mut() {
+            for hover_popover in hover_popovers.iter_mut() {
+                hover_popover.layout(
+                    SizeConstraint {
+                        min: Vector2F::zero(),
+                        max: vec2f(
+                            (120. * em_width) // Default size
+                                .min(size.x() / 2.) // Shrink to half of the editor width
+                                .max(MIN_POPOVER_CHARACTER_WIDTH * em_width), // Apply minimum width of 20 characters
+                            (16. * line_height) // Default size
+                                .min(size.y() / 2.) // Shrink to half of the editor height
+                                .max(MIN_POPOVER_LINE_HEIGHT * line_height), // Apply minimum height of 4 lines
+                        ),
+                    },
+                    cx,
+                );
+            }
         }
 
         (
@@ -1409,7 +1450,7 @@ impl Element for EditorElement {
                 selections,
                 context_menu,
                 code_actions_indicator,
-                hover,
+                hover_popovers: hover,
             },
         )
     }
@@ -1434,7 +1475,7 @@ impl Element for EditorElement {
             gutter_bounds,
             text_bounds,
             context_menu_bounds: None,
-            hover_bounds: None,
+            hover_popover_bounds: Default::default(),
         };
 
         self.paint_background(gutter_bounds, text_bounds, layout, cx);
@@ -1475,9 +1516,11 @@ impl Element for EditorElement {
             }
         }
 
-        if let Some((_, hover)) = &mut layout.hover {
-            if hover.dispatch_event(event, cx) {
-                return true;
+        if let Some((_, popover_elements)) = &mut layout.hover_popovers {
+            for popover_element in popover_elements.iter_mut() {
+                if popover_element.dispatch_event(event, cx) {
+                    return true;
+                }
             }
         }
 
@@ -1572,7 +1615,7 @@ pub struct LayoutState {
     selections: Vec<(ReplicaId, Vec<SelectionLayout>)>,
     context_menu: Option<(DisplayPoint, ElementBox)>,
     code_actions_indicator: Option<(u32, ElementBox)>,
-    hover: Option<(DisplayPoint, ElementBox)>,
+    hover_popovers: Option<(DisplayPoint, Vec<ElementBox>)>,
 }
 
 struct BlockLayout {
@@ -1617,7 +1660,7 @@ pub struct PaintState {
     gutter_bounds: RectF,
     text_bounds: RectF,
     context_menu_bounds: Option<RectF>,
-    hover_bounds: Option<RectF>,
+    hover_popover_bounds: Vec<RectF>,
 }
 
 impl PaintState {

crates/editor/src/hover_popover.rs 🔗

@@ -3,9 +3,10 @@ use gpui::{
     elements::{Flex, MouseEventHandler, Padding, Text},
     impl_internal_actions,
     platform::CursorStyle,
-    Axis, Element, ElementBox, ModelHandle, MutableAppContext, RenderContext, Task, ViewContext,
+    Axis, Element, ElementBox, ModelHandle, MouseButton, MutableAppContext, RenderContext, Task,
+    ViewContext,
 };
-use language::Bias;
+use language::{Bias, DiagnosticEntry, DiagnosticSeverity};
 use project::{HoverBlock, Project};
 use settings::Settings;
 use std::{ops::Range, time::Duration};
@@ -13,7 +14,7 @@ use util::TryFutureExt;
 
 use crate::{
     display_map::ToDisplayPoint, Anchor, AnchorRangeExt, DisplayPoint, Editor, EditorSnapshot,
-    EditorStyle,
+    EditorStyle, GoToDiagnostic, RangeToAnchorExt,
 };
 
 pub const HOVER_DELAY_MILLIS: u64 = 350;
@@ -32,21 +33,6 @@ pub fn init(cx: &mut MutableAppContext) {
     cx.add_action(hover_at);
 }
 
-#[derive(Default)]
-pub struct HoverState {
-    pub info_popover: Option<InfoPopover>,
-    pub diagnostic_popover: Option<DiagnosticPopover>,
-    pub triggered_from: Option<Anchor>,
-    pub symbol_range: Option<Range<Anchor>>,
-    pub task: Option<Task<Option<()>>>,
-}
-
-impl HoverState {
-    pub fn visible(&self) -> bool {
-        self.info_popover.is_some()
-    }
-}
-
 /// Bindable action which uses the most recent selection head to trigger a hover
 pub fn hover(editor: &mut Editor, _: &Hover, cx: &mut ViewContext<Editor>) {
     let head = editor.selections.newest_display(cx).head();
@@ -69,17 +55,11 @@ pub fn hover_at(editor: &mut Editor, action: &HoverAt, cx: &mut ViewContext<Edit
 /// Triggered by the `Hover` action when the cursor is not over a symbol or when the
 /// selections changed.
 pub fn hide_hover(editor: &mut Editor, cx: &mut ViewContext<Editor>) -> bool {
-    let mut did_hide = false;
+    let did_hide = editor.hover_state.info_popover.take().is_some()
+        | editor.hover_state.diagnostic_popover.take().is_some();
 
-    // only notify the context once
-    if editor.hover_state.info_popover.is_some() {
-        editor.hover_state.info_popover = None;
-        did_hide = true;
-        cx.notify();
-    }
-    editor.hover_state.task = None;
+    editor.hover_state.info_task = None;
     editor.hover_state.triggered_from = None;
-    editor.hover_state.symbol_range = None;
 
     editor.clear_background_highlights::<HoverState>(cx);
 
@@ -129,8 +109,8 @@ fn show_hover(
     };
 
     if !ignore_timeout {
-        if let Some(range) = &editor.hover_state.symbol_range {
-            if range
+        if let Some(InfoPopover { symbol_range, .. }) = &editor.hover_state.info_popover {
+            if symbol_range
                 .to_offset(&snapshot.buffer_snapshot)
                 .contains(&multibuffer_offset)
             {
@@ -187,10 +167,37 @@ fn show_hover(
             }
 
             // If there's a diagnostic, assign it on the hover state and notify
-            let diagnostic = snapshot
+            let local_diagnostic = snapshot
                 .buffer_snapshot
                 .diagnostics_in_range::<_, usize>(multibuffer_offset..multibuffer_offset, false)
-                .next();
+                // Find the entry with the most specific range
+                .min_by_key(|entry| entry.range.end - entry.range.start)
+                .map(|entry| DiagnosticEntry {
+                    diagnostic: entry.diagnostic,
+                    range: entry.range.to_anchors(&snapshot.buffer_snapshot),
+                });
+
+            // Pull the primary diagnostic out so we can jump to it if the popover is clicked
+            let primary_diagnostic = local_diagnostic.as_ref().and_then(|local_diagnostic| {
+                snapshot
+                    .buffer_snapshot
+                    .diagnostic_group::<usize>(local_diagnostic.diagnostic.group_id)
+                    .find(|diagnostic| diagnostic.diagnostic.is_primary)
+                    .map(|entry| DiagnosticEntry {
+                        diagnostic: entry.diagnostic,
+                        range: entry.range.to_anchors(&snapshot.buffer_snapshot),
+                    })
+            });
+
+            if let Some(this) = this.upgrade(&cx) {
+                this.update(&mut cx, |this, _| {
+                    this.hover_state.diagnostic_popover =
+                        local_diagnostic.map(|local_diagnostic| DiagnosticPopover {
+                            local_diagnostic,
+                            primary_diagnostic,
+                        });
+                });
+            }
 
             // Construct new hover popover from hover request
             let hover_popover = hover_request.await.ok().flatten().and_then(|hover_result| {
@@ -213,41 +220,28 @@ fn show_hover(
                     anchor.clone()..anchor.clone()
                 };
 
-                if let Some(this) = this.upgrade(&cx) {
-                    this.update(&mut cx, |this, _| {
-                        this.hover_state.symbol_range = Some(range.clone());
-                    });
-                }
-
                 Some(InfoPopover {
                     project: project.clone(),
-                    anchor: range.start.clone(),
+                    symbol_range: range.clone(),
                     contents: hover_result.contents,
                 })
             });
 
             if let Some(this) = this.upgrade(&cx) {
                 this.update(&mut cx, |this, cx| {
-                    if hover_popover.is_some() {
+                    if let Some(hover_popover) = hover_popover.as_ref() {
                         // Highlight the selected symbol using a background highlight
-                        if let Some(range) = this.hover_state.symbol_range.clone() {
-                            this.highlight_background::<HoverState>(
-                                vec![range],
-                                |theme| theme.editor.hover_popover.highlight,
-                                cx,
-                            );
-                        }
-                        this.hover_state.info_popover = hover_popover;
-                        cx.notify();
+                        this.highlight_background::<HoverState>(
+                            vec![hover_popover.symbol_range.clone()],
+                            |theme| theme.editor.hover_popover.highlight,
+                            cx,
+                        );
                     } else {
-                        if this.hover_state.visible() {
-                            // Popover was visible, but now is hidden. Dismiss it
-                            hide_hover(this, cx);
-                        } else {
-                            // Clear selected symbol range for future requests
-                            this.hover_state.symbol_range = None;
-                        }
+                        this.clear_background_highlights::<HoverState>(cx);
                     }
+
+                    this.hover_state.info_popover = hover_popover;
+                    cx.notify();
                 });
             }
             Ok::<_, anyhow::Error>(())
@@ -255,24 +249,70 @@ fn show_hover(
         .log_err()
     });
 
-    editor.hover_state.task = Some(task);
+    editor.hover_state.info_task = Some(task);
+}
+
+#[derive(Default)]
+pub struct HoverState {
+    pub info_popover: Option<InfoPopover>,
+    pub diagnostic_popover: Option<DiagnosticPopover>,
+    pub triggered_from: Option<Anchor>,
+    pub info_task: Option<Task<Option<()>>>,
+}
+
+impl HoverState {
+    pub fn visible(&self) -> bool {
+        self.info_popover.is_some() || self.diagnostic_popover.is_some()
+    }
+
+    pub fn render(
+        &self,
+        snapshot: &EditorSnapshot,
+        style: &EditorStyle,
+        visible_rows: Range<u32>,
+        cx: &mut RenderContext<Editor>,
+    ) -> Option<(DisplayPoint, Vec<ElementBox>)> {
+        // If there is a diagnostic, position the popovers based on that.
+        // Otherwise use the start of the hover range
+        let anchor = self
+            .diagnostic_popover
+            .as_ref()
+            .map(|diagnostic_popover| &diagnostic_popover.local_diagnostic.range.start)
+            .or_else(|| {
+                self.info_popover
+                    .as_ref()
+                    .map(|info_popover| &info_popover.symbol_range.start)
+            })?;
+        let point = anchor.to_display_point(&snapshot.display_snapshot);
+
+        // Don't render if the relevant point isn't on screen
+        if !self.visible() || !visible_rows.contains(&point.row()) {
+            return None;
+        }
+
+        let mut elements = Vec::new();
+
+        if let Some(diagnostic_popover) = self.diagnostic_popover.as_ref() {
+            elements.push(diagnostic_popover.render(style, cx));
+        }
+        if let Some(info_popover) = self.info_popover.as_ref() {
+            elements.push(info_popover.render(style, cx));
+        }
+
+        Some((point, elements))
+    }
 }
 
 #[derive(Debug, Clone)]
 pub struct InfoPopover {
     pub project: ModelHandle<Project>,
-    pub anchor: Anchor,
+    pub symbol_range: Range<Anchor>,
     pub contents: Vec<HoverBlock>,
 }
 
 impl InfoPopover {
-    pub fn render(
-        &self,
-        snapshot: &EditorSnapshot,
-        style: EditorStyle,
-        cx: &mut RenderContext<Editor>,
-    ) -> (DisplayPoint, ElementBox) {
-        let element = MouseEventHandler::new::<InfoPopover, _, _>(0, cx, |_, cx| {
+    pub fn render(&self, style: &EditorStyle, cx: &mut RenderContext<Editor>) -> ElementBox {
+        MouseEventHandler::new::<InfoPopover, _, _>(0, cx, |_, cx| {
             let mut flex = Flex::new(Axis::Vertical).scrollable::<HoverBlock, _>(1, None, cx);
             flex.extend(self.contents.iter().map(|content| {
                 let project = self.project.read(cx);
@@ -316,15 +356,63 @@ impl InfoPopover {
             top: 5.,
             ..Default::default()
         })
-        .boxed();
-
-        let display_point = self.anchor.to_display_point(&snapshot.display_snapshot);
-        (display_point, element)
+        .boxed()
     }
 }
 
 #[derive(Debug, Clone)]
-pub struct DiagnosticPopover {}
+pub struct DiagnosticPopover {
+    local_diagnostic: DiagnosticEntry<Anchor>,
+    primary_diagnostic: Option<DiagnosticEntry<Anchor>>,
+}
+
+impl DiagnosticPopover {
+    pub fn render(&self, style: &EditorStyle, cx: &mut RenderContext<Editor>) -> ElementBox {
+        enum PrimaryDiagnostic {}
+
+        let mut text_style = style.hover_popover.prose.clone();
+        text_style.font_size = style.text.font_size;
+
+        let container_style = match self.local_diagnostic.diagnostic.severity {
+            DiagnosticSeverity::HINT => style.hover_popover.info_container,
+            DiagnosticSeverity::INFORMATION => style.hover_popover.info_container,
+            DiagnosticSeverity::WARNING => style.hover_popover.warning_container,
+            DiagnosticSeverity::ERROR => style.hover_popover.error_container,
+            _ => style.hover_popover.container,
+        };
+
+        let tooltip_style = cx.global::<Settings>().theme.tooltip.clone();
+
+        MouseEventHandler::new::<DiagnosticPopover, _, _>(0, cx, |_, _| {
+            Text::new(self.local_diagnostic.diagnostic.message.clone(), text_style)
+                .with_soft_wrap(true)
+                .contained()
+                .with_style(container_style)
+                .boxed()
+        })
+        .on_click(MouseButton::Left, |_, cx| {
+            cx.dispatch_action(GoToDiagnostic)
+        })
+        .with_cursor_style(CursorStyle::PointingHand)
+        .with_tooltip::<PrimaryDiagnostic, _>(
+            0,
+            "Go To Diagnostic".to_string(),
+            Some(Box::new(crate::GoToDiagnostic)),
+            tooltip_style,
+            cx,
+        )
+        .boxed()
+    }
+
+    pub fn activation_info(&self) -> (usize, Anchor) {
+        let entry = self
+            .primary_diagnostic
+            .as_ref()
+            .unwrap_or(&self.local_diagnostic);
+
+        (entry.diagnostic.group_id, entry.range.start.clone())
+    }
+}
 
 #[cfg(test)]
 mod tests {

crates/text/src/selection.rs 🔗

@@ -54,6 +54,13 @@ impl<T: Clone> Selection<T> {
             goal: self.goal,
         }
     }
+
+    pub fn collapse_to(&mut self, point: T, new_goal: SelectionGoal) {
+        self.start = point.clone();
+        self.end = point;
+        self.goal = new_goal;
+        self.reversed = false;
+    }
 }
 
 impl<T: Copy + Ord> Selection<T> {
@@ -78,13 +85,6 @@ impl<T: Copy + Ord> Selection<T> {
         self.goal = new_goal;
     }
 
-    pub fn collapse_to(&mut self, point: T, new_goal: SelectionGoal) {
-        self.start = point;
-        self.end = point;
-        self.goal = new_goal;
-        self.reversed = false;
-    }
-
     pub fn range(&self) -> Range<T> {
         self.start..self.end
     }

crates/theme/src/theme.rs 🔗

@@ -627,6 +627,9 @@ impl<'de> Deserialize<'de> for SyntaxTheme {
 #[derive(Clone, Deserialize, Default)]
 pub struct HoverPopover {
     pub container: ContainerStyle,
+    pub info_container: ContainerStyle,
+    pub warning_container: ContainerStyle,
+    pub error_container: ContainerStyle,
     pub block_style: ContainerStyle,
     pub prose: TextStyle,
     pub highlight: Color,

crates/zed/src/menus.rs 🔗

@@ -281,7 +281,7 @@ pub fn menus() -> Vec<Menu<'static>> {
                 MenuItem::Separator,
                 MenuItem::Action {
                     name: "Next Problem",
-                    action: Box::new(editor::GoToNextDiagnostic),
+                    action: Box::new(editor::GoToDiagnostic),
                 },
                 MenuItem::Action {
                     name: "Previous Problem",

styles/src/styleTree/hoverPopover.ts 🔗

@@ -2,22 +2,48 @@ import Theme from "../themes/common/theme";
 import { backgroundColor, border, popoverShadow, text } from "./components";
 
 export default function HoverPopover(theme: Theme) {
+  let baseContainer = {
+    background: backgroundColor(theme, "on500"),
+    cornerRadius: 8,
+    padding: {
+      left: 8,
+      right: 8,
+      top: 4,
+      bottom: 4
+    },
+    shadow: popoverShadow(theme),
+    border: border(theme, "secondary"),
+    margin: {
+      left: -8,
+    },
+  };
+
   return {
-    container: {
-      background: backgroundColor(theme, "on500"),
-      cornerRadius: 8,
-      padding: {
-        left: 8,
-        right: 8,
-        top: 4,
-        bottom: 4,
+    container: baseContainer,
+    infoContainer: {
+      ...baseContainer,
+      background: backgroundColor(theme, "on500Info"),
+      border: {
+        color: theme.ramps.blue(0.2).hex(),
+        width: 1,
       },
-      shadow: popoverShadow(theme),
-      border: border(theme, "primary"),
-      margin: {
-        left: -8,
+    },
+    warningContainer: {
+      ...baseContainer,
+      background: backgroundColor(theme, "on500Warning"),
+      border: {
+        color: theme.ramps.yellow(0.2).hex(),
+        width: 1,
       },
     },
+    errorContainer: {
+      ...baseContainer,
+      background: backgroundColor(theme, "on500Error"),
+      border: {
+        color: theme.ramps.red(0.2).hex(),
+        width: 1,
+      }
+    },
     block_style: {
       padding: { top: 4 },
     },

styles/src/themes/common/base16.ts 🔗

@@ -88,16 +88,31 @@ export function createTheme(
       hovered: withOpacity(sample(ramps.red, 0.5), 0.2),
       active: withOpacity(sample(ramps.red, 0.5), 0.25),
     },
+    on500Error: {
+      base: sample(ramps.red, 0.1),
+      hovered: sample(ramps.red, 0.15),
+      active: sample(ramps.red, 0.2),
+    },
     warning: {
       base: withOpacity(sample(ramps.yellow, 0.5), 0.15),
       hovered: withOpacity(sample(ramps.yellow, 0.5), 0.2),
       active: withOpacity(sample(ramps.yellow, 0.5), 0.25),
     },
+    on500Warning: {
+      base: sample(ramps.yellow, 0.1),
+      hovered: sample(ramps.yellow, 0.15),
+      active: sample(ramps.yellow, 0.2),
+    },
     info: {
       base: withOpacity(sample(ramps.blue, 0.5), 0.15),
       hovered: withOpacity(sample(ramps.blue, 0.5), 0.2),
       active: withOpacity(sample(ramps.blue, 0.5), 0.25),
     },
+    on500Info: {
+      base: sample(ramps.blue, 0.1),
+      hovered: sample(ramps.blue, 0.15),
+      active: sample(ramps.blue, 0.2),
+    },
   };
 
   const borderColor = {
@@ -106,10 +121,10 @@ export function createTheme(
     muted: sample(ramps.neutral, isLight ? 1 : 3),
     active: sample(ramps.neutral, isLight ? 4 : 3),
     onMedia: withOpacity(darkest, 0.1),
-    ok: withOpacity(sample(ramps.green, 0.5), 0.15),
-    error: withOpacity(sample(ramps.red, 0.5), 0.15),
-    warning: withOpacity(sample(ramps.yellow, 0.5), 0.15),
-    info: withOpacity(sample(ramps.blue, 0.5), 0.15),
+    ok: sample(ramps.green, 0.3),
+    error: sample(ramps.red, 0.3),
+    warning: sample(ramps.yellow, 0.3),
+    info: sample(ramps.blue, 0.3),
   };
 
   const textColor = {

styles/src/themes/common/theme.ts 🔗

@@ -79,8 +79,11 @@ export default interface Theme {
     on500: BackgroundColorSet;
     ok: BackgroundColorSet;
     error: BackgroundColorSet;
+    on500Error: BackgroundColorSet;
     warning: BackgroundColorSet;
+    on500Warning: BackgroundColorSet;
     info: BackgroundColorSet;
+    on500Info: BackgroundColorSet;
   };
   borderColor: {
     primary: string;