Keith Simmons created
Diagnostics Popoverassets/keymaps/default.json | 2
crates/diagnostics/src/items.rs | 8
crates/editor/src/editor.rs | 30 +
crates/editor/src/element.rs | 152 ++++++++----
crates/editor/src/hover_popover.rs | 368 ++++++++++++++++++++++-------
crates/editor/src/test.rs | 78 +++++
crates/language/src/buffer.rs | 6
crates/text/src/selection.rs | 14
crates/theme/src/theme.rs | 3
crates/zed/src/menus.rs | 2
styles/src/styleTree/hoverPopover.ts | 50 +++
styles/src/themes/common/base16.ts | 23 +
styles/src/themes/common/theme.ts | 3
13 files changed, 539 insertions(+), 200 deletions(-)
@@ -190,7 +190,7 @@
"alt-down": "editor::SelectSmallerSyntaxNode",
"cmd-u": "editor::UndoSelection",
"cmd-shift-U": "editor::RedoSelection",
- "f8": "editor::GoToNextDiagnostic",
+ "f8": "editor::GoToDiagnostic",
"shift-f8": "editor::GoToPrevDiagnostic",
"f2": "editor::Rename",
"f12": "editor::GoToDefinition",
@@ -1,5 +1,5 @@
use collections::HashSet;
-use editor::{Editor, GoToNextDiagnostic};
+use editor::{Editor, GoToDiagnostic};
use gpui::{
elements::*, platform::CursorStyle, serde_json, Entity, ModelHandle, MouseButton,
MutableAppContext, RenderContext, Subscription, View, ViewContext, ViewHandle, WeakViewHandle,
@@ -48,10 +48,10 @@ impl DiagnosticIndicator {
}
}
- fn go_to_next_diagnostic(&mut self, _: &GoToNextDiagnostic, cx: &mut ViewContext<Self>) {
+ fn go_to_next_diagnostic(&mut self, _: &GoToDiagnostic, cx: &mut ViewContext<Self>) {
if let Some(editor) = self.active_editor.as_ref().and_then(|e| e.upgrade(cx)) {
editor.update(cx, |editor, cx| {
- editor.go_to_diagnostic(editor::Direction::Next, cx);
+ editor.go_to_diagnostic_impl(editor::Direction::Next, cx);
})
}
}
@@ -202,7 +202,7 @@ impl View for DiagnosticIndicator {
})
.with_cursor_style(CursorStyle::PointingHand)
.on_click(MouseButton::Left, |_, cx| {
- cx.dispatch_action(GoToNextDiagnostic)
+ cx.dispatch_action(GoToDiagnostic)
})
.boxed(),
);
@@ -82,9 +82,6 @@ pub struct SelectNext {
pub replace_newest: bool,
}
-#[derive(Clone, PartialEq)]
-pub struct GoToDiagnostic(pub Direction);
-
#[derive(Clone, PartialEq)]
pub struct Scroll(pub Vector2F);
@@ -138,7 +135,7 @@ actions!(
Backspace,
Delete,
Newline,
- GoToNextDiagnostic,
+ GoToDiagnostic,
GoToPrevDiagnostic,
Indent,
Outdent,
@@ -300,7 +297,7 @@ pub fn init(cx: &mut MutableAppContext) {
cx.add_action(Editor::move_to_enclosing_bracket);
cx.add_action(Editor::undo_selection);
cx.add_action(Editor::redo_selection);
- cx.add_action(Editor::go_to_next_diagnostic);
+ cx.add_action(Editor::go_to_diagnostic);
cx.add_action(Editor::go_to_prev_diagnostic);
cx.add_action(Editor::go_to_definition);
cx.add_action(Editor::page_up);
@@ -4564,17 +4561,32 @@ impl Editor {
self.selection_history.mode = SelectionHistoryMode::Normal;
}
- fn go_to_next_diagnostic(&mut self, _: &GoToNextDiagnostic, cx: &mut ViewContext<Self>) {
- self.go_to_diagnostic(Direction::Next, cx)
+ fn go_to_diagnostic(&mut self, _: &GoToDiagnostic, cx: &mut ViewContext<Self>) {
+ self.go_to_diagnostic_impl(Direction::Next, cx)
}
fn go_to_prev_diagnostic(&mut self, _: &GoToPrevDiagnostic, cx: &mut ViewContext<Self>) {
- self.go_to_diagnostic(Direction::Prev, cx)
+ self.go_to_diagnostic_impl(Direction::Prev, cx)
}
- pub fn go_to_diagnostic(&mut self, direction: Direction, cx: &mut ViewContext<Self>) {
+ pub fn go_to_diagnostic_impl(&mut self, direction: Direction, cx: &mut ViewContext<Self>) {
let buffer = self.buffer.read(cx).snapshot(cx);
let selection = self.selections.newest::<usize>(cx);
+
+ // If there is an active Diagnostic Popover. Jump to it's diagnostic instead.
+ if direction == Direction::Next {
+ if let Some(popover) = self.hover_state.diagnostic_popover.as_ref() {
+ let (group_id, jump_to) = popover.activation_info();
+ self.activate_diagnostics(group_id, cx);
+ self.change_selections(Some(Autoscroll::Center), cx, |s| {
+ let mut new_selection = s.newest_anchor().clone();
+ new_selection.collapse_to(jump_to, SelectionGoal::None);
+ s.select_anchors(vec![new_selection.clone()]);
+ });
+ return;
+ }
+ }
+
let mut active_primary_range = self.active_diagnostics.as_ref().map(|active_diagnostics| {
active_diagnostics
.primary_range
@@ -41,6 +41,10 @@ use std::{
ops::Range,
};
+const MIN_POPOVER_CHARACTER_WIDTH: f32 = 20.;
+const MIN_POPOVER_LINE_HEIGHT: f32 = 4.;
+const HOVER_POPOVER_GAP: f32 = 10.;
+
struct SelectionLayout {
head: DisplayPoint,
range: Range<DisplayPoint>,
@@ -268,8 +272,9 @@ impl EditorElement {
}
if paint
- .hover_bounds
- .map_or(false, |hover_bounds| hover_bounds.contains_point(position))
+ .hover_popover_bounds
+ .iter()
+ .any(|hover_bounds| hover_bounds.contains_point(position))
{
return false;
}
@@ -600,34 +605,77 @@ impl EditorElement {
cx.scene.pop_stacking_context();
}
- if let Some((position, hover_popover)) = layout.hover.as_mut() {
+ if let Some((position, hover_popovers)) = layout.hover_popovers.as_mut() {
cx.scene.push_stacking_context(None);
// This is safe because we check on layout whether the required row is available
let hovered_row_layout = &layout.line_layouts[(position.row() - start_row) as usize];
- let size = hover_popover.size();
+
+ // Minimum required size: Take the first popover, and add 1.5 times the minimum popover
+ // height. This is the size we will use to decide whether to render popovers above or below
+ // the hovered line.
+ let first_size = hover_popovers[0].size();
+ let height_to_reserve =
+ first_size.y() + 1.5 * MIN_POPOVER_LINE_HEIGHT as f32 * layout.line_height;
+
+ // Compute Hovered Point
let x = hovered_row_layout.x_for_index(position.column() as usize) - scroll_left;
- let y = position.row() as f32 * layout.line_height - scroll_top - size.y();
- let mut popover_origin = content_origin + vec2f(x, y);
+ let y = position.row() as f32 * layout.line_height - scroll_top;
+ let hovered_point = content_origin + vec2f(x, y);
- if popover_origin.y() < 0.0 {
- popover_origin.set_y(popover_origin.y() + layout.line_height + size.y());
- }
+ paint.hover_popover_bounds.clear();
- 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);
- }
+ if hovered_point.y() - height_to_reserve > 0.0 {
+ // There is enough space above. Render popovers above the hovered point
+ let mut current_y = hovered_point.y();
+ for hover_popover in hover_popovers {
+ let size = hover_popover.size();
+ let mut popover_origin = vec2f(hovered_point.x(), current_y - size.y());
- hover_popover.paint(
- popover_origin,
- RectF::from_points(Vector2F::zero(), vec2f(f32::MAX, f32::MAX)), // Let content bleed outside of editor
- cx,
- );
+ 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);
+ }
- paint.hover_bounds = Some(
- RectF::new(popover_origin, hover_popover.size()).dilate(Vector2F::new(0., 5.)),
- );
+ hover_popover.paint(
+ popover_origin,
+ RectF::from_points(Vector2F::zero(), vec2f(f32::MAX, f32::MAX)), // Let content bleed outside of editor
+ cx,
+ );
+
+ paint.hover_popover_bounds.push(
+ RectF::new(popover_origin, hover_popover.size())
+ .dilate(Vector2F::new(0., 5.)),
+ );
+
+ current_y = popover_origin.y() - HOVER_POPOVER_GAP;
+ }
+ } else {
+ // There is not enough space above. Render popovers below the hovered point
+ let mut current_y = hovered_point.y() + layout.line_height;
+ for hover_popover in hover_popovers {
+ let size = hover_popover.size();
+ let mut popover_origin = vec2f(hovered_point.x(), current_y);
+
+ 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(
+ popover_origin,
+ RectF::from_points(Vector2F::zero(), vec2f(f32::MAX, f32::MAX)), // Let content bleed outside of editor
+ cx,
+ );
+
+ paint.hover_popover_bounds.push(
+ RectF::new(popover_origin, hover_popover.size())
+ .dilate(Vector2F::new(0., 5.)),
+ );
+
+ current_y = popover_origin.y() + size.y() + HOVER_POPOVER_GAP;
+ }
+ }
cx.scene.pop_stacking_context();
}
@@ -1162,6 +1210,8 @@ impl Element for EditorElement {
});
let scroll_position = snapshot.scroll_position();
+ // The scroll position is a fractional point, the whole number of which represents
+ // the top of the window in terms of display rows.
let start_row = scroll_position.y() as u32;
let scroll_top = scroll_position.y() * line_height;
@@ -1335,16 +1385,8 @@ impl Element for EditorElement {
.map(|indicator| (newest_selection_head.row(), indicator));
}
- hover = view.hover_state.popover.clone().and_then(|hover| {
- let (point, rendered) = hover.render(&snapshot, 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
- });
+ let visible_rows = start_row..start_row + line_layouts.len() as u32;
+ hover = view.hover_state.render(&snapshot, &style, visible_rows, cx);
});
if let Some((_, context_menu)) = context_menu.as_mut() {
@@ -1367,21 +1409,23 @@ impl Element for EditorElement {
);
}
- if let Some((_, hover)) = hover.as_mut() {
- hover.layout(
- SizeConstraint {
- min: Vector2F::zero(),
- max: vec2f(
- (120. * em_width) // Default size
- .min(size.x() / 2.) // Shrink to half of the editor width
- .max(20. * em_width), // Apply minimum width of 20 characters
- (16. * line_height) // Default size
- .min(size.y() / 2.) // Shrink to half of the editor height
- .max(4. * line_height), // Apply minimum height of 4 lines
- ),
- },
- cx,
- );
+ if let Some((_, hover_popovers)) = hover.as_mut() {
+ for hover_popover in hover_popovers.iter_mut() {
+ hover_popover.layout(
+ SizeConstraint {
+ min: Vector2F::zero(),
+ max: vec2f(
+ (120. * em_width) // Default size
+ .min(size.x() / 2.) // Shrink to half of the editor width
+ .max(MIN_POPOVER_CHARACTER_WIDTH * em_width), // Apply minimum width of 20 characters
+ (16. * line_height) // Default size
+ .min(size.y() / 2.) // Shrink to half of the editor height
+ .max(MIN_POPOVER_LINE_HEIGHT * line_height), // Apply minimum height of 4 lines
+ ),
+ },
+ cx,
+ );
+ }
}
(
@@ -1406,7 +1450,7 @@ impl Element for EditorElement {
selections,
context_menu,
code_actions_indicator,
- hover,
+ hover_popovers: hover,
},
)
}
@@ -1431,7 +1475,7 @@ impl Element for EditorElement {
gutter_bounds,
text_bounds,
context_menu_bounds: None,
- hover_bounds: None,
+ hover_popover_bounds: Default::default(),
};
self.paint_background(gutter_bounds, text_bounds, layout, cx);
@@ -1472,9 +1516,11 @@ impl Element for EditorElement {
}
}
- if let Some((_, hover)) = &mut layout.hover {
- if hover.dispatch_event(event, cx) {
- return true;
+ if let Some((_, popover_elements)) = &mut layout.hover_popovers {
+ for popover_element in popover_elements.iter_mut() {
+ if popover_element.dispatch_event(event, cx) {
+ return true;
+ }
}
}
@@ -1569,7 +1615,7 @@ pub struct LayoutState {
selections: Vec<(ReplicaId, Vec<SelectionLayout>)>,
context_menu: Option<(DisplayPoint, ElementBox)>,
code_actions_indicator: Option<(u32, ElementBox)>,
- hover: Option<(DisplayPoint, ElementBox)>,
+ hover_popovers: Option<(DisplayPoint, Vec<ElementBox>)>,
}
struct BlockLayout {
@@ -1614,7 +1660,7 @@ pub struct PaintState {
gutter_bounds: RectF,
text_bounds: RectF,
context_menu_bounds: Option<RectF>,
- hover_bounds: Option<RectF>,
+ hover_popover_bounds: Vec<RectF>,
}
impl PaintState {
@@ -3,9 +3,10 @@ use gpui::{
elements::{Flex, MouseEventHandler, Padding, Text},
impl_internal_actions,
platform::CursorStyle,
- Axis, Element, ElementBox, ModelHandle, MutableAppContext, RenderContext, Task, ViewContext,
+ Axis, Element, ElementBox, ModelHandle, MouseButton, MutableAppContext, RenderContext, Task,
+ ViewContext,
};
-use language::Bias;
+use language::{Bias, DiagnosticEntry, DiagnosticSeverity};
use project::{HoverBlock, Project};
use settings::Settings;
use std::{ops::Range, time::Duration};
@@ -13,7 +14,7 @@ use util::TryFutureExt;
use crate::{
display_map::ToDisplayPoint, Anchor, AnchorRangeExt, DisplayPoint, Editor, EditorSnapshot,
- EditorStyle,
+ EditorStyle, GoToDiagnostic, RangeToAnchorExt,
};
pub const HOVER_DELAY_MILLIS: u64 = 350;
@@ -54,17 +55,11 @@ pub fn hover_at(editor: &mut Editor, action: &HoverAt, cx: &mut ViewContext<Edit
/// Triggered by the `Hover` action when the cursor is not over a symbol or when the
/// selections changed.
pub fn hide_hover(editor: &mut Editor, cx: &mut ViewContext<Editor>) -> bool {
- let mut did_hide = false;
+ let did_hide = editor.hover_state.info_popover.take().is_some()
+ | editor.hover_state.diagnostic_popover.take().is_some();
- // only notify the context once
- if editor.hover_state.popover.is_some() {
- editor.hover_state.popover = None;
- did_hide = true;
- cx.notify();
- }
- editor.hover_state.task = None;
+ editor.hover_state.info_task = None;
editor.hover_state.triggered_from = None;
- editor.hover_state.symbol_range = None;
editor.clear_background_highlights::<HoverState>(cx);
@@ -114,8 +109,8 @@ fn show_hover(
};
if !ignore_timeout {
- if let Some(range) = &editor.hover_state.symbol_range {
- if range
+ if let Some(InfoPopover { symbol_range, .. }) = &editor.hover_state.info_popover {
+ if symbol_range
.to_offset(&snapshot.buffer_snapshot)
.contains(&multibuffer_offset)
{
@@ -167,6 +162,43 @@ fn show_hover(
})
});
+ if let Some(delay) = delay {
+ delay.await;
+ }
+
+ // If there's a diagnostic, assign it on the hover state and notify
+ let local_diagnostic = snapshot
+ .buffer_snapshot
+ .diagnostics_in_range::<_, usize>(multibuffer_offset..multibuffer_offset, false)
+ // Find the entry with the most specific range
+ .min_by_key(|entry| entry.range.end - entry.range.start)
+ .map(|entry| DiagnosticEntry {
+ diagnostic: entry.diagnostic,
+ range: entry.range.to_anchors(&snapshot.buffer_snapshot),
+ });
+
+ // Pull the primary diagnostic out so we can jump to it if the popover is clicked
+ let primary_diagnostic = local_diagnostic.as_ref().and_then(|local_diagnostic| {
+ snapshot
+ .buffer_snapshot
+ .diagnostic_group::<usize>(local_diagnostic.diagnostic.group_id)
+ .find(|diagnostic| diagnostic.diagnostic.is_primary)
+ .map(|entry| DiagnosticEntry {
+ diagnostic: entry.diagnostic,
+ range: entry.range.to_anchors(&snapshot.buffer_snapshot),
+ })
+ });
+
+ if let Some(this) = this.upgrade(&cx) {
+ this.update(&mut cx, |this, _| {
+ this.hover_state.diagnostic_popover =
+ local_diagnostic.map(|local_diagnostic| DiagnosticPopover {
+ local_diagnostic,
+ primary_diagnostic,
+ });
+ });
+ }
+
// Construct new hover popover from hover request
let hover_popover = hover_request.await.ok().flatten().and_then(|hover_result| {
if hover_result.contents.is_empty() {
@@ -188,45 +220,28 @@ fn show_hover(
anchor.clone()..anchor.clone()
};
- if let Some(this) = this.upgrade(&cx) {
- this.update(&mut cx, |this, _| {
- this.hover_state.symbol_range = Some(range.clone());
- });
- }
-
- Some(HoverPopover {
+ Some(InfoPopover {
project: project.clone(),
- anchor: range.start.clone(),
+ symbol_range: range.clone(),
contents: hover_result.contents,
})
});
- if let Some(delay) = delay {
- delay.await;
- }
-
if let Some(this) = this.upgrade(&cx) {
this.update(&mut cx, |this, cx| {
- if hover_popover.is_some() {
+ if let Some(hover_popover) = hover_popover.as_ref() {
// Highlight the selected symbol using a background highlight
- if let Some(range) = this.hover_state.symbol_range.clone() {
- this.highlight_background::<HoverState>(
- vec![range],
- |theme| theme.editor.hover_popover.highlight,
- cx,
- );
- }
- this.hover_state.popover = hover_popover;
- cx.notify();
+ this.highlight_background::<HoverState>(
+ vec![hover_popover.symbol_range.clone()],
+ |theme| theme.editor.hover_popover.highlight,
+ cx,
+ );
} else {
- if this.hover_state.visible() {
- // 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;
- }
+ this.clear_background_highlights::<HoverState>(cx);
}
+
+ this.hover_state.info_popover = hover_popover;
+ cx.notify();
});
}
Ok::<_, anyhow::Error>(())
@@ -234,38 +249,70 @@ fn show_hover(
.log_err()
});
- editor.hover_state.task = Some(task);
+ editor.hover_state.info_task = Some(task);
}
#[derive(Default)]
pub struct HoverState {
- pub popover: Option<HoverPopover>,
+ pub info_popover: Option<InfoPopover>,
+ pub diagnostic_popover: Option<DiagnosticPopover>,
pub triggered_from: Option<Anchor>,
- pub symbol_range: Option<Range<Anchor>>,
- pub task: Option<Task<Option<()>>>,
+ pub info_task: Option<Task<Option<()>>>,
}
impl HoverState {
pub fn visible(&self) -> bool {
- self.popover.is_some()
+ self.info_popover.is_some() || self.diagnostic_popover.is_some()
+ }
+
+ pub fn render(
+ &self,
+ snapshot: &EditorSnapshot,
+ style: &EditorStyle,
+ visible_rows: Range<u32>,
+ cx: &mut RenderContext<Editor>,
+ ) -> Option<(DisplayPoint, Vec<ElementBox>)> {
+ // If there is a diagnostic, position the popovers based on that.
+ // Otherwise use the start of the hover range
+ let anchor = self
+ .diagnostic_popover
+ .as_ref()
+ .map(|diagnostic_popover| &diagnostic_popover.local_diagnostic.range.start)
+ .or_else(|| {
+ self.info_popover
+ .as_ref()
+ .map(|info_popover| &info_popover.symbol_range.start)
+ })?;
+ let point = anchor.to_display_point(&snapshot.display_snapshot);
+
+ // Don't render if the relevant point isn't on screen
+ if !self.visible() || !visible_rows.contains(&point.row()) {
+ return None;
+ }
+
+ let mut elements = Vec::new();
+
+ if let Some(diagnostic_popover) = self.diagnostic_popover.as_ref() {
+ elements.push(diagnostic_popover.render(style, cx));
+ }
+ if let Some(info_popover) = self.info_popover.as_ref() {
+ elements.push(info_popover.render(style, cx));
+ }
+
+ Some((point, elements))
}
}
#[derive(Debug, Clone)]
-pub struct HoverPopover {
+pub struct InfoPopover {
pub project: ModelHandle<Project>,
- pub anchor: Anchor,
+ pub symbol_range: Range<Anchor>,
pub contents: Vec<HoverBlock>,
}
-impl HoverPopover {
- pub fn render(
- &self,
- snapshot: &EditorSnapshot,
- style: EditorStyle,
- cx: &mut RenderContext<Editor>,
- ) -> (DisplayPoint, ElementBox) {
- let element = MouseEventHandler::new::<HoverPopover, _, _>(0, cx, |_, cx| {
+impl InfoPopover {
+ pub fn render(&self, style: &EditorStyle, cx: &mut RenderContext<Editor>) -> ElementBox {
+ MouseEventHandler::new::<InfoPopover, _, _>(0, cx, |_, cx| {
let mut flex = Flex::new(Axis::Vertical).scrollable::<HoverBlock, _>(1, None, cx);
flex.extend(self.contents.iter().map(|content| {
let project = self.project.read(cx);
@@ -309,10 +356,61 @@ impl HoverPopover {
top: 5.,
..Default::default()
})
- .boxed();
+ .boxed()
+ }
+}
+
+#[derive(Debug, Clone)]
+pub struct DiagnosticPopover {
+ local_diagnostic: DiagnosticEntry<Anchor>,
+ primary_diagnostic: Option<DiagnosticEntry<Anchor>>,
+}
+
+impl DiagnosticPopover {
+ pub fn render(&self, style: &EditorStyle, cx: &mut RenderContext<Editor>) -> ElementBox {
+ enum PrimaryDiagnostic {}
+
+ let mut text_style = style.hover_popover.prose.clone();
+ text_style.font_size = style.text.font_size;
+
+ let container_style = match self.local_diagnostic.diagnostic.severity {
+ DiagnosticSeverity::HINT => style.hover_popover.info_container,
+ DiagnosticSeverity::INFORMATION => style.hover_popover.info_container,
+ DiagnosticSeverity::WARNING => style.hover_popover.warning_container,
+ DiagnosticSeverity::ERROR => style.hover_popover.error_container,
+ _ => style.hover_popover.container,
+ };
- let display_point = self.anchor.to_display_point(&snapshot.display_snapshot);
- (display_point, element)
+ let tooltip_style = cx.global::<Settings>().theme.tooltip.clone();
+
+ MouseEventHandler::new::<DiagnosticPopover, _, _>(0, cx, |_, _| {
+ Text::new(self.local_diagnostic.diagnostic.message.clone(), text_style)
+ .with_soft_wrap(true)
+ .contained()
+ .with_style(container_style)
+ .boxed()
+ })
+ .on_click(MouseButton::Left, |_, cx| {
+ cx.dispatch_action(GoToDiagnostic)
+ })
+ .with_cursor_style(CursorStyle::PointingHand)
+ .with_tooltip::<PrimaryDiagnostic, _>(
+ 0,
+ "Go To Diagnostic".to_string(),
+ Some(Box::new(crate::GoToDiagnostic)),
+ tooltip_style,
+ cx,
+ )
+ .boxed()
+ }
+
+ pub fn activation_info(&self) -> (usize, Anchor) {
+ let entry = self
+ .primary_diagnostic
+ .as_ref()
+ .unwrap_or(&self.local_diagnostic);
+
+ (entry.diagnostic.group_id, entry.range.start.clone())
}
}
@@ -321,6 +419,7 @@ mod tests {
use futures::StreamExt;
use indoc::indoc;
+ use language::{Diagnostic, DiagnosticSet};
use project::HoverBlock;
use crate::test::EditorLspTestContext;
@@ -328,7 +427,7 @@ mod tests {
use super::*;
#[gpui::test]
- async fn test_hover_popover(cx: &mut gpui::TestAppContext) {
+ async fn test_mouse_hover_info_popover(cx: &mut gpui::TestAppContext) {
let mut cx = EditorLspTestContext::new_rust(
lsp::ServerCapabilities {
hover_provider: Some(lsp::HoverProviderCapability::Simple(true)),
@@ -362,19 +461,18 @@ mod tests {
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! {"
+ cx.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),
- }))
- });
+ .to_string(),
+ }),
+ range: Some(symbol_range),
+ }))
+ });
cx.foreground()
.advance_clock(Duration::from_millis(HOVER_DELAY_MILLIS + 100));
requests.next().await;
@@ -382,7 +480,7 @@ mod tests {
cx.editor(|editor, _| {
assert!(editor.hover_state.visible());
assert_eq!(
- editor.hover_state.popover.clone().unwrap().contents,
+ editor.hover_state.info_popover.clone().unwrap().contents,
vec![
HoverBlock {
text: "Some basic docs".to_string(),
@@ -400,6 +498,9 @@ mod tests {
let hover_point = cx.display_point(indoc! {"
fn te|st()
println!();"});
+ let mut request = cx
+ .lsp
+ .handle_request::<lsp::request::HoverRequest, _, _>(|_, _| async move { Ok(None) });
cx.update_editor(|editor, cx| {
hover_at(
editor,
@@ -409,15 +510,24 @@ mod tests {
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());
});
+ }
+
+ #[gpui::test]
+ async fn test_keyboard_hover_info_popover(cx: &mut gpui::TestAppContext) {
+ let mut cx = EditorLspTestContext::new_rust(
+ lsp::ServerCapabilities {
+ hover_provider: Some(lsp::HoverProviderCapability::Simple(true)),
+ ..Default::default()
+ },
+ cx,
+ )
+ .await;
// Hover with keyboard has no delay
cx.set_state(indoc! {"
@@ -427,26 +537,25 @@ mod tests {
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.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.condition(|editor, _| editor.hover_state.visible()).await;
cx.editor(|editor, _| {
- assert!(editor.hover_state.visible());
assert_eq!(
- editor.hover_state.popover.clone().unwrap().contents,
+ editor.hover_state.info_popover.clone().unwrap().contents,
vec![
HoverBlock {
text: "Some other basic docs".to_string(),
@@ -460,4 +569,73 @@ mod tests {
)
});
}
+
+ #[gpui::test]
+ async fn test_hover_diagnostic_and_info_popovers(cx: &mut gpui::TestAppContext) {
+ let mut cx = EditorLspTestContext::new_rust(
+ lsp::ServerCapabilities {
+ hover_provider: Some(lsp::HoverProviderCapability::Simple(true)),
+ ..Default::default()
+ },
+ cx,
+ )
+ .await;
+
+ // Hover with just diagnostic, pops DiagnosticPopover immediately and then
+ // info popover once request completes
+ cx.set_state(indoc! {"
+ fn te|st()
+ println!();"});
+
+ // Send diagnostic to client
+ let range = cx.text_anchor_range(indoc! {"
+ fn [test]()
+ println!();"});
+ cx.update_buffer(|buffer, cx| {
+ let snapshot = buffer.text_snapshot();
+ let set = DiagnosticSet::from_sorted_entries(
+ vec![DiagnosticEntry {
+ range,
+ diagnostic: Diagnostic {
+ message: "A test diagnostic message.".to_string(),
+ ..Default::default()
+ },
+ }],
+ &snapshot,
+ );
+ buffer.update_diagnostics(set, cx);
+ });
+
+ // Hover pops diagnostic immediately
+ cx.update_editor(|editor, cx| hover(editor, &Hover, cx));
+ cx.foreground().run_until_parked();
+
+ cx.editor(|Editor { hover_state, .. }, _| {
+ assert!(hover_state.diagnostic_popover.is_some() && hover_state.info_popover.is_none())
+ });
+
+ // Info Popover shows after request responded to
+ let range = cx.lsp_range(indoc! {"
+ fn [test]()
+ println!();"});
+ cx.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(range),
+ }))
+ });
+ cx.foreground()
+ .advance_clock(Duration::from_millis(HOVER_DELAY_MILLIS + 100));
+
+ cx.foreground().run_until_parked();
+ cx.editor(|Editor { hover_state, .. }, _| {
+ hover_state.diagnostic_popover.is_some() && hover_state.info_task.is_some()
+ });
+ }
}
@@ -9,9 +9,13 @@ use futures::{Future, StreamExt};
use indoc::indoc;
use collections::BTreeMap;
-use gpui::{json, keymap::Keystroke, AppContext, ModelHandle, ViewContext, ViewHandle};
-use language::{point_to_lsp, FakeLspAdapter, Language, LanguageConfig, Selection};
-use lsp::request;
+use gpui::{
+ json, keymap::Keystroke, AppContext, ModelContext, ModelHandle, ViewContext, ViewHandle,
+};
+use language::{
+ point_to_lsp, Buffer, BufferSnapshot, FakeLspAdapter, Language, LanguageConfig, Selection,
+};
+use lsp::{notification, request};
use project::Project;
use settings::Settings;
use util::{
@@ -119,7 +123,7 @@ impl<'a> EditorTestContext<'a> {
self.editor.condition(self.cx, predicate)
}
- pub fn editor<F, T>(&mut self, read: F) -> T
+ pub fn editor<F, T>(&self, read: F) -> T
where
F: FnOnce(&Editor, &AppContext) -> T,
{
@@ -133,12 +137,48 @@ impl<'a> EditorTestContext<'a> {
self.editor.update(self.cx, update)
}
- pub fn buffer_text(&mut self) -> String {
- self.editor.read_with(self.cx, |editor, cx| {
- editor.buffer.read(cx).snapshot(cx).text()
+ pub fn multibuffer<F, T>(&self, read: F) -> T
+ where
+ F: FnOnce(&MultiBuffer, &AppContext) -> T,
+ {
+ self.editor(|editor, cx| read(editor.buffer().read(cx), cx))
+ }
+
+ pub fn update_multibuffer<F, T>(&mut self, update: F) -> T
+ where
+ F: FnOnce(&mut MultiBuffer, &mut ModelContext<MultiBuffer>) -> T,
+ {
+ self.update_editor(|editor, cx| editor.buffer().update(cx, update))
+ }
+
+ pub fn buffer_text(&self) -> String {
+ self.multibuffer(|buffer, cx| buffer.snapshot(cx).text())
+ }
+
+ pub fn buffer<F, T>(&self, read: F) -> T
+ where
+ F: FnOnce(&Buffer, &AppContext) -> T,
+ {
+ self.multibuffer(|multibuffer, cx| {
+ let buffer = multibuffer.as_singleton().unwrap().read(cx);
+ read(buffer, cx)
})
}
+ pub fn update_buffer<F, T>(&mut self, update: F) -> T
+ where
+ F: FnOnce(&mut Buffer, &mut ModelContext<Buffer>) -> T,
+ {
+ self.update_multibuffer(|multibuffer, cx| {
+ let buffer = multibuffer.as_singleton().unwrap();
+ buffer.update(cx, update)
+ })
+ }
+
+ pub fn buffer_snapshot(&self) -> BufferSnapshot {
+ self.buffer(|buffer, _| buffer.snapshot())
+ }
+
pub fn simulate_keystroke(&mut self, keystroke_text: &str) {
let keystroke = Keystroke::parse(keystroke_text).unwrap();
let input = if keystroke.modified() {
@@ -164,6 +204,18 @@ impl<'a> EditorTestContext<'a> {
locations[0].to_display_point(&snapshot.display_snapshot)
}
+ // Returns anchors for the current buffer using `[`..`]`
+ pub fn text_anchor_range(&self, marked_text: &str) -> Range<language::Anchor> {
+ let range_marker: TextRangeMarker = ('[', ']').into();
+ let (unmarked_text, mut ranges) =
+ marked_text_ranges_by(&marked_text, vec![range_marker.clone()]);
+ assert_eq!(self.buffer_text(), unmarked_text);
+ let offset_range = ranges.remove(&range_marker).unwrap()[0].clone();
+ let snapshot = self.buffer_snapshot();
+
+ snapshot.anchor_before(offset_range.start)..snapshot.anchor_after(offset_range.end)
+ }
+
// Sets the editor state via a marked string.
// `|` characters represent empty selections
// `[` to `}` represents a non empty selection with the head at `}`
@@ -433,7 +485,7 @@ pub struct EditorLspTestContext<'a> {
pub cx: EditorTestContext<'a>,
pub lsp: lsp::FakeLanguageServer,
pub workspace: ViewHandle<Workspace>,
- pub editor_lsp_url: lsp::Url,
+ pub buffer_lsp_url: lsp::Url,
}
impl<'a> EditorLspTestContext<'a> {
@@ -507,7 +559,7 @@ impl<'a> EditorLspTestContext<'a> {
},
lsp,
workspace,
- editor_lsp_url: lsp::Url::from_file_path("/root/dir/file.rs").unwrap(),
+ buffer_lsp_url: lsp::Url::from_file_path("/root/dir/file.rs").unwrap(),
}
}
@@ -530,7 +582,7 @@ impl<'a> EditorLspTestContext<'a> {
// 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.cx.buffer_text());
+ assert_eq!(unmarked, self.buffer_text());
let offset_range = ranges.remove(&('[', ']').into()).unwrap()[0].clone();
self.to_lsp_range(offset_range)
}
@@ -594,12 +646,16 @@ impl<'a> EditorLspTestContext<'a> {
F: 'static + Send + FnMut(lsp::Url, T::Params, gpui::AsyncAppContext) -> Fut,
Fut: 'static + Send + Future<Output = Result<T::Result>>,
{
- let url = self.editor_lsp_url.clone();
+ let url = self.buffer_lsp_url.clone();
self.lsp.handle_request::<T, _, _>(move |params, cx| {
let url = url.clone();
handler(url, params, cx)
})
}
+
+ pub fn notify<T: notification::Notification>(&self, params: T::Params) {
+ self.lsp.notify::<T>(params);
+ }
}
impl<'a> Deref for EditorLspTestContext<'a> {
@@ -2462,11 +2462,11 @@ impl operation_queue::Operation for Operation {
impl Default for Diagnostic {
fn default() -> Self {
Self {
- code: Default::default(),
+ code: None,
severity: DiagnosticSeverity::ERROR,
message: Default::default(),
- group_id: Default::default(),
- is_primary: Default::default(),
+ group_id: 0,
+ is_primary: false,
is_valid: true,
is_disk_based: false,
is_unnecessary: false,
@@ -54,6 +54,13 @@ impl<T: Clone> Selection<T> {
goal: self.goal,
}
}
+
+ pub fn collapse_to(&mut self, point: T, new_goal: SelectionGoal) {
+ self.start = point.clone();
+ self.end = point;
+ self.goal = new_goal;
+ self.reversed = false;
+ }
}
impl<T: Copy + Ord> Selection<T> {
@@ -78,13 +85,6 @@ impl<T: Copy + Ord> Selection<T> {
self.goal = new_goal;
}
- pub fn collapse_to(&mut self, point: T, new_goal: SelectionGoal) {
- self.start = point;
- self.end = point;
- self.goal = new_goal;
- self.reversed = false;
- }
-
pub fn range(&self) -> Range<T> {
self.start..self.end
}
@@ -629,6 +629,9 @@ impl<'de> Deserialize<'de> for SyntaxTheme {
#[derive(Clone, Deserialize, Default)]
pub struct HoverPopover {
pub container: ContainerStyle,
+ pub info_container: ContainerStyle,
+ pub warning_container: ContainerStyle,
+ pub error_container: ContainerStyle,
pub block_style: ContainerStyle,
pub prose: TextStyle,
pub highlight: Color,
@@ -281,7 +281,7 @@ pub fn menus() -> Vec<Menu<'static>> {
MenuItem::Separator,
MenuItem::Action {
name: "Next Problem",
- action: Box::new(editor::GoToNextDiagnostic),
+ action: Box::new(editor::GoToDiagnostic),
},
MenuItem::Action {
name: "Previous Problem",
@@ -2,22 +2,48 @@ import Theme from "../themes/common/theme";
import { backgroundColor, border, popoverShadow, text } from "./components";
export default function HoverPopover(theme: Theme) {
+ let baseContainer = {
+ background: backgroundColor(theme, "on500"),
+ cornerRadius: 8,
+ padding: {
+ left: 8,
+ right: 8,
+ top: 4,
+ bottom: 4
+ },
+ shadow: popoverShadow(theme),
+ border: border(theme, "secondary"),
+ margin: {
+ left: -8,
+ },
+ };
+
return {
- container: {
- background: backgroundColor(theme, "on500"),
- cornerRadius: 8,
- padding: {
- left: 8,
- right: 8,
- top: 4,
- bottom: 4,
+ container: baseContainer,
+ infoContainer: {
+ ...baseContainer,
+ background: backgroundColor(theme, "on500Info"),
+ border: {
+ color: theme.ramps.blue(0).hex(),
+ width: 1,
},
- shadow: popoverShadow(theme),
- border: border(theme, "primary"),
- margin: {
- left: -8,
+ },
+ warningContainer: {
+ ...baseContainer,
+ background: backgroundColor(theme, "on500Warning"),
+ border: {
+ color: theme.ramps.yellow(0).hex(),
+ width: 1,
},
},
+ errorContainer: {
+ ...baseContainer,
+ background: backgroundColor(theme, "on500Error"),
+ border: {
+ color: theme.ramps.red(0).hex(),
+ width: 1,
+ }
+ },
block_style: {
padding: { top: 4 },
},
@@ -88,16 +88,31 @@ export function createTheme(
hovered: withOpacity(sample(ramps.red, 0.5), 0.2),
active: withOpacity(sample(ramps.red, 0.5), 0.25),
},
+ on500Error: {
+ base: sample(ramps.red, 0.05),
+ hovered: sample(ramps.red, 0.1),
+ active: sample(ramps.red, 0.15),
+ },
warning: {
base: withOpacity(sample(ramps.yellow, 0.5), 0.15),
hovered: withOpacity(sample(ramps.yellow, 0.5), 0.2),
active: withOpacity(sample(ramps.yellow, 0.5), 0.25),
},
+ on500Warning: {
+ base: sample(ramps.yellow, 0.05),
+ hovered: sample(ramps.yellow, 0.1),
+ active: sample(ramps.yellow, 0.15),
+ },
info: {
base: withOpacity(sample(ramps.blue, 0.5), 0.15),
hovered: withOpacity(sample(ramps.blue, 0.5), 0.2),
active: withOpacity(sample(ramps.blue, 0.5), 0.25),
},
+ on500Info: {
+ base: sample(ramps.blue, 0.05),
+ hovered: sample(ramps.blue, 0.1),
+ active: sample(ramps.blue, 0.15),
+ },
};
const borderColor = {
@@ -106,10 +121,10 @@ export function createTheme(
muted: sample(ramps.neutral, isLight ? 1 : 3),
active: sample(ramps.neutral, isLight ? 4 : 3),
onMedia: withOpacity(darkest, 0.1),
- ok: withOpacity(sample(ramps.green, 0.5), 0.15),
- error: withOpacity(sample(ramps.red, 0.5), 0.15),
- warning: withOpacity(sample(ramps.yellow, 0.5), 0.15),
- info: withOpacity(sample(ramps.blue, 0.5), 0.15),
+ ok: sample(ramps.green, 0.3),
+ error: sample(ramps.red, 0.3),
+ warning: sample(ramps.yellow, 0.3),
+ info: sample(ramps.blue, 0.3),
};
const textColor = {
@@ -79,8 +79,11 @@ export default interface Theme {
on500: BackgroundColorSet;
ok: BackgroundColorSet;
error: BackgroundColorSet;
+ on500Error: BackgroundColorSet;
warning: BackgroundColorSet;
+ on500Warning: BackgroundColorSet;
info: BackgroundColorSet;
+ on500Info: BackgroundColorSet;
};
borderColor: {
primary: string;