editor: Render diagnostic popover even if the source is out of view (#41449)

Lukas Wirth created 2 months ago

This happens quite often with cargo based diagnostics which may spawn
several lines (sometimes the entire screen), forcing the user to scroll
up to the start of the diagnostic just to see the hover message is not
great.

Release Notes:

- Fixed diagnostics hovers not working if the diagnostic spans out of
view

Change summary

crates/editor/src/element.rs       |  1 
crates/editor/src/hover_popover.rs | 33 +++++++++++++++++++++++++++----
crates/gpui/src/executor.rs        |  5 ++-
3 files changed, 32 insertions(+), 7 deletions(-)

Detailed changes

crates/editor/src/element.rs

@@ -5107,6 +5107,7 @@ impl EditorElement {
                 snapshot,
                 visible_display_row_range.clone(),
                 max_size,
+                &editor.text_layout_details(window),
                 window,
                 cx,
             )

crates/editor/src/hover_popover.rs

@@ -3,6 +3,7 @@ use crate::{
     EditorSnapshot, GlobalDiagnosticRenderer, Hover,
     display_map::{InlayOffset, ToDisplayPoint, invisibles::is_invisible},
     hover_links::{InlayHighlight, RangeInEditor},
+    movement::TextLayoutDetails,
     scroll::ScrollAmount,
 };
 use anyhow::Context as _;
@@ -766,9 +767,13 @@ impl HoverState {
         snapshot: &EditorSnapshot,
         visible_rows: Range<DisplayRow>,
         max_size: Size<Pixels>,
+        text_layout_details: &TextLayoutDetails,
         window: &mut Window,
         cx: &mut Context<Editor>,
     ) -> Option<(DisplayPoint, Vec<AnyElement>)> {
+        if !self.visible() {
+            return None;
+        }
         // If there is a diagnostic, position the popovers based on that.
         // Otherwise use the start of the hover range
         let anchor = self
@@ -791,11 +796,29 @@ impl HoverState {
                     }
                 })
             })?;
-        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 point = anchor.to_display_point(&snapshot.display_snapshot);
+
+        // Clamp the point within the visible rows in case the popup source spans multiple lines
+        if point.row() < visible_rows.start {
+            point = crate::movement::down_by_rows(
+                &snapshot.display_snapshot,
+                point,
+                (visible_rows.start - point.row()).0,
+                text::SelectionGoal::None,
+                true,
+                text_layout_details,
+            )
+            .0;
+        } else if visible_rows.end <= point.row() {
+            point = crate::movement::up_by_rows(
+                &snapshot.display_snapshot,
+                point,
+                (visible_rows.end - point.row()).0,
+                text::SelectionGoal::None,
+                true,
+                text_layout_details,
+            )
+            .0;
         }
 
         let mut elements = Vec::new();

crates/gpui/src/executor.rs

@@ -281,7 +281,8 @@ impl BackgroundExecutor {
         });
         let mut cx = std::task::Context::from_waker(&waker);
 
-        let mut test_should_end_by = Instant::now() + Duration::from_secs(500);
+        let duration = Duration::from_secs(500);
+        let mut test_should_end_by = Instant::now() + duration;
 
         loop {
             match future.as_mut().poll(&mut cx) {
@@ -319,7 +320,7 @@ impl BackgroundExecutor {
                             test_should_end_by.saturating_duration_since(Instant::now()),
                         );
                         if Instant::now() > test_should_end_by {
-                            panic!("test timed out with allow_parking")
+                            panic!("test timed out after {duration:?} with allow_parking")
                         }
                     }
                 }