From 6c00cd8a354b2c5041aefc6c270128a6b5bbc188 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 11:37:26 +0300 Subject: [PATCH 01/15] Do not combine inlay and text highlights on the *Map level --- crates/editor/src/display_map.rs | 43 ++-- crates/editor/src/display_map/inlay_map.rs | 61 ++---- crates/editor/src/editor.rs | 119 ++++------- crates/editor/src/hover_popover.rs | 81 ++++---- crates/editor/src/link_go_to_definition.rs | 187 ++++++++---------- crates/editor/src/test/editor_test_context.rs | 2 - 6 files changed, 191 insertions(+), 302 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index f306692b5e420ac7101e95ec2a38cc5d8f00669c..d0cb7fd045812b0bca1043a35ab7518000e1905f 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -5,8 +5,8 @@ mod tab_map; mod wrap_map; use crate::{ - link_go_to_definition::{DocumentRange, InlayRange}, - Anchor, AnchorRangeExt, InlayId, MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint, + link_go_to_definition::InlayRange, Anchor, AnchorRangeExt, InlayId, MultiBuffer, + MultiBufferSnapshot, ToOffset, ToPoint, }; pub use block_map::{BlockMap, BlockPoint}; use collections::{HashMap, HashSet}; @@ -43,7 +43,8 @@ pub trait ToDisplayPoint { fn to_display_point(&self, map: &DisplaySnapshot) -> DisplayPoint; } -type TextHighlights = TreeMap, Arc<(HighlightStyle, Vec)>>; +// TODO kb InlayHighlights = ... ? +type TextHighlights = TreeMap, Arc<(HighlightStyle, Vec>)>>; pub struct DisplayMap { buffer: ModelHandle, @@ -215,10 +216,8 @@ impl DisplayMap { ranges: Vec>, style: HighlightStyle, ) { - self.text_highlights.insert( - Some(type_id), - Arc::new((style, ranges.into_iter().map(DocumentRange::Text).collect())), - ); + self.text_highlights + .insert(Some(type_id), Arc::new((style, ranges))); } pub fn highlight_inlays( @@ -227,16 +226,17 @@ impl DisplayMap { ranges: Vec, style: HighlightStyle, ) { - self.text_highlights.insert( - Some(type_id), - Arc::new(( - style, - ranges.into_iter().map(DocumentRange::Inlay).collect(), - )), - ); + // TODO kb + // self.text_highlights.insert( + // Some(type_id), + // Arc::new(( + // style, + // ranges.into_iter().map(DocumentRange::Inlay).collect(), + // )), + // ); } - pub fn text_highlights(&self, type_id: TypeId) -> Option<(HighlightStyle, &[DocumentRange])> { + pub fn text_highlights(&self, type_id: TypeId) -> Option<(HighlightStyle, &[Range])> { let highlights = self.text_highlights.get(&Some(type_id))?; Some((highlights.0, &highlights.1)) } @@ -244,7 +244,7 @@ impl DisplayMap { pub fn clear_text_highlights( &mut self, type_id: TypeId, - ) -> Option)>> { + ) -> Option>)>> { self.text_highlights.remove(&Some(type_id)) } @@ -422,15 +422,6 @@ impl DisplaySnapshot { .to_inlay_offset(anchor.to_offset(&self.buffer_snapshot)) } - pub fn inlay_offset_to_display_point(&self, offset: InlayOffset, bias: Bias) -> DisplayPoint { - let inlay_point = self.inlay_snapshot.to_point(offset); - let fold_point = self.fold_snapshot.to_fold_point(inlay_point, bias); - let tab_point = self.tab_snapshot.to_tab_point(fold_point); - let wrap_point = self.wrap_snapshot.tab_point_to_wrap_point(tab_point); - let block_point = self.block_snapshot.to_block_point(wrap_point); - DisplayPoint(block_point) - } - fn display_point_to_inlay_point(&self, point: DisplayPoint, bias: Bias) -> InlayPoint { let block_point = point.0; let wrap_point = self.block_snapshot.to_wrap_point(block_point); @@ -754,7 +745,7 @@ impl DisplaySnapshot { #[cfg(any(test, feature = "test-support"))] pub fn highlight_ranges( &self, - ) -> Option)>> { + ) -> Option>)>> { let type_id = TypeId::of::(); self.text_highlights.get(&Some(type_id)).cloned() } diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 25b8d3aef6a28b959a6092e1cfba4adf031dd125..45cb7ec7f3394d0b80252a118a1ff76a9109da60 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -1,5 +1,4 @@ use crate::{ - link_go_to_definition::DocumentRange, multi_buffer::{MultiBufferChunks, MultiBufferRows}, Anchor, InlayId, MultiBufferSnapshot, ToOffset, }; @@ -1006,9 +1005,6 @@ impl InlaySnapshot { let transform_start = self.buffer.anchor_after( self.to_buffer_offset(cmp::max(range.start, cursor.start().0)), ); - let transform_start = - self.to_inlay_offset(transform_start.to_offset(&self.buffer)); - let transform_end = { let overshoot = InlayOffset(range.end.0 - cursor.start().0 .0); self.buffer.anchor_before(self.to_buffer_offset(cmp::min( @@ -1016,17 +1012,13 @@ impl InlaySnapshot { cursor.start().0 + overshoot, ))) }; - let transform_end = self.to_inlay_offset(transform_end.to_offset(&self.buffer)); for (tag, text_highlights) in text_highlights.iter() { let style = text_highlights.0; let ranges = &text_highlights.1; let start_ix = match ranges.binary_search_by(|probe| { - let cmp = self - .document_to_inlay_range(probe) - .end - .cmp(&transform_start); + let cmp = probe.end.cmp(&transform_start, &self.buffer); if cmp.is_gt() { cmp::Ordering::Greater } else { @@ -1036,19 +1028,18 @@ impl InlaySnapshot { Ok(i) | Err(i) => i, }; for range in &ranges[start_ix..] { - let range = self.document_to_inlay_range(range); - if range.start.cmp(&transform_end).is_ge() { + if range.start.cmp(&transform_end, &self.buffer).is_ge() { break; } highlight_endpoints.push(HighlightEndpoint { - offset: range.start, + offset: self.to_inlay_offset(range.start.to_offset(&self.buffer)), is_start: true, tag: *tag, style, }); highlight_endpoints.push(HighlightEndpoint { - offset: range.end, + offset: self.to_inlay_offset(range.end.to_offset(&self.buffer)), is_start: false, tag: *tag, style, @@ -1082,18 +1073,6 @@ impl InlaySnapshot { } } - fn document_to_inlay_range(&self, range: &DocumentRange) -> Range { - match range { - DocumentRange::Text(text_range) => { - self.to_inlay_offset(text_range.start.to_offset(&self.buffer)) - ..self.to_inlay_offset(text_range.end.to_offset(&self.buffer)) - } - DocumentRange::Inlay(inlay_range) => { - inlay_range.highlight_start..inlay_range.highlight_end - } - } - } - #[cfg(test)] pub fn text(&self) -> String { self.chunks(Default::default()..self.len(), false, None, None, None) @@ -1144,7 +1123,7 @@ fn push_isomorphic(sum_tree: &mut SumTree, summary: TextSummary) { #[cfg(test)] mod tests { use super::*; - use crate::{link_go_to_definition::InlayRange, InlayId, MultiBuffer}; + use crate::{InlayId, MultiBuffer}; use gpui::AppContext; use project::{InlayHint, InlayHintLabel, ResolveState}; use rand::prelude::*; @@ -1646,27 +1625,15 @@ mod tests { .map(|_| buffer_snapshot.random_byte_range(0, &mut rng)) .collect::>(); highlight_ranges.sort_by_key(|range| (range.start, Reverse(range.end))); - log::info!("highlighting ranges {:?}", highlight_ranges); - let highlight_ranges = if rng.gen_bool(0.5) { - highlight_ranges - .into_iter() - .map(|range| InlayRange { - inlay_position: buffer_snapshot.anchor_before(range.start), - highlight_start: inlay_snapshot.to_inlay_offset(range.start), - highlight_end: inlay_snapshot.to_inlay_offset(range.end), - }) - .map(DocumentRange::Inlay) - .collect::>() - } else { - highlight_ranges - .into_iter() - .map(|range| { - buffer_snapshot.anchor_before(range.start) - ..buffer_snapshot.anchor_after(range.end) - }) - .map(DocumentRange::Text) - .collect::>() - }; + log::info!("highlighting ranges {highlight_ranges:?}"); + // TODO kb add inlay ranges into the tests + let highlight_ranges = highlight_ranges + .into_iter() + .map(|range| { + buffer_snapshot.anchor_before(range.start) + ..buffer_snapshot.anchor_after(range.end) + }) + .collect::>(); highlights.insert( Some(TypeId::of::<()>()), Arc::new((HighlightStyle::default(), highlight_ranges)), diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 3f94f815f2f346916c3b215acead0d71469e6bcc..985ca7875e531551e08d7ce68661b220c673c674 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -66,7 +66,7 @@ use language::{ TransactionId, }; use link_go_to_definition::{ - hide_link_definition, show_link_definition, DocumentRange, GoToDefinitionLink, InlayRange, + hide_link_definition, show_link_definition, GoToDefinitionLink, InlayRange, LinkGoToDefinitionState, }; use log::error; @@ -548,7 +548,7 @@ type CompletionId = usize; type GetFieldEditorTheme = dyn Fn(&theme::Theme) -> theme::FieldEditor; type OverrideTextStyle = dyn Fn(&EditorStyle) -> Option; -type BackgroundHighlight = (fn(&Theme) -> Color, Vec); +type BackgroundHighlight = (fn(&Theme) -> Color, Vec>); pub struct Editor { handle: WeakViewHandle, @@ -7074,12 +7074,7 @@ impl Editor { .display_map .update(cx, |display_map, cx| display_map.snapshot(cx)); let mut buffer_highlights = this - .document_highlights_for_position( - selection.head(), - &buffer, - &display_snapshot, - ) - .filter_map(|highlight| highlight.as_text_range()) + .document_highlights_for_position(selection.head(), &buffer) .filter(|highlight| { highlight.start.excerpt_id() == selection.head().excerpt_id() && highlight.end.excerpt_id() == selection.head().excerpt_id() @@ -7134,15 +7129,11 @@ impl Editor { let ranges = this .clear_background_highlights::(cx) .into_iter() - .flat_map(|(_, ranges)| { - ranges.into_iter().filter_map(|range| range.as_text_range()) - }) + .flat_map(|(_, ranges)| ranges.into_iter()) .chain( this.clear_background_highlights::(cx) .into_iter() - .flat_map(|(_, ranges)| { - ranges.into_iter().filter_map(|range| range.as_text_range()) - }), + .flat_map(|(_, ranges)| ranges.into_iter()), ) .collect(); @@ -7820,13 +7811,8 @@ impl Editor { color_fetcher: fn(&Theme) -> Color, cx: &mut ViewContext, ) { - self.background_highlights.insert( - TypeId::of::(), - ( - color_fetcher, - ranges.into_iter().map(DocumentRange::Text).collect(), - ), - ); + self.background_highlights + .insert(TypeId::of::(), (color_fetcher, ranges)); cx.notify(); } @@ -7836,13 +7822,14 @@ impl Editor { color_fetcher: fn(&Theme) -> Color, cx: &mut ViewContext, ) { - self.background_highlights.insert( - TypeId::of::(), - ( - color_fetcher, - ranges.into_iter().map(DocumentRange::Inlay).collect(), - ), - ); + // TODO kb + // self.background_highlights.insert( + // TypeId::of::(), + // ( + // color_fetcher, + // ranges.into_iter().map(DocumentRange::Inlay).collect(), + // ), + // ); cx.notify(); } @@ -7874,8 +7861,7 @@ impl Editor { &'a self, position: Anchor, buffer: &'a MultiBufferSnapshot, - display_snapshot: &'a DisplaySnapshot, - ) -> impl 'a + Iterator { + ) -> impl 'a + Iterator> { let read_highlights = self .background_highlights .get(&TypeId::of::()) @@ -7884,16 +7870,14 @@ impl Editor { .background_highlights .get(&TypeId::of::()) .map(|h| &h.1); - let left_position = display_snapshot.anchor_to_inlay_offset(position.bias_left(buffer)); - let right_position = display_snapshot.anchor_to_inlay_offset(position.bias_right(buffer)); + let left_position = position.bias_left(buffer); + let right_position = position.bias_right(buffer); read_highlights .into_iter() .chain(write_highlights) .flat_map(move |ranges| { let start_ix = match ranges.binary_search_by(|probe| { - let cmp = document_to_inlay_range(probe, display_snapshot) - .end - .cmp(&left_position); + let cmp = probe.end.cmp(&left_position, buffer); if cmp.is_ge() { Ordering::Greater } else { @@ -7904,12 +7888,9 @@ impl Editor { }; let right_position = right_position.clone(); - ranges[start_ix..].iter().take_while(move |range| { - document_to_inlay_range(range, display_snapshot) - .start - .cmp(&right_position) - .is_le() - }) + ranges[start_ix..] + .iter() + .take_while(move |range| range.start.cmp(&right_position, buffer).is_le()) }) } @@ -7919,15 +7900,13 @@ impl Editor { display_snapshot: &DisplaySnapshot, theme: &Theme, ) -> Vec<(Range, Color)> { - let search_range = display_snapshot.anchor_to_inlay_offset(search_range.start) - ..display_snapshot.anchor_to_inlay_offset(search_range.end); let mut results = Vec::new(); for (color_fetcher, ranges) in self.background_highlights.values() { let color = color_fetcher(theme); let start_ix = match ranges.binary_search_by(|probe| { - let cmp = document_to_inlay_range(probe, display_snapshot) + let cmp = probe .end - .cmp(&search_range.start); + .cmp(&search_range.start, &display_snapshot.buffer_snapshot); if cmp.is_gt() { Ordering::Greater } else { @@ -7937,13 +7916,16 @@ impl Editor { Ok(i) | Err(i) => i, }; for range in &ranges[start_ix..] { - let range = document_to_inlay_range(range, display_snapshot); - if range.start.cmp(&search_range.end).is_ge() { + if range + .start + .cmp(&search_range.end, &display_snapshot.buffer_snapshot) + .is_ge() + { break; } - let start = display_snapshot.inlay_offset_to_display_point(range.start, Bias::Left); - let end = display_snapshot.inlay_offset_to_display_point(range.end, Bias::Right); + let start = range.start.to_display_point(&display_snapshot); + let end = range.end.to_display_point(&display_snapshot); results.push((start..end, color)) } } @@ -7956,17 +7938,15 @@ impl Editor { display_snapshot: &DisplaySnapshot, count: usize, ) -> Vec> { - let search_range = display_snapshot.anchor_to_inlay_offset(search_range.start) - ..display_snapshot.anchor_to_inlay_offset(search_range.end); let mut results = Vec::new(); let Some((_, ranges)) = self.background_highlights.get(&TypeId::of::()) else { return vec![]; }; let start_ix = match ranges.binary_search_by(|probe| { - let cmp = document_to_inlay_range(probe, display_snapshot) + let cmp = probe .end - .cmp(&search_range.start); + .cmp(&search_range.start, &display_snapshot.buffer_snapshot); if cmp.is_gt() { Ordering::Greater } else { @@ -7989,22 +7969,20 @@ impl Editor { return Vec::new(); } for range in &ranges[start_ix..] { - let range = document_to_inlay_range(range, display_snapshot); - if range.start.cmp(&search_range.end).is_ge() { + if range + .start + .cmp(&search_range.end, &display_snapshot.buffer_snapshot) + .is_ge() + { break; } - let end = display_snapshot - .inlay_offset_to_display_point(range.end, Bias::Right) - .to_point(display_snapshot); + let end = range.end.to_point(&display_snapshot.buffer_snapshot); if let Some(current_row) = &end_row { if end.row == current_row.row { continue; } } - let start = display_snapshot - .inlay_offset_to_display_point(range.start, Bias::Left) - .to_point(display_snapshot); - + let start = range.start.to_point(&display_snapshot.buffer_snapshot); if start_row.is_none() { assert_eq!(end_row, None); start_row = Some(start); @@ -8056,7 +8034,7 @@ impl Editor { pub fn text_highlights<'a, T: 'static>( &'a self, cx: &'a AppContext, - ) -> Option<(HighlightStyle, &'a [DocumentRange])> { + ) -> Option<(HighlightStyle, &'a [Range])> { self.display_map.read(cx).text_highlights(TypeId::of::()) } @@ -8281,7 +8259,6 @@ impl Editor { Some( ranges .iter() - .filter_map(|range| range.as_text_range()) .map(move |range| { range.start.to_offset_utf16(&snapshot)..range.end.to_offset_utf16(&snapshot) }) @@ -8496,19 +8473,6 @@ impl Editor { } } -fn document_to_inlay_range( - range: &DocumentRange, - snapshot: &DisplaySnapshot, -) -> Range { - match range { - DocumentRange::Text(text_range) => { - snapshot.anchor_to_inlay_offset(text_range.start) - ..snapshot.anchor_to_inlay_offset(text_range.end) - } - DocumentRange::Inlay(inlay_range) => inlay_range.highlight_start..inlay_range.highlight_end, - } -} - fn inlay_hint_settings( location: Anchor, snapshot: &MultiBufferSnapshot, @@ -8788,7 +8752,6 @@ impl View for Editor { fn marked_text_range(&self, cx: &AppContext) -> Option> { let snapshot = self.buffer.read(cx).read(cx); let range = self.text_highlights::(cx)?.1.get(0)?; - let range = range.as_text_range()?; Some(range.start.to_offset_utf16(&snapshot).0..range.end.to_offset_utf16(&snapshot).0) } diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index 3d5b1d21134c90554cbb273f7d058e3db621ad07..d7f78f74f06995931d3066ba4023eca0ee793e23 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -1,6 +1,6 @@ use crate::{ display_map::{InlayOffset, ToDisplayPoint}, - link_go_to_definition::{DocumentRange, InlayRange}, + link_go_to_definition::{InlayRange, RangeInEditor}, Anchor, AnchorRangeExt, DisplayPoint, Editor, EditorSettings, EditorSnapshot, EditorStyle, ExcerptId, RangeToAnchorExt, }; @@ -50,19 +50,18 @@ pub fn hover_at(editor: &mut Editor, point: Option, cx: &mut ViewC pub struct InlayHover { pub excerpt: ExcerptId, - pub triggered_from: InlayOffset, pub range: InlayRange, pub tooltip: HoverBlock, } pub fn find_hovered_hint_part( label_parts: Vec, - hint_range: Range, + hint_start: InlayOffset, hovered_offset: InlayOffset, ) -> Option<(InlayHintLabelPart, Range)> { - if hovered_offset >= hint_range.start && hovered_offset <= hint_range.end { - let mut hovered_character = (hovered_offset - hint_range.start).0; - let mut part_start = hint_range.start; + if hovered_offset >= hint_start { + let mut hovered_character = (hovered_offset - hint_start).0; + let mut part_start = hint_start; for part in label_parts { let part_len = part.value.chars().count(); if hovered_character > part_len { @@ -88,10 +87,8 @@ pub fn hover_at_inlay(editor: &mut Editor, inlay_hover: InlayHover, cx: &mut Vie }; if let Some(InfoPopover { symbol_range, .. }) = &editor.hover_state.info_popover { - if let DocumentRange::Inlay(range) = symbol_range { - if (range.highlight_start..range.highlight_end) - .contains(&inlay_hover.triggered_from) - { + if let RangeInEditor::Inlay(range) = symbol_range { + if range == &inlay_hover.range { // Hover triggered from same location as last time. Don't show again. return; } @@ -99,18 +96,6 @@ pub fn hover_at_inlay(editor: &mut Editor, inlay_hover: InlayHover, cx: &mut Vie hide_hover(editor, cx); } - let snapshot = editor.snapshot(cx); - // Don't request again if the location is the same as the previous request - if let Some(triggered_from) = editor.hover_state.triggered_from { - if inlay_hover.triggered_from - == snapshot - .display_snapshot - .anchor_to_inlay_offset(triggered_from) - { - return; - } - } - let task = cx.spawn(|this, mut cx| { async move { cx.background() @@ -122,7 +107,7 @@ pub fn hover_at_inlay(editor: &mut Editor, inlay_hover: InlayHover, cx: &mut Vie let hover_popover = InfoPopover { project: project.clone(), - symbol_range: DocumentRange::Inlay(inlay_hover.range), + symbol_range: RangeInEditor::Inlay(inlay_hover.range), blocks: vec![inlay_hover.tooltip], language: None, rendered_content: None, @@ -326,7 +311,7 @@ fn show_hover( Some(InfoPopover { project: project.clone(), - symbol_range: DocumentRange::Text(range), + symbol_range: RangeInEditor::Text(range), blocks: hover_result.contents, language: hover_result.language, rendered_content: None, @@ -608,8 +593,8 @@ impl HoverState { self.info_popover .as_ref() .map(|info_popover| match &info_popover.symbol_range { - DocumentRange::Text(range) => &range.start, - DocumentRange::Inlay(range) => &range.inlay_position, + RangeInEditor::Text(range) => &range.start, + RangeInEditor::Inlay(range) => &range.inlay_position, }) })?; let point = anchor.to_display_point(&snapshot.display_snapshot); @@ -635,7 +620,7 @@ impl HoverState { #[derive(Debug, Clone)] pub struct InfoPopover { pub project: ModelHandle, - symbol_range: DocumentRange, + symbol_range: RangeInEditor, pub blocks: Vec, language: Option>, rendered_content: Option, @@ -1488,17 +1473,18 @@ mod tests { ); let expected_new_type_label_start = InlayOffset(entire_inlay_start.0 + ": ".len()); - assert_eq!( - popover.symbol_range, - DocumentRange::Inlay(InlayRange { - inlay_position: buffer_snapshot.anchor_at(inlay_range.start, Bias::Right), - highlight_start: expected_new_type_label_start, - highlight_end: InlayOffset( - expected_new_type_label_start.0 + new_type_label.len() - ), - }), - "Popover range should match the new type label part" - ); + // TODO kb + // assert_eq!( + // popover.symbol_range, + // RangeInEditor::Inlay(InlayRange { + // inlay_position: buffer_snapshot.anchor_at(inlay_range.start, Bias::Right), + // highlight_start: expected_new_type_label_start, + // highlight_end: InlayOffset( + // expected_new_type_label_start.0 + new_type_label.len() + // ), + // }), + // "Popover range should match the new type label part" + // ); assert_eq!( popover .rendered_content @@ -1554,15 +1540,16 @@ mod tests { ); let expected_struct_label_start = InlayOffset(entire_inlay_start.0 + ": ".len() + new_type_label.len() + "<".len()); - assert_eq!( - popover.symbol_range, - DocumentRange::Inlay(InlayRange { - inlay_position: buffer_snapshot.anchor_at(inlay_range.start, Bias::Right), - highlight_start: expected_struct_label_start, - highlight_end: InlayOffset(expected_struct_label_start.0 + struct_label.len()), - }), - "Popover range should match the struct label part" - ); + // TODO kb + // assert_eq!( + // popover.symbol_range, + // RangeInEditor::Inlay(InlayRange { + // inlay_position: buffer_snapshot.anchor_at(inlay_range.start, Bias::Right), + // highlight_start: expected_struct_label_start, + // highlight_end: InlayOffset(expected_struct_label_start.0 + struct_label.len()), + // }), + // "Popover range should match the struct label part" + // ); assert_eq!( popover .rendered_content diff --git a/crates/editor/src/link_go_to_definition.rs b/crates/editor/src/link_go_to_definition.rs index 1f9a3aab730d4d6077df836e54ba09bc12f397d1..5663fe146981110c5405752916871a28fe51880a 100644 --- a/crates/editor/src/link_go_to_definition.rs +++ b/crates/editor/src/link_go_to_definition.rs @@ -1,8 +1,8 @@ use crate::{ - display_map::{DisplaySnapshot, InlayOffset}, + display_map::DisplaySnapshot, element::PointForPosition, hover_popover::{self, InlayHover}, - Anchor, DisplayPoint, Editor, EditorSnapshot, SelectPhase, + Anchor, DisplayPoint, Editor, EditorSnapshot, InlayId, SelectPhase, }; use gpui::{Task, ViewContext}; use language::{Bias, ToOffset}; @@ -17,12 +17,43 @@ use util::TryFutureExt; #[derive(Debug, Default)] pub struct LinkGoToDefinitionState { pub last_trigger_point: Option, - pub symbol_range: Option, + pub symbol_range: Option, pub kind: Option, pub definitions: Vec, pub task: Option>>, } +#[derive(Debug, Eq, PartialEq, Clone)] +pub enum RangeInEditor { + Text(Range), + Inlay(InlayRange), +} + +impl RangeInEditor { + pub fn as_text_range(&self) -> Option> { + match self { + Self::Text(range) => Some(range.clone()), + Self::Inlay(_) => None, + } + } + + fn point_within_range(&self, trigger_point: &TriggerPoint, snapshot: &EditorSnapshot) -> bool { + match (self, trigger_point) { + (Self::Text(range), TriggerPoint::Text(point)) => { + let point_after_start = range.start.cmp(point, &snapshot.buffer_snapshot).is_le(); + point_after_start && range.end.cmp(point, &snapshot.buffer_snapshot).is_ge() + } + (Self::Inlay(range), TriggerPoint::InlayHint(point, _, _)) => { + range.inlay == point.inlay + && range.highlight_start.cmp(&point.highlight_end).is_le() + && range.highlight_end.cmp(&point.highlight_end).is_ge() + } + (Self::Inlay(_), TriggerPoint::Text(_)) + | (Self::Text(_), TriggerPoint::InlayHint(_, _, _)) => false, + } + } +} + #[derive(Debug)] pub enum GoToDefinitionTrigger { Text(DisplayPoint), @@ -37,9 +68,11 @@ pub enum GoToDefinitionLink { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct InlayRange { + pub inlay: InlayId, + // TODO kb look up inlays by id instead? pub inlay_position: Anchor, - pub highlight_start: InlayOffset, - pub highlight_end: InlayOffset, + pub highlight_start: usize, + pub highlight_end: usize, } #[derive(Debug, Clone)] @@ -48,44 +81,7 @@ pub enum TriggerPoint { InlayHint(InlayRange, lsp::Location, LanguageServerId), } -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum DocumentRange { - Text(Range), - Inlay(InlayRange), -} - -impl DocumentRange { - pub fn as_text_range(&self) -> Option> { - match self { - Self::Text(range) => Some(range.clone()), - Self::Inlay(_) => None, - } - } - - fn point_within_range(&self, trigger_point: &TriggerPoint, snapshot: &EditorSnapshot) -> bool { - match (self, trigger_point) { - (DocumentRange::Text(range), TriggerPoint::Text(point)) => { - let point_after_start = range.start.cmp(point, &snapshot.buffer_snapshot).is_le(); - point_after_start && range.end.cmp(point, &snapshot.buffer_snapshot).is_ge() - } - (DocumentRange::Inlay(range), TriggerPoint::InlayHint(point, _, _)) => { - range.highlight_start.cmp(&point.highlight_end).is_le() - && range.highlight_end.cmp(&point.highlight_end).is_ge() - } - (DocumentRange::Inlay(_), TriggerPoint::Text(_)) - | (DocumentRange::Text(_), TriggerPoint::InlayHint(_, _, _)) => false, - } - } -} - impl TriggerPoint { - fn anchor(&self) -> &Anchor { - match self { - TriggerPoint::Text(anchor) => anchor, - TriggerPoint::InlayHint(range, _, _) => &range.inlay_position, - } - } - pub fn definition_kind(&self, shift: bool) -> LinkDefinitionKind { match self { TriggerPoint::Text(_) => { @@ -98,6 +94,13 @@ impl TriggerPoint { TriggerPoint::InlayHint(_, _, _) => LinkDefinitionKind::Type, } } + + fn anchor(&self) -> &Anchor { + match self { + TriggerPoint::Text(anchor) => anchor, + TriggerPoint::InlayHint(inlay_range, _, _) => &inlay_range.inlay_position, + } + } } pub fn update_go_to_definition_link( @@ -135,11 +138,7 @@ pub fn update_go_to_definition_link( } } (TriggerPoint::InlayHint(range_a, _, _), TriggerPoint::InlayHint(range_b, _, _)) => { - if range_a - .inlay_position - .cmp(&range_b.inlay_position, &snapshot.buffer_snapshot) - .is_eq() - { + if range_a == range_b { return; } } @@ -173,10 +172,6 @@ pub fn update_inlay_link_and_hover_points( shift_held: bool, cx: &mut ViewContext<'_, '_, Editor>, ) { - let hint_start_offset = - snapshot.display_point_to_inlay_offset(point_for_position.previous_valid, Bias::Left); - let hint_end_offset = - snapshot.display_point_to_inlay_offset(point_for_position.next_valid, Bias::Right); let hovered_offset = if point_for_position.column_overshoot_after_line_end == 0 { Some(snapshot.display_point_to_inlay_offset(point_for_position.exact_unclipped, Bias::Left)) } else { @@ -195,6 +190,7 @@ pub fn update_inlay_link_and_hover_points( Bias::Right, ); if let Some(hovered_hint) = editor + // TODO kb look up by position with binary search .visible_inlay_hints(cx) .into_iter() .skip_while(|hint| { @@ -224,15 +220,15 @@ pub fn update_inlay_link_and_hover_points( } } ResolveState::Resolved => { - let mut actual_hint_start = hint_start_offset; - let mut actual_hint_end = hint_end_offset; + let mut extra_shift_left = 0; + let mut extra_shift_right = 0; if cached_hint.padding_left { - actual_hint_start.0 += 1; - actual_hint_end.0 += 1; + extra_shift_left += 1; + extra_shift_right += 1; } + // TODO kb is it right for label part cases? for `\n` in hints and fold cases? if cached_hint.padding_right { - actual_hint_start.0 += 1; - actual_hint_end.0 += 1; + extra_shift_right += 1; } match cached_hint.label { project::InlayHintLabel::String(_) => { @@ -253,11 +249,12 @@ pub fn update_inlay_link_and_hover_points( } } }, - triggered_from: hovered_offset, range: InlayRange { + inlay: hovered_hint.id, inlay_position: hovered_hint.position, - highlight_start: actual_hint_start, - highlight_end: actual_hint_end, + highlight_start: extra_shift_left, + highlight_end: hovered_hint.text.len() + + extra_shift_right, }, }, cx, @@ -266,13 +263,23 @@ pub fn update_inlay_link_and_hover_points( } } project::InlayHintLabel::LabelParts(label_parts) => { + let hint_start = + snapshot.anchor_to_inlay_offset(hovered_hint.position); if let Some((hovered_hint_part, part_range)) = hover_popover::find_hovered_hint_part( label_parts, - actual_hint_start..actual_hint_end, + hint_start, hovered_offset, ) { + let range = InlayRange { + inlay: hovered_hint.id, + inlay_position: hovered_hint.position, + highlight_start: (part_range.start - hint_start).0 + + extra_shift_left, + highlight_end: (part_range.end - hint_start).0 + + extra_shift_right, + }; if let Some(tooltip) = hovered_hint_part.tooltip { hover_popover::hover_at_inlay( editor, @@ -292,12 +299,7 @@ pub fn update_inlay_link_and_hover_points( kind: content.kind, }, }, - triggered_from: hovered_offset, - range: InlayRange { - inlay_position: hovered_hint.position, - highlight_start: part_range.start, - highlight_end: part_range.end, - }, + range, }, cx, ); @@ -310,11 +312,7 @@ pub fn update_inlay_link_and_hover_points( update_go_to_definition_link( editor, Some(GoToDefinitionTrigger::InlayHint( - InlayRange { - inlay_position: hovered_hint.position, - highlight_start: part_range.start, - highlight_end: part_range.end, - }, + range, location, language_server_id, )), @@ -425,7 +423,7 @@ pub fn show_link_definition( let end = snapshot .buffer_snapshot .anchor_in_excerpt(excerpt_id.clone(), origin.range.end); - DocumentRange::Text(start..end) + RangeInEditor::Text(start..end) }) }), definition_result @@ -436,7 +434,7 @@ pub fn show_link_definition( }) } TriggerPoint::InlayHint(trigger_source, lsp_location, server_id) => Some(( - Some(DocumentRange::Inlay(*trigger_source)), + Some(RangeInEditor::Inlay(*trigger_source)), vec![GoToDefinitionLink::InlayHint( lsp_location.clone(), *server_id, @@ -498,24 +496,24 @@ pub fn show_link_definition( // If no symbol range returned from language server, use the surrounding word. let (offset_range, _) = snapshot.surrounding_word(*trigger_anchor); - DocumentRange::Text( + RangeInEditor::Text( snapshot.anchor_before(offset_range.start) ..snapshot.anchor_after(offset_range.end), ) } TriggerPoint::InlayHint(inlay_coordinates, _, _) => { - DocumentRange::Inlay(*inlay_coordinates) + RangeInEditor::Inlay(*inlay_coordinates) } }); match highlight_range { - DocumentRange::Text(text_range) => this + RangeInEditor::Text(text_range) => this .highlight_text::( vec![text_range], style, cx, ), - DocumentRange::Inlay(inlay_coordinates) => this + RangeInEditor::Inlay(inlay_coordinates) => this .highlight_inlays::( vec![inlay_coordinates], style, @@ -1202,27 +1200,20 @@ mod tests { let actual_ranges = snapshot .highlight_ranges::() .map(|ranges| ranges.as_ref().clone().1) - .unwrap_or_default() - .into_iter() - .map(|range| match range { - DocumentRange::Text(range) => { - panic!("Unexpected regular text selection range {range:?}") - } - DocumentRange::Inlay(inlay_range) => inlay_range, - }) - .collect::>(); + .unwrap_or_default(); let buffer_snapshot = editor.buffer().update(cx, |buffer, cx| buffer.snapshot(cx)); let expected_highlight_start = snapshot.display_point_to_inlay_offset( inlay_range.start.to_display_point(&snapshot), Bias::Left, ); - let expected_ranges = vec![InlayRange { - inlay_position: buffer_snapshot.anchor_at(inlay_range.start, Bias::Right), - highlight_start: expected_highlight_start, - highlight_end: InlayOffset(expected_highlight_start.0 + hint_label.len()), - }]; - assert_set_eq!(actual_ranges, expected_ranges); + // TODO kb + // let expected_ranges = vec![InlayRange { + // inlay_position: buffer_snapshot.anchor_at(inlay_range.start, Bias::Right), + // highlight_start: expected_highlight_start, + // highlight_end: InlayOffset(expected_highlight_start.0 + hint_label.len()), + // }]; + // assert_set_eq!(actual_ranges, expected_ranges); }); // Unpress cmd causes highlight to go away @@ -1244,15 +1235,7 @@ mod tests { let actual_ranges = snapshot .highlight_ranges::() .map(|ranges| ranges.as_ref().clone().1) - .unwrap_or_default() - .into_iter() - .map(|range| match range { - DocumentRange::Text(range) => { - panic!("Unexpected regular text selection range {range:?}") - } - DocumentRange::Inlay(inlay_range) => inlay_range, - }) - .collect::>(); + .unwrap_or_default(); assert!(actual_ranges.is_empty(), "When no cmd is pressed, should have no hint label selected, but got: {actual_ranges:?}"); }); diff --git a/crates/editor/src/test/editor_test_context.rs b/crates/editor/src/test/editor_test_context.rs index 033525395e17f0db865fff79c225338f282ec889..118cddaa9226a543ca479f577428237d77539d5d 100644 --- a/crates/editor/src/test/editor_test_context.rs +++ b/crates/editor/src/test/editor_test_context.rs @@ -225,7 +225,6 @@ impl<'a> EditorTestContext<'a> { .map(|h| h.1.clone()) .unwrap_or_default() .into_iter() - .filter_map(|range| range.as_text_range()) .map(|range| range.to_offset(&snapshot.buffer_snapshot)) .collect() }); @@ -241,7 +240,6 @@ impl<'a> EditorTestContext<'a> { .map(|ranges| ranges.as_ref().clone().1) .unwrap_or_default() .into_iter() - .filter_map(|range| range.as_text_range()) .map(|range| range.to_offset(&snapshot.buffer_snapshot)) .collect(); assert_set_eq!(actual_ranges, expected_ranges); From 9f5314e938924ba57ac1c6d043b4ea34769fe7c3 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 12:01:00 +0300 Subject: [PATCH 02/15] Unify highlights in *Map --- crates/ai/src/assistant.rs | 2 +- crates/editor/src/display_map.rs | 53 ++++++++++--------- crates/editor/src/display_map/block_map.rs | 20 +++---- crates/editor/src/display_map/fold_map.rs | 22 ++++---- crates/editor/src/display_map/inlay_map.rs | 33 ++++++------ crates/editor/src/display_map/tab_map.rs | 40 ++++++-------- crates/editor/src/display_map/wrap_map.rs | 24 +++------ crates/editor/src/editor.rs | 14 ++--- crates/editor/src/link_go_to_definition.rs | 8 +-- crates/editor/src/test/editor_test_context.rs | 2 +- 10 files changed, 100 insertions(+), 118 deletions(-) diff --git a/crates/ai/src/assistant.rs b/crates/ai/src/assistant.rs index 6d4fce2f6d09de085f2e78e8f10aeaad8c0bd16c..2376a1ff8764665d7a63a86a975f7820c789961e 100644 --- a/crates/ai/src/assistant.rs +++ b/crates/ai/src/assistant.rs @@ -702,7 +702,7 @@ impl AssistantPanel { } if foreground_ranges.is_empty() { - editor.clear_text_highlights::(cx); + editor.clear_highlights::(cx); } else { editor.highlight_text::( foreground_ranges, diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index d0cb7fd045812b0bca1043a35ab7518000e1905f..3ead02408d79febc1a074747bd5e1412985ccf43 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -43,8 +43,8 @@ pub trait ToDisplayPoint { fn to_display_point(&self, map: &DisplaySnapshot) -> DisplayPoint; } -// TODO kb InlayHighlights = ... ? type TextHighlights = TreeMap, Arc<(HighlightStyle, Vec>)>>; +type InlayHighlights = TreeMap, Arc<(HighlightStyle, Vec)>>; pub struct DisplayMap { buffer: ModelHandle, @@ -55,6 +55,7 @@ pub struct DisplayMap { wrap_map: ModelHandle, block_map: BlockMap, text_highlights: TextHighlights, + inlay_highlights: InlayHighlights, pub clip_at_line_ends: bool, } @@ -90,6 +91,7 @@ impl DisplayMap { wrap_map, block_map, text_highlights: Default::default(), + inlay_highlights: Default::default(), clip_at_line_ends: false, } } @@ -114,6 +116,7 @@ impl DisplayMap { wrap_snapshot, block_snapshot, text_highlights: self.text_highlights.clone(), + inlay_highlights: self.inlay_highlights.clone(), clip_at_line_ends: self.clip_at_line_ends, } } @@ -226,26 +229,18 @@ impl DisplayMap { ranges: Vec, style: HighlightStyle, ) { - // TODO kb - // self.text_highlights.insert( - // Some(type_id), - // Arc::new(( - // style, - // ranges.into_iter().map(DocumentRange::Inlay).collect(), - // )), - // ); + self.inlay_highlights + .insert(Some(type_id), Arc::new((style, ranges))); } pub fn text_highlights(&self, type_id: TypeId) -> Option<(HighlightStyle, &[Range])> { let highlights = self.text_highlights.get(&Some(type_id))?; Some((highlights.0, &highlights.1)) } - - pub fn clear_text_highlights( - &mut self, - type_id: TypeId, - ) -> Option>)>> { - self.text_highlights.remove(&Some(type_id)) + pub fn clear_highlights(&mut self, type_id: TypeId) -> bool { + let mut cleared = self.text_highlights.remove(&Some(type_id)).is_some(); + cleared |= self.inlay_highlights.remove(&Some(type_id)).is_none(); + cleared } pub fn set_font(&self, font_id: FontId, font_size: f32, cx: &mut ModelContext) -> bool { @@ -317,9 +312,18 @@ pub struct DisplaySnapshot { wrap_snapshot: wrap_map::WrapSnapshot, block_snapshot: block_map::BlockSnapshot, text_highlights: TextHighlights, + inlay_highlights: InlayHighlights, clip_at_line_ends: bool, } +#[derive(Debug, Default)] +pub struct Highlights<'a> { + pub text_highlights: Option<&'a TextHighlights>, + pub inlay_highlights: Option<&'a InlayHighlights>, + pub inlay_highlight_style: Option, + pub suggestion_highlight_style: Option, +} + impl DisplaySnapshot { #[cfg(test)] pub fn fold_count(&self) -> usize { @@ -454,9 +458,7 @@ impl DisplaySnapshot { .chunks( display_row..self.max_point().row() + 1, false, - None, - None, - None, + Highlights::default(), ) .map(|h| h.text) } @@ -465,7 +467,7 @@ impl DisplaySnapshot { pub fn reverse_text_chunks(&self, display_row: u32) -> impl Iterator { (0..=display_row).into_iter().rev().flat_map(|row| { self.block_snapshot - .chunks(row..row + 1, false, None, None, None) + .chunks(row..row + 1, false, Highlights::default()) .map(|h| h.text) .collect::>() .into_iter() @@ -477,15 +479,18 @@ impl DisplaySnapshot { &self, display_rows: Range, language_aware: bool, - hint_highlight_style: Option, + inlay_highlight_style: Option, suggestion_highlight_style: Option, ) -> DisplayChunks<'_> { self.block_snapshot.chunks( display_rows, language_aware, - Some(&self.text_highlights), - hint_highlight_style, - suggestion_highlight_style, + Highlights { + text_highlights: Some(&self.text_highlights), + inlay_highlights: Some(&self.inlay_highlights), + inlay_highlight_style, + suggestion_highlight_style, + }, ) } @@ -743,7 +748,7 @@ impl DisplaySnapshot { } #[cfg(any(test, feature = "test-support"))] - pub fn highlight_ranges( + pub fn text_highlight_ranges( &self, ) -> Option>)>> { let type_id = TypeId::of::(); diff --git a/crates/editor/src/display_map/block_map.rs b/crates/editor/src/display_map/block_map.rs index 741507004cc9bc0064ba682701310b832111438f..e54ac04d89c4d1f919730dc20b8844132f4c393d 100644 --- a/crates/editor/src/display_map/block_map.rs +++ b/crates/editor/src/display_map/block_map.rs @@ -1,10 +1,10 @@ use super::{ wrap_map::{self, WrapEdit, WrapPoint, WrapSnapshot}, - TextHighlights, + Highlights, }; use crate::{Anchor, Editor, ExcerptId, ExcerptRange, ToPoint as _}; use collections::{Bound, HashMap, HashSet}; -use gpui::{fonts::HighlightStyle, AnyElement, ViewContext}; +use gpui::{AnyElement, ViewContext}; use language::{BufferSnapshot, Chunk, Patch, Point}; use parking_lot::Mutex; use std::{ @@ -576,9 +576,7 @@ impl BlockSnapshot { self.chunks( 0..self.transforms.summary().output_rows, false, - None, - None, - None, + Highlights::default(), ) .map(|chunk| chunk.text) .collect() @@ -588,9 +586,7 @@ impl BlockSnapshot { &'a self, rows: Range, language_aware: bool, - text_highlights: Option<&'a TextHighlights>, - hint_highlight_style: Option, - suggestion_highlight_style: Option, + highlights: Highlights<'a>, ) -> BlockChunks<'a> { let max_output_row = cmp::min(rows.end, self.transforms.summary().output_rows); let mut cursor = self.transforms.cursor::<(BlockRow, WrapRow)>(); @@ -622,9 +618,7 @@ impl BlockSnapshot { input_chunks: self.wrap_snapshot.chunks( input_start..input_end, language_aware, - text_highlights, - hint_highlight_style, - suggestion_highlight_style, + highlights, ), input_chunk: Default::default(), transforms: cursor, @@ -1501,9 +1495,7 @@ mod tests { .chunks( start_row as u32..blocks_snapshot.max_point().row + 1, false, - None, - None, - None, + Highlights::default(), ) .map(|chunk| chunk.text) .collect::(); diff --git a/crates/editor/src/display_map/fold_map.rs b/crates/editor/src/display_map/fold_map.rs index d5473027a6b0145bad28f21c1e91ce7491f9eb63..8faa0c3ec2cc8cc74a87d7d98e2ee8181127e960 100644 --- a/crates/editor/src/display_map/fold_map.rs +++ b/crates/editor/src/display_map/fold_map.rs @@ -1,6 +1,6 @@ use super::{ inlay_map::{InlayBufferRows, InlayChunks, InlayEdit, InlayOffset, InlayPoint, InlaySnapshot}, - TextHighlights, + Highlights, }; use crate::{Anchor, AnchorRangeExt, MultiBufferSnapshot, ToOffset}; use gpui::{color::Color, fonts::HighlightStyle}; @@ -475,7 +475,7 @@ pub struct FoldSnapshot { impl FoldSnapshot { #[cfg(test)] pub fn text(&self) -> String { - self.chunks(FoldOffset(0)..self.len(), false, None, None, None) + self.chunks(FoldOffset(0)..self.len(), false, Highlights::default()) .map(|c| c.text) .collect() } @@ -651,9 +651,7 @@ impl FoldSnapshot { &'a self, range: Range, language_aware: bool, - text_highlights: Option<&'a TextHighlights>, - hint_highlight_style: Option, - suggestion_highlight_style: Option, + highlights: Highlights<'a>, ) -> FoldChunks<'a> { let mut transform_cursor = self.transforms.cursor::<(FoldOffset, InlayOffset)>(); @@ -674,9 +672,7 @@ impl FoldSnapshot { inlay_chunks: self.inlay_snapshot.chunks( inlay_start..inlay_end, language_aware, - text_highlights, - hint_highlight_style, - suggestion_highlight_style, + highlights, ), inlay_chunk: None, inlay_offset: inlay_start, @@ -687,8 +683,12 @@ impl FoldSnapshot { } pub fn chars_at(&self, start: FoldPoint) -> impl '_ + Iterator { - self.chunks(start.to_offset(self)..self.len(), false, None, None, None) - .flat_map(|chunk| chunk.text.chars()) + self.chunks( + start.to_offset(self)..self.len(), + false, + Highlights::default(), + ) + .flat_map(|chunk| chunk.text.chars()) } #[cfg(test)] @@ -1496,7 +1496,7 @@ mod tests { let text = &expected_text[start.0..end.0]; assert_eq!( snapshot - .chunks(start..end, false, None, None, None) + .chunks(start..end, false, Highlights::default()) .map(|c| c.text) .collect::(), text, diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 45cb7ec7f3394d0b80252a118a1ff76a9109da60..51b6806cd1809e1139323b747f3d7f1cd90d68e3 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -15,7 +15,7 @@ use std::{ use sum_tree::{Bias, Cursor, SumTree}; use text::{Patch, Rope}; -use super::TextHighlights; +use super::Highlights; pub struct InlayMap { snapshot: InlaySnapshot, @@ -213,7 +213,7 @@ pub struct InlayChunks<'a> { inlay_chunk: Option<&'a str>, output_offset: InlayOffset, max_output_offset: InlayOffset, - hint_highlight_style: Option, + inlay_highlight_style: Option, suggestion_highlight_style: Option, highlight_endpoints: Peekable>, active_highlights: BTreeMap, HighlightStyle>, @@ -314,7 +314,7 @@ impl<'a> Iterator for InlayChunks<'a> { self.output_offset.0 += chunk.len(); let mut highlight_style = match inlay.id { InlayId::Suggestion(_) => self.suggestion_highlight_style, - InlayId::Hint(_) => self.hint_highlight_style, + InlayId::Hint(_) => self.inlay_highlight_style, }; if !self.active_highlights.is_empty() { for active_highlight in self.active_highlights.values() { @@ -991,15 +991,13 @@ impl InlaySnapshot { &'a self, range: Range, language_aware: bool, - text_highlights: Option<&'a TextHighlights>, - hint_highlight_style: Option, - suggestion_highlight_style: Option, + highlights: Highlights<'a>, ) -> InlayChunks<'a> { let mut cursor = self.transforms.cursor::<(InlayOffset, usize)>(); cursor.seek(&range.start, Bias::Right, &()); let mut highlight_endpoints = Vec::new(); - if let Some(text_highlights) = text_highlights { + if let Some(text_highlights) = highlights.text_highlights { if !text_highlights.is_empty() { while cursor.start().0 < range.end { let transform_start = self.buffer.anchor_after( @@ -1065,8 +1063,8 @@ impl InlaySnapshot { buffer_chunk: None, output_offset: range.start, max_output_offset: range.end, - hint_highlight_style, - suggestion_highlight_style, + inlay_highlight_style: highlights.inlay_highlight_style, + suggestion_highlight_style: highlights.suggestion_highlight_style, highlight_endpoints: highlight_endpoints.into_iter().peekable(), active_highlights: Default::default(), snapshot: self, @@ -1075,7 +1073,7 @@ impl InlaySnapshot { #[cfg(test)] pub fn text(&self) -> String { - self.chunks(Default::default()..self.len(), false, None, None, None) + self.chunks(Default::default()..self.len(), false, Highlights::default()) .map(|chunk| chunk.text) .collect() } @@ -1123,7 +1121,7 @@ fn push_isomorphic(sum_tree: &mut SumTree, summary: TextSummary) { #[cfg(test)] mod tests { use super::*; - use crate::{InlayId, MultiBuffer}; + use crate::{display_map::TextHighlights, InlayId, MultiBuffer}; use gpui::AppContext; use project::{InlayHint, InlayHintLabel, ResolveState}; use rand::prelude::*; @@ -1619,7 +1617,7 @@ mod tests { ); } - let mut highlights = TextHighlights::default(); + let mut text_highlights = TextHighlights::default(); let highlight_count = rng.gen_range(0_usize..10); let mut highlight_ranges = (0..highlight_count) .map(|_| buffer_snapshot.random_byte_range(0, &mut rng)) @@ -1634,7 +1632,7 @@ mod tests { ..buffer_snapshot.anchor_after(range.end) }) .collect::>(); - highlights.insert( + text_highlights.insert( Some(TypeId::of::<()>()), Arc::new((HighlightStyle::default(), highlight_ranges)), ); @@ -1649,9 +1647,12 @@ mod tests { .chunks( InlayOffset(start)..InlayOffset(end), false, - Some(&highlights), - None, - None, + Highlights { + text_highlights: Some(&text_highlights), + inlay_highlights: None, + inlay_highlight_style: None, + suggestion_highlight_style: None, + }, ) .map(|chunk| chunk.text) .collect::(); diff --git a/crates/editor/src/display_map/tab_map.rs b/crates/editor/src/display_map/tab_map.rs index 2cf0471b37889a5cf5d3db26cfe3d1de91dc8e20..6b38ea2d243e77fcb8ea16e8d2d29ba83c4b003b 100644 --- a/crates/editor/src/display_map/tab_map.rs +++ b/crates/editor/src/display_map/tab_map.rs @@ -1,9 +1,8 @@ use super::{ fold_map::{self, FoldChunks, FoldEdit, FoldPoint, FoldSnapshot}, - TextHighlights, + Highlights, }; use crate::MultiBufferSnapshot; -use gpui::fonts::HighlightStyle; use language::{Chunk, Point}; use std::{cmp, mem, num::NonZeroU32, ops::Range}; use sum_tree::Bias; @@ -68,9 +67,7 @@ impl TabMap { 'outer: for chunk in old_snapshot.fold_snapshot.chunks( fold_edit.old.end..old_end_row_successor_offset, false, - None, - None, - None, + Highlights::default(), ) { for (ix, _) in chunk.text.match_indices('\t') { let offset_from_edit = offset_from_edit + (ix as u32); @@ -183,7 +180,7 @@ impl TabSnapshot { self.max_point() }; for c in self - .chunks(range.start..line_end, false, None, None, None) + .chunks(range.start..line_end, false, Highlights::default()) .flat_map(|chunk| chunk.text.chars()) { if c == '\n' { @@ -200,9 +197,7 @@ impl TabSnapshot { .chunks( TabPoint::new(range.end.row(), 0)..range.end, false, - None, - None, - None, + Highlights::default(), ) .flat_map(|chunk| chunk.text.chars()) { @@ -223,9 +218,7 @@ impl TabSnapshot { &'a self, range: Range, language_aware: bool, - text_highlights: Option<&'a TextHighlights>, - hint_highlight_style: Option, - suggestion_highlight_style: Option, + highlights: Highlights<'a>, ) -> TabChunks<'a> { let (input_start, expanded_char_column, to_next_stop) = self.to_fold_point(range.start, Bias::Left); @@ -245,9 +238,7 @@ impl TabSnapshot { fold_chunks: self.fold_snapshot.chunks( input_start..input_end, language_aware, - text_highlights, - hint_highlight_style, - suggestion_highlight_style, + highlights, ), input_column, column: expanded_char_column, @@ -270,9 +261,13 @@ impl TabSnapshot { #[cfg(test)] pub fn text(&self) -> String { - self.chunks(TabPoint::zero()..self.max_point(), false, None, None, None) - .map(|chunk| chunk.text) - .collect() + self.chunks( + TabPoint::zero()..self.max_point(), + false, + Highlights::default(), + ) + .map(|chunk| chunk.text) + .collect() } pub fn max_point(&self) -> TabPoint { @@ -597,9 +592,7 @@ mod tests { .chunks( TabPoint::new(0, ix as u32)..tab_snapshot.max_point(), false, - None, - None, - None, + Highlights::default(), ) .map(|c| c.text) .collect::(), @@ -674,7 +667,8 @@ mod tests { let mut chunks = Vec::new(); let mut was_tab = false; let mut text = String::new(); - for chunk in snapshot.chunks(start..snapshot.max_point(), false, None, None, None) { + for chunk in snapshot.chunks(start..snapshot.max_point(), false, Highlights::default()) + { if chunk.is_tab != was_tab { if !text.is_empty() { chunks.push((mem::take(&mut text), was_tab)); @@ -743,7 +737,7 @@ mod tests { let expected_summary = TextSummary::from(expected_text.as_str()); assert_eq!( tabs_snapshot - .chunks(start..end, false, None, None, None) + .chunks(start..end, false, Highlights::default()) .map(|c| c.text) .collect::(), expected_text, diff --git a/crates/editor/src/display_map/wrap_map.rs b/crates/editor/src/display_map/wrap_map.rs index f3600936f9bf77df6773ad14fd39f4e465398e15..60337661c1abfed638dd861a4c2e9464f70cf2ca 100644 --- a/crates/editor/src/display_map/wrap_map.rs +++ b/crates/editor/src/display_map/wrap_map.rs @@ -1,13 +1,11 @@ use super::{ fold_map::FoldBufferRows, tab_map::{self, TabEdit, TabPoint, TabSnapshot}, - TextHighlights, + Highlights, }; use crate::MultiBufferSnapshot; use gpui::{ - fonts::{FontId, HighlightStyle}, - text_layout::LineWrapper, - AppContext, Entity, ModelContext, ModelHandle, Task, + fonts::FontId, text_layout::LineWrapper, AppContext, Entity, ModelContext, ModelHandle, Task, }; use language::{Chunk, Point}; use lazy_static::lazy_static; @@ -444,9 +442,7 @@ impl WrapSnapshot { let mut chunks = new_tab_snapshot.chunks( TabPoint::new(edit.new_rows.start, 0)..new_tab_snapshot.max_point(), false, - None, - None, - None, + Highlights::default(), ); let mut edit_transforms = Vec::::new(); for _ in edit.new_rows.start..edit.new_rows.end { @@ -575,9 +571,7 @@ impl WrapSnapshot { &'a self, rows: Range, language_aware: bool, - text_highlights: Option<&'a TextHighlights>, - hint_highlight_style: Option, - suggestion_highlight_style: Option, + highlights: Highlights<'a>, ) -> WrapChunks<'a> { let output_start = WrapPoint::new(rows.start, 0); let output_end = WrapPoint::new(rows.end, 0); @@ -594,9 +588,7 @@ impl WrapSnapshot { input_chunks: self.tab_snapshot.chunks( input_start..input_end, language_aware, - text_highlights, - hint_highlight_style, - suggestion_highlight_style, + highlights, ), input_chunk: Default::default(), output_position: output_start, @@ -1323,9 +1315,7 @@ mod tests { self.chunks( wrap_row..self.max_point().row() + 1, false, - None, - None, - None, + Highlights::default(), ) .map(|h| h.text) } @@ -1350,7 +1340,7 @@ mod tests { } let actual_text = self - .chunks(start_row..end_row, true, None, None, None) + .chunks(start_row..end_row, true, Highlights::default()) .map(|c| c.text) .collect::(); assert_eq!( diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 985ca7875e531551e08d7ce68661b220c673c674..5e6eb7f74d0e156be3158e5679f10af1237049cf 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -7234,7 +7234,7 @@ impl Editor { Some(Autoscroll::fit()), cx, ); - self.clear_text_highlights::(cx); + self.clear_highlights::(cx); self.show_local_selections = true; if moving_cursor { @@ -8038,11 +8038,11 @@ impl Editor { self.display_map.read(cx).text_highlights(TypeId::of::()) } - pub fn clear_text_highlights(&mut self, cx: &mut ViewContext) { - let text_highlights = self + pub fn clear_highlights(&mut self, cx: &mut ViewContext) { + let cleared = self .display_map - .update(cx, |map, _| map.clear_text_highlights(TypeId::of::())); - if text_highlights.is_some() { + .update(cx, |map, _| map.clear_highlights(TypeId::of::())); + if cleared { cx.notify(); } } @@ -8683,7 +8683,7 @@ impl View for Editor { self.link_go_to_definition_state.task = None; - self.clear_text_highlights::(cx); + self.clear_highlights::(cx); } false @@ -8756,7 +8756,7 @@ impl View for Editor { } fn unmark_text(&mut self, cx: &mut ViewContext) { - self.clear_text_highlights::(cx); + self.clear_highlights::(cx); self.ime_transaction.take(); } diff --git a/crates/editor/src/link_go_to_definition.rs b/crates/editor/src/link_go_to_definition.rs index 5663fe146981110c5405752916871a28fe51880a..544a696e0b05fe7b865ee5dcb55ac8bf57407361 100644 --- a/crates/editor/src/link_go_to_definition.rs +++ b/crates/editor/src/link_go_to_definition.rs @@ -444,7 +444,7 @@ pub fn show_link_definition( this.update(&mut cx, |this, cx| { // Clear any existing highlights - this.clear_text_highlights::(cx); + this.clear_highlights::(cx); this.link_go_to_definition_state.kind = Some(definition_kind); this.link_go_to_definition_state.symbol_range = result .as_ref() @@ -545,7 +545,7 @@ pub fn hide_link_definition(editor: &mut Editor, cx: &mut ViewContext) { editor.link_go_to_definition_state.task = None; - editor.clear_text_highlights::(cx); + editor.clear_highlights::(cx); } pub fn go_to_fetched_definition( @@ -1198,7 +1198,7 @@ mod tests { cx.update_editor(|editor, cx| { let snapshot = editor.snapshot(cx); let actual_ranges = snapshot - .highlight_ranges::() + .text_highlight_ranges::() .map(|ranges| ranges.as_ref().clone().1) .unwrap_or_default(); @@ -1233,7 +1233,7 @@ mod tests { cx.update_editor(|editor, cx| { let snapshot = editor.snapshot(cx); let actual_ranges = snapshot - .highlight_ranges::() + .text_highlight_ranges::() .map(|ranges| ranges.as_ref().clone().1) .unwrap_or_default(); diff --git a/crates/editor/src/test/editor_test_context.rs b/crates/editor/src/test/editor_test_context.rs index 118cddaa9226a543ca479f577428237d77539d5d..0bae32f1f7087b05b67f166554671ca6359a6104 100644 --- a/crates/editor/src/test/editor_test_context.rs +++ b/crates/editor/src/test/editor_test_context.rs @@ -236,7 +236,7 @@ impl<'a> EditorTestContext<'a> { let expected_ranges = self.ranges(marked_text); let snapshot = self.update_editor(|editor, cx| editor.snapshot(cx)); let actual_ranges: Vec> = snapshot - .highlight_ranges::() + .text_highlight_ranges::() .map(|ranges| ranges.as_ref().clone().1) .unwrap_or_default() .into_iter() From 890a5872547335cba5ccc1dc54bd2956ead77dac Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 12:28:42 +0300 Subject: [PATCH 03/15] Use standalone inlay background highlights --- crates/editor/src/display_map.rs | 15 +++++++------ crates/editor/src/display_map/inlay_map.rs | 4 +--- crates/editor/src/editor.rs | 25 +++++++++------------- crates/editor/src/element.rs | 11 ++++++++-- crates/editor/src/link_go_to_definition.rs | 1 - crates/search/src/buffer_search.rs | 12 +++++------ crates/search/src/project_search.rs | 2 +- crates/vim/src/normal/search.rs | 2 +- crates/vim/src/test.rs | 2 +- 9 files changed, 38 insertions(+), 36 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index 3ead02408d79febc1a074747bd5e1412985ccf43..00ce8fb0ed236207bcbd4c6cbbaeb4e56ecb21e5 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -5,11 +5,11 @@ mod tab_map; mod wrap_map; use crate::{ - link_go_to_definition::InlayRange, Anchor, AnchorRangeExt, InlayId, MultiBuffer, - MultiBufferSnapshot, ToOffset, ToPoint, + link_go_to_definition::InlayRange, Anchor, AnchorRangeExt, InlayBackgroundHighlight, InlayId, + MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint, }; pub use block_map::{BlockMap, BlockPoint}; -use collections::{HashMap, HashSet}; +use collections::{BTreeMap, HashMap, HashSet}; use fold_map::FoldMap; use gpui::{ color::Color, @@ -320,6 +320,7 @@ pub struct DisplaySnapshot { pub struct Highlights<'a> { pub text_highlights: Option<&'a TextHighlights>, pub inlay_highlights: Option<&'a InlayHighlights>, + pub inlay_background_highlights: Option<&'a BTreeMap>, pub inlay_highlight_style: Option, pub suggestion_highlight_style: Option, } @@ -475,10 +476,11 @@ impl DisplaySnapshot { }) } - pub fn chunks( - &self, + pub fn chunks<'a>( + &'a self, display_rows: Range, language_aware: bool, + inlay_background_highlights: Option<&'a BTreeMap>, inlay_highlight_style: Option, suggestion_highlight_style: Option, ) -> DisplayChunks<'_> { @@ -488,6 +490,7 @@ impl DisplaySnapshot { Highlights { text_highlights: Some(&self.text_highlights), inlay_highlights: Some(&self.inlay_highlights), + inlay_background_highlights, inlay_highlight_style, suggestion_highlight_style, }, @@ -1699,7 +1702,7 @@ pub mod tests { ) -> Vec<(String, Option, Option)> { let snapshot = map.update(cx, |map, cx| map.snapshot(cx)); let mut chunks: Vec<(String, Option, Option)> = Vec::new(); - for chunk in snapshot.chunks(rows, true, None, None) { + for chunk in snapshot.chunks(rows, true, None, None, None) { let syntax_color = chunk .syntax_highlight_id .and_then(|id| id.style(theme)?.color); diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 51b6806cd1809e1139323b747f3d7f1cd90d68e3..7ec7bd1234167d0ae017c8a547621071382abf15 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -1649,9 +1649,7 @@ mod tests { false, Highlights { text_highlights: Some(&text_highlights), - inlay_highlights: None, - inlay_highlight_style: None, - suggestion_highlight_style: None, + ..Highlights::default() }, ) .map(|chunk| chunk.text) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 5e6eb7f74d0e156be3158e5679f10af1237049cf..ca9295227a266bd48306a3e41e218d0b1602442a 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -549,6 +549,7 @@ type GetFieldEditorTheme = dyn Fn(&theme::Theme) -> theme::FieldEditor; type OverrideTextStyle = dyn Fn(&EditorStyle) -> Option; type BackgroundHighlight = (fn(&Theme) -> Color, Vec>); +type InlayBackgroundHighlight = (fn(&Theme) -> Color, Vec); pub struct Editor { handle: WeakViewHandle, @@ -580,6 +581,7 @@ pub struct Editor { placeholder_text: Option>, highlighted_rows: Option>, background_highlights: BTreeMap, + inlay_background_highlights: BTreeMap, nav_history: Option, context_menu: Option, mouse_context_menu: ViewHandle, @@ -1523,6 +1525,7 @@ impl Editor { placeholder_text: None, highlighted_rows: None, background_highlights: Default::default(), + inlay_background_highlights: Default::default(), nav_history: None, context_menu: None, mouse_context_menu: cx @@ -7070,9 +7073,6 @@ impl Editor { } else { this.update(&mut cx, |this, cx| { let buffer = this.buffer.read(cx).snapshot(cx); - let display_snapshot = this - .display_map - .update(cx, |display_map, cx| display_map.snapshot(cx)); let mut buffer_highlights = this .document_highlights_for_position(selection.head(), &buffer) .filter(|highlight| { @@ -7822,14 +7822,8 @@ impl Editor { color_fetcher: fn(&Theme) -> Color, cx: &mut ViewContext, ) { - // TODO kb - // self.background_highlights.insert( - // TypeId::of::(), - // ( - // color_fetcher, - // ranges.into_iter().map(DocumentRange::Inlay).collect(), - // ), - // ); + self.inlay_background_highlights + .insert(TypeId::of::(), (color_fetcher, ranges)); cx.notify(); } @@ -7837,15 +7831,16 @@ impl Editor { &mut self, cx: &mut ViewContext, ) -> Option { - let highlights = self.background_highlights.remove(&TypeId::of::()); - if highlights.is_some() { + let text_highlights = self.background_highlights.remove(&TypeId::of::()); + let inlay_highlights = self.inlay_background_highlights.remove(&TypeId::of::()); + if text_highlights.is_some() || inlay_highlights.is_some() { cx.notify(); } - highlights + text_highlights } #[cfg(feature = "test-support")] - pub fn all_background_highlights( + pub fn all_text_background_highlights( &mut self, cx: &mut ViewContext, ) -> Vec<(Range, Color)> { diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index b7e34fda5377d6370d33cdec35087a4e544cd7d9..5eb4775ef047f1ca3b2c9046b6d4475827268746 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1547,6 +1547,7 @@ impl EditorElement { &mut self, rows: Range, line_number_layouts: &[Option], + editor: &mut Editor, snapshot: &EditorSnapshot, cx: &ViewContext, ) -> Vec { @@ -1595,6 +1596,7 @@ impl EditorElement { .chunks( rows.clone(), true, + Some(&editor.inlay_background_highlights), Some(style.theme.hint), Some(style.theme.suggestion), ) @@ -2355,8 +2357,13 @@ impl Element for EditorElement { let scrollbar_row_range = scroll_position.y()..(scroll_position.y() + height_in_lines); let mut max_visible_line_width = 0.0; - let line_layouts = - self.layout_lines(start_row..end_row, &line_number_layouts, &snapshot, cx); + let line_layouts = self.layout_lines( + start_row..end_row, + &line_number_layouts, + editor, + &snapshot, + cx, + ); for line_with_invisibles in &line_layouts { if line_with_invisibles.line.width() > max_visible_line_width { max_visible_line_width = line_with_invisibles.line.width(); diff --git a/crates/editor/src/link_go_to_definition.rs b/crates/editor/src/link_go_to_definition.rs index 544a696e0b05fe7b865ee5dcb55ac8bf57407361..f0b4ffaa663f2b66611192f9d82c93401fe53e23 100644 --- a/crates/editor/src/link_go_to_definition.rs +++ b/crates/editor/src/link_go_to_definition.rs @@ -69,7 +69,6 @@ pub enum GoToDefinitionLink { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct InlayRange { pub inlay: InlayId, - // TODO kb look up inlays by id instead? pub inlay_position: Anchor, pub highlight_start: usize, pub highlight_end: usize, diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index 9ae2b20f7af49626732697f7fdf418a9b4764fa7..fa26e926e72e52aada9bfbf098631c1ab4fae111 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -992,7 +992,7 @@ mod tests { .unwrap(); editor.update(cx, |editor, cx| { assert_eq!( - editor.all_background_highlights(cx), + editor.all_text_background_highlights(cx), &[ ( DisplayPoint::new(2, 17)..DisplayPoint::new(2, 19), @@ -1013,7 +1013,7 @@ mod tests { editor.next_notification(cx).await; editor.update(cx, |editor, cx| { assert_eq!( - editor.all_background_highlights(cx), + editor.all_text_background_highlights(cx), &[( DisplayPoint::new(2, 43)..DisplayPoint::new(2, 45), Color::red(), @@ -1029,7 +1029,7 @@ mod tests { .unwrap(); editor.update(cx, |editor, cx| { assert_eq!( - editor.all_background_highlights(cx), + editor.all_text_background_highlights(cx), &[ ( DisplayPoint::new(0, 24)..DisplayPoint::new(0, 26), @@ -1070,7 +1070,7 @@ mod tests { editor.next_notification(cx).await; editor.update(cx, |editor, cx| { assert_eq!( - editor.all_background_highlights(cx), + editor.all_text_background_highlights(cx), &[ ( DisplayPoint::new(0, 41)..DisplayPoint::new(0, 43), @@ -1281,7 +1281,7 @@ mod tests { .unwrap(); editor.update(cx, |editor, cx| { assert_eq!( - editor.all_background_highlights(cx), + editor.all_text_background_highlights(cx), &[( DisplayPoint::new(2, 43)..DisplayPoint::new(2, 45), Color::red(), @@ -1308,7 +1308,7 @@ mod tests { editor.next_notification(cx).await; editor.update(cx, |editor, cx| { assert_eq!( - editor.all_background_highlights(cx), + editor.all_text_background_highlights(cx), &[( DisplayPoint::new(0, 35)..DisplayPoint::new(0, 40), Color::red(), diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 6ca492880385437d8cc254dd1dd702c1d639f8e6..2bd128aa291ad831b1089d699448219a7abaa874 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -1724,7 +1724,7 @@ pub mod tests { assert_eq!( search_view .results_editor - .update(cx, |editor, cx| editor.all_background_highlights(cx)), + .update(cx, |editor, cx| editor.all_text_background_highlights(cx)), &[ ( DisplayPoint::new(2, 32)..DisplayPoint::new(2, 35), diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index d0c8a56249447100409bfd5cf385b2278752ca3d..c9c04007d1a4c0237047bb24f686668d40002290 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -227,7 +227,7 @@ mod test { deterministic.run_until_parked(); cx.update_editor(|editor, cx| { - let highlights = editor.all_background_highlights(cx); + let highlights = editor.all_text_background_highlights(cx); assert_eq!(3, highlights.len()); assert_eq!( DisplayPoint::new(2, 0)..DisplayPoint::new(2, 2), diff --git a/crates/vim/src/test.rs b/crates/vim/src/test.rs index a708d3c12adece2d758e95ffd762f86bae5b5fc8..559e065694615fd2bb7e4d7f27feaf37ce77c62f 100644 --- a/crates/vim/src/test.rs +++ b/crates/vim/src/test.rs @@ -190,7 +190,7 @@ async fn test_selection_on_search(cx: &mut gpui::TestAppContext) { search_bar.next_notification(&cx).await; cx.update_editor(|editor, cx| { - let highlights = editor.all_background_highlights(cx); + let highlights = editor.all_text_background_highlights(cx); assert_eq!(3, highlights.len()); assert_eq!( DisplayPoint::new(2, 0)..DisplayPoint::new(2, 2), From 42bd2be2f3de3c146fff0ece2640b10071ad6a75 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 14:46:14 +0300 Subject: [PATCH 04/15] Implement inlay highlighting --- crates/editor/src/display_map.rs | 29 +-- crates/editor/src/display_map/inlay_map.rs | 251 ++++++++++++++++----- crates/editor/src/editor.rs | 9 +- crates/editor/src/element.rs | 20 +- 4 files changed, 231 insertions(+), 78 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index 00ce8fb0ed236207bcbd4c6cbbaeb4e56ecb21e5..4241cfbc1465771dd89c2f5716294d5db9455a41 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -5,11 +5,11 @@ mod tab_map; mod wrap_map; use crate::{ - link_go_to_definition::InlayRange, Anchor, AnchorRangeExt, InlayBackgroundHighlight, InlayId, - MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint, + link_go_to_definition::InlayRange, Anchor, AnchorRangeExt, InlayId, MultiBuffer, + MultiBufferSnapshot, ToOffset, ToPoint, }; pub use block_map::{BlockMap, BlockPoint}; -use collections::{BTreeMap, HashMap, HashSet}; +use collections::{HashMap, HashSet}; use fold_map::FoldMap; use gpui::{ color::Color, @@ -304,6 +304,16 @@ impl DisplayMap { } } +#[derive(Debug, Default)] +pub struct Highlights<'a> { + pub text_highlights: Option<&'a TextHighlights>, + pub inlay_highlights: Option<&'a InlayHighlights>, + pub inlay_background_highlights: + Option, Arc<(HighlightStyle, &'a [InlayRange])>>>, + pub inlay_highlight_style: Option, + pub suggestion_highlight_style: Option, +} + pub struct DisplaySnapshot { pub buffer_snapshot: MultiBufferSnapshot, pub fold_snapshot: fold_map::FoldSnapshot, @@ -316,15 +326,6 @@ pub struct DisplaySnapshot { clip_at_line_ends: bool, } -#[derive(Debug, Default)] -pub struct Highlights<'a> { - pub text_highlights: Option<&'a TextHighlights>, - pub inlay_highlights: Option<&'a InlayHighlights>, - pub inlay_background_highlights: Option<&'a BTreeMap>, - pub inlay_highlight_style: Option, - pub suggestion_highlight_style: Option, -} - impl DisplaySnapshot { #[cfg(test)] pub fn fold_count(&self) -> usize { @@ -480,7 +481,9 @@ impl DisplaySnapshot { &'a self, display_rows: Range, language_aware: bool, - inlay_background_highlights: Option<&'a BTreeMap>, + inlay_background_highlights: Option< + TreeMap, Arc<(HighlightStyle, &'a [InlayRange])>>, + >, inlay_highlight_style: Option, suggestion_highlight_style: Option, ) -> DisplayChunks<'_> { diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 7ec7bd1234167d0ae017c8a547621071382abf15..02fae216487a377d431b78fbd3bc1dbf135505fd 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -1,4 +1,5 @@ use crate::{ + link_go_to_definition::InlayRange, multi_buffer::{MultiBufferChunks, MultiBufferRows}, Anchor, InlayId, MultiBufferSnapshot, ToOffset, }; @@ -10,22 +11,23 @@ use std::{ cmp, iter::Peekable, ops::{Add, AddAssign, Range, Sub, SubAssign}, + sync::Arc, vec, }; -use sum_tree::{Bias, Cursor, SumTree}; +use sum_tree::{Bias, Cursor, SumTree, TreeMap}; use text::{Patch, Rope}; use super::Highlights; pub struct InlayMap { snapshot: InlaySnapshot, - inlays: Vec, } #[derive(Clone)] pub struct InlaySnapshot { pub buffer: MultiBufferSnapshot, transforms: SumTree, + inlays: Vec, pub version: usize, } @@ -399,13 +401,13 @@ impl InlayMap { let snapshot = InlaySnapshot { buffer: buffer.clone(), transforms: SumTree::from_iter(Some(Transform::Isomorphic(buffer.text_summary())), &()), + inlays: Vec::new(), version, }; ( Self { snapshot: snapshot.clone(), - inlays: Vec::new(), }, snapshot, ) @@ -474,7 +476,7 @@ impl InlayMap { ); let new_start = InlayOffset(new_transforms.summary().output.len); - let start_ix = match self.inlays.binary_search_by(|probe| { + let start_ix = match snapshot.inlays.binary_search_by(|probe| { probe .position .to_offset(&buffer_snapshot) @@ -484,7 +486,7 @@ impl InlayMap { Ok(ix) | Err(ix) => ix, }; - for inlay in &self.inlays[start_ix..] { + for inlay in &snapshot.inlays[start_ix..] { let buffer_offset = inlay.position.to_offset(&buffer_snapshot); if buffer_offset > buffer_edit.new.end { break; @@ -554,7 +556,7 @@ impl InlayMap { let snapshot = &mut self.snapshot; let mut edits = BTreeSet::new(); - self.inlays.retain(|inlay| { + snapshot.inlays.retain(|inlay| { let retain = !to_remove.contains(&inlay.id); if !retain { let offset = inlay.position.to_offset(&snapshot.buffer); @@ -570,13 +572,13 @@ impl InlayMap { } let offset = inlay_to_insert.position.to_offset(&snapshot.buffer); - match self.inlays.binary_search_by(|probe| { + match snapshot.inlays.binary_search_by(|probe| { probe .position .cmp(&inlay_to_insert.position, &snapshot.buffer) }) { Ok(ix) | Err(ix) => { - self.inlays.insert(ix, inlay_to_insert); + snapshot.inlays.insert(ix, inlay_to_insert); } } @@ -596,7 +598,7 @@ impl InlayMap { } pub fn current_inlays(&self) -> impl Iterator { - self.inlays.iter() + self.snapshot.inlays.iter() } #[cfg(test)] @@ -612,7 +614,7 @@ impl InlayMap { let mut to_insert = Vec::new(); let snapshot = &mut self.snapshot; for i in 0..rng.gen_range(1..=5) { - if self.inlays.is_empty() || rng.gen() { + if snapshot.inlays.is_empty() || rng.gen() { let position = snapshot.buffer.random_byte_range(0, rng).start; let bias = if rng.gen() { Bias::Left } else { Bias::Right }; let len = if rng.gen_bool(0.01) { @@ -643,7 +645,8 @@ impl InlayMap { }); } else { to_remove.push( - self.inlays + snapshot + .inlays .iter() .choose(rng) .map(|inlay| inlay.id) @@ -997,61 +1000,50 @@ impl InlaySnapshot { cursor.seek(&range.start, Bias::Right, &()); let mut highlight_endpoints = Vec::new(); + // TODO kb repeat this for all other highlights? if let Some(text_highlights) = highlights.text_highlights { if !text_highlights.is_empty() { - while cursor.start().0 < range.end { - let transform_start = self.buffer.anchor_after( - self.to_buffer_offset(cmp::max(range.start, cursor.start().0)), - ); - let transform_end = { - let overshoot = InlayOffset(range.end.0 - cursor.start().0 .0); - self.buffer.anchor_before(self.to_buffer_offset(cmp::min( - cursor.end(&()).0, - cursor.start().0 + overshoot, - ))) - }; - - for (tag, text_highlights) in text_highlights.iter() { - let style = text_highlights.0; - let ranges = &text_highlights.1; - - let start_ix = match ranges.binary_search_by(|probe| { - let cmp = probe.end.cmp(&transform_start, &self.buffer); - if cmp.is_gt() { - cmp::Ordering::Greater - } else { - cmp::Ordering::Less - } - }) { - Ok(i) | Err(i) => i, - }; - for range in &ranges[start_ix..] { - if range.start.cmp(&transform_end, &self.buffer).is_ge() { - break; - } - - highlight_endpoints.push(HighlightEndpoint { - offset: self.to_inlay_offset(range.start.to_offset(&self.buffer)), - is_start: true, - tag: *tag, - style, - }); - highlight_endpoints.push(HighlightEndpoint { - offset: self.to_inlay_offset(range.end.to_offset(&self.buffer)), - is_start: false, - tag: *tag, - style, - }); - } - } - - cursor.next(&()); - } - highlight_endpoints.sort(); + self.apply_text_highlights( + &mut cursor, + &range, + text_highlights, + &mut highlight_endpoints, + ); + cursor.seek(&range.start, Bias::Right, &()); + } + } + if let Some(inlay_highlights) = highlights.inlay_highlights { + let adjusted_highlights = TreeMap::from_ordered_entries(inlay_highlights.iter().map( + |(type_id, styled_ranges)| { + ( + *type_id, + Arc::new((styled_ranges.0, styled_ranges.1.as_slice())), + ) + }, + )); + if !inlay_highlights.is_empty() { + self.apply_inlay_highlights( + &mut cursor, + &range, + &adjusted_highlights, + &mut highlight_endpoints, + ); + cursor.seek(&range.start, Bias::Right, &()); + } + } + if let Some(inlay_background_highlights) = highlights.inlay_background_highlights.as_ref() { + if !inlay_background_highlights.is_empty() { + self.apply_inlay_highlights( + &mut cursor, + &range, + inlay_background_highlights, + &mut highlight_endpoints, + ); cursor.seek(&range.start, Bias::Right, &()); } } + highlight_endpoints.sort(); let buffer_range = self.to_buffer_offset(range.start)..self.to_buffer_offset(range.end); let buffer_chunks = self.buffer.chunks(buffer_range, language_aware); @@ -1071,6 +1063,137 @@ impl InlaySnapshot { } } + fn apply_text_highlights( + &self, + cursor: &mut Cursor<'_, Transform, (InlayOffset, usize)>, + range: &Range, + text_highlights: &TreeMap, Arc<(HighlightStyle, Vec>)>>, + highlight_endpoints: &mut Vec, + ) { + while cursor.start().0 < range.end { + let transform_start = self + .buffer + .anchor_after(self.to_buffer_offset(cmp::max(range.start, cursor.start().0))); + let transform_end = + { + let overshoot = InlayOffset(range.end.0 - cursor.start().0 .0); + self.buffer.anchor_before(self.to_buffer_offset(cmp::min( + cursor.end(&()).0, + cursor.start().0 + overshoot, + ))) + }; + + for (tag, text_highlights) in text_highlights.iter() { + let style = text_highlights.0; + let ranges = &text_highlights.1; + + let start_ix = match ranges.binary_search_by(|probe| { + let cmp = probe.end.cmp(&transform_start, &self.buffer); + if cmp.is_gt() { + cmp::Ordering::Greater + } else { + cmp::Ordering::Less + } + }) { + Ok(i) | Err(i) => i, + }; + for range in &ranges[start_ix..] { + if range.start.cmp(&transform_end, &self.buffer).is_ge() { + break; + } + + highlight_endpoints.push(HighlightEndpoint { + offset: self.to_inlay_offset(range.start.to_offset(&self.buffer)), + is_start: true, + tag: *tag, + style, + }); + highlight_endpoints.push(HighlightEndpoint { + offset: self.to_inlay_offset(range.end.to_offset(&self.buffer)), + is_start: false, + tag: *tag, + style, + }); + } + } + + cursor.next(&()); + } + } + + fn apply_inlay_highlights( + &self, + cursor: &mut Cursor<'_, Transform, (InlayOffset, usize)>, + range: &Range, + inlay_highlights: &TreeMap, Arc<(HighlightStyle, &[InlayRange])>>, + highlight_endpoints: &mut Vec, + ) { + while cursor.start().0 < range.end { + let transform_start = self + .buffer + .anchor_after(self.to_buffer_offset(cmp::max(range.start, cursor.start().0))); + let transform_start = self.to_inlay_offset(transform_start.to_offset(&self.buffer)); + let transform_end = + { + let overshoot = InlayOffset(range.end.0 - cursor.start().0 .0); + self.buffer.anchor_before(self.to_buffer_offset(cmp::min( + cursor.end(&()).0, + cursor.start().0 + overshoot, + ))) + }; + let transform_end = self.to_inlay_offset(transform_end.to_offset(&self.buffer)); + + // TODO kb add a map + let hint_for_id = |id| self.inlays.iter().find(|inlay| inlay.id == id); + + for (tag, inlay_highlights) in inlay_highlights.iter() { + let style = inlay_highlights.0; + let ranges = inlay_highlights + .1 + .iter() + .filter_map(|range| Some((hint_for_id(range.inlay)?, range))) + .map(|(hint, range)| { + let hint_start = + self.to_inlay_offset(hint.position.to_offset(&self.buffer)); + let highlight_start = InlayOffset(hint_start.0 + range.highlight_start); + let highlight_end = InlayOffset(hint_start.0 + range.highlight_end); + highlight_start..highlight_end + }) + .collect::>(); + + let start_ix = match ranges.binary_search_by(|probe| { + if probe.end > transform_start { + cmp::Ordering::Greater + } else { + cmp::Ordering::Less + } + }) { + Ok(i) | Err(i) => i, + }; + for range in &ranges[start_ix..] { + if range.start >= transform_end { + break; + } + + highlight_endpoints.push(HighlightEndpoint { + offset: range.start, + is_start: true, + tag: *tag, + style, + }); + highlight_endpoints.push(HighlightEndpoint { + offset: range.end, + is_start: false, + tag: *tag, + style, + }); + } + } + + cursor.next(&()); + } + } + #[cfg(test)] pub fn text(&self) -> String { self.chunks(Default::default()..self.len(), false, Highlights::default()) @@ -1495,7 +1618,12 @@ mod tests { // The inlays can be manually removed. let (inlay_snapshot, _) = inlay_map.splice( - inlay_map.inlays.iter().map(|inlay| inlay.id).collect(), + inlay_map + .snapshot + .inlays + .iter() + .map(|inlay| inlay.id) + .collect(), Vec::new(), ); assert_eq!(inlay_snapshot.text(), "abxJKLyDzefghi"); @@ -1587,6 +1715,7 @@ mod tests { log::info!("inlay text: {:?}", inlay_snapshot.text()); let inlays = inlay_map + .snapshot .inlays .iter() .filter(|inlay| inlay.position.is_valid(&buffer_snapshot)) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index ca9295227a266bd48306a3e41e218d0b1602442a..14a421b2f994cb9a77546586ffe892883fca2447 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -99,6 +99,7 @@ use std::{ time::{Duration, Instant}, }; pub use sum_tree::Bias; +use sum_tree::TreeMap; use text::Rope; use theme::{DiagnosticStyle, Theme, ThemeSettings}; use util::{post_inc, RangeExt, ResultExt, TryFutureExt}; @@ -581,7 +582,7 @@ pub struct Editor { placeholder_text: Option>, highlighted_rows: Option>, background_highlights: BTreeMap, - inlay_background_highlights: BTreeMap, + inlay_background_highlights: TreeMap, InlayBackgroundHighlight>, nav_history: Option, context_menu: Option, mouse_context_menu: ViewHandle, @@ -7823,7 +7824,7 @@ impl Editor { cx: &mut ViewContext, ) { self.inlay_background_highlights - .insert(TypeId::of::(), (color_fetcher, ranges)); + .insert(Some(TypeId::of::()), (color_fetcher, ranges)); cx.notify(); } @@ -7832,7 +7833,9 @@ impl Editor { cx: &mut ViewContext, ) -> Option { let text_highlights = self.background_highlights.remove(&TypeId::of::()); - let inlay_highlights = self.inlay_background_highlights.remove(&TypeId::of::()); + let inlay_highlights = self + .inlay_background_highlights + .remove(&Some(TypeId::of::())); if text_highlights.is_some() || inlay_highlights.is_some() { cx.notify(); } diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 5eb4775ef047f1ca3b2c9046b6d4475827268746..bc3ca9f7b13593f909c751ac12cc6df965a06fbc 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -54,6 +54,7 @@ use std::{ ops::Range, sync::Arc, }; +use sum_tree::TreeMap; use text::Point; use workspace::item::Item; @@ -1592,11 +1593,28 @@ impl EditorElement { .collect() } else { let style = &self.style; + let theme = theme::current(cx); + let inlay_background_highlights = + TreeMap::from_ordered_entries(editor.inlay_background_highlights.iter().map( + |(type_id, (color_fetcher, ranges))| { + let color = Some(color_fetcher(&theme)); + ( + *type_id, + Arc::new(( + HighlightStyle { + color, + ..HighlightStyle::default() + }, + ranges.as_slice(), + )), + ) + }, + )); let chunks = snapshot .chunks( rows.clone(), true, - Some(&editor.inlay_background_highlights), + Some(inlay_background_highlights), Some(style.theme.hint), Some(style.theme.suggestion), ) From 80b96eb05be5f6d9971b00faeb5cd40d0a52314b Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 15:27:44 +0300 Subject: [PATCH 05/15] Add inlay highlight test --- crates/editor/src/display_map/inlay_map.rs | 90 +++++++++++++++++----- 1 file changed, 70 insertions(+), 20 deletions(-) diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 02fae216487a377d431b78fbd3bc1dbf135505fd..0ecbc3342599f41f148aa385aefe65e420931515 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -1244,7 +1244,10 @@ fn push_isomorphic(sum_tree: &mut SumTree, summary: TextSummary) { #[cfg(test)] mod tests { use super::*; - use crate::{display_map::TextHighlights, InlayId, MultiBuffer}; + use crate::{ + display_map::{InlayHighlights, TextHighlights}, + InlayId, MultiBuffer, + }; use gpui::AppContext; use project::{InlayHint, InlayHintLabel, ResolveState}; use rand::prelude::*; @@ -1725,8 +1728,8 @@ mod tests { }) .collect::>(); let mut expected_text = Rope::from(buffer_snapshot.text()); - for (offset, inlay) in inlays.into_iter().rev() { - expected_text.replace(offset..offset, &inlay.text.to_string()); + for (offset, inlay) in inlays.iter().rev() { + expected_text.replace(*offset..*offset, &inlay.text.to_string()); } assert_eq!(inlay_snapshot.text(), expected_text.to_string()); @@ -1747,24 +1750,70 @@ mod tests { } let mut text_highlights = TextHighlights::default(); + let mut inlay_highlights = InlayHighlights::default(); let highlight_count = rng.gen_range(0_usize..10); - let mut highlight_ranges = (0..highlight_count) - .map(|_| buffer_snapshot.random_byte_range(0, &mut rng)) - .collect::>(); - highlight_ranges.sort_by_key(|range| (range.start, Reverse(range.end))); - log::info!("highlighting ranges {highlight_ranges:?}"); - // TODO kb add inlay ranges into the tests - let highlight_ranges = highlight_ranges - .into_iter() - .map(|range| { - buffer_snapshot.anchor_before(range.start) - ..buffer_snapshot.anchor_after(range.end) - }) - .collect::>(); - text_highlights.insert( - Some(TypeId::of::<()>()), - Arc::new((HighlightStyle::default(), highlight_ranges)), - ); + if rng.gen_bool(0.5) { + let mut highlight_ranges = (0..highlight_count) + .map(|_| buffer_snapshot.random_byte_range(0, &mut rng)) + .collect::>(); + highlight_ranges.sort_by_key(|range| (range.start, Reverse(range.end))); + log::info!("highlighting text ranges {highlight_ranges:?}"); + text_highlights.insert( + Some(TypeId::of::<()>()), + Arc::new(( + HighlightStyle::default(), + highlight_ranges + .into_iter() + .map(|range| { + buffer_snapshot.anchor_before(range.start) + ..buffer_snapshot.anchor_after(range.end) + }) + .collect(), + )), + ); + } else { + let mut inlay_indices = BTreeSet::default(); + while inlay_indices.len() < highlight_count.min(inlays.len()) { + inlay_indices.insert(rng.gen_range(0..inlays.len())); + } + let highlight_ranges = inlay_indices + .into_iter() + .filter_map(|i| { + let (_, inlay) = &inlays[i]; + let inlay_text_len = inlay.text.len(); + match inlay_text_len { + 0 => None, + 1 => Some(InlayRange { + inlay: inlay.id, + inlay_position: inlay.position, + highlight_start: 0, + highlight_end: 1, + }), + n => { + let inlay_text = inlay.text.to_string(); + let mut highlight_end_offset_increment = rng.gen_range(1..n); + while !inlay_text.is_char_boundary(highlight_end_offset_increment) { + highlight_end_offset_increment += 1; + } + Some(InlayRange { + inlay: inlay.id, + inlay_position: inlay.position, + highlight_start: 0, + // TODO kb + // highlight_end_offset_increment: highlight_end_offset_increment, + highlight_end: inlay.text.len(), + }) + } + } + }) + .collect(); + + log::info!("highlighting inlay ranges {highlight_ranges:?}"); + inlay_highlights.insert( + Some(TypeId::of::<()>()), + Arc::new((HighlightStyle::default(), highlight_ranges)), + ); + }; for _ in 0..5 { let mut end = rng.gen_range(0..=inlay_snapshot.len().0); @@ -1778,6 +1827,7 @@ mod tests { false, Highlights { text_highlights: Some(&text_highlights), + inlay_highlights: Some(&inlay_highlights), ..Highlights::default() }, ) From a9de6c3dba11629cb231677588a0ea27563a3f28 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 17:01:36 +0300 Subject: [PATCH 06/15] Properly handle inlay highlights in the InlayMap Co-Authored-By: Antonio Scandurra --- crates/editor/src/display_map.rs | 11 +- crates/editor/src/display_map/inlay_map.rs | 224 +++++++++------------ crates/editor/src/editor.rs | 8 +- crates/editor/src/hover_popover.rs | 4 +- crates/editor/src/link_go_to_definition.rs | 13 +- crates/vim/src/state.rs | 1 - 6 files changed, 114 insertions(+), 147 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index 4241cfbc1465771dd89c2f5716294d5db9455a41..ed57e537bd5f46661f6747fb4cd5d09fa4ef32d6 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -5,7 +5,7 @@ mod tab_map; mod wrap_map; use crate::{ - link_go_to_definition::InlayRange, Anchor, AnchorRangeExt, InlayId, MultiBuffer, + link_go_to_definition::InlayHighlight, Anchor, AnchorRangeExt, InlayId, MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint, }; pub use block_map::{BlockMap, BlockPoint}; @@ -44,7 +44,7 @@ pub trait ToDisplayPoint { } type TextHighlights = TreeMap, Arc<(HighlightStyle, Vec>)>>; -type InlayHighlights = TreeMap, Arc<(HighlightStyle, Vec)>>; +type InlayHighlights = TreeMap, Arc<(HighlightStyle, Vec)>>; pub struct DisplayMap { buffer: ModelHandle, @@ -226,7 +226,7 @@ impl DisplayMap { pub fn highlight_inlays( &mut self, type_id: TypeId, - ranges: Vec, + ranges: Vec, style: HighlightStyle, ) { self.inlay_highlights @@ -308,8 +308,9 @@ impl DisplayMap { pub struct Highlights<'a> { pub text_highlights: Option<&'a TextHighlights>, pub inlay_highlights: Option<&'a InlayHighlights>, + // TODO kb remove, backgrounds are not handled in the *Map codegi pub inlay_background_highlights: - Option, Arc<(HighlightStyle, &'a [InlayRange])>>>, + Option, Arc<(HighlightStyle, &'a [InlayHighlight])>>>, pub inlay_highlight_style: Option, pub suggestion_highlight_style: Option, } @@ -482,7 +483,7 @@ impl DisplaySnapshot { display_rows: Range, language_aware: bool, inlay_background_highlights: Option< - TreeMap, Arc<(HighlightStyle, &'a [InlayRange])>>, + TreeMap, Arc<(HighlightStyle, &'a [InlayHighlight])>>, >, inlay_highlight_style: Option, suggestion_highlight_style: Option, diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 0ecbc3342599f41f148aa385aefe65e420931515..6c92f9b3d48b615a219a1e697fde8da8e760e993 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -1,5 +1,5 @@ use crate::{ - link_go_to_definition::InlayRange, + link_go_to_definition::InlayHighlight, multi_buffer::{MultiBufferChunks, MultiBufferRows}, Anchor, InlayId, MultiBufferSnapshot, ToOffset, }; @@ -219,6 +219,7 @@ pub struct InlayChunks<'a> { suggestion_highlight_style: Option, highlight_endpoints: Peekable>, active_highlights: BTreeMap, HighlightStyle>, + highlights: Highlights<'a>, snapshot: &'a InlaySnapshot, } @@ -294,6 +295,44 @@ impl<'a> Iterator for InlayChunks<'a> { prefix } Transform::Inlay(inlay) => { + let mut inlay_highlight_style_and_range = None; + if let Some(inlay_highlights) = self.highlights.inlay_highlights { + for (_, style_and_ranges) in inlay_highlights.iter() { + let highlight_style: &HighlightStyle = &style_and_ranges.0; + let inlay_ranges: &Vec = &style_and_ranges.1; + // TODO kb: turn into a map. + if let Some(range) = + inlay_ranges.iter().find(|range| range.inlay == inlay.id) + { + inlay_highlight_style_and_range = Some((highlight_style, range)); + break; + } + } + } + + let mut highlight_style = match inlay.id { + InlayId::Suggestion(_) => self.suggestion_highlight_style, + InlayId::Hint(_) => self.inlay_highlight_style, + }; + let next_inlay_highlight_endpoint; + if let Some((style, inlay_range)) = inlay_highlight_style_and_range { + let offset_in_inlay = (self.output_offset - self.transforms.start().0).0; + let range = inlay_range.highlight_start..inlay_range.highlight_end; + if offset_in_inlay < range.start { + next_inlay_highlight_endpoint = range.start; + } else if offset_in_inlay >= range.end { + next_inlay_highlight_endpoint = usize::MAX; + } else { + next_inlay_highlight_endpoint = range.end; + highlight_style + .get_or_insert_with(|| Default::default()) + .highlight(style.clone()); + } + } else { + next_inlay_highlight_endpoint = usize::MAX; + } + + // let inlay_chunks = self.inlay_chunks.get_or_insert_with(|| { let start = self.output_offset - self.transforms.start().0; let end = cmp::min(self.max_output_offset, self.transforms.end(&()).0) @@ -303,21 +342,15 @@ impl<'a> Iterator for InlayChunks<'a> { let inlay_chunk = self .inlay_chunk .get_or_insert_with(|| inlay_chunks.next().unwrap()); - let (chunk, remainder) = inlay_chunk.split_at( - inlay_chunk - .len() - .min(next_highlight_endpoint.0 - self.output_offset.0), - ); + let (chunk, remainder) = + inlay_chunk.split_at(inlay_chunk.len().min(next_inlay_highlight_endpoint)); *inlay_chunk = remainder; if inlay_chunk.is_empty() { self.inlay_chunk = None; } self.output_offset.0 += chunk.len(); - let mut highlight_style = match inlay.id { - InlayId::Suggestion(_) => self.suggestion_highlight_style, - InlayId::Hint(_) => self.inlay_highlight_style, - }; + if !self.active_highlights.is_empty() { for active_highlight in self.active_highlights.values() { highlight_style @@ -626,18 +659,20 @@ impl InlayMap { .filter(|ch| *ch != '\r') .take(len) .collect::(); - log::info!( - "creating inlay at buffer offset {} with bias {:?} and text {:?}", - position, - bias, - text - ); let inlay_id = if i % 2 == 0 { InlayId::Hint(post_inc(next_inlay_id)) } else { InlayId::Suggestion(post_inc(next_inlay_id)) }; + log::info!( + "creating inlay {:?} at buffer offset {} with bias {:?} and text {:?}", + inlay_id, + position, + bias, + text + ); + to_insert.push(Inlay { id: inlay_id, position: snapshot.buffer.anchor_at(position, bias), @@ -1012,38 +1047,39 @@ impl InlaySnapshot { cursor.seek(&range.start, Bias::Right, &()); } } - if let Some(inlay_highlights) = highlights.inlay_highlights { - let adjusted_highlights = TreeMap::from_ordered_entries(inlay_highlights.iter().map( - |(type_id, styled_ranges)| { - ( - *type_id, - Arc::new((styled_ranges.0, styled_ranges.1.as_slice())), - ) - }, - )); - if !inlay_highlights.is_empty() { - self.apply_inlay_highlights( - &mut cursor, - &range, - &adjusted_highlights, - &mut highlight_endpoints, - ); - cursor.seek(&range.start, Bias::Right, &()); - } - } - if let Some(inlay_background_highlights) = highlights.inlay_background_highlights.as_ref() { - if !inlay_background_highlights.is_empty() { - self.apply_inlay_highlights( - &mut cursor, - &range, - inlay_background_highlights, - &mut highlight_endpoints, - ); - cursor.seek(&range.start, Bias::Right, &()); - } - } - highlight_endpoints.sort(); + + // if let Some(inlay_highlights) = highlights.inlay_highlights { + // let adjusted_highlights = TreeMap::from_ordered_entries(inlay_highlights.iter().map( + // |(type_id, styled_ranges)| { + // ( + // *type_id, + // Arc::new((styled_ranges.0, styled_ranges.1.as_slice())), + // ) + // }, + // )); + // if !inlay_highlights.is_empty() { + // self.apply_inlay_highlights( + // &mut cursor, + // &range, + // &adjusted_highlights, + // &mut highlight_endpoints, + // ); + // cursor.seek(&range.start, Bias::Right, &()); + // } + // } + // if let Some(inlay_background_highlights) = highlights.inlay_background_highlights.as_ref() { + // if !inlay_background_highlights.is_empty() { + // self.apply_inlay_highlights( + // &mut cursor, + // &range, + // inlay_background_highlights, + // &mut highlight_endpoints, + // ); + // cursor.seek(&range.start, Bias::Right, &()); + // } + // } + let buffer_range = self.to_buffer_offset(range.start)..self.to_buffer_offset(range.end); let buffer_chunks = self.buffer.chunks(buffer_range, language_aware); @@ -1059,6 +1095,7 @@ impl InlaySnapshot { suggestion_highlight_style: highlights.suggestion_highlight_style, highlight_endpoints: highlight_endpoints.into_iter().peekable(), active_highlights: Default::default(), + highlights, snapshot: self, } } @@ -1121,79 +1158,6 @@ impl InlaySnapshot { } } - fn apply_inlay_highlights( - &self, - cursor: &mut Cursor<'_, Transform, (InlayOffset, usize)>, - range: &Range, - inlay_highlights: &TreeMap, Arc<(HighlightStyle, &[InlayRange])>>, - highlight_endpoints: &mut Vec, - ) { - while cursor.start().0 < range.end { - let transform_start = self - .buffer - .anchor_after(self.to_buffer_offset(cmp::max(range.start, cursor.start().0))); - let transform_start = self.to_inlay_offset(transform_start.to_offset(&self.buffer)); - let transform_end = - { - let overshoot = InlayOffset(range.end.0 - cursor.start().0 .0); - self.buffer.anchor_before(self.to_buffer_offset(cmp::min( - cursor.end(&()).0, - cursor.start().0 + overshoot, - ))) - }; - let transform_end = self.to_inlay_offset(transform_end.to_offset(&self.buffer)); - - // TODO kb add a map - let hint_for_id = |id| self.inlays.iter().find(|inlay| inlay.id == id); - - for (tag, inlay_highlights) in inlay_highlights.iter() { - let style = inlay_highlights.0; - let ranges = inlay_highlights - .1 - .iter() - .filter_map(|range| Some((hint_for_id(range.inlay)?, range))) - .map(|(hint, range)| { - let hint_start = - self.to_inlay_offset(hint.position.to_offset(&self.buffer)); - let highlight_start = InlayOffset(hint_start.0 + range.highlight_start); - let highlight_end = InlayOffset(hint_start.0 + range.highlight_end); - highlight_start..highlight_end - }) - .collect::>(); - - let start_ix = match ranges.binary_search_by(|probe| { - if probe.end > transform_start { - cmp::Ordering::Greater - } else { - cmp::Ordering::Less - } - }) { - Ok(i) | Err(i) => i, - }; - for range in &ranges[start_ix..] { - if range.start >= transform_end { - break; - } - - highlight_endpoints.push(HighlightEndpoint { - offset: range.start, - is_start: true, - tag: *tag, - style, - }); - highlight_endpoints.push(HighlightEndpoint { - offset: range.end, - is_start: false, - tag: *tag, - style, - }); - } - } - - cursor.next(&()); - } - } - #[cfg(test)] pub fn text(&self) -> String { self.chunks(Default::default()..self.len(), false, Highlights::default()) @@ -1752,7 +1716,7 @@ mod tests { let mut text_highlights = TextHighlights::default(); let mut inlay_highlights = InlayHighlights::default(); let highlight_count = rng.gen_range(0_usize..10); - if rng.gen_bool(0.5) { + if false && rng.gen_bool(0.5) { let mut highlight_ranges = (0..highlight_count) .map(|_| buffer_snapshot.random_byte_range(0, &mut rng)) .collect::>(); @@ -1783,7 +1747,7 @@ mod tests { let inlay_text_len = inlay.text.len(); match inlay_text_len { 0 => None, - 1 => Some(InlayRange { + 1 => Some(InlayHighlight { inlay: inlay.id, inlay_position: inlay.position, highlight_start: 0, @@ -1791,17 +1755,17 @@ mod tests { }), n => { let inlay_text = inlay.text.to_string(); - let mut highlight_end_offset_increment = rng.gen_range(1..n); - while !inlay_text.is_char_boundary(highlight_end_offset_increment) { - highlight_end_offset_increment += 1; + let mut highlight_end = rng.gen_range(1..n); + while !inlay_text.is_char_boundary(highlight_end) { + highlight_end += 1; } - Some(InlayRange { + Some(InlayHighlight { inlay: inlay.id, inlay_position: inlay.position, highlight_start: 0, // TODO kb - // highlight_end_offset_increment: highlight_end_offset_increment, - highlight_end: inlay.text.len(), + // highlight_end + highlight_end: inlay_text.len(), }) } } @@ -1821,9 +1785,11 @@ mod tests { let mut start = rng.gen_range(0..=end); start = expected_text.clip_offset(start, Bias::Right); + let range = InlayOffset(start)..InlayOffset(end); + log::info!("calling inlay_snapshot.chunks({:?})", range); let actual_text = inlay_snapshot .chunks( - InlayOffset(start)..InlayOffset(end), + range, false, Highlights { text_highlights: Some(&text_highlights), diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 14a421b2f994cb9a77546586ffe892883fca2447..67560b1b033eaacf7c60324439fb551a6244f855 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -66,7 +66,7 @@ use language::{ TransactionId, }; use link_go_to_definition::{ - hide_link_definition, show_link_definition, GoToDefinitionLink, InlayRange, + hide_link_definition, show_link_definition, GoToDefinitionLink, InlayHighlight, LinkGoToDefinitionState, }; use log::error; @@ -550,7 +550,7 @@ type GetFieldEditorTheme = dyn Fn(&theme::Theme) -> theme::FieldEditor; type OverrideTextStyle = dyn Fn(&EditorStyle) -> Option; type BackgroundHighlight = (fn(&Theme) -> Color, Vec>); -type InlayBackgroundHighlight = (fn(&Theme) -> Color, Vec); +type InlayBackgroundHighlight = (fn(&Theme) -> Color, Vec); pub struct Editor { handle: WeakViewHandle, @@ -7819,7 +7819,7 @@ impl Editor { pub fn highlight_inlay_background( &mut self, - ranges: Vec, + ranges: Vec, color_fetcher: fn(&Theme) -> Color, cx: &mut ViewContext, ) { @@ -8019,7 +8019,7 @@ impl Editor { pub fn highlight_inlays( &mut self, - ranges: Vec, + ranges: Vec, style: HighlightStyle, cx: &mut ViewContext, ) { diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index d7f78f74f06995931d3066ba4023eca0ee793e23..1cec238e4a6d97a9d30ec8de1dbbd554506311ec 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -1,6 +1,6 @@ use crate::{ display_map::{InlayOffset, ToDisplayPoint}, - link_go_to_definition::{InlayRange, RangeInEditor}, + link_go_to_definition::{InlayHighlight, RangeInEditor}, Anchor, AnchorRangeExt, DisplayPoint, Editor, EditorSettings, EditorSnapshot, EditorStyle, ExcerptId, RangeToAnchorExt, }; @@ -50,7 +50,7 @@ pub fn hover_at(editor: &mut Editor, point: Option, cx: &mut ViewC pub struct InlayHover { pub excerpt: ExcerptId, - pub range: InlayRange, + pub range: InlayHighlight, pub tooltip: HoverBlock, } diff --git a/crates/editor/src/link_go_to_definition.rs b/crates/editor/src/link_go_to_definition.rs index f0b4ffaa663f2b66611192f9d82c93401fe53e23..3797de5e57876148c8ade870bc554afa93426532 100644 --- a/crates/editor/src/link_go_to_definition.rs +++ b/crates/editor/src/link_go_to_definition.rs @@ -26,7 +26,7 @@ pub struct LinkGoToDefinitionState { #[derive(Debug, Eq, PartialEq, Clone)] pub enum RangeInEditor { Text(Range), - Inlay(InlayRange), + Inlay(InlayHighlight), } impl RangeInEditor { @@ -57,7 +57,7 @@ impl RangeInEditor { #[derive(Debug)] pub enum GoToDefinitionTrigger { Text(DisplayPoint), - InlayHint(InlayRange, lsp::Location, LanguageServerId), + InlayHint(InlayHighlight, lsp::Location, LanguageServerId), } #[derive(Debug, Clone)] @@ -67,9 +67,10 @@ pub enum GoToDefinitionLink { } #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct InlayRange { +pub struct InlayHighlight { pub inlay: InlayId, pub inlay_position: Anchor, + // TODO kb turn into Range pub highlight_start: usize, pub highlight_end: usize, } @@ -77,7 +78,7 @@ pub struct InlayRange { #[derive(Debug, Clone)] pub enum TriggerPoint { Text(Anchor), - InlayHint(InlayRange, lsp::Location, LanguageServerId), + InlayHint(InlayHighlight, lsp::Location, LanguageServerId), } impl TriggerPoint { @@ -248,7 +249,7 @@ pub fn update_inlay_link_and_hover_points( } } }, - range: InlayRange { + range: InlayHighlight { inlay: hovered_hint.id, inlay_position: hovered_hint.position, highlight_start: extra_shift_left, @@ -271,7 +272,7 @@ pub fn update_inlay_link_and_hover_points( hovered_offset, ) { - let range = InlayRange { + let range = InlayHighlight { inlay: hovered_hint.id, inlay_position: hovered_hint.position, highlight_start: (part_range.start - hint_start).0 diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index 2cb5e058e8726e88a8282b4969b8f3b2b659b6ed..8fd4049767bfada65bbfd57142eb96c43c310366 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -186,7 +186,6 @@ impl EditorState { if self.active_operator().is_none() && self.pre_count.is_some() || self.active_operator().is_some() && self.post_count.is_some() { - dbg!("VimCount"); context.add_identifier("VimCount"); } From 129fb62182850e5b9605c31387b0afaf91614ab7 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 21:39:22 +0300 Subject: [PATCH 07/15] Consider offsets in inlay chunks --- crates/editor/src/display_map/inlay_map.rs | 48 +++------------------- 1 file changed, 6 insertions(+), 42 deletions(-) diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 6c92f9b3d48b615a219a1e697fde8da8e760e993..53c833a1b008b76c8e62683357d3479363a387ff 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -315,15 +315,15 @@ impl<'a> Iterator for InlayChunks<'a> { InlayId::Hint(_) => self.inlay_highlight_style, }; let next_inlay_highlight_endpoint; + let offset_in_inlay = self.output_offset - self.transforms.start().0; if let Some((style, inlay_range)) = inlay_highlight_style_and_range { - let offset_in_inlay = (self.output_offset - self.transforms.start().0).0; let range = inlay_range.highlight_start..inlay_range.highlight_end; - if offset_in_inlay < range.start { + if offset_in_inlay.0 < range.start { next_inlay_highlight_endpoint = range.start; - } else if offset_in_inlay >= range.end { + } else if offset_in_inlay.0 >= range.end { next_inlay_highlight_endpoint = usize::MAX; } else { - next_inlay_highlight_endpoint = range.end; + next_inlay_highlight_endpoint = range.end - offset_in_inlay.0; highlight_style .get_or_insert_with(|| Default::default()) .highlight(style.clone()); @@ -332,9 +332,8 @@ impl<'a> Iterator for InlayChunks<'a> { next_inlay_highlight_endpoint = usize::MAX; } - // let inlay_chunks = self.inlay_chunks.get_or_insert_with(|| { - let start = self.output_offset - self.transforms.start().0; + let start = offset_in_inlay; let end = cmp::min(self.max_output_offset, self.transforms.end(&()).0) - self.transforms.start().0; inlay.text.chunks_in_range(start.0..end.0) @@ -1035,7 +1034,6 @@ impl InlaySnapshot { cursor.seek(&range.start, Bias::Right, &()); let mut highlight_endpoints = Vec::new(); - // TODO kb repeat this for all other highlights? if let Some(text_highlights) = highlights.text_highlights { if !text_highlights.is_empty() { self.apply_text_highlights( @@ -1048,38 +1046,6 @@ impl InlaySnapshot { } } highlight_endpoints.sort(); - - // if let Some(inlay_highlights) = highlights.inlay_highlights { - // let adjusted_highlights = TreeMap::from_ordered_entries(inlay_highlights.iter().map( - // |(type_id, styled_ranges)| { - // ( - // *type_id, - // Arc::new((styled_ranges.0, styled_ranges.1.as_slice())), - // ) - // }, - // )); - // if !inlay_highlights.is_empty() { - // self.apply_inlay_highlights( - // &mut cursor, - // &range, - // &adjusted_highlights, - // &mut highlight_endpoints, - // ); - // cursor.seek(&range.start, Bias::Right, &()); - // } - // } - // if let Some(inlay_background_highlights) = highlights.inlay_background_highlights.as_ref() { - // if !inlay_background_highlights.is_empty() { - // self.apply_inlay_highlights( - // &mut cursor, - // &range, - // inlay_background_highlights, - // &mut highlight_endpoints, - // ); - // cursor.seek(&range.start, Bias::Right, &()); - // } - // } - let buffer_range = self.to_buffer_offset(range.start)..self.to_buffer_offset(range.end); let buffer_chunks = self.buffer.chunks(buffer_range, language_aware); @@ -1763,9 +1729,7 @@ mod tests { inlay: inlay.id, inlay_position: inlay.position, highlight_start: 0, - // TODO kb - // highlight_end - highlight_end: inlay_text.len(), + highlight_end, }) } } From 47e0535f1c7efc87449473292420f76dbbbfee74 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 21:57:05 +0300 Subject: [PATCH 08/15] Randomize inlay highlight range start --- crates/editor/src/display_map/inlay_map.rs | 17 ++++---- crates/editor/src/hover_popover.rs | 2 +- crates/editor/src/link_go_to_definition.rs | 46 +++++++++++----------- 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 53c833a1b008b76c8e62683357d3479363a387ff..5efa15a1c2b54704c80ad66cd532537abbf97aff 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -316,10 +316,10 @@ impl<'a> Iterator for InlayChunks<'a> { }; let next_inlay_highlight_endpoint; let offset_in_inlay = self.output_offset - self.transforms.start().0; - if let Some((style, inlay_range)) = inlay_highlight_style_and_range { - let range = inlay_range.highlight_start..inlay_range.highlight_end; + if let Some((style, highlight)) = inlay_highlight_style_and_range { + let range = &highlight.range; if offset_in_inlay.0 < range.start { - next_inlay_highlight_endpoint = range.start; + next_inlay_highlight_endpoint = range.start - offset_in_inlay.0; } else if offset_in_inlay.0 >= range.end { next_inlay_highlight_endpoint = usize::MAX; } else { @@ -1711,25 +1711,28 @@ mod tests { .filter_map(|i| { let (_, inlay) = &inlays[i]; let inlay_text_len = inlay.text.len(); + // TODO kb gen_range match inlay_text_len { 0 => None, 1 => Some(InlayHighlight { inlay: inlay.id, inlay_position: inlay.position, - highlight_start: 0, - highlight_end: 1, + range: 0..1, }), n => { let inlay_text = inlay.text.to_string(); let mut highlight_end = rng.gen_range(1..n); + let mut highlight_start = rng.gen_range(0..highlight_end); while !inlay_text.is_char_boundary(highlight_end) { highlight_end += 1; } + while !inlay_text.is_char_boundary(highlight_start) { + highlight_start -= 1; + } Some(InlayHighlight { inlay: inlay.id, inlay_position: inlay.position, - highlight_start: 0, - highlight_end, + range: highlight_start..highlight_end, }) } } diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index 1cec238e4a6d97a9d30ec8de1dbbd554506311ec..79de3786a7befa7281f53ed1753ea2368361daca 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -107,7 +107,7 @@ pub fn hover_at_inlay(editor: &mut Editor, inlay_hover: InlayHover, cx: &mut Vie let hover_popover = InfoPopover { project: project.clone(), - symbol_range: RangeInEditor::Inlay(inlay_hover.range), + symbol_range: RangeInEditor::Inlay(inlay_hover.range.clone()), blocks: vec![inlay_hover.tooltip], language: None, rendered_content: None, diff --git a/crates/editor/src/link_go_to_definition.rs b/crates/editor/src/link_go_to_definition.rs index 3797de5e57876148c8ade870bc554afa93426532..581bda7e58c78070ebeca40485fe45d3e9ed93bb 100644 --- a/crates/editor/src/link_go_to_definition.rs +++ b/crates/editor/src/link_go_to_definition.rs @@ -43,10 +43,10 @@ impl RangeInEditor { let point_after_start = range.start.cmp(point, &snapshot.buffer_snapshot).is_le(); point_after_start && range.end.cmp(point, &snapshot.buffer_snapshot).is_ge() } - (Self::Inlay(range), TriggerPoint::InlayHint(point, _, _)) => { - range.inlay == point.inlay - && range.highlight_start.cmp(&point.highlight_end).is_le() - && range.highlight_end.cmp(&point.highlight_end).is_ge() + (Self::Inlay(highlight), TriggerPoint::InlayHint(point, _, _)) => { + highlight.inlay == point.inlay + && highlight.range.contains(&point.range.start) + && highlight.range.contains(&point.range.end) } (Self::Inlay(_), TriggerPoint::Text(_)) | (Self::Text(_), TriggerPoint::InlayHint(_, _, _)) => false, @@ -66,13 +66,11 @@ pub enum GoToDefinitionLink { InlayHint(lsp::Location, LanguageServerId), } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct InlayHighlight { pub inlay: InlayId, pub inlay_position: Anchor, - // TODO kb turn into Range - pub highlight_start: usize, - pub highlight_end: usize, + pub range: Range, } #[derive(Debug, Clone)] @@ -252,9 +250,8 @@ pub fn update_inlay_link_and_hover_points( range: InlayHighlight { inlay: hovered_hint.id, inlay_position: hovered_hint.position, - highlight_start: extra_shift_left, - highlight_end: hovered_hint.text.len() - + extra_shift_right, + range: extra_shift_left + ..hovered_hint.text.len() + extra_shift_right, }, }, cx, @@ -272,13 +269,14 @@ pub fn update_inlay_link_and_hover_points( hovered_offset, ) { - let range = InlayHighlight { + let highlight_start = + (part_range.start - hint_start).0 + extra_shift_left; + let highlight_end = + (part_range.end - hint_start).0 + extra_shift_right; + let highlight = InlayHighlight { inlay: hovered_hint.id, inlay_position: hovered_hint.position, - highlight_start: (part_range.start - hint_start).0 - + extra_shift_left, - highlight_end: (part_range.end - hint_start).0 - + extra_shift_right, + range: highlight_start..highlight_end, }; if let Some(tooltip) = hovered_hint_part.tooltip { hover_popover::hover_at_inlay( @@ -299,7 +297,7 @@ pub fn update_inlay_link_and_hover_points( kind: content.kind, }, }, - range, + range: highlight.clone(), }, cx, ); @@ -312,7 +310,7 @@ pub fn update_inlay_link_and_hover_points( update_go_to_definition_link( editor, Some(GoToDefinitionTrigger::InlayHint( - range, + highlight, location, language_server_id, )), @@ -433,8 +431,8 @@ pub fn show_link_definition( ) }) } - TriggerPoint::InlayHint(trigger_source, lsp_location, server_id) => Some(( - Some(RangeInEditor::Inlay(*trigger_source)), + TriggerPoint::InlayHint(highlight, lsp_location, server_id) => Some(( + Some(RangeInEditor::Inlay(highlight.clone())), vec![GoToDefinitionLink::InlayHint( lsp_location.clone(), *server_id, @@ -501,8 +499,8 @@ pub fn show_link_definition( ..snapshot.anchor_after(offset_range.end), ) } - TriggerPoint::InlayHint(inlay_coordinates, _, _) => { - RangeInEditor::Inlay(*inlay_coordinates) + TriggerPoint::InlayHint(highlight, _, _) => { + RangeInEditor::Inlay(highlight.clone()) } }); @@ -513,9 +511,9 @@ pub fn show_link_definition( style, cx, ), - RangeInEditor::Inlay(inlay_coordinates) => this + RangeInEditor::Inlay(highlight) => this .highlight_inlays::( - vec![inlay_coordinates], + vec![highlight], style, cx, ), From 396efec6e1c465f6faf54f20d082d389fb8bbb5c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 22:05:02 +0300 Subject: [PATCH 09/15] Uncomment the rest of the tests --- crates/editor/src/display_map.rs | 8 ++++ crates/editor/src/display_map/inlay_map.rs | 1 - crates/editor/src/hover_popover.rs | 52 ++++++++-------------- crates/editor/src/link_go_to_definition.rs | 19 +++----- 4 files changed, 33 insertions(+), 47 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index ed57e537bd5f46661f6747fb4cd5d09fa4ef32d6..88cdd9ac3668c43b1ebc34ed45f40440d96f27c1 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -761,6 +761,14 @@ impl DisplaySnapshot { let type_id = TypeId::of::(); self.text_highlights.get(&Some(type_id)).cloned() } + + #[cfg(any(test, feature = "test-support"))] + pub fn inlay_highlight_ranges( + &self, + ) -> Option)>> { + let type_id = TypeId::of::(); + self.inlay_highlights.get(&Some(type_id)).cloned() + } } #[derive(Copy, Clone, Default, Eq, Ord, PartialOrd, PartialEq)] diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 5efa15a1c2b54704c80ad66cd532537abbf97aff..62ec3f6eef005431625574efecec6401b5c75b08 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -1711,7 +1711,6 @@ mod tests { .filter_map(|i| { let (_, inlay) = &inlays[i]; let inlay_text_len = inlay.text.len(); - // TODO kb gen_range match inlay_text_len { 0 => None, 1 => Some(InlayHighlight { diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index 79de3786a7befa7281f53ed1753ea2368361daca..f460b18bce30d23a9aad2da737e57fc7e474747a 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -796,6 +796,7 @@ mod tests { inlay_hint_cache::tests::{cached_hint_labels, visible_hint_labels}, link_go_to_definition::update_inlay_link_and_hover_points, test::editor_lsp_test_context::EditorLspTestContext, + InlayId, }; use collections::BTreeSet; use gpui::fonts::Weight; @@ -1462,29 +1463,19 @@ mod tests { .advance_clock(Duration::from_millis(HOVER_DELAY_MILLIS + 100)); cx.foreground().run_until_parked(); cx.update_editor(|editor, cx| { - let snapshot = editor.snapshot(cx); let hover_state = &editor.hover_state; assert!(hover_state.diagnostic_popover.is_none() && hover_state.info_popover.is_some()); let popover = hover_state.info_popover.as_ref().unwrap(); let buffer_snapshot = editor.buffer().update(cx, |buffer, cx| buffer.snapshot(cx)); - let entire_inlay_start = snapshot.display_point_to_inlay_offset( - inlay_range.start.to_display_point(&snapshot), - Bias::Left, + assert_eq!( + popover.symbol_range, + RangeInEditor::Inlay(InlayHighlight { + inlay: InlayId::Hint(0), + inlay_position: buffer_snapshot.anchor_at(inlay_range.start, Bias::Right), + range: ": ".len()..": ".len() + new_type_label.len(), + }), + "Popover range should match the new type label part" ); - - let expected_new_type_label_start = InlayOffset(entire_inlay_start.0 + ": ".len()); - // TODO kb - // assert_eq!( - // popover.symbol_range, - // RangeInEditor::Inlay(InlayRange { - // inlay_position: buffer_snapshot.anchor_at(inlay_range.start, Bias::Right), - // highlight_start: expected_new_type_label_start, - // highlight_end: InlayOffset( - // expected_new_type_label_start.0 + new_type_label.len() - // ), - // }), - // "Popover range should match the new type label part" - // ); assert_eq!( popover .rendered_content @@ -1529,27 +1520,20 @@ mod tests { .advance_clock(Duration::from_millis(HOVER_DELAY_MILLIS + 100)); cx.foreground().run_until_parked(); cx.update_editor(|editor, cx| { - let snapshot = editor.snapshot(cx); let hover_state = &editor.hover_state; assert!(hover_state.diagnostic_popover.is_none() && hover_state.info_popover.is_some()); let popover = hover_state.info_popover.as_ref().unwrap(); let buffer_snapshot = editor.buffer().update(cx, |buffer, cx| buffer.snapshot(cx)); - let entire_inlay_start = snapshot.display_point_to_inlay_offset( - inlay_range.start.to_display_point(&snapshot), - Bias::Left, + assert_eq!( + popover.symbol_range, + RangeInEditor::Inlay(InlayHighlight { + inlay: InlayId::Hint(0), + inlay_position: buffer_snapshot.anchor_at(inlay_range.start, Bias::Right), + range: ": ".len() + new_type_label.len() + "<".len() + ..": ".len() + new_type_label.len() + "<".len() + struct_label.len(), + }), + "Popover range should match the struct label part" ); - let expected_struct_label_start = - InlayOffset(entire_inlay_start.0 + ": ".len() + new_type_label.len() + "<".len()); - // TODO kb - // assert_eq!( - // popover.symbol_range, - // RangeInEditor::Inlay(InlayRange { - // inlay_position: buffer_snapshot.anchor_at(inlay_range.start, Bias::Right), - // highlight_start: expected_struct_label_start, - // highlight_end: InlayOffset(expected_struct_label_start.0 + struct_label.len()), - // }), - // "Popover range should match the struct label part" - // ); assert_eq!( popover .rendered_content diff --git a/crates/editor/src/link_go_to_definition.rs b/crates/editor/src/link_go_to_definition.rs index 581bda7e58c78070ebeca40485fe45d3e9ed93bb..3de6fb872e5583f3d51f96c43df6bf47b2c8dc49 100644 --- a/crates/editor/src/link_go_to_definition.rs +++ b/crates/editor/src/link_go_to_definition.rs @@ -1196,22 +1196,17 @@ mod tests { cx.update_editor(|editor, cx| { let snapshot = editor.snapshot(cx); let actual_ranges = snapshot - .text_highlight_ranges::() + .inlay_highlight_ranges::() .map(|ranges| ranges.as_ref().clone().1) .unwrap_or_default(); let buffer_snapshot = editor.buffer().update(cx, |buffer, cx| buffer.snapshot(cx)); - let expected_highlight_start = snapshot.display_point_to_inlay_offset( - inlay_range.start.to_display_point(&snapshot), - Bias::Left, - ); - // TODO kb - // let expected_ranges = vec![InlayRange { - // inlay_position: buffer_snapshot.anchor_at(inlay_range.start, Bias::Right), - // highlight_start: expected_highlight_start, - // highlight_end: InlayOffset(expected_highlight_start.0 + hint_label.len()), - // }]; - // assert_set_eq!(actual_ranges, expected_ranges); + let expected_ranges = vec![InlayHighlight { + inlay: InlayId::Hint(0), + inlay_position: buffer_snapshot.anchor_at(inlay_range.start, Bias::Right), + range: 0..hint_label.len(), + }]; + assert_set_eq!(actual_ranges, expected_ranges); }); // Unpress cmd causes highlight to go away From 9b43acfc887017a235e3b1f3db1fad7755fac1bf Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 22:08:02 +0300 Subject: [PATCH 10/15] Remove useless background highlights code --- crates/editor/src/display_map.rs | 9 +------ crates/editor/src/editor.rs | 1 + crates/editor/src/element.rs | 29 ++-------------------- crates/editor/src/inlay_hint_cache.rs | 1 + crates/editor/src/link_go_to_definition.rs | 1 - 5 files changed, 5 insertions(+), 36 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index 88cdd9ac3668c43b1ebc34ed45f40440d96f27c1..20e2a2a266feabba8aae590740e15b66add5f102 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -308,9 +308,6 @@ impl DisplayMap { pub struct Highlights<'a> { pub text_highlights: Option<&'a TextHighlights>, pub inlay_highlights: Option<&'a InlayHighlights>, - // TODO kb remove, backgrounds are not handled in the *Map codegi - pub inlay_background_highlights: - Option, Arc<(HighlightStyle, &'a [InlayHighlight])>>>, pub inlay_highlight_style: Option, pub suggestion_highlight_style: Option, } @@ -482,9 +479,6 @@ impl DisplaySnapshot { &'a self, display_rows: Range, language_aware: bool, - inlay_background_highlights: Option< - TreeMap, Arc<(HighlightStyle, &'a [InlayHighlight])>>, - >, inlay_highlight_style: Option, suggestion_highlight_style: Option, ) -> DisplayChunks<'_> { @@ -494,7 +488,6 @@ impl DisplaySnapshot { Highlights { text_highlights: Some(&self.text_highlights), inlay_highlights: Some(&self.inlay_highlights), - inlay_background_highlights, inlay_highlight_style, suggestion_highlight_style, }, @@ -1714,7 +1707,7 @@ pub mod tests { ) -> Vec<(String, Option, Option)> { let snapshot = map.update(cx, |map, cx| map.snapshot(cx)); let mut chunks: Vec<(String, Option, Option)> = Vec::new(); - for chunk in snapshot.chunks(rows, true, None, None, None) { + for chunk in snapshot.chunks(rows, true, None, None) { let syntax_color = chunk .syntax_highlight_id .and_then(|id| id.style(theme)?.color); diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 67560b1b033eaacf7c60324439fb551a6244f855..e61a41f3efaa26d94a957629f3b5e9d82aa15499 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -7823,6 +7823,7 @@ impl Editor { color_fetcher: fn(&Theme) -> Color, cx: &mut ViewContext, ) { + // TODO: no actual highlights happen for inlays currently, find a way to do that self.inlay_background_highlights .insert(Some(TypeId::of::()), (color_fetcher, ranges)); cx.notify(); diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index bc3ca9f7b13593f909c751ac12cc6df965a06fbc..b7e34fda5377d6370d33cdec35087a4e544cd7d9 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -54,7 +54,6 @@ use std::{ ops::Range, sync::Arc, }; -use sum_tree::TreeMap; use text::Point; use workspace::item::Item; @@ -1548,7 +1547,6 @@ impl EditorElement { &mut self, rows: Range, line_number_layouts: &[Option], - editor: &mut Editor, snapshot: &EditorSnapshot, cx: &ViewContext, ) -> Vec { @@ -1593,28 +1591,10 @@ impl EditorElement { .collect() } else { let style = &self.style; - let theme = theme::current(cx); - let inlay_background_highlights = - TreeMap::from_ordered_entries(editor.inlay_background_highlights.iter().map( - |(type_id, (color_fetcher, ranges))| { - let color = Some(color_fetcher(&theme)); - ( - *type_id, - Arc::new(( - HighlightStyle { - color, - ..HighlightStyle::default() - }, - ranges.as_slice(), - )), - ) - }, - )); let chunks = snapshot .chunks( rows.clone(), true, - Some(inlay_background_highlights), Some(style.theme.hint), Some(style.theme.suggestion), ) @@ -2375,13 +2355,8 @@ impl Element for EditorElement { let scrollbar_row_range = scroll_position.y()..(scroll_position.y() + height_in_lines); let mut max_visible_line_width = 0.0; - let line_layouts = self.layout_lines( - start_row..end_row, - &line_number_layouts, - editor, - &snapshot, - cx, - ); + let line_layouts = + self.layout_lines(start_row..end_row, &line_number_layouts, &snapshot, cx); for line_with_invisibles in &line_layouts { if line_with_invisibles.line.width() > max_visible_line_width { max_visible_line_width = line_with_invisibles.line.width(); diff --git a/crates/editor/src/inlay_hint_cache.rs b/crates/editor/src/inlay_hint_cache.rs index 34898aea2efe7ec45229cdb4e63de13cd48217f9..9f9491d3de85b179e1da9c02cadfa112a134ce4e 100644 --- a/crates/editor/src/inlay_hint_cache.rs +++ b/crates/editor/src/inlay_hint_cache.rs @@ -485,6 +485,7 @@ impl InlayHintCache { self.hints.clear(); } + // TODO kb have a map pub fn hint_by_id(&self, excerpt_id: ExcerptId, hint_id: InlayId) -> Option { self.hints .get(&excerpt_id)? diff --git a/crates/editor/src/link_go_to_definition.rs b/crates/editor/src/link_go_to_definition.rs index 3de6fb872e5583f3d51f96c43df6bf47b2c8dc49..8786fc8086c018782075716482e58c5dd80c2f46 100644 --- a/crates/editor/src/link_go_to_definition.rs +++ b/crates/editor/src/link_go_to_definition.rs @@ -224,7 +224,6 @@ pub fn update_inlay_link_and_hover_points( extra_shift_left += 1; extra_shift_right += 1; } - // TODO kb is it right for label part cases? for `\n` in hints and fold cases? if cached_hint.padding_right { extra_shift_right += 1; } From 8ae3f792350d5d96af8b1014c573f857b9bec977 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 23:05:22 +0300 Subject: [PATCH 11/15] Restructure inlay highlights data for proper access --- crates/editor/src/display_map.rs | 22 +++++++++------- crates/editor/src/display_map/inlay_map.rs | 30 +++++++++------------- crates/editor/src/editor.rs | 4 +-- crates/editor/src/link_go_to_definition.rs | 15 ++++++----- 4 files changed, 35 insertions(+), 36 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index 20e2a2a266feabba8aae590740e15b66add5f102..d97db9695ac4052f647a58d90fa1f23b4188004d 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -9,7 +9,7 @@ use crate::{ MultiBufferSnapshot, ToOffset, ToPoint, }; pub use block_map::{BlockMap, BlockPoint}; -use collections::{HashMap, HashSet}; +use collections::{BTreeMap, HashMap, HashSet}; use fold_map::FoldMap; use gpui::{ color::Color, @@ -44,7 +44,7 @@ pub trait ToDisplayPoint { } type TextHighlights = TreeMap, Arc<(HighlightStyle, Vec>)>>; -type InlayHighlights = TreeMap, Arc<(HighlightStyle, Vec)>>; +type InlayHighlights = BTreeMap>; pub struct DisplayMap { buffer: ModelHandle, @@ -226,11 +226,15 @@ impl DisplayMap { pub fn highlight_inlays( &mut self, type_id: TypeId, - ranges: Vec, + highlights: Vec, style: HighlightStyle, ) { - self.inlay_highlights - .insert(Some(type_id), Arc::new((style, ranges))); + for highlight in highlights { + self.inlay_highlights + .entry(type_id) + .or_default() + .insert(highlight.inlay, (style, highlight)); + } } pub fn text_highlights(&self, type_id: TypeId) -> Option<(HighlightStyle, &[Range])> { @@ -239,7 +243,7 @@ impl DisplayMap { } pub fn clear_highlights(&mut self, type_id: TypeId) -> bool { let mut cleared = self.text_highlights.remove(&Some(type_id)).is_some(); - cleared |= self.inlay_highlights.remove(&Some(type_id)).is_none(); + cleared |= self.inlay_highlights.remove(&type_id).is_none(); cleared } @@ -756,11 +760,11 @@ impl DisplaySnapshot { } #[cfg(any(test, feature = "test-support"))] - pub fn inlay_highlight_ranges( + pub fn inlay_highlights( &self, - ) -> Option)>> { + ) -> Option<&HashMap> { let type_id = TypeId::of::(); - self.inlay_highlights.get(&Some(type_id)).cloned() + self.inlay_highlights.get(&type_id) } } diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 62ec3f6eef005431625574efecec6401b5c75b08..bb0057b2d720c7ca0c558f7e599a5c5b6e6e9668 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -1,5 +1,4 @@ use crate::{ - link_go_to_definition::InlayHighlight, multi_buffer::{MultiBufferChunks, MultiBufferRows}, Anchor, InlayId, MultiBufferSnapshot, ToOffset, }; @@ -295,16 +294,13 @@ impl<'a> Iterator for InlayChunks<'a> { prefix } Transform::Inlay(inlay) => { - let mut inlay_highlight_style_and_range = None; + let mut inlay_style_and_highlight = None; + // type InlayHighlights = BTreeMap>; if let Some(inlay_highlights) = self.highlights.inlay_highlights { - for (_, style_and_ranges) in inlay_highlights.iter() { - let highlight_style: &HighlightStyle = &style_and_ranges.0; - let inlay_ranges: &Vec = &style_and_ranges.1; - // TODO kb: turn into a map. - if let Some(range) = - inlay_ranges.iter().find(|range| range.inlay == inlay.id) - { - inlay_highlight_style_and_range = Some((highlight_style, range)); + for (_, inlay_id_to_data) in inlay_highlights.iter() { + let style_and_highlight = inlay_id_to_data.get(&inlay.id); + if style_and_highlight.is_some() { + inlay_style_and_highlight = style_and_highlight; break; } } @@ -316,7 +312,7 @@ impl<'a> Iterator for InlayChunks<'a> { }; let next_inlay_highlight_endpoint; let offset_in_inlay = self.output_offset - self.transforms.start().0; - if let Some((style, highlight)) = inlay_highlight_style_and_range { + if let Some((style, highlight)) = inlay_style_and_highlight { let range = &highlight.range; if offset_in_inlay.0 < range.start { next_inlay_highlight_endpoint = range.start - offset_in_inlay.0; @@ -1176,6 +1172,7 @@ mod tests { use super::*; use crate::{ display_map::{InlayHighlights, TextHighlights}, + link_go_to_definition::InlayHighlight, InlayId, MultiBuffer, }; use gpui::AppContext; @@ -1706,7 +1703,7 @@ mod tests { while inlay_indices.len() < highlight_count.min(inlays.len()) { inlay_indices.insert(rng.gen_range(0..inlays.len())); } - let highlight_ranges = inlay_indices + let new_highlights = inlay_indices .into_iter() .filter_map(|i| { let (_, inlay) = &inlays[i]; @@ -1736,13 +1733,10 @@ mod tests { } } }) + .map(|highlight| (highlight.inlay, (HighlightStyle::default(), highlight))) .collect(); - - log::info!("highlighting inlay ranges {highlight_ranges:?}"); - inlay_highlights.insert( - Some(TypeId::of::<()>()), - Arc::new((HighlightStyle::default(), highlight_ranges)), - ); + log::info!("highlighting inlay ranges {new_highlights:?}"); + inlay_highlights.insert(TypeId::of::<()>(), new_highlights); }; for _ in 0..5 { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index e61a41f3efaa26d94a957629f3b5e9d82aa15499..26f30a75a8ab1aea77d4c579a7324db256a6a48a 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -8020,12 +8020,12 @@ impl Editor { pub fn highlight_inlays( &mut self, - ranges: Vec, + highlights: Vec, style: HighlightStyle, cx: &mut ViewContext, ) { self.display_map.update(cx, |map, _| { - map.highlight_inlays(TypeId::of::(), ranges, style) + map.highlight_inlays(TypeId::of::(), highlights, style) }); cx.notify(); } diff --git a/crates/editor/src/link_go_to_definition.rs b/crates/editor/src/link_go_to_definition.rs index 8786fc8086c018782075716482e58c5dd80c2f46..f7655b140f57999d5f9d2d75f10f7246deca0daf 100644 --- a/crates/editor/src/link_go_to_definition.rs +++ b/crates/editor/src/link_go_to_definition.rs @@ -1194,18 +1194,19 @@ mod tests { cx.foreground().run_until_parked(); cx.update_editor(|editor, cx| { let snapshot = editor.snapshot(cx); - let actual_ranges = snapshot - .inlay_highlight_ranges::() - .map(|ranges| ranges.as_ref().clone().1) - .unwrap_or_default(); + let actual_highlights = snapshot + .inlay_highlights::() + .into_iter() + .flat_map(|highlights| highlights.values().map(|(_, highlight)| highlight)) + .collect::>(); let buffer_snapshot = editor.buffer().update(cx, |buffer, cx| buffer.snapshot(cx)); - let expected_ranges = vec![InlayHighlight { + let expected_highlight = InlayHighlight { inlay: InlayId::Hint(0), inlay_position: buffer_snapshot.anchor_at(inlay_range.start, Bias::Right), range: 0..hint_label.len(), - }]; - assert_set_eq!(actual_ranges, expected_ranges); + }; + assert_set_eq!(actual_highlights, vec![&expected_highlight]); }); // Unpress cmd causes highlight to go away From 4e9f0adcef7b791ba8ee24ba65d92df4c0f9499f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 23:24:55 +0300 Subject: [PATCH 12/15] Improve inlay hint cache lookup --- crates/editor/src/display_map/inlay_map.rs | 1 - crates/editor/src/inlay_hint_cache.rs | 101 ++++++++++++--------- crates/editor/src/link_go_to_definition.rs | 1 - 3 files changed, 58 insertions(+), 45 deletions(-) diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index bb0057b2d720c7ca0c558f7e599a5c5b6e6e9668..1aac71337123fb767ed9a04a4a0f7d5f2c6105cb 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -295,7 +295,6 @@ impl<'a> Iterator for InlayChunks<'a> { } Transform::Inlay(inlay) => { let mut inlay_style_and_highlight = None; - // type InlayHighlights = BTreeMap>; if let Some(inlay_highlights) = self.highlights.inlay_highlights { for (_, inlay_id_to_data) in inlay_highlights.iter() { let style_and_highlight = inlay_id_to_data.get(&inlay.id); diff --git a/crates/editor/src/inlay_hint_cache.rs b/crates/editor/src/inlay_hint_cache.rs index 9f9491d3de85b179e1da9c02cadfa112a134ce4e..eceae96856f81aa2603fb88cc4699429be23395f 100644 --- a/crates/editor/src/inlay_hint_cache.rs +++ b/crates/editor/src/inlay_hint_cache.rs @@ -43,7 +43,8 @@ pub struct CachedExcerptHints { version: usize, buffer_version: Global, buffer_id: u64, - hints: Vec<(InlayId, InlayHint)>, + ordered_hints: Vec, + hints_by_id: HashMap, } #[derive(Debug, Clone, Copy)] @@ -316,7 +317,7 @@ impl InlayHintCache { self.hints.retain(|cached_excerpt, cached_hints| { let retain = excerpts_to_query.contains_key(cached_excerpt); if !retain { - invalidated_hints.extend(cached_hints.read().hints.iter().map(|&(id, _)| id)); + invalidated_hints.extend(cached_hints.read().ordered_hints.iter().copied()); } retain }); @@ -384,7 +385,7 @@ impl InlayHintCache { let shown_excerpt_hints_to_remove = shown_hints_to_remove.entry(*excerpt_id).or_default(); let excerpt_cached_hints = excerpt_cached_hints.read(); - let mut excerpt_cache = excerpt_cached_hints.hints.iter().fuse().peekable(); + let mut excerpt_cache = excerpt_cached_hints.ordered_hints.iter().fuse().peekable(); shown_excerpt_hints_to_remove.retain(|(shown_anchor, shown_hint_id)| { let Some(buffer) = shown_anchor .buffer_id @@ -395,7 +396,8 @@ impl InlayHintCache { let buffer_snapshot = buffer.read(cx).snapshot(); loop { match excerpt_cache.peek() { - Some((cached_hint_id, cached_hint)) => { + Some(&cached_hint_id) => { + let cached_hint = &excerpt_cached_hints.hints_by_id[cached_hint_id]; if cached_hint_id == shown_hint_id { excerpt_cache.next(); return !new_kinds.contains(&cached_hint.kind); @@ -428,7 +430,8 @@ impl InlayHintCache { } }); - for (cached_hint_id, maybe_missed_cached_hint) in excerpt_cache { + for cached_hint_id in excerpt_cache { + let maybe_missed_cached_hint = &excerpt_cached_hints.hints_by_id[cached_hint_id]; let cached_hint_kind = maybe_missed_cached_hint.kind; if !old_kinds.contains(&cached_hint_kind) && new_kinds.contains(&cached_hint_kind) { to_insert.push(Inlay::hint( @@ -463,7 +466,7 @@ impl InlayHintCache { self.update_tasks.remove(&excerpt_to_remove); if let Some(cached_hints) = self.hints.remove(&excerpt_to_remove) { let cached_hints = cached_hints.read(); - to_remove.extend(cached_hints.hints.iter().map(|(id, _)| *id)); + to_remove.extend(cached_hints.ordered_hints.iter().copied()); } } if to_remove.is_empty() { @@ -485,15 +488,12 @@ impl InlayHintCache { self.hints.clear(); } - // TODO kb have a map pub fn hint_by_id(&self, excerpt_id: ExcerptId, hint_id: InlayId) -> Option { self.hints .get(&excerpt_id)? .read() - .hints - .iter() - .find(|&(id, _)| id == &hint_id) - .map(|(_, hint)| hint) + .hints_by_id + .get(&hint_id) .cloned() } @@ -501,7 +501,13 @@ impl InlayHintCache { let mut hints = Vec::new(); for excerpt_hints in self.hints.values() { let excerpt_hints = excerpt_hints.read(); - hints.extend(excerpt_hints.hints.iter().map(|(_, hint)| hint).cloned()); + hints.extend( + excerpt_hints + .ordered_hints + .iter() + .map(|id| &excerpt_hints.hints_by_id[id]) + .cloned(), + ); } hints } @@ -519,12 +525,7 @@ impl InlayHintCache { ) { if let Some(excerpt_hints) = self.hints.get(&excerpt_id) { let mut guard = excerpt_hints.write(); - if let Some(cached_hint) = guard - .hints - .iter_mut() - .find(|(hint_id, _)| hint_id == &id) - .map(|(_, hint)| hint) - { + if let Some(cached_hint) = guard.hints_by_id.get_mut(&id) { if let ResolveState::CanResolve(server_id, _) = &cached_hint.resolve_state { let hint_to_resolve = cached_hint.clone(); let server_id = *server_id; @@ -556,12 +557,7 @@ impl InlayHintCache { editor.inlay_hint_cache.hints.get(&excerpt_id) { let mut guard = excerpt_hints.write(); - if let Some(cached_hint) = guard - .hints - .iter_mut() - .find(|(hint_id, _)| hint_id == &id) - .map(|(_, hint)| hint) - { + if let Some(cached_hint) = guard.hints_by_id.get_mut(&id) { if cached_hint.resolve_state == ResolveState::Resolving { resolved_hint.resolve_state = ResolveState::Resolved; *cached_hint = resolved_hint; @@ -987,12 +983,20 @@ fn calculate_hint_updates( let missing_from_cache = match &cached_excerpt_hints { Some(cached_excerpt_hints) => { let cached_excerpt_hints = cached_excerpt_hints.read(); - match cached_excerpt_hints.hints.binary_search_by(|probe| { - probe.1.position.cmp(&new_hint.position, buffer_snapshot) - }) { + match cached_excerpt_hints + .ordered_hints + .binary_search_by(|probe| { + cached_excerpt_hints + .hints_by_id + .get(probe) + .unwrap() + .position + .cmp(&new_hint.position, buffer_snapshot) + }) { Ok(ix) => { let mut missing_from_cache = true; - for (cached_inlay_id, cached_hint) in &cached_excerpt_hints.hints[ix..] { + for id in &cached_excerpt_hints.ordered_hints[ix..] { + let cached_hint = &cached_excerpt_hints.hints_by_id[id]; if new_hint .position .cmp(&cached_hint.position, buffer_snapshot) @@ -1001,7 +1005,7 @@ fn calculate_hint_updates( break; } if cached_hint == &new_hint { - excerpt_hints_to_persist.insert(*cached_inlay_id, cached_hint.kind); + excerpt_hints_to_persist.insert(*id, cached_hint.kind); missing_from_cache = false; } } @@ -1032,12 +1036,12 @@ fn calculate_hint_updates( let cached_excerpt_hints = cached_excerpt_hints.read(); remove_from_cache.extend( cached_excerpt_hints - .hints + .ordered_hints .iter() - .filter(|(cached_inlay_id, _)| { + .filter(|cached_inlay_id| { !excerpt_hints_to_persist.contains_key(cached_inlay_id) }) - .map(|(cached_inlay_id, _)| *cached_inlay_id), + .copied(), ); } } @@ -1081,7 +1085,8 @@ fn apply_hint_update( version: query.cache_version, buffer_version: buffer_snapshot.version().clone(), buffer_id: query.buffer_id, - hints: Vec::new(), + ordered_hints: Vec::new(), + hints_by_id: HashMap::default(), })) }); let mut cached_excerpt_hints = cached_excerpt_hints.write(); @@ -1094,20 +1099,24 @@ fn apply_hint_update( let mut cached_inlays_changed = !new_update.remove_from_cache.is_empty(); cached_excerpt_hints - .hints - .retain(|(hint_id, _)| !new_update.remove_from_cache.contains(hint_id)); + .ordered_hints + .retain(|hint_id| !new_update.remove_from_cache.contains(hint_id)); let mut splice = InlaySplice { to_remove: new_update.remove_from_visible, to_insert: Vec::new(), }; for new_hint in new_update.add_to_cache { - let cached_hints = &mut cached_excerpt_hints.hints; - let insert_position = match cached_hints - .binary_search_by(|probe| probe.1.position.cmp(&new_hint.position, &buffer_snapshot)) - { + let insert_position = match cached_excerpt_hints + .ordered_hints + .binary_search_by(|probe| { + cached_excerpt_hints.hints_by_id[probe] + .position + .cmp(&new_hint.position, &buffer_snapshot) + }) { Ok(i) => { let mut insert_position = Some(i); - for (_, cached_hint) in &cached_hints[i..] { + for id in &cached_excerpt_hints.ordered_hints[i..] { + let cached_hint = &cached_excerpt_hints.hints_by_id[id]; if new_hint .position .cmp(&cached_hint.position, &buffer_snapshot) @@ -1138,7 +1147,11 @@ fn apply_hint_update( .to_insert .push(Inlay::hint(new_inlay_id, new_hint_position, &new_hint)); } - cached_hints.insert(insert_position, (InlayId::Hint(new_inlay_id), new_hint)); + let new_id = InlayId::Hint(new_inlay_id); + cached_excerpt_hints.hints_by_id.insert(new_id, new_hint); + cached_excerpt_hints + .ordered_hints + .insert(insert_position, new_id); cached_inlays_changed = true; } } @@ -1158,7 +1171,7 @@ fn apply_hint_update( outdated_excerpt_caches.insert(*excerpt_id); splice .to_remove - .extend(excerpt_hints.hints.iter().map(|(id, _)| id)); + .extend(excerpt_hints.ordered_hints.iter().copied()); } } cached_inlays_changed |= !outdated_excerpt_caches.is_empty(); @@ -3312,7 +3325,9 @@ all hints should be invalidated and requeried for all of its visible excerpts" pub fn cached_hint_labels(editor: &Editor) -> Vec { let mut labels = Vec::new(); for (_, excerpt_hints) in &editor.inlay_hint_cache().hints { - for (_, inlay) in &excerpt_hints.read().hints { + let excerpt_hints = excerpt_hints.read(); + for id in &excerpt_hints.ordered_hints { + let inlay = excerpt_hints.hints_by_id.get(id).unwrap(); labels.push(inlay.text()); } } diff --git a/crates/editor/src/link_go_to_definition.rs b/crates/editor/src/link_go_to_definition.rs index f7655b140f57999d5f9d2d75f10f7246deca0daf..7da0b88622ff4152127b46714c62394210b4f4d0 100644 --- a/crates/editor/src/link_go_to_definition.rs +++ b/crates/editor/src/link_go_to_definition.rs @@ -188,7 +188,6 @@ pub fn update_inlay_link_and_hover_points( Bias::Right, ); if let Some(hovered_hint) = editor - // TODO kb look up by position with binary search .visible_inlay_hints(cx) .into_iter() .skip_while(|hint| { From f9b70718ac7aca02f904ac833b1e63d07ce58644 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 23:36:33 +0300 Subject: [PATCH 13/15] Store hints in the map, not the snapshot --- crates/editor/src/display_map/inlay_map.rs | 29 ++++++++-------------- 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 1aac71337123fb767ed9a04a4a0f7d5f2c6105cb..a67e8484eb1c9a36b53a7b5e9d1987247b18a333 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -20,13 +20,13 @@ use super::Highlights; pub struct InlayMap { snapshot: InlaySnapshot, + inlays: Vec, } #[derive(Clone)] pub struct InlaySnapshot { pub buffer: MultiBufferSnapshot, transforms: SumTree, - inlays: Vec, pub version: usize, } @@ -428,13 +428,13 @@ impl InlayMap { let snapshot = InlaySnapshot { buffer: buffer.clone(), transforms: SumTree::from_iter(Some(Transform::Isomorphic(buffer.text_summary())), &()), - inlays: Vec::new(), version, }; ( Self { snapshot: snapshot.clone(), + inlays: Vec::new(), }, snapshot, ) @@ -503,7 +503,7 @@ impl InlayMap { ); let new_start = InlayOffset(new_transforms.summary().output.len); - let start_ix = match snapshot.inlays.binary_search_by(|probe| { + let start_ix = match self.inlays.binary_search_by(|probe| { probe .position .to_offset(&buffer_snapshot) @@ -513,7 +513,7 @@ impl InlayMap { Ok(ix) | Err(ix) => ix, }; - for inlay in &snapshot.inlays[start_ix..] { + for inlay in &self.inlays[start_ix..] { let buffer_offset = inlay.position.to_offset(&buffer_snapshot); if buffer_offset > buffer_edit.new.end { break; @@ -583,7 +583,7 @@ impl InlayMap { let snapshot = &mut self.snapshot; let mut edits = BTreeSet::new(); - snapshot.inlays.retain(|inlay| { + self.inlays.retain(|inlay| { let retain = !to_remove.contains(&inlay.id); if !retain { let offset = inlay.position.to_offset(&snapshot.buffer); @@ -599,13 +599,13 @@ impl InlayMap { } let offset = inlay_to_insert.position.to_offset(&snapshot.buffer); - match snapshot.inlays.binary_search_by(|probe| { + match self.inlays.binary_search_by(|probe| { probe .position .cmp(&inlay_to_insert.position, &snapshot.buffer) }) { Ok(ix) | Err(ix) => { - snapshot.inlays.insert(ix, inlay_to_insert); + self.inlays.insert(ix, inlay_to_insert); } } @@ -625,7 +625,7 @@ impl InlayMap { } pub fn current_inlays(&self) -> impl Iterator { - self.snapshot.inlays.iter() + self.inlays.iter() } #[cfg(test)] @@ -641,7 +641,7 @@ impl InlayMap { let mut to_insert = Vec::new(); let snapshot = &mut self.snapshot; for i in 0..rng.gen_range(1..=5) { - if snapshot.inlays.is_empty() || rng.gen() { + if self.inlays.is_empty() || rng.gen() { let position = snapshot.buffer.random_byte_range(0, rng).start; let bias = if rng.gen() { Bias::Left } else { Bias::Right }; let len = if rng.gen_bool(0.01) { @@ -674,8 +674,7 @@ impl InlayMap { }); } else { to_remove.push( - snapshot - .inlays + self.inlays .iter() .choose(rng) .map(|inlay| inlay.id) @@ -1547,12 +1546,7 @@ mod tests { // The inlays can be manually removed. let (inlay_snapshot, _) = inlay_map.splice( - inlay_map - .snapshot - .inlays - .iter() - .map(|inlay| inlay.id) - .collect(), + inlay_map.inlays.iter().map(|inlay| inlay.id).collect(), Vec::new(), ); assert_eq!(inlay_snapshot.text(), "abxJKLyDzefghi"); @@ -1644,7 +1638,6 @@ mod tests { log::info!("inlay text: {:?}", inlay_snapshot.text()); let inlays = inlay_map - .snapshot .inlays .iter() .filter(|inlay| inlay.position.is_valid(&buffer_snapshot)) From e7b5880af0599aab13567db8fb2b55c13abc3fa4 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 14 Sep 2023 23:53:56 +0300 Subject: [PATCH 14/15] Combine both text and inlay highlights in randomized tests --- crates/editor/src/display_map/inlay_map.rs | 49 +++++++++++----------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index a67e8484eb1c9a36b53a7b5e9d1987247b18a333..124b32c2343bf87300146972ca30d7d47e237367 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -1669,30 +1669,31 @@ mod tests { } let mut text_highlights = TextHighlights::default(); + let text_highlight_count = rng.gen_range(0_usize..10); + let mut text_highlight_ranges = (0..text_highlight_count) + .map(|_| buffer_snapshot.random_byte_range(0, &mut rng)) + .collect::>(); + text_highlight_ranges.sort_by_key(|range| (range.start, Reverse(range.end))); + log::info!("highlighting text ranges {text_highlight_ranges:?}"); + text_highlights.insert( + Some(TypeId::of::<()>()), + Arc::new(( + HighlightStyle::default(), + text_highlight_ranges + .into_iter() + .map(|range| { + buffer_snapshot.anchor_before(range.start) + ..buffer_snapshot.anchor_after(range.end) + }) + .collect(), + )), + ); + let mut inlay_highlights = InlayHighlights::default(); - let highlight_count = rng.gen_range(0_usize..10); - if false && rng.gen_bool(0.5) { - let mut highlight_ranges = (0..highlight_count) - .map(|_| buffer_snapshot.random_byte_range(0, &mut rng)) - .collect::>(); - highlight_ranges.sort_by_key(|range| (range.start, Reverse(range.end))); - log::info!("highlighting text ranges {highlight_ranges:?}"); - text_highlights.insert( - Some(TypeId::of::<()>()), - Arc::new(( - HighlightStyle::default(), - highlight_ranges - .into_iter() - .map(|range| { - buffer_snapshot.anchor_before(range.start) - ..buffer_snapshot.anchor_after(range.end) - }) - .collect(), - )), - ); - } else { + if !inlays.is_empty() { + let inlay_highlight_count = rng.gen_range(0..inlays.len()); let mut inlay_indices = BTreeSet::default(); - while inlay_indices.len() < highlight_count.min(inlays.len()) { + while inlay_indices.len() < inlay_highlight_count { inlay_indices.insert(rng.gen_range(0..inlays.len())); } let new_highlights = inlay_indices @@ -1729,7 +1730,7 @@ mod tests { .collect(); log::info!("highlighting inlay ranges {new_highlights:?}"); inlay_highlights.insert(TypeId::of::<()>(), new_highlights); - }; + } for _ in 0..5 { let mut end = rng.gen_range(0..=inlay_snapshot.len().0); @@ -1738,7 +1739,7 @@ mod tests { start = expected_text.clip_offset(start, Bias::Right); let range = InlayOffset(start)..InlayOffset(end); - log::info!("calling inlay_snapshot.chunks({:?})", range); + log::info!("calling inlay_snapshot.chunks({range:?})"); let actual_text = inlay_snapshot .chunks( range, From 8c1df5afa2ed444da0391889f47b45d2ecfc9e50 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 15 Sep 2023 10:21:57 +0300 Subject: [PATCH 15/15] Empty both hint cache storages correctly --- crates/editor/src/inlay_hint_cache.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/editor/src/inlay_hint_cache.rs b/crates/editor/src/inlay_hint_cache.rs index eceae96856f81aa2603fb88cc4699429be23395f..8aa7a1e40edb872917ce44132a24ae832da4d3b2 100644 --- a/crates/editor/src/inlay_hint_cache.rs +++ b/crates/editor/src/inlay_hint_cache.rs @@ -986,10 +986,7 @@ fn calculate_hint_updates( match cached_excerpt_hints .ordered_hints .binary_search_by(|probe| { - cached_excerpt_hints - .hints_by_id - .get(probe) - .unwrap() + cached_excerpt_hints.hints_by_id[probe] .position .cmp(&new_hint.position, buffer_snapshot) }) { @@ -1101,6 +1098,9 @@ fn apply_hint_update( cached_excerpt_hints .ordered_hints .retain(|hint_id| !new_update.remove_from_cache.contains(hint_id)); + cached_excerpt_hints + .hints_by_id + .retain(|hint_id, _| !new_update.remove_from_cache.contains(hint_id)); let mut splice = InlaySplice { to_remove: new_update.remove_from_visible, to_insert: Vec::new(), @@ -3327,8 +3327,7 @@ all hints should be invalidated and requeried for all of its visible excerpts" for (_, excerpt_hints) in &editor.inlay_hint_cache().hints { let excerpt_hints = excerpt_hints.read(); for id in &excerpt_hints.ordered_hints { - let inlay = excerpt_hints.hints_by_id.get(id).unwrap(); - labels.push(inlay.text()); + labels.push(excerpt_hints.hints_by_id[id].text()); } }