From 9ea9b046342f68d9915d86b614b91ef4b7ca222a Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Wed, 6 May 2026 13:31:59 +0900 Subject: [PATCH 1/3] editor: Fix inlay-hint hover targeting and popover position MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When two type hints sit one source character apart (e.g. a `::T` hint just before a closing token and another `::U` hint right after it), `clip_point(_, Bias::Right)` skips through the lone source character so both hints fall inside the buffer-position window used to find a hovered hint. The previous `max_by_key(|hint| hint.id)` heuristic then picked an arbitrary one, which often was not the inlay actually under the mouse. Filter the candidate hints by the actual mouse `hovered_offset` so the hint whose rendered text is under the cursor is the one resolved for hover and link navigation. Additionally, the popover previously used the inlay's buffer anchor directly, which in display space always lands at the inlay's leading edge — so for long or multi-part inlays the popover would appear at the start of the inlay regardless of which character was hovered. Resolve the actual display position by adding the in-inlay byte offset onto the inlay's start. --- crates/editor/src/display_map.rs | 32 ++- crates/editor/src/hover_popover.rs | 267 +++++++++++++++++++++--- crates/editor/src/inlays/inlay_hints.rs | 16 +- 3 files changed, 275 insertions(+), 40 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index 143963a7f5e..32918c9e7e4 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -1668,19 +1668,11 @@ impl DisplaySnapshot { inlay_snapshot .buffer_offset_to_inlay_ranges(range) .map(|inlay_range| { - let inlay_point_to_display_point = |inlay_point: InlayPoint, bias: Bias| { - let fold_point = self.fold_snapshot().to_fold_point(inlay_point, bias); - let tab_point = self.tab_snapshot().fold_point_to_tab_point(fold_point); - let wrap_point = self.wrap_snapshot().tab_point_to_wrap_point(tab_point); - let block_point = self.block_snapshot.to_block_point(wrap_point); - DisplayPoint(block_point) - }; - - let start = inlay_point_to_display_point( + let start = self.inlay_point_to_display_point( inlay_snapshot.to_point(inlay_range.start), Bias::Left, ); - let end = inlay_point_to_display_point( + let end = self.inlay_point_to_display_point( inlay_snapshot.to_point(inlay_range.end), Bias::Left, ); @@ -1704,6 +1696,26 @@ impl DisplaySnapshot { .to_inlay_offset(anchor.to_offset(self.buffer_snapshot())) } + pub fn inlay_point_to_display_point( + &self, + inlay_point: InlayPoint, + bias: Bias, + ) -> DisplayPoint { + let fold_point = self.fold_snapshot().to_fold_point(inlay_point, bias); + let tab_point = self.tab_snapshot().fold_point_to_tab_point(fold_point); + let wrap_point = self.wrap_snapshot().tab_point_to_wrap_point(tab_point); + let block_point = self.block_snapshot.to_block_point(wrap_point); + DisplayPoint(block_point) + } + + pub fn inlay_offset_to_display_point( + &self, + inlay_offset: InlayOffset, + bias: Bias, + ) -> DisplayPoint { + self.inlay_point_to_display_point(self.inlay_snapshot().to_point(inlay_offset), bias) + } + pub fn display_point_to_anchor(&self, point: DisplayPoint, bias: Bias) -> Anchor { self.buffer_snapshot() .anchor_at(point.to_offset(self, bias), bias) diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index cfee889ac27..316b04508cb 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -27,6 +27,7 @@ use std::{ }; use std::{ops::Range, sync::Arc, time::Duration}; use std::{path::PathBuf, rc::Rc}; +use text::Bias; use theme_settings::ThemeSettings; use ui::{CopyButton, Scrollbars, WithScrollbar, prelude::*, theme_is_transparent}; use url::Url; @@ -882,6 +883,48 @@ impl HoverState { px((dx.powi(2) + dy.powi(2)).sqrt()) } + /// Resolves the display point that the active popover should anchor to. + /// + /// Diagnostic and text-symbol hovers anchor at the start of the hover range. + /// For inlay-hint hovers, however, the buffer anchor alone always maps to the + /// inlay's leading edge in display space, so the popover would pin to the start + /// of the inlay regardless of which character was hovered. Add the in-inlay + /// byte offset onto the inlay's start so the popover lands at the hovered position. + pub(crate) fn popover_anchor_display_point( + &self, + snapshot: &EditorSnapshot, + ) -> Option { + let display_snapshot = &snapshot.display_snapshot; + if let Some(diagnostic_popover) = self.diagnostic_popover.as_ref() { + return Some( + diagnostic_popover + .local_diagnostic + .range + .start + .to_display_point(display_snapshot), + ); + } + if let Some(text_anchor) = + self.info_popovers + .iter() + .find_map(|info_popover| match &info_popover.symbol_range { + RangeInEditor::Text(range) => Some(&range.start), + RangeInEditor::Inlay(_) => None, + }) + { + return Some(text_anchor.to_display_point(display_snapshot)); + } + let highlight = self.info_popovers.iter().find_map(|info_popover| { + match &info_popover.symbol_range { + RangeInEditor::Text(_) => None, + RangeInEditor::Inlay(range) => Some(range), + } + })?; + let hint_start = display_snapshot.anchor_to_inlay_offset(highlight.inlay_position); + let hovered_offset = InlayOffset(hint_start.0 + highlight.range.start); + Some(display_snapshot.inlay_offset_to_display_point(hovered_offset, Bias::Left)) + } + pub(crate) fn render( &mut self, snapshot: &EditorSnapshot, @@ -894,29 +937,7 @@ impl HoverState { 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 - .diagnostic_popover - .as_ref() - .map(|diagnostic_popover| &diagnostic_popover.local_diagnostic.range.start) - .or_else(|| { - self.info_popovers.iter().find_map(|info_popover| { - match &info_popover.symbol_range { - RangeInEditor::Text(range) => Some(&range.start), - RangeInEditor::Inlay(_) => None, - } - }) - }) - .or_else(|| { - self.info_popovers.iter().find_map(|info_popover| { - match &info_popover.symbol_range { - RangeInEditor::Text(_) => None, - RangeInEditor::Inlay(range) => Some(&range.inlay_position), - } - }) - })?; - let mut point = anchor.to_display_point(&snapshot.display_snapshot); + let mut point = self.popover_anchor_display_point(snapshot)?; // Clamp the point within the visible rows in case the popup source spans multiple lines if visible_rows.end <= point.row() { point = crate::movement::up_by_rows( @@ -2038,7 +2059,7 @@ mod tests { cx.background_executor .advance_clock(Duration::from_millis(get_hover_popover_delay(&cx) + 100)); cx.background_executor.run_until_parked(); - cx.update_editor(|editor, _, cx| { + cx.update_editor(|editor, window, cx| { let hover_state = &editor.hover_state; assert!( hover_state.diagnostic_popover.is_none() && hover_state.info_popovers.len() == 1 @@ -2059,6 +2080,20 @@ mod tests { popover.get_rendered_text(cx), format!("A tooltip for {new_type_label}"), ); + + let snapshot = editor.snapshot(window, cx); + let popover_point = hover_state + .popover_anchor_display_point(&snapshot) + .expect("popover should resolve to a display point"); + let inlay_start_display = + MultiBufferOffset(inlay_range.start).to_display_point(&snapshot); + assert_eq!(popover_point.row(), inlay_start_display.row()); + assert_eq!( + popover_point.column(), + inlay_start_display.column() + ": ".len() as u32, + "Popover should anchor at the start of the hovered label part inside the inlay, \ + not at the start of the inlay", + ); }); let struct_hint_part_hover_position = cx.update_editor(|editor, window, cx| { @@ -2095,21 +2130,21 @@ mod tests { cx.background_executor .advance_clock(Duration::from_millis(get_hover_popover_delay(&cx) + 100)); cx.background_executor.run_until_parked(); - cx.update_editor(|editor, _, cx| { + cx.update_editor(|editor, window, cx| { let hover_state = &editor.hover_state; assert!( hover_state.diagnostic_popover.is_none() && hover_state.info_popovers.len() == 1 ); let popover = hover_state.info_popovers.first().unwrap(); let buffer_snapshot = editor.buffer().update(cx, |buffer, cx| buffer.snapshot(cx)); + let struct_part_offset = ": ".len() + new_type_label.len() + "<".len(); assert_eq!( popover.symbol_range, RangeInEditor::Inlay(InlayHighlight { inlay: InlayId::Hint(0), inlay_position: buffer_snapshot .anchor_after(MultiBufferOffset(inlay_range.start)), - range: ": ".len() + new_type_label.len() + "<".len() - ..": ".len() + new_type_label.len() + "<".len() + struct_label.len(), + range: struct_part_offset..struct_part_offset + struct_label.len(), }), "Popover range should match the struct label part" ); @@ -2118,6 +2153,184 @@ mod tests { format!("A tooltip for {struct_label}"), "Rendered markdown element should remove backticks from text" ); + + let snapshot = editor.snapshot(window, cx); + let popover_point = hover_state + .popover_anchor_display_point(&snapshot) + .expect("popover should resolve to a display point"); + let inlay_start_display = + MultiBufferOffset(inlay_range.start).to_display_point(&snapshot); + assert_eq!(popover_point.row(), inlay_start_display.row()); + assert_eq!( + popover_point.column(), + inlay_start_display.column() + struct_part_offset as u32, + "Popover should anchor at the start of the hovered label part inside the inlay, \ + not at the start of the inlay", + ); + }); + } + + /// When two type hints sit one source character apart (e.g. a `::T` hint + /// just before a closing token and another `::U` hint right after it), + /// `clip_point` skips through the lone source character with `Bias::Right`, + /// so both hints fall inside the buffer-position window used to find a + /// hovered hint. The hovered-offset filter must then disambiguate them by + /// the inlay text actually under the mouse. + #[gpui::test] + async fn test_hover_inlay_picks_inlay_under_cursor(cx: &mut gpui::TestAppContext) { + init_test(cx, |settings| { + settings.defaults.inlay_hints = Some(InlayHintSettingsContent { + show_value_hints: Some(true), + enabled: Some(true), + edit_debounce_ms: Some(0), + scroll_debounce_ms: Some(0), + show_type_hints: Some(true), + show_parameter_hints: Some(false), + show_other_hints: Some(false), + show_background: Some(false), + toggle_on_modifiers_press: None, + }) + }); + + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + inlay_hint_provider: Some(lsp::OneOf::Right( + lsp::InlayHintServerCapabilities::Options(lsp::InlayHintOptions { + resolve_provider: Some(false), + ..Default::default() + }), + )), + ..Default::default() + }, + cx, + ) + .await; + + cx.set_state(indoc! {" + fn main() { + let xˇ = a; + } + "}); + + // Anchor positions for the two adjacent type hints. The first hint is + // anchored right before `;` and the second right after it, so they end + // up rendered as `a::Foo;::Bar` with only `;` between them in display. + let first_hint_offset = cx.ranges(indoc! {" + fn main() { + let x = a«»; + } + "})[0] + .start; + let second_hint_offset = cx.ranges(indoc! {" + fn main() { + let x = a;«» + } + "})[0] + .start; + let first_position = cx.to_lsp(MultiBufferOffset(first_hint_offset)); + let second_position = cx.to_lsp(MultiBufferOffset(second_hint_offset)); + + let first_label = "::Foo"; + let second_label = "::Bar"; + cx.lsp + .set_request_handler::(move |_, _| async move { + Ok(Some(vec![ + lsp::InlayHint { + position: first_position, + label: lsp::InlayHintLabel::String(first_label.to_string()), + kind: Some(lsp::InlayHintKind::TYPE), + text_edits: None, + tooltip: Some(lsp::InlayHintTooltip::String("Foo".to_string())), + padding_left: Some(false), + padding_right: Some(false), + data: None, + }, + lsp::InlayHint { + position: second_position, + label: lsp::InlayHintLabel::String(second_label.to_string()), + kind: Some(lsp::InlayHintKind::TYPE), + text_edits: None, + tooltip: Some(lsp::InlayHintTooltip::String("Bar".to_string())), + padding_left: Some(false), + padding_right: Some(false), + data: None, + }, + ])) + }) + .next() + .await; + cx.background_executor.run_until_parked(); + cx.update_editor(|editor, _, cx| { + assert_eq!( + vec![first_label.to_string(), second_label.to_string()], + visible_hint_labels(editor, cx), + ); + }); + + // Hover in the middle of the first inlay's rendered text. + let hover_position = cx.update_editor(|editor, window, cx| { + let snapshot = editor.snapshot(window, cx); + let inlay_start = MultiBufferOffset(first_hint_offset).to_display_point(&snapshot); + let exact_unclipped = DisplayPoint::new( + inlay_start.row(), + inlay_start.column() + (first_label.len() as u32 / 2), + ); + let previous_valid = snapshot.clip_point(exact_unclipped, Bias::Left); + let next_valid = snapshot.clip_point(exact_unclipped, Bias::Right); + let nearest_valid = if previous_valid == next_valid { + previous_valid + } else { + match snapshot.inlay_bias_at(exact_unclipped) { + Some(Bias::Left) => next_valid, + Some(Bias::Right) => previous_valid, + None => previous_valid, + } + }; + PointForPosition { + previous_valid, + next_valid, + nearest_valid, + exact_unclipped, + column_overshoot_after_line_end: 0, + } + }); + cx.update_editor(|editor, window, cx| { + editor.update_inlay_link_and_hover_points( + &editor.snapshot(window, cx), + hover_position, + None, + true, + false, + window, + cx, + ); + }); + cx.background_executor + .advance_clock(Duration::from_millis(get_hover_popover_delay(&cx) + 100)); + cx.background_executor.run_until_parked(); + + cx.update_editor(|editor, _, cx| { + let hover_state = &editor.hover_state; + assert_eq!( + hover_state.info_popovers.len(), + 1, + "expected exactly one popover", + ); + let popover = hover_state.info_popovers.first().unwrap(); + assert_eq!( + popover.get_rendered_text(cx), + "Foo", + "popover content should match the first inlay (the one under the cursor), \ + not the adjacent inlay one source character past it", + ); + let RangeInEditor::Inlay(highlight) = &popover.symbol_range else { + panic!("expected an inlay popover, got {:?}", popover.symbol_range); + }; + assert_eq!( + highlight.inlay, + InlayId::Hint(0), + "popover should reference the first inlay", + ); }); } diff --git a/crates/editor/src/inlays/inlay_hints.rs b/crates/editor/src/inlays/inlay_hints.rs index ac3133ea89c..117f731a09b 100644 --- a/crates/editor/src/inlays/inlay_hints.rs +++ b/crates/editor/src/inlays/inlay_hints.rs @@ -604,7 +604,13 @@ impl Editor { point_for_position.next_valid.to_point(snapshot), Bias::Right, ); - if let Some(hovered_hint) = Self::visible_inlay_hints(self.display_map.read(cx)) + // The buffer-position window can include neighbouring inlays + // (e.g. a `::Type` hint after a closing paren and another hint just + // past it both pass the take_while when `clip_point` skips through + // a one-character source span between them). Use the actual mouse + // `hovered_offset` to pick the inlay whose rendered text is under + // the cursor. + let hovered_hint = Self::visible_inlay_hints(self.display_map.read(cx)) .filter(|hint| snapshot.can_resolve(&hint.position)) .skip_while(|hint| { hint.position @@ -616,8 +622,12 @@ impl Editor { .cmp(&next_valid_anchor, &buffer_snapshot) .is_le() }) - .max_by_key(|hint| hint.id) - { + .find(|hint| { + let hint_start = snapshot.anchor_to_inlay_offset(hint.position); + let hint_end = InlayOffset(hint_start.0 + hint.text().len()); + hovered_offset >= hint_start && hovered_offset < hint_end + }); + if let Some(hovered_hint) = hovered_hint { if let Some(ResolvedHint::Resolved(cached_hint)) = buffer_snapshot .anchor_to_buffer_anchor(hovered_hint.position) .and_then(|(anchor, _)| { From b4f3ea7871f8968afede33d7d1b2dd215507546c Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Thu, 21 May 2026 21:21:31 +0900 Subject: [PATCH 2/3] Address review feedbacks on comments --- crates/editor/src/hover_popover.rs | 5 ++--- crates/editor/src/inlays/inlay_hints.rs | 6 ------ 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index 316b04508cb..506153546e3 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -888,8 +888,7 @@ impl HoverState { /// Diagnostic and text-symbol hovers anchor at the start of the hover range. /// For inlay-hint hovers, however, the buffer anchor alone always maps to the /// inlay's leading edge in display space, so the popover would pin to the start - /// of the inlay regardless of which character was hovered. Add the in-inlay - /// byte offset onto the inlay's start so the popover lands at the hovered position. + /// of the inlay regardless of which character was hovered. pub(crate) fn popover_anchor_display_point( &self, snapshot: &EditorSnapshot, @@ -2175,7 +2174,7 @@ mod tests { /// `clip_point` skips through the lone source character with `Bias::Right`, /// so both hints fall inside the buffer-position window used to find a /// hovered hint. The hovered-offset filter must then disambiguate them by - /// the inlay text actually under the mouse. + /// checking which hint's inlay-offset range contains the mouse offset. #[gpui::test] async fn test_hover_inlay_picks_inlay_under_cursor(cx: &mut gpui::TestAppContext) { init_test(cx, |settings| { diff --git a/crates/editor/src/inlays/inlay_hints.rs b/crates/editor/src/inlays/inlay_hints.rs index 117f731a09b..98a4fdba7cd 100644 --- a/crates/editor/src/inlays/inlay_hints.rs +++ b/crates/editor/src/inlays/inlay_hints.rs @@ -604,12 +604,6 @@ impl Editor { point_for_position.next_valid.to_point(snapshot), Bias::Right, ); - // The buffer-position window can include neighbouring inlays - // (e.g. a `::Type` hint after a closing paren and another hint just - // past it both pass the take_while when `clip_point` skips through - // a one-character source span between them). Use the actual mouse - // `hovered_offset` to pick the inlay whose rendered text is under - // the cursor. let hovered_hint = Self::visible_inlay_hints(self.display_map.read(cx)) .filter(|hint| snapshot.can_resolve(&hint.position)) .skip_while(|hint| { From 5f371d806d404a087fef614c8914b00a0b783f55 Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Thu, 21 May 2026 21:33:46 +0900 Subject: [PATCH 3/3] Add symmetric assertion for inlay-hint hover disambiguation --- crates/editor/src/hover_popover.rs | 153 +++++++++++++++++------------ 1 file changed, 90 insertions(+), 63 deletions(-) diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index 506153546e3..2a24071a231 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -2266,71 +2266,98 @@ mod tests { ); }); - // Hover in the middle of the first inlay's rendered text. - let hover_position = cx.update_editor(|editor, window, cx| { - let snapshot = editor.snapshot(window, cx); - let inlay_start = MultiBufferOffset(first_hint_offset).to_display_point(&snapshot); - let exact_unclipped = DisplayPoint::new( - inlay_start.row(), - inlay_start.column() + (first_label.len() as u32 / 2), - ); - let previous_valid = snapshot.clip_point(exact_unclipped, Bias::Left); - let next_valid = snapshot.clip_point(exact_unclipped, Bias::Right); - let nearest_valid = if previous_valid == next_valid { - previous_valid - } else { - match snapshot.inlay_bias_at(exact_unclipped) { - Some(Bias::Left) => next_valid, - Some(Bias::Right) => previous_valid, - None => previous_valid, + async fn assert_hovered_inlay( + cx: &mut EditorLspTestContext, + hint_offset: usize, + label_len: u32, + expected_tooltip: &str, + expected_inlay_id: InlayId, + ) { + let hover_position = cx.update_editor(|editor, window, cx| { + let snapshot = editor.snapshot(window, cx); + let inlay_start = MultiBufferOffset(hint_offset).to_display_point(&snapshot); + let exact_unclipped = + DisplayPoint::new(inlay_start.row(), inlay_start.column() + label_len / 2); + let previous_valid = snapshot.clip_point(exact_unclipped, Bias::Left); + let next_valid = snapshot.clip_point(exact_unclipped, Bias::Right); + let nearest_valid = if previous_valid == next_valid { + previous_valid + } else { + match snapshot.inlay_bias_at(exact_unclipped) { + Some(Bias::Left) => next_valid, + Some(Bias::Right) => previous_valid, + None => previous_valid, + } + }; + PointForPosition { + previous_valid, + next_valid, + nearest_valid, + exact_unclipped, + column_overshoot_after_line_end: 0, } - }; - PointForPosition { - previous_valid, - next_valid, - nearest_valid, - exact_unclipped, - column_overshoot_after_line_end: 0, - } - }); - cx.update_editor(|editor, window, cx| { - editor.update_inlay_link_and_hover_points( - &editor.snapshot(window, cx), - hover_position, - None, - true, - false, - window, - cx, - ); - }); - cx.background_executor - .advance_clock(Duration::from_millis(get_hover_popover_delay(&cx) + 100)); - cx.background_executor.run_until_parked(); + }); + cx.update_editor(|editor, window, cx| { + editor.hover_state = HoverState::default(); + editor.update_inlay_link_and_hover_points( + &editor.snapshot(window, cx), + hover_position, + None, + true, + false, + window, + cx, + ); + }); + cx.background_executor + .advance_clock(Duration::from_millis(get_hover_popover_delay(cx) + 100)); + cx.background_executor.run_until_parked(); - cx.update_editor(|editor, _, cx| { - let hover_state = &editor.hover_state; - assert_eq!( - hover_state.info_popovers.len(), - 1, - "expected exactly one popover", - ); - let popover = hover_state.info_popovers.first().unwrap(); - assert_eq!( - popover.get_rendered_text(cx), - "Foo", - "popover content should match the first inlay (the one under the cursor), \ - not the adjacent inlay one source character past it", - ); - let RangeInEditor::Inlay(highlight) = &popover.symbol_range else { - panic!("expected an inlay popover, got {:?}", popover.symbol_range); - }; - assert_eq!( - highlight.inlay, - InlayId::Hint(0), - "popover should reference the first inlay", - ); - }); + cx.update_editor(|editor, _, cx| { + let hover_state = &editor.hover_state; + assert_eq!( + hover_state.info_popovers.len(), + 1, + "expected exactly one popover for inlay {expected_inlay_id:?}", + ); + let popover = hover_state.info_popovers.first().unwrap(); + assert_eq!( + popover.get_rendered_text(cx), + expected_tooltip, + "popover content should match the inlay under the cursor, \ + not the adjacent inlay one source character away", + ); + let RangeInEditor::Inlay(highlight) = &popover.symbol_range else { + panic!("expected an inlay popover, got {:?}", popover.symbol_range); + }; + assert_eq!( + highlight.inlay, expected_inlay_id, + "popover should reference the hovered inlay", + ); + }); + } + + // Hover the first inlay (`::Foo`): the buffer-position window also covers + // the second hint past `;`, so the hovered-offset filter must pick the first. + assert_hovered_inlay( + &mut cx, + first_hint_offset, + first_label.len() as u32, + "Foo", + InlayId::Hint(0), + ) + .await; + + // Symmetric check: hover the second inlay (`::Bar`) and verify the filter + // picks the second hint despite the first hint also being in range. + assert_hovered_inlay( + &mut cx, + second_hint_offset, + second_label.len() as u32, + "Bar", + InlayId::Hint(1), + ) + .await; } #[test]