Selectable diagnostic popover text (#14518)

Ephram and Conrad Irwin created

Release Notes:

- Added the ability to select and copy text from diagnostic popovers

- Fixed #12695




https://github.com/user-attachments/assets/b896bacf-ecbc-48f5-8228-a3821f0b1a4e

---------

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>

Change summary

crates/editor/src/element.rs       |  11 -
crates/editor/src/hover_popover.rs | 206 ++++++++++++++++++++-----------
crates/markdown/src/markdown.rs    |  36 +++++
crates/markdown/src/parser.rs      |  35 +++++
4 files changed, 205 insertions(+), 83 deletions(-)

Detailed changes

crates/editor/src/element.rs 🔗

@@ -2718,14 +2718,9 @@ impl EditorElement {
         );
 
         let hover_popovers = self.editor.update(cx, |editor, cx| {
-            editor.hover_state.render(
-                &snapshot,
-                &self.style,
-                visible_display_row_range.clone(),
-                max_size,
-                editor.workspace.as_ref().map(|(w, _)| w.clone()),
-                cx,
-            )
+            editor
+                .hover_state
+                .render(&snapshot, visible_display_row_range.clone(), max_size, cx)
         });
         let Some((position, hover_popovers)) = hover_popovers else {
             return;

crates/editor/src/hover_popover.rs 🔗

@@ -3,13 +3,12 @@ use crate::{
     hover_links::{InlayHighlight, RangeInEditor},
     scroll::ScrollAmount,
     Anchor, AnchorRangeExt, DisplayPoint, DisplayRow, Editor, EditorSettings, EditorSnapshot,
-    EditorStyle, Hover, RangeToAnchorExt,
+    Hover, RangeToAnchorExt,
 };
 use gpui::{
-    div, px, AnyElement, AsyncWindowContext, CursorStyle, FontWeight, Hsla, InteractiveElement,
-    IntoElement, MouseButton, ParentElement, Pixels, ScrollHandle, SharedString, Size,
-    StatefulInteractiveElement, StyleRefinement, Styled, Task, TextStyleRefinement, View,
-    ViewContext, WeakView,
+    div, px, AnyElement, AsyncWindowContext, FontWeight, Hsla, InteractiveElement, IntoElement,
+    MouseButton, ParentElement, Pixels, ScrollHandle, Size, StatefulInteractiveElement,
+    StyleRefinement, Styled, Task, TextStyleRefinement, View, ViewContext,
 };
 use itertools::Itertools;
 use language::{DiagnosticEntry, Language, LanguageRegistry};
@@ -22,9 +21,8 @@ use std::rc::Rc;
 use std::{borrow::Cow, cell::RefCell};
 use std::{ops::Range, sync::Arc, time::Duration};
 use theme::ThemeSettings;
-use ui::{prelude::*, window_is_transparent, Tooltip};
+use ui::{prelude::*, window_is_transparent};
 use util::TryFutureExt;
-use workspace::Workspace;
 pub const HOVER_DELAY_MILLIS: u64 = 350;
 pub const HOVER_REQUEST_DELAY_MILLIS: u64 = 200;
 
@@ -64,6 +62,18 @@ pub fn show_keyboard_hover(editor: &mut Editor, cx: &mut ViewContext<Editor>) ->
             }
         }
     }
+
+    let diagnostic_popover = editor.hover_state.diagnostic_popover.clone();
+    if let Some(d) = diagnostic_popover {
+        let keyboard_grace = d.keyboard_grace.borrow();
+        if *keyboard_grace {
+            if let Some(anchor) = d.anchor {
+                show_hover(editor, anchor, false, cx);
+                return true;
+            }
+        }
+    }
+
     return false;
 }
 
@@ -215,6 +225,7 @@ fn show_hover(
     if !ignore_timeout {
         if same_info_hover(editor, &snapshot, anchor)
             || same_diagnostic_hover(editor, &snapshot, anchor)
+            || editor.hover_state.diagnostic_popover.is_some()
         {
             // Hover triggered from same location as last time. Don't show again.
             return;
@@ -285,12 +296,85 @@ fn show_hover(
                     })
             });
 
