Add integration test and fix hovering over the wire

Keith Simmons created

Change summary

crates/collab/src/integration_tests.rs | 98 ++++++++++++++++++++++++++++
crates/collab/src/rpc.rs               |  1 
crates/editor/src/editor.rs            | 15 ++--
crates/editor/src/element.rs           | 83 ++++++++++++++---------
crates/project/src/lsp_command.rs      |  6 +
crates/project/src/project.rs          |  4 
styles/src/styleTree/contextMenu.ts    |  4 
styles/src/styleTree/tooltip.ts        |  4 
8 files changed, 165 insertions(+), 50 deletions(-)

Detailed changes

crates/collab/src/integration_tests.rs 🔗

@@ -2281,6 +2281,104 @@ async fn test_document_highlights(cx_a: &mut TestAppContext, cx_b: &mut TestAppC
     });
 }
 
+#[gpui::test(iterations = 10)]
+async fn test_lsp_hover(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) {
+    cx_a.foreground().forbid_parking();
+    let mut server = TestServer::start(cx_a.foreground(), cx_a.background()).await;
+    let client_a = server.create_client(cx_a, "user_a").await;
+    let client_b = server.create_client(cx_b, "user_b").await;
+    server
+        .make_contacts(vec![(&client_a, cx_a), (&client_b, cx_b)])
+        .await;
+
+    client_a
+        .fs
+        .insert_tree(
+            "/root-1",
+            json!({
+                "main.rs": "use std::collections::HashMap;",
+            }),
+        )
+        .await;
+
+    // Set up a fake language server.
+    let mut language = Language::new(
+        LanguageConfig {
+            name: "Rust".into(),
+            path_suffixes: vec!["rs".to_string()],
+            ..Default::default()
+        },
+        Some(tree_sitter_rust::language()),
+    );
+    let mut fake_language_servers = language.set_fake_lsp_adapter(Default::default());
+    client_a.language_registry.add(Arc::new(language));
+
+    let (project_a, worktree_id) = client_a.build_local_project("/root-1", cx_a).await;
+    let project_b = client_b.build_remote_project(&project_a, cx_a, cx_b).await;
+
+    // Open the file as the guest
+    let buffer_b = cx_b
+        .background()
+        .spawn(project_b.update(cx_b, |p, cx| p.open_buffer((worktree_id, "main.rs"), cx)))
+        .await
+        .unwrap();
+
+    // Request hover information as the guest.
+    let fake_language_server = fake_language_servers.next().await.unwrap();
+    fake_language_server.handle_request::<lsp::request::HoverRequest, _, _>(
+        |params, _| async move {
+            assert_eq!(
+                params
+                    .text_document_position_params
+                    .text_document
+                    .uri
+                    .as_str(),
+                "file:///root-1/main.rs"
+            );
+            assert_eq!(
+                params.text_document_position_params.position,
+                lsp::Position::new(0, 22)
+            );
+            Ok(Some(lsp::Hover {
+                contents: lsp::HoverContents::Array(vec![
+                    lsp::MarkedString::String("Test hover content.".to_string()),
+                    lsp::MarkedString::LanguageString(lsp::LanguageString {
+                        language: "Rust".to_string(),
+                        value: "let foo = 42;".to_string(),
+                    }),
+                ]),
+                range: Some(lsp::Range::new(
+                    lsp::Position::new(0, 22),
+                    lsp::Position::new(0, 29),
+                )),
+            }))
+        },
+    );
+
+    let hover_info = project_b
+        .update(cx_b, |p, cx| p.hover(&buffer_b, 22, cx))
+        .await
+        .unwrap()
+        .unwrap();
+    buffer_b.read_with(cx_b, |buffer, _| {
+        let snapshot = buffer.snapshot();
+        assert_eq!(hover_info.range.unwrap().to_offset(&snapshot), 22..29);
+        assert_eq!(
+            hover_info.contents,
+            vec![
+                project::HoverBlock {
+                    text: "Test hover content.".to_string(),
+                    language: None,
+                },
+                project::HoverBlock {
+                    text: "let foo = 42;".to_string(),
+                    language: Some("Rust".to_string()),
+                }
+            ]
+        );
+    });
+}
+
 #[gpui::test(iterations = 10)]
 async fn test_project_symbols(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) {
     cx_a.foreground().forbid_parking();

crates/collab/src/rpc.rs 🔗

@@ -150,6 +150,7 @@ impl Server {
             .add_message_handler(Server::start_language_server)
             .add_message_handler(Server::update_language_server)
             .add_message_handler(Server::update_diagnostic_summary)
+            .add_request_handler(Server::forward_project_request::<proto::GetHover>)
             .add_request_handler(Server::forward_project_request::<proto::GetDefinition>)
             .add_request_handler(Server::forward_project_request::<proto::GetReferences>)
             .add_request_handler(Server::forward_project_request::<proto::SearchProject>)

crates/editor/src/editor.rs 🔗

@@ -25,9 +25,9 @@ use gpui::{
     geometry::vector::{vec2f, Vector2F},
     impl_actions, impl_internal_actions,
     platform::CursorStyle,
-    text_layout, AppContext, AsyncAppContext, Axis, ClipboardItem, Element, ElementBox,
-    ElementStateContext, Entity, ModelHandle, MutableAppContext, ReadModel, RenderContext, Task,
-    View, ViewContext, ViewHandle, WeakViewHandle,
+    text_layout, AppContext, AsyncAppContext, Axis, ClipboardItem, Element, ElementBox, Entity,
+    ModelHandle, MutableAppContext, RenderContext, Task, View, ViewContext, ViewHandle,
+    WeakViewHandle,
 };
 pub use language::{char_kind, CharKind};
 use language::{
@@ -80,7 +80,7 @@ pub struct Scroll(pub Vector2F);
 #[derive(Clone, PartialEq)]
 pub struct Select(pub SelectPhase);
 
-#[derive(Clone)]
+#[derive(Clone, PartialEq)]
 pub struct HoverAt {
     point: Option<DisplayPoint>,
 }
@@ -215,7 +215,7 @@ impl_actions!(
     ]
 );
 
-impl_internal_actions!(editor, [Scroll, Select, HoverAt, GoToDefinitionAt]);
+impl_internal_actions!(editor, [Scroll, Select, HoverAt]);
 
 enum DocumentHighlightRead {}
 enum DocumentHighlightWrite {}
@@ -645,7 +645,6 @@ impl ContextMenu {
         match self {
             ContextMenu::Completions(menu) => (cursor_position, menu.render(style, cx)),
             ContextMenu::CodeActions(menu) => menu.render(cursor_position, style, cx),
-            ContextMenu::Hover(popover) => (cursor_position, popover.render(style)),
         }
     }
 }
@@ -899,10 +898,10 @@ pub(crate) struct HoverPopover {
 }
 
 impl HoverPopover {
-    fn render<C: ElementStateContext + ReadModel>(
+    fn render(
         &self,
         style: EditorStyle,
-        cx: &mut C,
+        cx: &mut RenderContext<Editor>,
     ) -> (DisplayPoint, ElementBox) {
         let element = MouseEventHandler::new::<HoverPopover, _, _>(0, cx, |_, cx| {
             let mut flex = Flex::new(Axis::Vertical).scrollable::<HoverBlock, _>(1, None, cx);

crates/editor/src/element.rs 🔗

@@ -5,7 +5,7 @@ use super::{
 };
 use crate::{
     display_map::{DisplaySnapshot, TransformBlock},
-    EditorStyle, GoToDefinition, HoverAt,
+    EditorStyle, HoverAt,
 };
 use clock::ReplicaId;
 use collections::{BTreeMap, HashMap};
@@ -102,7 +102,7 @@ impl EditorElement {
     fn mouse_down(
         &self,
         position: Vector2F,
-        cmd: bool,
+        _: bool,
         alt: bool,
         shift: bool,
         mut click_count: usize,
@@ -119,11 +119,7 @@ impl EditorElement {
         let snapshot = self.snapshot(cx.app);
         let (position, overshoot) = paint.point_for_position(&snapshot, layout, position);
 
-        if cmd {
-            cx.dispatch_action(GoToDefinitionAt {
-                location: Some(position),
-            });
-        } else if shift && alt {
+        if shift && alt {
             cx.dispatch_action(Select(SelectPhase::BeginColumnar {
                 position,
                 overshoot: overshoot.column(),
@@ -354,6 +350,7 @@ impl EditorElement {
         bounds: RectF,
         visible_bounds: RectF,
         layout: &mut LayoutState,
+        paint: &mut PaintState,
         cx: &mut PaintContext,
     ) {
         let view = self.view(cx.app);
@@ -533,6 +530,10 @@ impl EditorElement {
                 cx,
             );
 
+            paint.hover_bounds = Some(
+                RectF::new(popover_origin, hover_popover.size()).dilate(Vector2F::new(0., 5.)),
+            );
+
             cx.scene.pop_stacking_context();
         }
 
@@ -1109,6 +1110,7 @@ impl Element for EditorElement {
 
         let mut context_menu = None;
         let mut code_actions_indicator = None;
+        let mut hover = None;
         cx.render(&self.view.upgrade(cx).unwrap(), |view, cx| {
             let newest_selection_head = view
                 .selections
@@ -1127,6 +1129,17 @@ impl Element for EditorElement {
                     .render_code_actions_indicator(&style, cx)
                     .map(|indicator| (newest_selection_head.row(), indicator));
             }
+
+            hover = view.hover_popover().and_then(|hover| {
+                let (point, rendered) = hover.render(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));
+                    }
+                }
+
+                None
+            });
         });
 
         if let Some((_, context_menu)) = context_menu.as_mut() {
@@ -1149,27 +1162,18 @@ impl Element for EditorElement {
             );
         }
 
-        let hover = self.view(cx).hover_popover().and_then(|hover| {
-            let (point, mut rendered) = hover.render(style.clone(), cx);
-
-            if point.row() >= snapshot.scroll_position().y() as u32 {
-                if line_layouts.len() > (point.row() - start_row) as usize {
-                    rendered.layout(
-                        SizeConstraint {
-                            min: Vector2F::zero(),
-                            max: vec2f(
-                                (120. * em_width).min(size.x()),
-                                (size.y() - line_height) * 1. / 2.,
-                            ),
-                        },
-                        cx,
-                    );
-                    return Some((point, rendered));
-                }
-            }
-
-            None
-        });
+        if let Some((_, hover)) = hover.as_mut() {
+            hover.layout(
+                SizeConstraint {
+                    min: Vector2F::zero(),
+                    max: vec2f(
+                        (120. * em_width).min(size.x()),
+                        (size.y() - line_height) * 1. / 2.,
+                    ),
+                },
+                cx,
+            );
+        }
 
         let blocks = self.layout_blocks(
             start_row..end_row,
@@ -1227,11 +1231,18 @@ impl Element for EditorElement {
             layout.text_size,
         );
 
+        let mut paint_state = PaintState {
+            bounds,
+            gutter_bounds,
+            text_bounds,
+            hover_bounds: None,
+        };
+
         self.paint_background(gutter_bounds, text_bounds, layout, cx);
         if layout.gutter_size.x() > 0. {
             self.paint_gutter(gutter_bounds, visible_bounds, layout, cx);
         }
-        self.paint_text(text_bounds, visible_bounds, layout, cx);
+        self.paint_text(text_bounds, visible_bounds, layout, &mut paint_state, cx);
 
         if !layout.blocks.is_empty() {
             cx.scene.push_layer(Some(bounds));
@@ -1241,11 +1252,7 @@ impl Element for EditorElement {
 
         cx.scene.pop_layer();
 
-        PaintState {
-            bounds,
-            gutter_bounds,
-            text_bounds,
-        }
+        paint_state
     }
 
     fn dispatch_event(
@@ -1310,6 +1317,13 @@ 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
+                    .hover_bounds
+                    .map_or(false, |hover_bounds| hover_bounds.contains_point(*position))
+                {
+                    return false;
+                }
+
                 let point = if paint.text_bounds.contains_point(*position) {
                     let (point, overshoot) =
                         paint.point_for_position(&self.snapshot(cx), layout, *position);
@@ -1401,6 +1415,7 @@ pub struct PaintState {
     bounds: RectF,
     gutter_bounds: RectF,
     text_bounds: RectF,
+    hover_bounds: Option<RectF>,
 }
 
 impl PaintState {

crates/project/src/lsp_command.rs 🔗

@@ -857,6 +857,9 @@ impl LspCommand for GetHover {
                     let mut current_text = String::new();
                     for event in Parser::new_ext(&markup_content.value, Options::all()) {
                         match event {
+                            Event::SoftBreak => {
+                                current_text.push(' ');
+                            }
                             Event::Text(text) | Event::Code(text) => {
                                 current_text.push_str(&text.to_string());
                             }
@@ -875,7 +878,8 @@ impl LspCommand for GetHover {
                             Event::End(Tag::CodeBlock(_))
                             | Event::End(Tag::Paragraph)
                             | Event::End(Tag::Heading(_, _, _))
-                            | Event::End(Tag::BlockQuote) => {
+                            | Event::End(Tag::BlockQuote)
+                            | Event::HardBreak => {
                                 if !current_text.is_empty() {
                                     let text = std::mem::replace(&mut current_text, String::new());
                                     contents.push(HoverBlock { text, language });

crates/project/src/project.rs 🔗

@@ -219,7 +219,7 @@ pub struct Symbol {
     pub signature: [u8; 32],
 }
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialEq)]
 pub struct HoverBlock {
     pub text: String,
     pub language: Option<String>,
@@ -3766,10 +3766,8 @@ impl Project {
         } else if let Some(project_id) = self.remote_id() {
             let rpc = self.client.clone();
             let message = request.to_proto(project_id, buffer);
-            dbg!(&message);
             return cx.spawn(|this, cx| async move {
                 let response = rpc.request(message).await?;
-                dbg!(&response);
                 request
                     .response_from_proto(response, this, buffer_handle, cx)
                     .await

styles/src/styleTree/contextMenu.ts 🔗

@@ -1,12 +1,12 @@
 import Theme from "../themes/common/theme";
-import { backgroundColor, border, borderColor, shadow, text } from "./components";
+import { backgroundColor, border, borderColor, popoverShadow, text } from "./components";
 
 export default function contextMenu(theme: Theme) {
   return {
     background: backgroundColor(theme, 300, "base"),
     cornerRadius: 6,
     padding: 6,
-    shadow: shadow(theme),
+    shadow: popoverShadow(theme),
     border: border(theme, "primary"),
     item: {
       padding: { left: 4, right: 4, top: 2, bottom: 2 },

styles/src/styleTree/tooltip.ts 🔗

@@ -1,5 +1,5 @@
 import Theme from "../themes/common/theme";
-import { backgroundColor, border, shadow, text } from "./components";
+import { backgroundColor, border, popoverShadow, text } from "./components";
 
 export default function tooltip(theme: Theme) {
   return {
@@ -7,7 +7,7 @@ export default function tooltip(theme: Theme) {
     border: border(theme, "secondary"),
     padding: { top: 4, bottom: 4, left: 8, right: 8 },
     margin: { top: 6, left: 6 },
-    shadow: shadow(theme),
+    shadow: popoverShadow(theme),
     cornerRadius: 6,
     text: text(theme, "sans", "secondary", { size: "xs", weight: "bold" }),
     keystroke: {