From dddfc7beae080766492c4755fd933070bd1be881 Mon Sep 17 00:00:00 2001 From: Keith Simmons Date: Thu, 9 Jun 2022 10:53:58 -0700 Subject: [PATCH] Add hover test and tweak dismiss logic --- crates/editor/src/editor.rs | 191 ++++++++++++++++++++++++++++- crates/editor/src/hover_popover.rs | 62 ++++++---- crates/editor/src/test.rs | 168 ++++++++++++++++++++++++- crates/gpui/src/executor.rs | 13 ++ crates/language/src/language.rs | 4 + 5 files changed, 410 insertions(+), 28 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 6ec8852888..17fdcc49f0 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -6147,8 +6147,15 @@ pub fn styled_runs_for_code_label<'a>( #[cfg(test)] mod tests { - use crate::test::{ - assert_text_with_selections, build_editor, select_ranges, EditorTestContext, + use crate::{ + hover_popover::{ + hover, hover_at, HoverAt, HOVER_DELAY_MILLIS, HOVER_GRACE_MILLIS, + HOVER_REQUEST_DELAY_MILLIS, + }, + test::{ + assert_text_with_selections, build_editor, select_ranges, EditorLspTestContext, + EditorTestContext, + }, }; use super::*; @@ -6159,7 +6166,7 @@ mod tests { use indoc::indoc; use language::{FakeLspAdapter, LanguageConfig}; use lsp::FakeLanguageServer; - use project::FakeFs; + use project::{FakeFs, HoverBlock}; use settings::LanguageOverride; use smol::stream::StreamExt; use std::{cell::RefCell, rc::Rc, time::Instant}; @@ -9391,6 +9398,184 @@ 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!]();"}); + cx.handle_request::(move |_| { + 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), + }) + }) + .await; + cx.foreground() + .advance_clock(Duration::from_millis(HOVER_DELAY_MILLIS + 100)); + + 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.handle_request::(move |_| None) + .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.handle_request::(move |_| { + 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), + }) + }) + .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.handle_request::(move |_| { + 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), + }) + }) + .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))); diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index c883ff2e45..f4b4c3414c 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -19,6 +19,10 @@ use crate::{ EditorStyle, }; +pub const HOVER_DELAY_MILLIS: u64 = 500; +pub const HOVER_REQUEST_DELAY_MILLIS: u64 = 250; +pub const HOVER_GRACE_MILLIS: u64 = 250; + #[derive(Clone, PartialEq)] pub struct HoverAt { pub point: Option, @@ -33,14 +37,14 @@ pub fn init(cx: &mut MutableAppContext) { } /// Bindable action which uses the most recent selection head to trigger a hover -fn hover(editor: &mut Editor, _: &Hover, cx: &mut ViewContext) { +pub fn hover(editor: &mut Editor, _: &Hover, cx: &mut ViewContext) { 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) { +pub fn hover_at(editor: &mut Editor, action: &HoverAt, cx: &mut ViewContext) { if let Some(point) = action.point { show_hover(editor, point, false, cx); } else { @@ -57,12 +61,12 @@ pub fn hide_hover(editor: &mut Editor, cx: &mut ViewContext) -> bool { // 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; + editor.hover_state.hidden_at = Some(cx.background().now()); did_hide = true; cx.notify(); } editor.hover_state.task = None; + editor.hover_state.symbol_range = None; editor.clear_background_highlights::(cx); @@ -118,13 +122,13 @@ fn show_hover( // 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 + let should_delay = editor.hover_state.popover.is_none() // Hover not 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 + .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 { @@ -145,8 +149,18 @@ 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 { - Some(cx.background().timer(Duration::from_millis(500))) + // Construct delay task to wait for later + let total_delay = Some( + cx.background() + .timer(Duration::from_millis(HOVER_DELAY_MILLIS)), + ); + + cx.background() + .timer(Duration::from_millis(HOVER_REQUEST_DELAY_MILLIS)) + .await; + total_delay } else { None }; @@ -157,6 +171,8 @@ fn show_hover( return None; } + // Create symbol range of anchors for highlighting and filtering + // of future requests. let range = if let Some(range) = hover_result.range { let start = snapshot .buffer_snapshot @@ -189,7 +205,7 @@ fn show_hover( if let Some(this) = this.upgrade(&cx) { this.update(&mut cx, |this, cx| { - if this.hover_state.popover.is_some() || hover_popover.is_some() { + if 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::( @@ -198,18 +214,16 @@ fn show_hover( cx, ); } - this.hover_state.popover = hover_popover; - - if this.hover_state.popover.is_none() { - this.hover_state.hidden_at = Some(Instant::now()); - } - cx.notify(); - } - - if this.hover_state.popover.is_none() { - this.hover_state.symbol_range = None; + } else { + if this.hover_state.popover.is_some() { + // Popover was visible, but now is hidden. Dismiss it + hide_hover(this, cx); + } else { + // Clear selected symbol range for future requests + this.hover_state.symbol_range = None; + } } }); } @@ -229,7 +243,13 @@ pub struct HoverState { pub task: Option>>, } -#[derive(Clone)] +impl HoverState { + pub fn visible(&self) -> bool { + self.popover.is_some() + } +} + +#[derive(Debug, Clone)] pub struct HoverPopover { pub project: ModelHandle, pub anchor: Anchor, diff --git a/crates/editor/src/test.rs b/crates/editor/src/test.rs index 0f49576936..2222906cd4 100644 --- a/crates/editor/src/test.rs +++ b/crates/editor/src/test.rs @@ -1,10 +1,16 @@ -use std::ops::{Deref, DerefMut, Range}; +use std::{ + ops::{Deref, DerefMut, Range}, + sync::Arc, +}; +use futures::StreamExt; use indoc::indoc; use collections::BTreeMap; -use gpui::{keymap::Keystroke, ModelHandle, ViewContext, ViewHandle}; -use language::Selection; +use gpui::{keymap::Keystroke, AppContext, ModelHandle, ViewContext, ViewHandle}; +use language::{point_to_lsp, FakeLspAdapter, Language, LanguageConfig, Selection}; +use lsp::request; +use project::{FakeFs, Project}; use settings::Settings; use util::{ set_eq, @@ -13,7 +19,8 @@ use util::{ use crate::{ display_map::{DisplayMap, DisplaySnapshot, ToDisplayPoint}, - Autoscroll, DisplayPoint, Editor, EditorMode, MultiBuffer, + multi_buffer::ToPointUtf16, + Autoscroll, DisplayPoint, Editor, EditorMode, MultiBuffer, ToPoint, }; #[cfg(test)] @@ -102,6 +109,13 @@ impl<'a> EditorTestContext<'a> { } } + pub fn editor(&mut self, read: F) -> T + where + F: FnOnce(&Editor, &AppContext) -> T, + { + self.editor.read_with(self.cx, read) + } + pub fn update_editor(&mut self, update: F) -> T where F: FnOnce(&mut Editor, &mut ViewContext) -> T, @@ -132,6 +146,14 @@ impl<'a> EditorTestContext<'a> { } } + pub fn display_point(&mut self, cursor_location: &str) -> DisplayPoint { + let (_, locations) = marked_text(cursor_location); + let snapshot = self + .editor + .update(self.cx, |editor, cx| editor.snapshot(cx)); + locations[0].to_display_point(&snapshot.display_snapshot) + } + // Sets the editor state via a marked string. // `|` characters represent empty selections // `[` to `}` represents a non empty selection with the head at `}` @@ -365,3 +387,141 @@ impl<'a> DerefMut for EditorTestContext<'a> { &mut self.cx } } + +pub struct EditorLspTestContext<'a> { + pub cx: EditorTestContext<'a>, + lsp: lsp::FakeLanguageServer, +} + +impl<'a> EditorLspTestContext<'a> { + pub async fn new( + mut language: Language, + capabilities: lsp::ServerCapabilities, + cx: &'a mut gpui::TestAppContext, + ) -> EditorLspTestContext<'a> { + let file_name = format!( + "/file.{}", + language + .path_suffixes() + .first() + .unwrap_or(&"txt".to_string()) + ); + + let mut fake_servers = language.set_fake_lsp_adapter(FakeLspAdapter { + capabilities, + ..Default::default() + }); + + let fs = FakeFs::new(cx.background().clone()); + fs.insert_file(file_name.clone(), "".to_string()).await; + + let project = Project::test(fs, [file_name.as_ref()], cx).await; + project.update(cx, |project, _| project.languages().add(Arc::new(language))); + let buffer = project + .update(cx, |project, cx| project.open_local_buffer(file_name, cx)) + .await + .unwrap(); + + let (window_id, editor) = cx.update(|cx| { + cx.set_global(Settings::test(cx)); + crate::init(cx); + + let (window_id, editor) = cx.add_window(Default::default(), |cx| { + let buffer = cx.add_model(|cx| MultiBuffer::singleton(buffer, cx)); + + Editor::new(EditorMode::Full, buffer, Some(project), None, None, cx) + }); + + editor.update(cx, |_, cx| cx.focus_self()); + + (window_id, editor) + }); + + let lsp = fake_servers.next().await.unwrap(); + + Self { + cx: EditorTestContext { + cx, + window_id, + editor, + }, + lsp, + } + } + + pub async fn new_rust( + capabilities: lsp::ServerCapabilities, + cx: &'a mut gpui::TestAppContext, + ) -> EditorLspTestContext<'a> { + let language = Language::new( + LanguageConfig { + name: "Rust".into(), + path_suffixes: vec!["rs".to_string()], + ..Default::default() + }, + Some(tree_sitter_rust::language()), + ); + + Self::new(language, capabilities, cx).await + } + + pub async fn handle_request(&mut self, mut construct_result: F) + where + T: 'static + request::Request, + T::Params: 'static + Send, + T::Result: 'static + Send + Clone, + F: 'static + Send + FnMut(T::Params) -> T::Result, + { + self.lsp + .handle_request::(move |params, _| { + let result = construct_result(params); + async move { Ok(result.clone()) } + }) + .next() + .await; + } + + // Constructs lsp range using a marked string with '[', ']' range delimiters + pub fn lsp_range(&mut self, marked_text: &str) -> lsp::Range { + let (unmarked, mut ranges) = marked_text_ranges_by(marked_text, vec![('[', ']').into()]); + assert_eq!(unmarked, self.editor_text()); + let snapshot = self.update_editor(|editor, cx| editor.snapshot(cx)); + + let offset_range = ranges.remove(&('[', ']').into()).unwrap()[0].clone(); + let start_point = offset_range.start.to_point(&snapshot.buffer_snapshot); + let end_point = offset_range.end.to_point(&snapshot.buffer_snapshot); + self.editor(|editor, cx| { + let buffer = editor.buffer().read(cx); + let start = point_to_lsp( + buffer + .point_to_buffer_offset(start_point, cx) + .unwrap() + .1 + .to_point_utf16(&buffer.read(cx)), + ); + let end = point_to_lsp( + buffer + .point_to_buffer_offset(end_point, cx) + .unwrap() + .1 + .to_point_utf16(&buffer.read(cx)), + ); + + lsp::Range { start, end } + }) + } +} + +impl<'a> Deref for EditorLspTestContext<'a> { + type Target = EditorTestContext<'a>; + + fn deref(&self) -> &Self::Target { + &self.cx + } +} + +impl<'a> DerefMut for EditorLspTestContext<'a> { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.cx + } +} diff --git a/crates/gpui/src/executor.rs b/crates/gpui/src/executor.rs index 3126e18c7a..1bc8d61c44 100644 --- a/crates/gpui/src/executor.rs +++ b/crates/gpui/src/executor.rs @@ -330,6 +330,11 @@ impl Deterministic { Timer::Deterministic(DeterministicTimer { rx, id, state }) } + pub fn now(&self) -> std::time::Instant { + let state = self.state.lock(); + state.now.clone() + } + pub fn advance_clock(&self, duration: Duration) { let new_now = self.state.lock().now + duration; loop { @@ -647,6 +652,14 @@ impl Background { } } + pub fn now(&self) -> std::time::Instant { + match self { + Background::Production { .. } => std::time::Instant::now(), + #[cfg(any(test, feature = "test-support"))] + Background::Deterministic { executor } => executor.now(), + } + } + #[cfg(any(test, feature = "test-support"))] pub async fn simulate_random_delay(&self) { use rand::prelude::*; diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index cebe666da0..c8ce904a42 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -571,6 +571,10 @@ impl Language { &self.config.brackets } + pub fn path_suffixes(&self) -> &[String] { + &self.config.path_suffixes + } + pub fn should_autoclose_before(&self, c: char) -> bool { c.is_whitespace() || self.config.autoclose_before.contains(c) }