+            let diagnostic_popover = if let Some(local_diagnostic) = local_diagnostic {
+                let text = match local_diagnostic.diagnostic.source {
+                    Some(ref source) => {
+                        format!("{source}: {}", local_diagnostic.diagnostic.message)
+                    }
+                    None => local_diagnostic.diagnostic.message.clone(),
+                };
+
+                let mut border_color: Option<Hsla> = None;
+                let mut background_color: Option<Hsla> = None;
+
+                let parsed_content = cx
+                    .new_view(|cx| {
+                        let status_colors = cx.theme().status();
+
+                        match local_diagnostic.diagnostic.severity {
+                            DiagnosticSeverity::ERROR => {
+                                background_color = Some(status_colors.error_background);
+                                border_color = Some(status_colors.error_border);
+                            }
+                            DiagnosticSeverity::WARNING => {
+                                background_color = Some(status_colors.warning_background);
+                                border_color = Some(status_colors.warning_border);
+                            }
+                            DiagnosticSeverity::INFORMATION => {
+                                background_color = Some(status_colors.info_background);
+                                border_color = Some(status_colors.info_border);
+                            }
+                            DiagnosticSeverity::HINT => {
+                                background_color = Some(status_colors.hint_background);
+                                border_color = Some(status_colors.hint_border);
+                            }
+                            _ => {
+                                background_color = Some(status_colors.ignored_background);
+                                border_color = Some(status_colors.ignored_border);
+                            }
+                        };
+                        let settings = ThemeSettings::get_global(cx);
+                        let mut base_text_style = cx.text_style();
+                        base_text_style.refine(&TextStyleRefinement {
+                            font_family: Some(settings.ui_font.family.clone()),
+                            font_size: Some(settings.ui_font_size.into()),
+                            color: Some(cx.theme().colors().editor_foreground),
+                            background_color: Some(gpui::transparent_black()),
+
+                            ..Default::default()
+                        });
+                        let markdown_style = MarkdownStyle {
+                            base_text_style,
+                            selection_background_color: { cx.theme().players().local().selection },
+                            link: TextStyleRefinement {
+                                underline: Some(gpui::UnderlineStyle {
+                                    thickness: px(1.),
+                                    color: Some(cx.theme().colors().editor_foreground),
+                                    wavy: false,
+                                }),
+                                ..Default::default()
+                            },
+                            ..Default::default()
+                        };
+                        Markdown::new_text(text, markdown_style.clone(), None, cx, None)
+                    })
+                    .ok();
+
+                Some(DiagnosticPopover {
+                    local_diagnostic,
+                    primary_diagnostic,
+                    parsed_content,
+                    border_color,
+                    background_color,
+                    keyboard_grace: Rc::new(RefCell::new(ignore_timeout)),
+                    anchor: Some(anchor),
+                })
+            } else {
+                None
+            };
+
             this.update(&mut cx, |this, _| {
-                this.hover_state.diagnostic_popover =
-                    local_diagnostic.map(|local_diagnostic| DiagnosticPopover {
-                        local_diagnostic,
-                        primary_diagnostic,
-                    });
+                this.hover_state.diagnostic_popover = diagnostic_popover;
             })?;
 
             let hovers_response = hover_request.await;
@@ -495,10 +579,8 @@ impl HoverState {
     pub fn render(
         &mut self,
         snapshot: &EditorSnapshot,
-        style: &EditorStyle,
         visible_rows: Range<DisplayRow>,
         max_size: Size<Pixels>,
-        _workspace: Option<WeakView<Workspace>>,
         cx: &mut ViewContext<Editor>,
     ) -> Option<(DisplayPoint, Vec<AnyElement>)> {
         // If there is a diagnostic, position the popovers based on that.
@@ -533,7 +615,7 @@ impl HoverState {
         let mut elements = Vec::new();
 
         if let Some(diagnostic_popover) = self.diagnostic_popover.as_ref() {
-            elements.push(diagnostic_popover.render(style, max_size, cx));
+            elements.push(diagnostic_popover.render(max_size, cx));
         }
         for info_popover in &mut self.info_popovers {
             elements.push(info_popover.render(max_size, cx));
@@ -551,6 +633,13 @@ impl HoverState {
                 }
             }
         }
+        if let Some(diagnostic_popover) = &self.diagnostic_popover {
+            if let Some(markdown_view) = &diagnostic_popover.parsed_content {
+                if markdown_view.focus_handle(cx).is_focused(cx) {
+                    hover_popover_is_focused = true;
+                }
+            }
+        }
         return hover_popover_is_focused;
     }
 }
