Merge pull request #1166 from zed-industries/hover-fixes

Always delay hover display

crates/editor/src/editor.rs

@@ -6147,12 +6147,8 @@ pub fn styled_runs_for_code_label<'a>(
 
 #[cfg(test)]
 mod tests {
-    use crate::{
+    use crate::test::{
-        hover_popover::{hover, hover_at, HoverAt, HOVER_DELAY_MILLIS, HOVER_GRACE_MILLIS},
+        assert_text_with_selections, build_editor, select_ranges, EditorTestContext,
-        test::{
-            assert_text_with_selections, build_editor, select_ranges, EditorLspTestContext,
-            EditorTestContext,
-        },
     };
 
     use super::*;
@@ -6164,7 +6160,7 @@ mod tests {
     use indoc::indoc;
     use language::{FakeLspAdapter, LanguageConfig};
     use lsp::FakeLanguageServer;
-    use project::{FakeFs, HoverBlock};
+    use project::FakeFs;
     use settings::LanguageOverride;
     use std::{cell::RefCell, rc::Rc, time::Instant};
     use text::Point;
@@ -9395,193 +9391,6 @@ mod tests {
         }
     }
 
-    #[gpui::test]
-    async fn test_hover_popover(cx: &mut gpui::TestAppContext) {
-        let mut cx = EditorLspTestContext::new_rust(
-            lsp::ServerCapabilities {
-                hover_provider: Some(lsp::HoverProviderCapability::Simple(true)),
-                ..Default::default()
-            },
-            cx,
-        )
-        .await;
-
-        // Basic hover delays and then pops without moving the mouse
-        cx.set_state(indoc! {"
-            fn |test()
-                println!();"});
-        let hover_point = cx.display_point(indoc! {"
-            fn test()
-                print|ln!();"});
-
-        cx.update_editor(|editor, cx| {
-            hover_at(
-                editor,
-                &HoverAt {
-                    point: Some(hover_point),
-                },
-                cx,
-            )
-        });
-        assert!(!cx.editor(|editor, _| editor.hover_state.visible()));
-
-        // After delay, hover should be visible.
-        let symbol_range = cx.lsp_range(indoc! {"
-            fn test()
-                [println!]();"});
-        let mut requests =
-            cx.lsp
-                .handle_request::<lsp::request::HoverRequest, _, _>(move |_, _| async move {
-                    Ok(Some(lsp::Hover {
-                        contents: lsp::HoverContents::Markup(lsp::MarkupContent {
-                            kind: lsp::MarkupKind::Markdown,
-                            value: indoc! {"
-                        # Some basic docs
-                        Some test documentation"}
-                            .to_string(),
-                        }),
-                        range: Some(symbol_range),
-                    }))
-                });
-        cx.foreground()
-            .advance_clock(Duration::from_millis(HOVER_DELAY_MILLIS + 100));
-        requests.next().await;
-
-        cx.editor(|editor, _| {
-            assert!(editor.hover_state.visible());
-            assert_eq!(
-                editor.hover_state.popover.clone().unwrap().contents,
-                vec![
-                    HoverBlock {
-                        text: "Some basic docs".to_string(),
-                        language: None
-                    },
-                    HoverBlock {
-                        text: "Some test documentation".to_string(),
-                        language: None
-                    }
-                ]
-            )
-        });
-
-        // Mouse moved with no hover response dismisses
-        let hover_point = cx.display_point(indoc! {"
-            fn te|st()
-                println!();"});
-        cx.update_editor(|editor, cx| {
-            hover_at(
-                editor,
-                &HoverAt {
-                    point: Some(hover_point),
-                },
-                cx,
-            )
-        });
-        cx.lsp
-            .handle_request::<lsp::request::HoverRequest, _, _>(|_, _| async move { Ok(None) })
-            .next()
-            .await;
-        cx.foreground().run_until_parked();
-        cx.editor(|editor, _| {
-            assert!(!editor.hover_state.visible());
-        });
-        cx.foreground()
-            .advance_clock(Duration::from_millis(HOVER_GRACE_MILLIS + 100));
-
-        // Hover with keyboard has no delay
-        cx.set_state(indoc! {"
-            f|n test()
-                println!();"});
-        cx.update_editor(|editor, cx| hover(editor, &hover_popover::Hover, cx));
-        let symbol_range = cx.lsp_range(indoc! {"
-            [fn] test()
-                println!();"});
-        cx.lsp
-            .handle_request::<lsp::request::HoverRequest, _, _>(move |_, _| async move {
-                Ok(Some(lsp::Hover {
-                    contents: lsp::HoverContents::Markup(lsp::MarkupContent {
-                        kind: lsp::MarkupKind::Markdown,
-                        value: indoc! {"
-                        # Some other basic docs
-                        Some other test documentation"}
-                        .to_string(),
-                    }),
-                    range: Some(symbol_range),
-                }))
-            })
-            .next()
-            .await;
-        cx.foreground().run_until_parked();
-        cx.editor(|editor, _| {
-            assert!(editor.hover_state.visible());
-            assert_eq!(
-                editor.hover_state.popover.clone().unwrap().contents,
-                vec![
-                    HoverBlock {
-                        text: "Some other basic docs".to_string(),
-                        language: None
-                    },
-                    HoverBlock {
-                        text: "Some other test documentation".to_string(),
-                        language: None
-                    }
-                ]
-            )
-        });
-
-        // Open hover popover disables delay
-        let hover_point = cx.display_point(indoc! {"
-            fn test()
-                print|ln!();"});
-        cx.update_editor(|editor, cx| {
-            hover_at(
-                editor,
-                &HoverAt {
-                    point: Some(hover_point),
-                },
-                cx,
-            )
-        });
-
-        let symbol_range = cx.lsp_range(indoc! {"
-            fn test()
-                [println!]();"});
-        cx.lsp
-            .handle_request::<lsp::request::HoverRequest, _, _>(move |_, _| async move {
-                Ok(Some(lsp::Hover {
-                    contents: lsp::HoverContents::Markup(lsp::MarkupContent {
-                        kind: lsp::MarkupKind::Markdown,
-                        value: indoc! {"
-                        # Some third basic docs
-                        Some third test documentation"}
-                        .to_string(),
-                    }),
-                    range: Some(symbol_range),
-                }))
-            })
-            .next()
-            .await;
-        cx.foreground().run_until_parked();
-        // No delay as the popover is already visible
-
-        cx.editor(|editor, _| {
-            assert!(editor.hover_state.visible());
-            assert_eq!(
-                editor.hover_state.popover.clone().unwrap().contents,
-                vec![
-                    HoverBlock {
-                        text: "Some third basic docs".to_string(),
-                        language: None
-                    },
-                    HoverBlock {
-                        text: "Some third test documentation".to_string(),
-                        language: None
-                    }
-                ]
-            )
-        });
-    }
-
     #[gpui::test]
     async fn test_toggle_comment(cx: &mut gpui::TestAppContext) {
         cx.update(|cx| cx.set_global(Settings::test(cx)));

crates/editor/src/hover_popover.rs

@@ -19,9 +19,8 @@ use crate::{
     EditorStyle,
 };
 
-pub const HOVER_DELAY_MILLIS: u64 = 500;
+pub const HOVER_DELAY_MILLIS: u64 = 350;
-pub const HOVER_REQUEST_DELAY_MILLIS: u64 = 250;
+pub const HOVER_REQUEST_DELAY_MILLIS: u64 = 200;
-pub const HOVER_GRACE_MILLIS: u64 = 250;
 
 #[derive(Clone, PartialEq)]
 pub struct HoverAt {
@@ -116,17 +115,7 @@ fn show_hover(
         return;
     };
 
-    // We should only delay if the hover popover isn't visible, it wasn't recently hidden, and
+    if !ignore_timeout {
-    // the hover wasn't triggered from the keyboard
-    let should_delay = editor.hover_state.popover.is_none() // Hover not visible currently
-    && editor
-            .hover_state
-            .hidden_at
-            .map(|hidden| hidden.elapsed().as_millis() > HOVER_GRACE_MILLIS as u128)
-            .unwrap_or(true) // Hover wasn't recently visible
-        && !ignore_timeout; // Hover was not triggered from keyboard
-
-    if should_delay {
         if let Some(range) = &editor.hover_state.symbol_range {
             if range
                 .to_offset(&snapshot.buffer_snapshot)
@@ -134,6 +123,8 @@ fn show_hover(
             {
                 // Hover triggered from same location as last time. Don't show again.
                 return;
+            } else {
+                hide_hover(editor, cx);
             }
         }
     }
@@ -156,7 +147,7 @@ fn show_hover(
     let task = cx.spawn_weak(|this, mut cx| {
         async move {
             // If we need to delay, delay a set amount initially before making the lsp request
-            let delay = if should_delay {
+            let delay = if !ignore_timeout {
                 // Construct delay task to wait for later
                 let total_delay = Some(
                     cx.background()
@@ -327,3 +318,149 @@ impl HoverPopover {
         (display_point, element)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use futures::StreamExt;
+    use indoc::indoc;
+
+    use project::HoverBlock;
+
+    use crate::test::EditorLspTestContext;
+
+    use super::*;
+
+    #[gpui::test]
+    async fn test_hover_popover(cx: &mut gpui::TestAppContext) {
+        let mut cx = EditorLspTestContext::new_rust(
+            lsp::ServerCapabilities {
+                hover_provider: Some(lsp::HoverProviderCapability::Simple(true)),
+                ..Default::default()
+            },
+            cx,
+        )
+        .await;
+
+        // Basic hover delays and then pops without moving the mouse
+        cx.set_state(indoc! {"
+            fn |test()
+                println!();"});
+        let hover_point = cx.display_point(indoc! {"
+            fn test()
+                print|ln!();"});
+
+        cx.update_editor(|editor, cx| {
+            hover_at(
+                editor,
+                &HoverAt {
+                    point: Some(hover_point),
+                },
+                cx,
+            )
+        });
+        assert!(!cx.editor(|editor, _| editor.hover_state.visible()));
+
+        // After delay, hover should be visible.
+        let symbol_range = cx.lsp_range(indoc! {"
+            fn test()
+                [println!]();"});
+        let mut requests =
+            cx.lsp
+                .handle_request::<lsp::request::HoverRequest, _, _>(move |_, _| async move {
+                    Ok(Some(lsp::Hover {
+                        contents: lsp::HoverContents::Markup(lsp::MarkupContent {
+                            kind: lsp::MarkupKind::Markdown,
+                            value: indoc! {"
+                                # Some basic docs
+                                Some test documentation"}
+                            .to_string(),
+                        }),
+                        range: Some(symbol_range),
+                    }))
+                });
+        cx.foreground()
+            .advance_clock(Duration::from_millis(HOVER_DELAY_MILLIS + 100));
+        requests.next().await;
+
+        cx.editor(|editor, _| {
+            assert!(editor.hover_state.visible());
+            assert_eq!(
+                editor.hover_state.popover.clone().unwrap().contents,
+                vec![
+                    HoverBlock {
+                        text: "Some basic docs".to_string(),
+                        language: None
+                    },
+                    HoverBlock {
+                        text: "Some test documentation".to_string(),
+                        language: None
+                    }
+                ]
+            )
+        });
+
+        // Mouse moved with no hover response dismisses
+        let hover_point = cx.display_point(indoc! {"
+            fn te|st()
+                println!();"});
+        cx.update_editor(|editor, cx| {
+            hover_at(
+                editor,
+                &HoverAt {
+                    point: Some(hover_point),
+                },
+                cx,
+            )
+        });
+        let mut request = cx
+            .lsp
+            .handle_request::<lsp::request::HoverRequest, _, _>(|_, _| async move { Ok(None) });
+        cx.foreground()
+            .advance_clock(Duration::from_millis(HOVER_DELAY_MILLIS + 100));
+        request.next().await;
+        cx.editor(|editor, _| {
+            assert!(!editor.hover_state.visible());
+        });
+
+        // Hover with keyboard has no delay
+        cx.set_state(indoc! {"
+            f|n test()
+                println!();"});
+        cx.update_editor(|editor, cx| hover(editor, &Hover, cx));
+        let symbol_range = cx.lsp_range(indoc! {"
+            [fn] test()
+                println!();"});
+        cx.lsp
+            .handle_request::<lsp::request::HoverRequest, _, _>(move |_, _| async move {
+                Ok(Some(lsp::Hover {
+                    contents: lsp::HoverContents::Markup(lsp::MarkupContent {
+                        kind: lsp::MarkupKind::Markdown,
+                        value: indoc! {"
+                            # Some other basic docs
+                            Some other test documentation"}
+                        .to_string(),
+                    }),
+                    range: Some(symbol_range),
+                }))
+            })
+            .next()
+            .await;
+        cx.foreground().run_until_parked();
+        cx.editor(|editor, _| {
+            assert!(editor.hover_state.visible());
+            assert_eq!(
+                editor.hover_state.popover.clone().unwrap().contents,
+                vec![
+                    HoverBlock {
+                        text: "Some other basic docs".to_string(),
+                        language: None
+                    },
+                    HoverBlock {
+                        text: "Some other test documentation".to_string(),
+                        language: None
+                    }
+                ]
+            )
+        });
+    }
+}

crates/editor/src/link_go_to_definition.rs

@@ -0,0 +1,58 @@
+use std::{
+    ops::Range,
+    time::{Duration, 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 FetchDefinition {
+    pub point: Option<DisplayPoint>,
+}
+
+#[derive(Clone, PartialEq)]
+pub struct GoToFetchedDefinition {
+    pub point: Option<DisplayPoint>,
+}
+
+impl_internal_actions!(edtior, [FetchDefinition, GoToFetchedDefinition]);
+
+pub fn init(cx: &mut MutableAppContext) {
+    cx.add_action(fetch_definition);
+    cx.add_action(go_to_fetched_definition);
+}
+
+pub fn fetch_definition(
+    editor: &mut Editor,
+    FetchDefinition { point }: &FetchDefinition,
+    cx: &mut ViewContext<Editor>,
+) {
+}
+
+pub fn go_to_fetched_definition(
+    editor: &mut Editor,
+    GoToFetchedDefinition { point }: &GoToFetchedDefinition,
+    cx: &mut ViewContext<Editor>,
+) {
+}
+
+#[derive(Default)]
+pub struct LinkGoToDefinitionState {
+    pub triggered_from
+    pub symbol_range: Option<Range<Anchor>>,
+    pub task: Option<Task<Option<()>>>,
+}