Add basic debounce, fix flickering

Isaac Clayton created

Change summary

crates/editor/src/editor.rs          | 122 ++++++++++++++++++++++++-----
crates/editor/src/element.rs         |  57 ++++++++-----
crates/project/src/lsp_command.rs    |  16 +--
styles/src/styleTree/hoverPopover.ts |   4 
4 files changed, 142 insertions(+), 57 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -83,10 +83,15 @@ pub struct Select(pub SelectPhase);
 #[derive(Clone)]
 pub struct ShowHover(DisplayPoint);
 
+#[derive(Clone)]
+pub struct HideHover;
+
 #[derive(Clone)]
 pub struct Hover {
-    point: DisplayPoint,
-    overshoot: DisplayPoint,
+    point: Option<DisplayPoint>,
+    // visible: bool,
+    // TODO(isaac): remove overshoot
+    // overshoot: DisplayPoint,
 }
 
 #[derive(Clone, Deserialize, PartialEq)]
@@ -218,7 +223,7 @@ impl_actions!(
     ]
 );
 
-impl_internal_actions!(editor, [Scroll, Select, Hover, ShowHover, GoToDefinitionAt]);
+impl_internal_actions!(editor, [Scroll, Select, Hover, ShowHover, HideHover, GoToDefinitionAt]);
 
 enum DocumentHighlightRead {}
 enum DocumentHighlightWrite {}