@@ -606,84 +695,55 @@ impl InfoPopover {
 pub struct DiagnosticPopover {
     local_diagnostic: DiagnosticEntry<Anchor>,
     primary_diagnostic: Option<DiagnosticEntry<Anchor>>,
+    parsed_content: Option<View<Markdown>>,
+    border_color: Option<Hsla>,
+    background_color: Option<Hsla>,
+    pub keyboard_grace: Rc<RefCell<bool>>,
+    pub anchor: Option<Anchor>,
 }
 
 impl DiagnosticPopover {
-    pub fn render(
-        &self,
-        style: &EditorStyle,
-        max_size: Size<Pixels>,
-        cx: &mut ViewContext<Editor>,
-    ) -> AnyElement {
-        let text = match &self.local_diagnostic.diagnostic.source {
-            Some(source) => format!("{source}: {}", self.local_diagnostic.diagnostic.message),
-            None => self.local_diagnostic.diagnostic.message.clone(),
-        };
-
-        let status_colors = cx.theme().status();
+    pub fn render(&self, max_size: Size<Pixels>, cx: &mut ViewContext<Editor>) -> AnyElement {
+        let keyboard_grace = Rc::clone(&self.keyboard_grace);
+        let mut markdown_div = div().py_1().px_2();
+        if let Some(markdown) = &self.parsed_content {
+            markdown_div = markdown_div.child(markdown.clone());
+        }
 
-        struct DiagnosticColors {
-            pub background: Hsla,
-            pub border: Hsla,
+        if let Some(background_color) = &self.background_color {
+            markdown_div = markdown_div.bg(*background_color);
         }
 
-        let diagnostic_colors = match self.local_diagnostic.diagnostic.severity {
-            DiagnosticSeverity::ERROR => DiagnosticColors {
-                background: status_colors.error_background,
-                border: status_colors.error_border,
-            },
-            DiagnosticSeverity::WARNING => DiagnosticColors {
-                background: status_colors.warning_background,
-                border: status_colors.warning_border,
-            },
-            DiagnosticSeverity::INFORMATION => DiagnosticColors {
-                background: status_colors.info_background,
-                border: status_colors.info_border,
-            },
-            DiagnosticSeverity::HINT => DiagnosticColors {
-                background: status_colors.hint_background,
-                border: status_colors.hint_border,
-            },
-            _ => DiagnosticColors {
-                background: status_colors.ignored_background,
-                border: status_colors.ignored_border,
-            },
-        };
+        if let Some(border_color) = &self.border_color {
+            markdown_div = markdown_div
+                .border_1()
+                .border_color(*border_color)
+                .rounded_lg();
+        }
 
-        div()
+        let diagnostic_div = div()
             .id("diagnostic")
             .block()
+            .max_h(max_size.height)
             .elevation_2_borderless(cx)
             // Don't draw the background color if the theme
             // allows transparent surfaces.
             .when(window_is_transparent(cx), |this| {
                 this.bg(gpui::transparent_black())
             })
-            .cursor(CursorStyle::PointingHand)
-            .tooltip(move |cx| Tooltip::for_action("Go To Diagnostic", &crate::GoToDiagnostic, cx))
             // Prevent a mouse move on the popover from being propagated to the editor,
             // because that would dismiss the popover.
             .on_mouse_move(|_, cx| cx.stop_propagation())
             // Prevent a mouse down on the popover from being propagated to the editor,
             // because that would move the cursor.
-            .on_mouse_down(MouseButton::Left, |_, cx| cx.stop_propagation())
-            .on_click(cx.listener(|editor, _, cx| editor.go_to_diagnostic(&Default::default(), cx)))
-            .child(
-                div()
-                    .id("diagnostic-inner")
-                    .overflow_y_scroll()
-                    .max_w(max_size.width)
-                    .max_h(max_size.height)
-                    .px_2()
-                    .py_1()
-                    .bg(diagnostic_colors.background)
-                    .text_color(style.text.color)
-                    .border_1()
-                    .border_color(diagnostic_colors.border)
-                    .rounded_lg()
-                    .child(SharedString::from(text)),
-            )
-            .into_any_element()
+            .on_mouse_down(MouseButton::Left, move |_, cx| {
+                let mut keyboard_grace = keyboard_grace.borrow_mut();
+                *keyboard_grace = false;
+                cx.stop_propagation();
+            })
+            .child(markdown_div);
+
+        diagnostic_div.into_any_element()
     }
 
     pub fn activation_info(&self) -> (usize, Anchor) {

crates/markdown/src/markdown.rs 🔗

@@ -10,7 +10,7 @@ use gpui::{
     TextStyle, TextStyleRefinement, View,
 };
 use language::{Language, LanguageRegistry, Rope};
-use parser::{parse_markdown, MarkdownEvent, MarkdownTag, MarkdownTagEnd};
+use parser::{parse_links_only, parse_markdown, MarkdownEvent, MarkdownTag, MarkdownTagEnd};
 
 use std::{iter, mem, ops::Range, rc::Rc, sync::Arc};
 use theme::SyntaxTheme;
@@ -61,6 +61,7 @@ pub struct Markdown {
     focus_handle: FocusHandle,
     language_registry: Option<Arc<LanguageRegistry>>,
     fallback_code_block_language: Option<String>,
+    parse_links_only: bool,
 }
 
 actions!(markdown, [Copy]);
@@ -86,6 +87,33 @@ impl Markdown {
             focus_handle,
             language_registry,
             fallback_code_block_language,
+            parse_links_only: false,
+        };
+        this.parse(cx);
+        this
+    }
+
+    pub fn new_text(
+        source: String,
+        style: MarkdownStyle,
+        language_registry: Option<Arc<LanguageRegistry>>,
+        cx: &mut ViewContext<Self>,
+        fallback_code_block_language: Option<String>,
+    ) -> Self {
+        let focus_handle = cx.focus_handle();
+        let mut this = Self {
+            source,
+            selection: Selection::default(),
+            pressed_link: None,
+            autoscroll_request: None,
+            style,
+            should_reparse: false,
+            parsed_markdown: ParsedMarkdown::default(),
+            pending_parse: None,
+            focus_handle,
+            language_registry,
+            fallback_code_block_language,
+            parse_links_only: true,
         };
         this.parse(cx);
         this
@@ -136,9 +164,13 @@ impl Markdown {
         }
 
         let text = self.source.clone();
+        let parse_text_only = self.parse_links_only;
         let parsed = cx.background_executor().spawn(async move {
             let text = SharedString::from(text);
-            let events = Arc::from(parse_markdown(text.as_ref()));
+            let events = match parse_text_only {
+                true => Arc::from(parse_links_only(text.as_ref())),
+                false => Arc::from(parse_markdown(text.as_ref())),
+            };
             anyhow::Ok(ParsedMarkdown {
                 source: text,
                 events,

crates/markdown/src/parser.rs 🔗

@@ -88,6 +88,41 @@ pub fn parse_markdown(text: &str) -> Vec<(Range<usize>, MarkdownEvent)> {
     events
 }
 
+pub fn parse_links_only(text: &str) -> Vec<(Range<usize>, MarkdownEvent)> {
+    let mut events = Vec::new();
+    let mut finder = LinkFinder::new();
+    finder.kinds(&[linkify::LinkKind::Url]);
+    let mut text_range = Range {
+        start: 0,
+        end: text.len(),
+    };
+    for link in finder.links(&text[text_range.clone()]) {
+        let link_range = text_range.start + link.start()..text_range.start + link.end();
+
+        if link_range.start > text_range.start {
+            events.push((text_range.start..link_range.start, MarkdownEvent::Text));
+        }
+
+        events.push((
+            link_range.clone(),
+            MarkdownEvent::Start(MarkdownTag::Link {
+                link_type: LinkType::Autolink,
+                dest_url: SharedString::from(link.as_str().to_string()),
+                title: SharedString::default(),
+                id: SharedString::default(),
+            }),
+        ));
+        events.push((link_range.clone(), MarkdownEvent::Text));
+        events.push((link_range.clone(), MarkdownEvent::End(MarkdownTagEnd::Link)));
+
+        text_range.start = link_range.end;
+    }
+
+    events.push((text_range, MarkdownEvent::Text));
+
+    events
+}
+
 /// A static-lifetime equivalent of pulldown_cmark::Event so we can cache the
 /// parse result for rendering without resorting to unsafe lifetime coercion.
 #[derive(Clone, Debug, PartialEq)]