Rework hover delay, respect editor font size, and enable hover in multibuffers

crates/editor/src/editor.rs

@@ -1,6 +1,6 @@
 pub mod display_map;
 mod element;
-mod hover;
+mod hover_popover;
 pub mod items;
 pub mod movement;
 mod multi_buffer;
@@ -26,10 +26,11 @@ use gpui::{
     geometry::vector::{vec2f, Vector2F},
     impl_actions, impl_internal_actions,
     platform::CursorStyle,
-    text_layout, AppContext, AsyncAppContext, Axis, ClipboardItem, Element, ElementBox, Entity,
+    text_layout, AppContext, AsyncAppContext, ClipboardItem, Element, ElementBox, Entity,
     ModelHandle, MutableAppContext, RenderContext, Task, View, ViewContext, ViewHandle,
     WeakViewHandle,
 };
+use hover_popover::{hide_hover, HoverState};
 pub use language::{char_kind, CharKind};
 use language::{
     BracketPair, Buffer, CodeAction, CodeLabel, Completion, Diagnostic, DiagnosticSeverity,
@@ -83,11 +84,6 @@ pub struct Scroll(pub Vector2F);
 #[derive(Clone, PartialEq)]
 pub struct Select(pub SelectPhase);
 
-#[derive(Clone, PartialEq)]
-pub struct HoverAt {
-    point: Option<DisplayPoint>,
-}
-
 #[derive(Clone, Debug, PartialEq)]
 pub struct Jump {
     path: ProjectPath,
@@ -128,11 +124,6 @@ pub struct ConfirmCodeAction {
     pub item_ix: Option<usize>,
 }
 
-#[derive(Clone, Default)]
-pub struct GoToDefinitionAt {
-    pub location: Option<DisplayPoint>,
-}
-
 actions!(
     editor,
     [
@@ -225,7 +216,7 @@ impl_actions!(
     ]
 );
 
-impl_internal_actions!(editor, [Scroll, Select, HoverAt, Jump]);
+impl_internal_actions!(editor, [Scroll, Select, Jump]);
 
 enum DocumentHighlightRead {}
 enum DocumentHighlightWrite {}
@@ -312,8 +303,6 @@ pub fn init(cx: &mut MutableAppContext) {
     cx.add_action(Editor::fold_selected_ranges);
     cx.add_action(Editor::show_completions);
     cx.add_action(Editor::toggle_code_actions);
-    cx.add_action(Editor::hover);
-    cx.add_action(Editor::hover_at);
     cx.add_action(Editor::open_excerpts);
     cx.add_action(Editor::jump);
     cx.add_action(Editor::restart_language_server);
@@ -323,6 +312,8 @@ pub fn init(cx: &mut MutableAppContext) {
     cx.add_async_action(Editor::confirm_rename);
     cx.add_async_action(Editor::find_all_references);
 
+    hover_popover::init(cx);
+
     workspace::register_project_item::<Editor>(cx);
     workspace::register_followable_item::<Editor>(cx);
 }
@@ -1023,7 +1014,7 @@ impl Editor {
             next_completion_id: 0,
             available_code_actions: Default::default(),
             code_actions_task: Default::default(),
-            hover_task: Default::default(),
+
             document_highlights_task: Default::default(),
             pending_rename: Default::default(),
             searchable: true,
@@ -1032,11 +1023,7 @@ impl Editor {
             keymap_context_layers: Default::default(),
             input_enabled: true,
             leader_replica_id: None,
-            hover_state: HoverState {
+            hover_state: Default::default(),
-                popover: None,
-                last_hover: std::time::Instant::now(),
-                start_grace: std::time::Instant::now(),
-            },
         };
         this.end_selection(cx);
 
@@ -1159,6 +1146,8 @@ impl Editor {
         }
 
         self.autoscroll_request.take();
+        hide_hover(self, cx);
+
         cx.emit(Event::ScrollPositionChanged { local });
         cx.notify();
     }
@@ -1422,7 +1411,7 @@ impl Editor {
                 }
             }
 
-            self.hide_hover(cx);
+            hide_hover(self, cx);
 
             if old_cursor_position.to_display_point(&display_map).row()
                 != new_cursor_position.to_display_point(&display_map).row()
@@ -1785,7 +1774,7 @@ impl Editor {
             return;
         }
 
-        if self.hide_hover(cx) {
+        if hide_hover(self, cx) {
             return;
         }
 
@@ -2416,179 +2405,6 @@ impl Editor {
         }))
     }
 
-    /// Bindable action which uses the most recent selection head to trigger a hover
-    fn hover(&mut self, _: &Hover, cx: &mut ViewContext<Self>) {
-        let head = self.selections.newest_display(cx).head();
-        self.show_hover(head, true, cx);
-    }
-
-    /// The internal hover action dispatches between `show_hover` or `hide_hover`
-    /// depending on whether a point to hover over is provided.
-    fn hover_at(&mut self, action: &HoverAt, cx: &mut ViewContext<Self>) {
-        if let Some(point) = action.point {
-            self.show_hover(point, false, cx);
-        } else {
-            self.hide_hover(cx);
-        }
-    }
-
-    /// Hides the type information popup.
-    /// Triggered by the `Hover` action when the cursor is not over a symbol or when the
-    /// selecitons changed.
-    fn hide_hover(&mut self, cx: &mut ViewContext<Self>) -> bool {
-        // consistently keep track of state to make handoff smooth
-        self.hover_state.determine_state(false);
-
-        let mut did_hide = false;
-
-        // only notify the context once
-        if self.hover_state.popover.is_some() {
-            self.hover_state.popover = None;
-            did_hide = true;
-            cx.notify();
-        }
-
-        self.clear_background_highlights::<HoverState>(cx);
-
-        self.hover_task = None;
-
-        did_hide
-    }
-
-    /// Queries the LSP and shows type info and documentation
-    /// about the symbol the mouse is currently hovering over.
-    /// Triggered by the `Hover` action when the cursor may be over a symbol.
-    fn show_hover(
-        &mut self,
-        point: DisplayPoint,
-        ignore_timeout: bool,
-        cx: &mut ViewContext<Self>,
-    ) {
-        if self.pending_rename.is_some() {
-            return;
-        }
-
-        if let Some(hover) = &self.hover_state.popover {
-            if hover.hover_point == point {
-                // Hover triggered from same location as last time. Don't show again.
-                return;
-            }
-        }
-
-        let snapshot = self.snapshot(cx);
-        let (buffer, buffer_position) = if let Some(output) = self
-            .buffer
-            .read(cx)
-            .text_anchor_for_position(point.to_point(&snapshot.display_snapshot), cx)
-        {
-            output
-        } else {
-            return;
-        };
-
-        let project = if let Some(project) = self.project.clone() {
-            project
-        } else {
-            return;
-        };
-
-        // query the LSP for hover info
-        let hover_request = project.update(cx, |project, cx| {
-            project.hover(&buffer, buffer_position.clone(), cx)
-        });
-
-        let buffer_snapshot = buffer.read(cx).snapshot();
-
-        let task = cx.spawn_weak(|this, mut cx| {
-            async move {
-                // Construct new hover popover from hover request
-                let hover_popover = hover_request.await.ok().flatten().and_then(|hover_result| {
-                    if hover_result.contents.is_empty() {
-                        return None;
-                    }
-
-                    let range = if let Some(range) = hover_result.range {
-                        let offset_range = range.to_offset(&buffer_snapshot);
-                        if !offset_range
-                            .contains(&point.to_offset(&snapshot.display_snapshot, Bias::Left))
-                        {
-                            return None;
-                        }
-
-                        offset_range
-                            .start
-                            .to_display_point(&snapshot.display_snapshot)
-                            ..offset_range
-                                .end
-                                .to_display_point(&snapshot.display_snapshot)
-                    } else {
-                        point..point
-                    };
-
-                    Some(HoverPopover {
-                        project: project.clone(),
-                        hover_point: point,
-                        range,
-                        contents: hover_result.contents,
-                    })
-                });
-
-                if let Some(this) = this.upgrade(&cx) {
-                    this.update(&mut cx, |this, cx| {
-                        // this was trickier than expected, trying to do a couple things:
-                        //
-                        // 1. if you hover over a symbol, there should be a slight delay
-                        //    before the popover shows
-                        // 2. if you move to another symbol when the popover is showing,
-                        //    the popover should switch right away, and you should
-                        //    not have to wait for it to come up again
-                        let (recent_hover, in_grace) =
-                            this.hover_state.determine_state(hover_popover.is_some());
-                        let smooth_handoff =
-                            this.hover_state.popover.is_some() && hover_popover.is_some();
-                        let visible = this.hover_state.popover.is_some() || hover_popover.is_some();
-
-                        // `smooth_handoff` and `in_grace` determine whether to switch right away.
-                        // `recent_hover` will activate the handoff after the initial delay.
-                        // `ignore_timeout` is set when the user manually sent the hover action.
-                        if (ignore_timeout || smooth_handoff || !recent_hover || in_grace)
-                            && visible
-                        {
-                            // Highlight the selected symbol using a background highlight
-                            if let Some(display_range) =
-                                hover_popover.as_ref().map(|popover| popover.range.clone())
-                            {
-                                let start = snapshot.display_snapshot.buffer_snapshot.anchor_after(
-                                    display_range
-                                        .start
-                                        .to_offset(&snapshot.display_snapshot, Bias::Right),
-                                );
-                                let end = snapshot.display_snapshot.buffer_snapshot.anchor_before(
-                                    display_range
-                                        .end
-                                        .to_offset(&snapshot.display_snapshot, Bias::Left),
-                                );
-
-                                this.highlight_background::<HoverState>(
-                                    vec![start..end],
-                                    |theme| theme.editor.hover_popover.highlight,
-                                    cx,
-                                );
-                            }
-
-                            this.hover_state.popover = hover_popover;
-                            cx.notify();
-                        }
-                    });
-                }
-                Ok::<_, anyhow::Error>(())
-            }
-            .log_err()
-        });
-
-        self.hover_task = Some(task);
-    }
-
     async fn open_project_transaction(
         this: ViewHandle<Editor>,
         workspace: ViewHandle<Workspace>,
@@ -2614,7 +2430,7 @@ impl Editor {
                         .read(cx)
                         .excerpt_containing(editor.selections.newest_anchor().head(), cx)
                 });
-                if let Some((excerpted_buffer, excerpt_range)) = excerpt {
+                if let Some((_, excerpted_buffer, excerpt_range)) = excerpt {
                     if excerpted_buffer == *buffer {
                         let snapshot = buffer.read_with(&cx, |buffer, _| buffer.snapshot());
                         let excerpt_range = excerpt_range.to_offset(&snapshot);
@@ -2835,10 +2651,6 @@ impl Editor {
             .map(|menu| menu.render(cursor_position, style, cx))
     }
 
-    pub(crate) fn hover_popover(&self) -> Option<HoverPopover> {
-        self.hover_state.popover.clone()
-    }
-
     fn show_context_menu(&mut self, menu: ContextMenu, cx: &mut ViewContext<Self>) {
         if !matches!(menu, ContextMenu::Completions(_)) {
             self.completion_tasks.clear();

crates/editor/src/element.rs

@@ -3,9 +3,10 @@ use super::{
     Anchor, DisplayPoint, Editor, EditorMode, EditorSnapshot, Input, Scroll, Select, SelectPhase,
     SoftWrap, ToPoint, MAX_LINE_LEN,
 };
+use crate::hover_popover::HoverAt;
 use crate::{
     display_map::{DisplaySnapshot, TransformBlock},
-    EditorStyle, HoverAt,
+    EditorStyle,
 };
 use clock::ReplicaId;
 use collections::{BTreeMap, HashMap};
@@ -1196,8 +1197,8 @@ impl Element for EditorElement {
                     .map(|indicator| (newest_selection_head.row(), indicator));
             }
 
-            hover = view.hover_popover().and_then(|hover| {
+            hover = view.hover_state.popover.clone().and_then(|hover| {
-                let (point, rendered) = hover.render(style.clone(), cx);
+                let (point, rendered) = hover.render(&snapshot, style.clone(), cx);
                 if point.row() >= snapshot.scroll_position().y() as u32 {
                     if line_layouts.len() > (point.row() - start_row) as usize {
                         return Some((point, rendered));
@@ -1233,8 +1234,12 @@ impl Element for EditorElement {
                 SizeConstraint {
                     min: Vector2F::zero(),
                     max: vec2f(
-                        (120. * em_width).min(size.x()),
+                        (120. * em_width) // Default size
-                        (size.y() - line_height) * 1. / 2.,
+                            .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,

crates/editor/src/hover_popover.rs

@@ -1,48 +1,240 @@
-/// Keeps track of the state of the [`HoverPopover`].
+use std::{
-/// Times out the initial delay and the grace period.
+    ops::Range,
-pub struct HoverState {
+    time::{Duration, Instant},
-    popover: Option<HoverPopover>,
+};
-    last_hover: std::time::Instant,
+
-    start_grace: std::time::Instant,
+use gpui::{
+    actions,
+    elements::{Flex, MouseEventHandler, Padding, Text},
+    impl_internal_actions,
+    platform::CursorStyle,
+    Axis, Element, ElementBox, ModelHandle, MutableAppContext, RenderContext, Task, ViewContext,
+};
+use language::Bias;
+use project::{HoverBlock, Project};
+use util::TryFutureExt;
+
+use crate::{
+    display_map::ToDisplayPoint, Anchor, AnchorRangeExt, DisplayPoint, Editor, EditorSnapshot,
+    EditorStyle,
+};
+
+#[derive(Clone, PartialEq)]
+pub struct HoverAt {
+    pub point: Option<DisplayPoint>,
 }
 
-impl HoverState {
+actions!(editor, [Hover]);
-    /// Takes whether the cursor is currently hovering over a symbol,
+impl_internal_actions!(editor, [HoverAt]);
-    /// and returns a tuple containing whether there was a recent hover,
+
-    /// and whether the hover is still in the grace period.
+pub fn init(cx: &mut MutableAppContext) {
-    pub fn determine_state(&mut self, hovering: bool) -> (bool, bool) {
+    cx.add_action(hover);
-        // NOTE: We use some sane defaults, but it might be
+    cx.add_action(hover_at);
-        //       nice to make these values configurable.
+}
-        let recent_hover = self.last_hover.elapsed() < std::time::Duration::from_millis(500);
+
-        if !hovering {
+/// Bindable action which uses the most recent selection head to trigger a hover
-            self.last_hover = std::time::Instant::now();
+fn hover(editor: &mut Editor, _: &Hover, cx: &mut ViewContext<Editor>) {
+    let head = editor.selections.newest_display(cx).head();
+    show_hover(editor, head, true, cx);
+}
+
+/// The internal hover action dispatches between `show_hover` or `hide_hover`
+/// depending on whether a point to hover over is provided.
+fn hover_at(editor: &mut Editor, action: &HoverAt, cx: &mut ViewContext<Editor>) {
+    if let Some(point) = action.point {
+        show_hover(editor, point, false, cx);
+    } else {
+        hide_hover(editor, cx);
+    }
+}
+
+/// Hides the type information popup.
+/// 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;
+
+    // only notify the context once
+    if editor.hover_state.popover.is_some() {
+        editor.hover_state.popover = None;
+        editor.hover_state.hidden_at = Some(Instant::now());
+        editor.hover_state.symbol_range = None;
+        did_hide = true;
+        cx.notify();
     }
 
-        let in_grace = self.start_grace.elapsed() < std::time::Duration::from_millis(250);
+    editor.clear_background_highlights::<HoverState>(cx);
-        if hovering && !recent_hover {
+
-            self.start_grace = std::time::Instant::now();
+    editor.hover_state.task = None;
+
+    did_hide
+}
+
+/// Queries the LSP and shows type info and documentation
+/// about the symbol the mouse is currently hovering over.
+/// Triggered by the `Hover` action when the cursor may be over a symbol.
+fn show_hover(
+    editor: &mut Editor,
+    point: DisplayPoint,
+    ignore_timeout: bool,
+    cx: &mut ViewContext<Editor>,
+) {
+    if editor.pending_rename.is_some() {
+        return;
     }
 
-        return (recent_hover, in_grace);
+    let snapshot = editor.snapshot(cx);
+    let multibuffer_offset = point.to_offset(&snapshot.display_snapshot, Bias::Left);
+
+    if let Some(range) = &editor.hover_state.symbol_range {
+        if range
+            .to_offset(&snapshot.buffer_snapshot)
+            .contains(&multibuffer_offset)
+        {
+            // Hover triggered from same location as last time. Don't show again.
+            return;
+        }
     }
 
-    pub fn close(&mut self) {
+    let (buffer, buffer_position) = if let Some(output) = editor
-        self.popover.take();
+        .buffer
+        .read(cx)
+        .text_anchor_for_position(multibuffer_offset, cx)
+    {
+        output
+    } else {
+        return;
+    };
+
+    let excerpt_id = if let Some((excerpt_id, _, _)) = editor
+        .buffer()
+        .read(cx)
+        .excerpt_containing(multibuffer_offset, cx)
+    {
+        excerpt_id
+    } else {
+        return;
+    };
+
+    let project = if let Some(project) = editor.project.clone() {
+        project
+    } else {
+        return;
+    };
+
+    // query the LSP for hover info
+    let hover_request = project.update(cx, |project, cx| {
+        project.hover(&buffer, buffer_position.clone(), cx)
+    });
+
+    // We should only delay if the hover popover isn't visible, it wasn't recently hidden, and
+    // the hover wasn't triggered from the keyboard
+    let should_delay = editor.hover_state.popover.is_none() // Hover visible currently
+        && editor
+            .hover_state
+            .hidden_at
+            .map(|hidden| hidden.elapsed().as_millis() > 200)
+            .unwrap_or(true) // Hover was visible recently enough
+        && !ignore_timeout; // Hover triggered from keyboard
+
+    // Get input anchor
+    let anchor = snapshot
+        .buffer_snapshot
+        .anchor_at(multibuffer_offset, Bias::Left);
+
+    let task = cx.spawn_weak(|this, mut cx| {
+        async move {
+            let delay = if should_delay {
+                Some(cx.background().timer(Duration::from_millis(500)))
+            } else {
+                None
+            };
+
+            // Construct new hover popover from hover request
+            let hover_popover = hover_request.await.ok().flatten().and_then(|hover_result| {
+                if hover_result.contents.is_empty() {
+                    return None;
                 }
+
+                let range = if let Some(range) = hover_result.range {
+                    let start = snapshot
+                        .buffer_snapshot
+                        .anchor_in_excerpt(excerpt_id.clone(), range.start);
+                    let end = snapshot
+                        .buffer_snapshot
+                        .anchor_in_excerpt(excerpt_id.clone(), range.end);
+
+                    start..end
+                } else {
+                    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(HoverPopover {
+                    project: project.clone(),
+                    anchor: range.start.clone(),
+                    contents: hover_result.contents,
+                })
+            });
+
+            if let Some(delay) = delay {
+                delay.await;
+            }
+
+            if let Some(this) = this.upgrade(&cx) {
+                this.update(&mut cx, |this, cx| {
+                    if this.hover_state.popover.is_some() || hover_popover.is_some() {
+                        // 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.popover = hover_popover;
+
+                        if this.hover_state.popover.is_none() {
+                            this.hover_state.hidden_at = Some(Instant::now());
+                        }
+
+                        cx.notify();
+                    }
+                });
+            }
+            Ok::<_, anyhow::Error>(())
+        }
+        .log_err()
+    });
+
+    editor.hover_state.task = Some(task);
+}
+
+#[derive(Default)]
+pub struct HoverState {
+    pub popover: Option<HoverPopover>,
+    pub hidden_at: Option<Instant>,
+    pub symbol_range: Option<Range<Anchor>>,
+    pub task: Option<Task<Option<()>>>,
 }
 
 #[derive(Clone)]
-pub(crate) struct HoverPopover {
+pub struct HoverPopover {
     pub project: ModelHandle<Project>,
-    pub hover_point: DisplayPoint,
+    pub anchor: Anchor,
-    pub range: Range<DisplayPoint>,
     pub contents: Vec<HoverBlock>,
-    pub task: Option<Task<()>>,
 }
 
 impl HoverPopover {
-    fn render(
+    pub fn render(
         &self,
+        snapshot: &EditorSnapshot,
         style: EditorStyle,
         cx: &mut RenderContext<Editor>,
     ) -> (DisplayPoint, ElementBox) {
@@ -70,7 +262,10 @@ impl HoverPopover {
                         )
                         .boxed()
                 } else {
-                    Text::new(content.text.clone(), style.hover_popover.prose.clone())
+                    let mut text_style = style.hover_popover.prose.clone();
+                    text_style.font_size = style.text.font_size;
+
+                    Text::new(content.text.clone(), text_style)
                         .with_soft_wrap(true)
                         .contained()
                         .with_style(style.hover_popover.block_style)
@@ -89,6 +284,7 @@ impl HoverPopover {
         })
         .boxed();
 
-        (self.range.start, element)
+        let display_point = self.anchor.to_display_point(&snapshot.display_snapshot);
+        (display_point, element)
     }
 }

crates/editor/src/multi_buffer.rs

@@ -937,7 +937,7 @@ impl MultiBuffer {
         &self,
         position: impl ToOffset,
         cx: &AppContext,
-    ) -> Option<(ModelHandle<Buffer>, Range<text::Anchor>)> {
+    ) -> Option<(ExcerptId, ModelHandle<Buffer>, Range<text::Anchor>)> {
         let snapshot = self.read(cx);
         let position = position.to_offset(&snapshot);
 
@@ -945,6 +945,7 @@ impl MultiBuffer {
         cursor.seek(&position, Bias::Right, &());
         cursor.item().map(|excerpt| {
             (
+                excerpt.id.clone(),
                 self.buffers
                     .borrow()
                     .get(&excerpt.buffer_id)

styles/src/buildTokens.ts

@@ -30,7 +30,7 @@ function themeTokens(theme: Theme) {
       boolean: theme.syntax.boolean.color,
     },
     player: theme.player,
-    shadowAlpha: theme.shadowAlpha,
+    shadow: theme.shadow,
   };
 }