From bde7e55adb52bc1fda4b899629233047cfa09f85 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 29 Oct 2025 13:18:34 +0100 Subject: [PATCH] editor: Render diagnostic popover even if the source is out of view (#41449) 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 --- crates/editor/src/element.rs | 1 + crates/editor/src/hover_popover.rs | 31 ++++++++++++++++++++++++++---- crates/gpui/src/executor.rs | 5 +++-- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 1fcc90a863..101b424e4e 100644 --- a/crates/editor/src/element.rs +++ b/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, ) diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index e6eb5c1ea2..6227d90e9b 100644 --- a/crates/editor/src/hover_popover.rs +++ b/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, max_size: Size, + text_layout_details: &TextLayoutDetails, window: &mut Window, cx: &mut Context, ) -> Option<(DisplayPoint, Vec)> { + 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); + let mut 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; + // 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(); diff --git a/crates/gpui/src/executor.rs b/crates/gpui/src/executor.rs index 644bee6b8d..b820e120dd 100644 --- a/crates/gpui/src/executor.rs +++ b/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") } } }