Move highlighting to editor code and implement proto message types for hover response

crates/editor/src/editor.rs

@@ -39,7 +39,7 @@ pub use multi_buffer::{
     Anchor, AnchorRangeExt, ExcerptId, MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint,
 };
 use ordered_float::OrderedFloat;
-use project::{HoverContents, Project, ProjectTransaction};
+use project::{HoverBlock, Project, ProjectTransaction};
 use selections_collection::{resolve_multiple, MutableSelectionsCollection, SelectionsCollection};
 use serde::{Deserialize, Serialize};
 use settings::Settings;
@@ -427,7 +427,7 @@ pub struct Editor {
     keymap_context_layers: BTreeMap<TypeId, gpui::keymap::Context>,
     input_enabled: bool,
     leader_replica_id: Option<u16>,
-    hover_popover: HoverState,
+    hover_state: HoverState,
 }
 
 /// Keeps track of the state of the [`HoverPopover`].
@@ -457,6 +457,10 @@ impl HoverState {
 
         return (recent_hover, in_grace);
     }
+
+    pub fn close(&mut self) {
+        self.popover.take();
+    }
 }
 
 pub struct EditorSnapshot {
@@ -885,20 +889,26 @@ impl CodeActionsMenu {
 }
 
 struct HoverPopover {
-    pub point: DisplayPoint,
+    pub range: Range<DisplayPoint>,
-    pub contents: Vec<HoverContents>,
+    pub contents: Vec<HoverBlock>,
 }
 
 impl HoverPopover {
-    fn render(&self, style: EditorStyle) -> (DisplayPoint, ElementBox) {
+    fn render(&self, style: EditorStyle, project: &Project) -> (DisplayPoint, ElementBox) {
         let mut flex = Flex::new(Axis::Vertical);
         flex.extend(self.contents.iter().map(|content| {
+            if let Some(language) = content
+                .language
+                .clone()
+                .and_then(|language| project.languages().get_language(&language))
+            {
+                let runs =
+                    language.highlight_text(&content.text.as_str().into(), 0..content.text.len());
+
                 Text::new(content.text.clone(), style.text.clone())
                     .with_soft_wrap(true)
                     .with_highlights(
-                    content
+                        runs.iter()
-                        .runs
-                        .iter()
                             .filter_map(|(range, id)| {
                                 id.style(style.theme.syntax.as_ref())
                                     .map(|style| (range.clone(), style))
@@ -906,10 +916,19 @@ impl HoverPopover {
                             .collect(),
                     )
                     .boxed()
+            } else {
+                Text::new(content.text.clone(), style.hover_popover.prose.clone())
+                    .with_soft_wrap(true)
+                    .contained()
+                    .with_style(style.hover_popover.block_style)
+                    .boxed()
+            }
         }));
         (
-            self.point,
+            self.range.start,
-            flex.contained().with_style(style.hover_popover).boxed(),
+            flex.contained()
+                .with_style(style.hover_popover.container)
+                .boxed(),
         )
     }
 }
@@ -1080,7 +1099,7 @@ impl Editor {
             keymap_context_layers: Default::default(),
             input_enabled: true,
             leader_replica_id: None,
-            hover_popover: HoverState {
+            hover_state: HoverState {
                 popover: None,
                 last_hover: std::time::Instant::now(),
                 start_grace: std::time::Instant::now(),
@@ -1470,6 +1489,8 @@ impl Editor {
                 }
             }
 
+            self.hover_state.close();
+
             if old_cursor_position.to_display_point(&display_map).row()
                 != new_cursor_position.to_display_point(&display_map).row()
             {
@@ -2477,11 +2498,11 @@ impl Editor {
                 if let Some(this) = this.upgrade(&cx) {
                     this.update(&mut cx, |this, cx| {
                         // consistently keep track of state to make handoff smooth
-                        let (_recent_hover, _in_grace) = this.hover_popover.determine_state(false);
+                        let (_recent_hover, _in_grace) = this.hover_state.determine_state(false);
 
                         // only notify the context once
-                        if this.hover_popover.popover.is_some() {
+                        if this.hover_state.popover.is_some() {
-                            this.hover_popover.popover = None;
+                            this.hover_state.popover = None;
                             cx.notify();
                         }
                     });
@@ -2497,17 +2518,11 @@ impl Editor {
     /// 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, mut point: DisplayPoint, cx: &mut ViewContext<Self>) {
+    fn show_hover(&mut self, point: DisplayPoint, cx: &mut ViewContext<Self>) {
         if self.pending_rename.is_some() {
             return;
         }
 
-        let project = if let Some(project) = self.project.clone() {
-            project
-        } else {
-            return;
-        };
-
         let snapshot = self.snapshot(cx);
         let (buffer, buffer_position) = if let Some(output) = self
             .buffer
@@ -2521,6 +2536,19 @@ impl Editor {
 
         let buffer_snapshot = buffer.read(cx).snapshot();
 
+        if let Some(existing_popover) = &self.hover_state.popover {
+            if existing_popover.range.contains(&point) {
+                // Hover already contains value. No need to request a new one
+                return;
+            }
+        }
+
+        let project = if let Some(project) = self.project.clone() {
+            project
+        } else {
+            return;
+        };
+
         // query the LSP for hover info
         let hover = project.update(cx, |project, cx| {
             project.hover(&buffer, buffer_position.clone(), cx)
@@ -2535,6 +2563,8 @@ impl Editor {
                     Err(_) => None,
                 };
 
+                let mut symbol_range = point..point;
+
                 // determine the contents of the popover
                 if let Some(hover) = hover {
                     if hover.contents.is_empty() {
@@ -2547,8 +2577,11 @@ impl Editor {
                             if offset_range
                                 .contains(&point.to_offset(&snapshot.display_snapshot, Bias::Left))
                             {
-                                point = offset_range
+                                symbol_range = offset_range
                                     .start
+                                    .to_display_point(&snapshot.display_snapshot)
+                                    ..offset_range
+                                        .end
                                         .to_display_point(&snapshot.display_snapshot);
                             } else {
                                 contents = None;
@@ -2557,7 +2590,10 @@ impl Editor {
                     }
                 };
 
-                let hover_popover = contents.map(|contents| HoverPopover { point, contents });
+                let hover_popover = contents.map(|contents| HoverPopover {
+                    range: symbol_range,
+                    contents,
+                });
 
                 if let Some(this) = this.upgrade(&cx) {
                     this.update(&mut cx, |this, cx| {
@@ -2569,16 +2605,15 @@ impl Editor {
                         //    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_popover.determine_state(hover_popover.is_some());
+                            this.hover_state.determine_state(hover_popover.is_some());
                         let smooth_handoff =
-                            this.hover_popover.popover.is_some() && hover_popover.is_some();
+                            this.hover_state.popover.is_some() && hover_popover.is_some();
-                        let visible =
+                        let visible = this.hover_state.popover.is_some() || hover_popover.is_some();
-                            this.hover_popover.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.
                         if (smooth_handoff || !recent_hover || in_grace) && visible {
-                            this.hover_popover.popover = hover_popover;
+                            this.hover_state.popover = hover_popover;
                             cx.notify();
                         }
                     });
@@ -2837,11 +2872,15 @@ impl Editor {
             .map(|menu| menu.render(cursor_position, style))
     }
 
-    pub fn render_hover_popover(&self, style: EditorStyle) -> Option<(DisplayPoint, ElementBox)> {
+    pub fn render_hover_popover(
-        self.hover_popover
+        &self,
+        style: EditorStyle,
+        project: &Project,
+    ) -> Option<(DisplayPoint, ElementBox)> {
+        self.hover_state
             .popover
             .as_ref()
-            .map(|hover| hover.render(style))
+            .map(|hover| hover.render(style, project))
     }
 
     fn show_context_menu(&mut self, menu: ContextMenu, cx: &mut ViewContext<Self>) {

crates/editor/src/element.rs

@@ -495,7 +495,7 @@ impl EditorElement {
             let mut list_origin = content_origin + vec2f(x, y);
             let list_height = context_menu.size().y();
 
-            if list_origin.y() + list_height > bounds.lower_left().y() {
+            if list_origin.y() + list_height > bounds.max_y() {
                 list_origin.set_y(list_origin.y() - layout.line_height - list_height);
             }
 
@@ -516,14 +516,18 @@ impl EditorElement {
                 {
                     cx.scene.push_stacking_context(None);
 
+                    let size = hover_popover.size();
                     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 y = position.row() as f32 * layout.line_height - scroll_top - size.y();
                     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() {
+                    if popover_origin.y() < 0.0 {
-                        popover_origin
+                        popover_origin.set_y(popover_origin.y() + layout.line_height + size.y());
-                            .set_y(popover_origin.y() - layout.line_height - popover_height);
+                    }
+
+                    let x_out_of_bounds = bounds.max_x() - (popover_origin.x() + size.x());
+                    if x_out_of_bounds < 0.0 {
+                        popover_origin.set_x(popover_origin.x() + x_out_of_bounds);
                     }
 
                     hover_popover.paint(
@@ -1129,7 +1133,10 @@ impl Element for EditorElement {
                     .map(|indicator| (newest_selection_head.row(), indicator));
             }
 
-            hover = view.render_hover_popover(style);
+            if let Some(project) = view.project.clone() {
+                let project = project.read(cx);
+                hover = view.render_hover_popover(style, project);
+            }
         });
 
         if let Some((_, context_menu)) = context_menu.as_mut() {

crates/project/src/lsp_command.rs

@@ -1,4 +1,4 @@
-use crate::{DocumentHighlight, Hover, HoverContents, Location, Project, ProjectTransaction};
+use crate::{DocumentHighlight, Hover, HoverBlock, Location, Project, ProjectTransaction};
 use anyhow::{anyhow, Result};
 use async_trait::async_trait;
 use client::{proto, PeerId};
@@ -8,7 +8,7 @@ use language::{
     proto::{deserialize_anchor, deserialize_version, serialize_anchor, serialize_version},
     range_from_lsp, Anchor, Bias, Buffer, PointUtf16, ToPointUtf16,
 };
-use lsp::{DocumentHighlightKind, LanguageString, MarkedString, ServerCapabilities};
+use lsp::{DocumentHighlightKind, ServerCapabilities};
 use pulldown_cmark::{CodeBlockKind, Event, Options, Parser, Tag};
 use std::{cmp::Reverse, ops::Range, path::Path};
 
@@ -821,7 +821,7 @@ impl LspCommand for GetHover {
     async fn response_from_lsp(
         self,
         message: Option<lsp::Hover>,
-        project: ModelHandle<Project>,
+        _: ModelHandle<Project>,
         buffer: ModelHandle<Buffer>,
         mut cx: AsyncAppContext,
     ) -> Result<Self::Response> {
@@ -836,55 +836,14 @@ impl LspCommand for GetHover {
                 })
             });
 
-            fn text_and_language(marked_string: MarkedString) -> (String, Option<String>) {
+            let contents = cx.read(|_| match hover.contents {
-                match marked_string {
-                    MarkedString::LanguageString(LanguageString { language, value }) => {
-                        (value, Some(language))
-                    }
-                    MarkedString::String(text) => (text, None),
-                }
-            }
-
-            fn highlight(
-                text: String,
-                language: Option<String>,
-                project: &Project,
-            ) -> Option<HoverContents> {
-                let text = text.trim();
-                if text.is_empty() {
-                    return None;
-                }
-
-                if let Some(language) =
-                    language.and_then(|language| project.languages().get_language(&language))
-                {
-                    let runs = language.highlight_text(&text.into(), 0..text.len());
-                    Some(HoverContents {
-                        text: text.to_string(),
-                        runs,
-                    })
-                } else {
-                    Some(HoverContents {
-                        text: text.to_string(),
-                        runs: Vec::new(),
-                    })
-                }
-            }
-
-            let contents = cx.read(|cx| {
-                let project = project.read(cx);
-                match hover.contents {
                 lsp::HoverContents::Scalar(marked_string) => {
-                        let (text, language) = text_and_language(marked_string);
+                    HoverBlock::try_new(marked_string).map(|contents| vec![contents])
-                        highlight(text, language, project).map(|content| vec![content])
                 }
                 lsp::HoverContents::Array(marked_strings) => {
-                        let content: Vec<HoverContents> = marked_strings
+                    let content: Vec<HoverBlock> = marked_strings
                         .into_iter()
-                            .filter_map(|marked_string| {
+                        .filter_map(|marked_string| HoverBlock::try_new(marked_string))
-                                let (text, language) = text_and_language(marked_string);
-                                highlight(text, language, project)
-                            })
                         .collect();
                     if content.is_empty() {
                         None
@@ -901,14 +860,10 @@ impl LspCommand for GetHover {
                             Event::Text(text) | Event::Code(text) => {
                                 current_text.push_str(&text.to_string());
                             }
-                                Event::Start(Tag::CodeBlock(CodeBlockKind::Fenced(
+                            Event::Start(Tag::CodeBlock(CodeBlockKind::Fenced(new_language))) => {
-                                    new_language,
+                                if !current_text.is_empty() {
-                                ))) => {
+                                    let text = std::mem::replace(&mut current_text, String::new());
-                                    if let Some(content) =
+                                    contents.push(HoverBlock { text, language });
-                                        highlight(current_text.clone(), language, project)
-                                    {
-                                        contents.push(content);
-                                        current_text.clear();
                                 }
 
                                 language = if new_language.is_empty() {
@@ -917,23 +872,26 @@ impl LspCommand for GetHover {
                                     Some(new_language.to_string())
                                 };
                             }
-                                Event::End(Tag::CodeBlock(_)) => {
+                            Event::End(Tag::CodeBlock(_))
-                                    if let Some(content) =
+                            | Event::End(Tag::Paragraph)
-                                        highlight(current_text.clone(), language.clone(), project)
+                            | Event::End(Tag::Heading(_, _, _))
-                                    {
+                            | Event::End(Tag::BlockQuote) => {
-                                        contents.push(content);
+                                if !current_text.is_empty() {
+                                    let text = std::mem::replace(&mut current_text, String::new());
+                                    contents.push(HoverBlock { text, language });
                                     current_text.clear();
-                                        language = None;
                                 }
+                                language = None;
                             }
                             _ => {}
                         }
                     }
 
-                        if let Some(content) =
+                    if !current_text.is_empty() {
-                            highlight(current_text.clone(), language.clone(), project)
+                        contents.push(HoverBlock {
-                        {
+                            text: current_text,
-                            contents.push(content);
+                            language,
+                        });
                     }
 
                     if contents.is_empty() {
@@ -942,7 +900,6 @@ impl LspCommand for GetHover {
                         Some(contents)
                     }
                 }
-                }
             });
 
             contents.map(|contents| Hover { contents, range })
@@ -982,22 +939,72 @@ impl LspCommand for GetHover {
 
     fn response_to_proto(
         response: Self::Response,
-        project: &mut Project,
+        _: &mut Project,
-        peer_id: PeerId,
+        _: PeerId,
-        buffer_version: &clock::Global,
+        _: &clock::Global,
-        cx: &AppContext,
+        _: &AppContext,
     ) -> proto::GetHoverResponse {
-        todo!()
+        if let Some(response) = response {
+            let (start, end) = if let Some(range) = response.range {
+                (
+                    Some(language::proto::serialize_anchor(&range.start)),
+                    Some(language::proto::serialize_anchor(&range.end)),
+                )
+            } else {
+                (None, None)
+            };
+
+            let contents = response
+                .contents
+                .into_iter()
+                .map(|block| proto::HoverBlock {
+                    text: block.text,
+                    language: block.language,
+                })
+                .collect();
+
+            proto::GetHoverResponse {
+                start,
+                end,
+                contents,
+            }
+        } else {
+            proto::GetHoverResponse {
+                start: None,
+                end: None,
+                contents: Vec::new(),
+            }
+        }
     }
 
     async fn response_from_proto(
         self,
-        message: <Self::ProtoRequest as proto::RequestMessage>::Response,
+        message: proto::GetHoverResponse,
-        project: ModelHandle<Project>,
+        _: ModelHandle<Project>,
-        buffer: ModelHandle<Buffer>,
+        _: ModelHandle<Buffer>,
-        cx: AsyncAppContext,
+        _: AsyncAppContext,
     ) -> Result<Self::Response> {
-        todo!()
+        let range = if let (Some(start), Some(end)) = (message.start, message.end) {
+            language::proto::deserialize_anchor(start)
+                .and_then(|start| language::proto::deserialize_anchor(end).map(|end| start..end))
+        } else {
+            None
+        };
+
+        let contents: Vec<_> = message
+            .contents
+            .into_iter()
+            .map(|block| HoverBlock {
+                text: block.text,
+                language: block.language,
+            })
+            .collect();
+
+        Ok(if contents.is_empty() {
+            None
+        } else {
+            Some(Hover { contents, range })
+        })
     }
 
     fn buffer_id_from_proto(message: &Self::ProtoRequest) -> u64 {

crates/project/src/project.rs

@@ -19,11 +19,14 @@ use language::{
     point_to_lsp,
     proto::{deserialize_anchor, deserialize_version, serialize_anchor, serialize_version},
     range_from_lsp, range_to_lsp, Anchor, Bias, Buffer, CodeAction, CodeLabel, Completion,
-    Diagnostic, DiagnosticEntry, DiagnosticSet, Event as BufferEvent, File as _, HighlightId,
+    Diagnostic, DiagnosticEntry, DiagnosticSet, Event as BufferEvent, File as _, Language,
-    Language, LanguageRegistry, LanguageServerName, LocalFile, LspAdapter, OffsetRangeExt,
+    LanguageRegistry, LanguageServerName, LocalFile, LspAdapter, OffsetRangeExt, Operation, Patch,
-    Operation, Patch, PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, Transaction,
+    PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, Transaction,
+};
+use lsp::{
+    DiagnosticSeverity, DiagnosticTag, DocumentHighlightKind, LanguageServer, LanguageString,
+    MarkedString,
 };
-use lsp::{DiagnosticSeverity, DiagnosticTag, DocumentHighlightKind, LanguageServer};
 use lsp_command::*;
 use parking_lot::Mutex;
 use postage::stream::Stream;
@@ -217,14 +220,34 @@ pub struct Symbol {
 }
 
 #[derive(Debug)]
-pub struct HoverContents {
+pub struct HoverBlock {
     pub text: String,
-    pub runs: Vec<(Range<usize>, HighlightId)>,
+    pub language: Option<String>,
+}
+
+impl HoverBlock {
+    fn try_new(marked_string: MarkedString) -> Option<Self> {
+        let result = match marked_string {
+            MarkedString::LanguageString(LanguageString { language, value }) => HoverBlock {
+                text: value,
+                language: Some(language),
+            },
+            MarkedString::String(text) => HoverBlock {
+                text,
+                language: None,
+            },
+        };
+        if result.text.is_empty() {
+            None
+        } else {
+            Some(result)
+        }
+    }
 }
 
 #[derive(Debug)]
 pub struct Hover {
-    pub contents: Vec<HoverContents>,
+    pub contents: Vec<HoverBlock>,
     pub range: Option<Range<language::Anchor>>,
 }
 

crates/rpc/proto/zed.proto

@@ -436,8 +436,14 @@ message GetHover {
 }
 
 message GetHoverResponse {
-    repeated CodeAction actions = 1;
+    optional Anchor start = 1;
-    repeated VectorClockEntry version = 2;
+    optional Anchor end = 2;
+    repeated HoverBlock contents = 3;
+}
+
+message HoverBlock {
+    string text = 1;
+    optional string language = 2;
 }
 
 message ApplyCodeAction {

crates/theme/src/theme.rs

@@ -444,7 +444,7 @@ pub struct Editor {
     pub autocomplete: AutocompleteStyle,
     pub code_actions_indicator: Color,
     pub unnecessary_code_fade: f32,
-    pub hover_popover: ContainerStyle,
+    pub hover_popover: HoverPopover,
 }
 
 #[derive(Clone, Deserialize, Default)]
@@ -623,3 +623,10 @@ impl<'de> Deserialize<'de> for SyntaxTheme {
         Ok(result)
     }
 }
+
+#[derive(Clone, Deserialize, Default)]
+pub struct HoverPopover {
+    pub container: ContainerStyle,
+    pub block_style: ContainerStyle,
+    pub prose: TextStyle,
+}

styles/src/styleTree/hoverPopover.ts

@@ -1,8 +1,9 @@
 import Theme from "../themes/common/theme";
-import { backgroundColor, border, popoverShadow } from "./components";
+import { backgroundColor, border, popoverShadow, text } from "./components";
 
 export default function HoverPopover(theme: Theme) {
   return {
+    container: {
       background: backgroundColor(theme, "on500"),
       cornerRadius: 8,
       padding: {
@@ -16,5 +17,10 @@ export default function HoverPopover(theme: Theme) {
       margin: {
         left: -8,
       },
+    },
+    block_style: {
+      padding: { top: 4 },
+    },
+    prose: text(theme, "sans", "primary", { "size": "sm" })
   }
 }