@@ -307,6 +312,7 @@ pub fn init(cx: &mut MutableAppContext) {
     cx.add_action(Editor::toggle_code_actions);
     cx.add_action(Editor::hover);
     cx.add_action(Editor::show_hover);
+    cx.add_action(Editor::hide_hover);
     cx.add_action(Editor::open_excerpts);
     cx.add_action(Editor::restart_language_server);
     cx.add_async_action(Editor::confirm_completion);
@@ -432,7 +438,7 @@ pub struct Editor {
     keymap_context_layers: BTreeMap<TypeId, gpui::keymap::Context>,
     input_enabled: bool,
     leader_replica_id: Option<u16>,
-    hover_popover: Option<HoverPopover>,
+    hover_popover: (Option<HoverPopover>, std::time::Instant, std::time::Instant),
 }
 
 pub struct EditorSnapshot {
@@ -1047,7 +1053,7 @@ impl Editor {
             keymap_context_layers: Default::default(),
             input_enabled: true,
             leader_replica_id: None,
-            hover_popover: None,
+            hover_popover: (None, std::time::Instant::now(), std::time::Instant::now()),
         };
         this.end_selection(cx);
 
@@ -1433,8 +1439,6 @@ impl Editor {
                 }
             }
 
-            self.hover_popover.take();
-
             if old_cursor_position.to_display_point(&display_map).row()
                 != new_cursor_position.to_display_point(&display_map).row()
             {
@@ -2425,11 +2429,35 @@ impl Editor {
     }
 
     fn hover(&mut self, action: &Hover, cx: &mut ViewContext<Self>) {
-        if action.overshoot.is_zero() {
-            self.show_hover(&ShowHover(action.point), cx);
+        // dbg!("hover");
+        if let Some(point) = action.point {
+            self.show_hover(&ShowHover(point), cx);
+        } else {
+            self.hide_hover(&HideHover, cx);
         }
     }
 
+    fn hide_hover(&mut self, _: &HideHover, cx: &mut ViewContext<Self>) {
+        let task = cx.spawn_weak(|this, mut cx| {
+            async move {
+                if let Some(this) = this.upgrade(&cx) {
+                    this.update(&mut cx, |this, cx| {
+                        if this.hover_popover.0.is_some() {
+                            // dbg!("hidden");
+                            this.hover_popover.0 = None;
+                            this.hover_popover.1 = std::time::Instant::now();
+                            cx.notify();
+                        }
+                    });
+                }
+                Ok(())
+            }
+            .log_err()
+        });
+
+        self.hover_task = Some(task);
+    }
+
     fn show_hover(&mut self, action: &ShowHover, cx: &mut ViewContext<Self>) {
         if self.pending_rename.is_some() {
             return;
@@ -2441,29 +2469,38 @@ impl Editor {
             return;
         };
 
+        // we use the mouse cursor position by default
+        let mut point = action.0.clone();
+
         let snapshot = self.snapshot(cx);
         let (buffer, buffer_position) = if let Some(output) = self
             .buffer
             .read(cx)
-            .text_anchor_for_position(action.0.to_point(&snapshot.display_snapshot), cx)
+            .text_anchor_for_position(point.to_point(&snapshot.display_snapshot), cx)
         {
             output
         } else {
             return;
         };
 
+        let buffer_snapshot = buffer.read(cx).snapshot();
+
         let hover = project.update(cx, |project, cx| {
             project.hover(&buffer, buffer_position.clone(), cx)
         });
 
-        let point = action.0.clone();
-
         let task = cx.spawn_weak(|this, mut cx| {
             async move {
-                // TODO: what to show while language server is loading?
-                let text: String = match hover.await? {
-                    None => "Language server is warming up...".into(),
-                    Some(hover) => match hover.contents {
+                // TODO: what to show while LSP is loading?
+                let mut text = None;
+
+                let hover = match hover.await {
+                    Ok(hover) => hover,
+                    Err(_) => None,
+                };
+
+                if let Some(hover) = hover {
+                    text = Some(match hover.contents {
                         lsp::HoverContents::Scalar(marked_string) => match marked_string {
                             lsp::MarkedString::String(string) => string,
                             lsp::MarkedString::LanguageString(string) => string.value,
@@ -2473,23 +2510,57 @@ impl Editor {
                             todo!()
                         }
                         lsp::HoverContents::Markup(markup) => markup.value,
-                    },
+                    });
+
+                    if let Some(range) = hover.range {
+                        let offset_range = range.to_offset(&buffer_snapshot);
+                        if offset_range
+                            .contains(&point.to_offset(&snapshot.display_snapshot, Bias::Left))
+                        {
+                            point = offset_range
+                                .start
+                                .to_display_point(&snapshot.display_snapshot);
+                        } else {
+                            text = None;
+                        }
+                    }
                 };
 
-                let hover_popover = HoverPopover {
-                    // TODO: fix tooltip to beginning of symbol based on range
+                let hover_popover = text.map(|text| HoverPopover {
                     point,
                     text,
                     runs: Vec::new(),
-                };
+                });
 
                 if let Some(this) = this.upgrade(&cx) {
                     this.update(&mut cx, |this, cx| {
-                        if this.focused {
-                            this.hover_popover = Some(hover_popover);
+                        let item_hovered_recently =
+                            this.hover_popover.1.elapsed() < std::time::Duration::from_millis(200);
+                        if hover_popover.is_none() {
+                            this.hover_popover.1 = std::time::Instant::now();
                         }
 
-                        cx.notify();
+                        let in_grace_period =
+                            this.hover_popover.2.elapsed() < std::time::Duration::from_millis(100);
+                        if hover_popover.is_some() && !item_hovered_recently {
+                            this.hover_popover.2 = std::time::Instant::now();
+                        }
+
+                        let smooth_handoff =
+                            this.hover_popover.0.is_some() && hover_popover.is_some();
+
+                        let visible = this.hover_popover.0.is_some() || hover_popover.is_some();
+
+                        println!(
+                            "grace:    {}\nrecently: {}",
+                            in_grace_period, item_hovered_recently
+                        );
+
+                        if (smooth_handoff || !item_hovered_recently || in_grace_period) && visible
+                        {
+                            this.hover_popover.0 = hover_popover;
+                            cx.notify();
+                        }
                     });
                 }
                 Ok::<_, anyhow::Error>(())
@@ -2747,7 +2818,10 @@ impl Editor {
     }
 
     pub fn render_hover_popover(&self, style: EditorStyle) -> Option<(DisplayPoint, ElementBox)> {
-        self.hover_popover.as_ref().map(|hover| hover.render(style))
+        self.hover_popover
+            .0
+            .as_ref()
+            .map(|hover| hover.render(style))
     }
 
     fn show_context_menu(&mut self, menu: ContextMenu, cx: &mut ViewContext<Self>) {

crates/editor/src/element.rs 🔗

@@ -33,7 +33,7 @@ use std::{
     cmp::{self, Ordering},
     fmt::Write,
     iter,
-    ops::Range,
+    ops::{Not, Range},
 };
 
 struct SelectionLayout {
@@ -509,25 +509,32 @@ impl EditorElement {
         }
 
         if let Some((position, hover_popover)) = layout.hover.as_mut() {
-            cx.scene.push_stacking_context(None);
+            if position.row() >= start_row {
+                if let Some(cursor_row_layout) = &layout
+                    .line_layouts
+                    .get((position.row() - start_row) as usize)
+                {
+                    cx.scene.push_stacking_context(None);
+
+                    let x = cursor_row_layout.x_for_index(position.column() as usize) - scroll_left;
+                    let y = (position.row() + 1) as f32 * layout.line_height - scroll_top;
+                    let mut popover_origin = content_origin + vec2f(x, y);
+                    let popover_height = hover_popover.size().y();
+
+                    if popover_origin.y() + popover_height > bounds.lower_left().y() {
+                        popover_origin
+                            .set_y(popover_origin.y() - layout.line_height - popover_height);
+                    }
 
-            let cursor_row_layout = &layout.line_layouts[(position.row() - start_row) as usize];
-            let x = cursor_row_layout.x_for_index(position.column() as usize) - scroll_left;
-            let y = (position.row() + 1) as f32 * layout.line_height - scroll_top;
-            let mut popover_origin = content_origin + vec2f(x, y);
-            let popover_height = hover_popover.size().y();
+                    hover_popover.paint(
+                        popover_origin,
+                        RectF::from_points(Vector2F::zero(), vec2f(f32::MAX, f32::MAX)), // Let content bleed outside of editor
+                        cx,
+                    );
 
-            if popover_origin.y() + popover_height > bounds.lower_left().y() {
-                popover_origin.set_y(popover_origin.y() - layout.line_height - popover_height);
+                    cx.scene.pop_stacking_context();
+                }
             }
-
-            hover_popover.paint(
-                popover_origin,
-                RectF::from_points(Vector2F::zero(), vec2f(f32::MAX, f32::MAX)), // Let content bleed outside of editor
-                cx,
-            );
-
-            cx.scene.pop_stacking_context();
         }
 
         cx.scene.pop_layer();
@@ -1291,14 +1298,20 @@ impl Element for EditorElement {
             } => self.scroll(*position, *delta, *precise, layout, paint, cx),
             Event::KeyDown { input, .. } => self.key_down(input.as_deref(), cx),
             Event::MouseMoved { position, .. } => {
-                if paint.text_bounds.contains_point(*position) {
+                let point = if paint.text_bounds.contains_point(*position) {
                     let (point, overshoot) =
                         paint.point_for_position(&self.snapshot(cx), layout, *position);
-                    cx.dispatch_action(Hover { point, overshoot });
-                    true
+                    if overshoot.is_zero() {
+                        Some(point)
+                    } else {
+                        None
+                    }
                 } else {
-                    false
-                }
+                    None
+                };
+
+                cx.dispatch_action(Hover { point });
+                true
             }
             _ => false,
         }

crates/project/src/lsp_command.rs 🔗

@@ -1,4 +1,4 @@
-use crate::{DocumentHighlight, Location, Project, ProjectTransaction, Hover};
+use crate::{DocumentHighlight, Hover, Location, Project, ProjectTransaction};
 use anyhow::{anyhow, Result};
 use async_trait::async_trait;
 use client::{proto, PeerId};
@@ -828,18 +828,16 @@ impl LspCommand for GetHover {
             let range = hover.range.map(|range| {
                 cx.read(|cx| {
                     let buffer = buffer.read(cx);
-                    let token_start = buffer
-                        .clip_point_utf16(point_from_lsp(range.start), Bias::Left);
-                    let token_end = buffer
-                        .clip_point_utf16(point_from_lsp(range.end), Bias::Left);
-                    buffer.anchor_after(token_start)..
-                        buffer.anchor_before(token_end)
+                    let token_start =
+                        buffer.clip_point_utf16(point_from_lsp(range.start), Bias::Left);
+                    let token_end = buffer.clip_point_utf16(point_from_lsp(range.end), Bias::Left);
+                    buffer.anchor_after(token_start)..buffer.anchor_before(token_end)
                 })
             });
-            
+
             Hover {
                 contents: hover.contents,
-                range
+                range,
             }
         }))
     }

styles/src/styleTree/hoverPopover.ts 🔗

@@ -3,7 +3,7 @@ import { backgroundColor, border, popoverShadow } from "./components";
 
 export default function HoverPopover(theme: Theme) {
   return {
-    background: backgroundColor(theme, 500),
+    background: backgroundColor(theme, "on500"),
     cornerRadius: 8,
     padding: {
       left: 8,
@@ -14,7 +14,7 @@ export default function HoverPopover(theme: Theme) {
     shadow: popoverShadow(theme),
     border: border(theme, "primary"),
     margin: {
-      left: -14,
+      left: -8,
     },
   }
 }