From 8f68688a6465bdf22f7b1daccd2b75700dc59257 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 16 Jun 2023 23:59:22 +0300 Subject: [PATCH] Allow readding inlays with existing ids, move inlay types --- crates/editor/src/display_map.rs | 13 +- crates/editor/src/display_map/inlay_map.rs | 110 +++++++++++------ crates/editor/src/editor.rs | 135 ++++++++++++--------- crates/editor/src/inlay_cache.rs | 129 ++++++++++---------- crates/editor/src/multi_buffer.rs | 16 ++- crates/project/src/project.rs | 5 +- 6 files changed, 248 insertions(+), 160 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index b0b97fd70d49fe64f3bf4b5f70c4de2d06614d15..791665bf78374f1d4d5c874e49c123836bfde424 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -4,10 +4,7 @@ mod inlay_map; mod tab_map; mod wrap_map; -use crate::{ - inlay_cache::{InlayId, InlayProperties}, - Anchor, AnchorRangeExt, MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint, -}; +use crate::{Anchor, AnchorRangeExt, MultiBuffer, MultiBufferSnapshot, ToOffset, ToPoint}; pub use block_map::{BlockMap, BlockPoint}; use collections::{HashMap, HashSet}; use fold_map::FoldMap; @@ -31,6 +28,8 @@ pub use block_map::{ BlockDisposition, BlockId, BlockProperties, BlockStyle, RenderBlock, TransformBlock, }; +pub use self::inlay_map::{Inlay, InlayId, InlayProperties}; + #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum FoldStatus { Folded, @@ -243,10 +242,14 @@ impl DisplayMap { .update(cx, |map, cx| map.set_wrap_width(width, cx)) } + pub fn current_inlays(&self) -> impl Iterator { + self.inlay_map.current_inlays() + } + pub fn splice_inlays>( &mut self, to_remove: Vec, - to_insert: Vec>, + to_insert: Vec<(Option, InlayProperties)>, cx: &mut ModelContext, ) -> Vec { if to_remove.is_empty() && to_insert.is_empty() { diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 3571a603a9c8f485626b0ae73bc8d75c521dcd98..0c7d1bd60396fb95a6e2989bb7fbd23c2d7c080f 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -1,7 +1,6 @@ use crate::{ - inlay_cache::{Inlay, InlayId, InlayProperties}, multi_buffer::{MultiBufferChunks, MultiBufferRows}, - MultiBufferSnapshot, ToOffset, + Anchor, MultiBufferSnapshot, ToOffset, }; use collections::{BTreeSet, HashMap}; use gpui::fonts::HighlightStyle; @@ -35,6 +34,22 @@ enum Transform { Inlay(Inlay), } +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct InlayId(usize); + +#[derive(Debug, Clone)] +pub struct Inlay { + pub id: InlayId, + pub position: Anchor, + pub text: text::Rope, +} + +#[derive(Debug, Clone)] +pub struct InlayProperties { + pub position: Anchor, + pub text: T, +} + impl sum_tree::Item for Transform { type Summary = TransformSummary; @@ -446,7 +461,7 @@ impl InlayMap { pub fn splice>( &mut self, to_remove: Vec, - to_insert: Vec>, + to_insert: Vec<(Option, InlayProperties)>, ) -> (InlaySnapshot, Vec, Vec) { let snapshot = self.snapshot.lock(); let mut edits = BTreeSet::new(); @@ -460,13 +475,15 @@ impl InlayMap { } let mut new_inlay_ids = Vec::with_capacity(to_insert.len()); - for properties in to_insert { + for (existing_id, properties) in to_insert { let inlay = Inlay { - id: InlayId(post_inc(&mut self.next_inlay_id)), + id: existing_id.unwrap_or_else(|| InlayId(post_inc(&mut self.next_inlay_id))), position: properties.position, text: properties.text.into(), }; - new_inlay_ids.push(inlay.id); + if existing_id.is_none() { + new_inlay_ids.push(inlay.id); + } self.inlays_by_id.insert(inlay.id, inlay.clone()); match self .inlays @@ -494,6 +511,10 @@ impl InlayMap { (snapshot, edits, new_inlay_ids) } + pub fn current_inlays(&self) -> impl Iterator { + self.inlays.iter() + } + #[cfg(test)] pub(crate) fn randomly_mutate( &mut self, @@ -519,10 +540,13 @@ impl InlayMap { bias, text ); - to_insert.push(InlayProperties { - position: snapshot.buffer.anchor_at(position, bias), - text, - }); + to_insert.push(( + None, + InlayProperties { + position: snapshot.buffer.anchor_at(position, bias), + text, + }, + )); } else { to_remove.push(*self.inlays_by_id.keys().choose(rng).unwrap()); } @@ -852,10 +876,13 @@ mod tests { let (inlay_snapshot, _, _) = inlay_map.splice( Vec::new(), - vec![InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_after(3), - text: "|123|", - }], + vec![( + None, + InlayProperties { + position: buffer.read(cx).snapshot(cx).anchor_after(3), + text: "|123|", + }, + )], ); assert_eq!(inlay_snapshot.text(), "abc|123|defghi"); assert_eq!( @@ -928,14 +955,20 @@ mod tests { let (inlay_snapshot, _, _) = inlay_map.splice( Vec::new(), vec![ - InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_before(3), - text: "|123|", - }, - InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_after(3), - text: "|456|", - }, + ( + None, + InlayProperties { + position: buffer.read(cx).snapshot(cx).anchor_before(3), + text: "|123|", + }, + ), + ( + None, + InlayProperties { + position: buffer.read(cx).snapshot(cx).anchor_after(3), + text: "|456|", + }, + ), ], ); assert_eq!(inlay_snapshot.text(), "abx|123||456|yDzefghi"); @@ -963,18 +996,27 @@ mod tests { let (inlay_snapshot, _, _) = inlay_map.splice( Vec::new(), vec![ - InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_before(0), - text: "|123|\n", - }, - InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_before(4), - text: "|456|", - }, - InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_before(7), - text: "\n|567|\n", - }, + ( + None, + InlayProperties { + position: buffer.read(cx).snapshot(cx).anchor_before(0), + text: "|123|\n", + }, + ), + ( + None, + InlayProperties { + position: buffer.read(cx).snapshot(cx).anchor_before(4), + text: "|456|", + }, + ), + ( + None, + InlayProperties { + position: buffer.read(cx).snapshot(cx).anchor_before(7), + text: "\n|567|\n", + }, + ), ], ); assert_eq!(inlay_snapshot.text(), "|123|\nabc\n|456|def\n|567|\n\nghi"); diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 4a40d6ef2f2885e980adfddb1d9ac9ee91046836..d32dc47154eb62592a0826578d0c05089e0b50df 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -54,9 +54,7 @@ use gpui::{ }; use highlight_matching_bracket::refresh_matching_bracket_highlights; use hover_popover::{hide_hover, HoverState}; -use inlay_cache::{ - Inlay, InlayCache, InlayHintQuery, InlayId, InlayProperties, InlayRefreshReason, InlaySplice, -}; +use inlay_cache::{InlayCache, InlayHintQuery, InlayRefreshReason, InlaySplice}; pub use items::MAX_TAB_TITLE_LEN; use itertools::Itertools; pub use language::{char_kind, CharKind}; @@ -2606,57 +2604,74 @@ impl Editor { } let multi_buffer_handle = self.buffer().clone(); + let currently_visible_ranges = self.excerpt_visible_offsets(&multi_buffer_handle, cx); + let currently_shown_inlays = self + .display_map + .read(cx) + .current_inlays() + .map(|inlay| (inlay.position, inlay.id)) + .collect::>(); match reason { InlayRefreshReason::SettingsChange(new_settings) => { - let InlaySplice { + if let Some(InlaySplice { to_remove, to_insert, - } = self.inlay_cache.apply_settings(new_settings); - self.splice_inlay_hints(to_remove, to_insert, cx); + }) = self.inlay_cache.apply_settings( + multi_buffer_handle, + new_settings, + currently_visible_ranges, + currently_shown_inlays, + cx, + ) { + self.splice_inlay_hints(to_remove, to_insert, cx); + } } InlayRefreshReason::Scroll(scrolled_to) => { - let addition_queries = self - .excerpt_visible_offsets(&multi_buffer_handle, cx) - .into_iter() - .find_map(|(buffer, excerpt_visible_offset_range, excerpt_id)| { + if let Some(updated_range_query) = currently_visible_ranges.iter().find_map( + |(buffer, excerpt_visible_offset_range, excerpt_id)| { let buffer_id = scrolled_to.anchor.buffer_id?; if buffer_id == buffer.read(cx).remote_id() - && scrolled_to.anchor.excerpt_id == excerpt_id + && &scrolled_to.anchor.excerpt_id == excerpt_id { - inlay_hint_query(&buffer, excerpt_id, excerpt_visible_offset_range, cx) + Some(inlay_hint_query( + buffer, + *excerpt_id, + excerpt_visible_offset_range, + cx, + )) } else { None } - }) - .into_iter(); - - cx.spawn(|editor, mut cx| async move { - let InlaySplice { - to_remove, - to_insert, - } = editor - .update(&mut cx, |editor, cx| { - editor.inlay_cache.append_inlays( - multi_buffer_handle, - addition_queries, - cx, - ) - })? - .await - .context("inlay cache hint fetch")?; + }, + ) { + cx.spawn(|editor, mut cx| async move { + let InlaySplice { + to_remove, + to_insert, + } = editor + .update(&mut cx, |editor, cx| { + editor.inlay_cache.append_inlays( + multi_buffer_handle, + std::iter::once(updated_range_query), + currently_shown_inlays, + cx, + ) + })? + .await + .context("inlay cache hint fetch")?; - editor.update(&mut cx, |editor, cx| { - editor.splice_inlay_hints(to_remove, to_insert, cx) + editor.update(&mut cx, |editor, cx| { + editor.splice_inlay_hints(to_remove, to_insert, cx) + }) }) - }) - .detach_and_log_err(cx); + .detach_and_log_err(cx); + } } InlayRefreshReason::VisibleExcerptsChange => { - let replacement_queries = self - .excerpt_visible_offsets(&multi_buffer_handle, cx) - .into_iter() - .filter_map(|(buffer, excerpt_visible_offset_range, excerpt_id)| { - inlay_hint_query(&buffer, excerpt_id, excerpt_visible_offset_range, cx) + let replacement_queries = currently_visible_ranges + .iter() + .map(|(buffer, excerpt_visible_offset_range, excerpt_id)| { + inlay_hint_query(buffer, *excerpt_id, excerpt_visible_offset_range, cx) }) .collect::>(); cx.spawn(|editor, mut cx| async move { @@ -2668,6 +2683,7 @@ impl Editor { editor.inlay_cache.replace_inlays( multi_buffer_handle, replacement_queries.into_iter(), + currently_shown_inlays, cx, ) })? @@ -2709,13 +2725,13 @@ impl Editor { fn splice_inlay_hints( &self, to_remove: Vec, - to_insert: Vec<(Anchor, project::InlayHint)>, + to_insert: Vec<(Option, Anchor, project::InlayHint)>, cx: &mut ViewContext, ) { let buffer = self.buffer.read(cx).read(cx); let new_inlays = to_insert .into_iter() - .map(|(hint_anchor, hint)| { + .map(|(id, hint_anchor, hint)| { let mut text = hint.text(); // TODO kb styling instead? if hint.padding_right { @@ -2725,10 +2741,13 @@ impl Editor { text.insert(0, ' '); } - InlayProperties { - position: hint_anchor.bias_left(&buffer), - text, - } + ( + id, + InlayProperties { + position: hint_anchor.bias_left(&buffer), + text, + }, + ) }) .collect(); drop(buffer); @@ -3482,15 +3501,23 @@ impl Editor { to_remove.push(suggestion.id); } - let to_insert = vec![InlayProperties { - position: cursor, - text: text.clone(), - }]; - self.display_map.update(cx, move |map, cx| { + let to_insert = vec![( + None, + InlayProperties { + position: cursor, + text: text.clone(), + }, + )]; + let new_inlay_ids = self.display_map.update(cx, move |map, cx| { map.splice_inlays(to_remove, to_insert, cx) }); + assert_eq!( + new_inlay_ids.len(), + 1, + "Expecting only copilot suggestion id generated" + ); self.copilot_state.suggestion = Some(Inlay { - id: InlayId(usize::MAX), + id: new_inlay_ids.into_iter().next().unwrap(), position: cursor, text, }); @@ -7652,9 +7679,9 @@ impl Editor { fn inlay_hint_query( buffer: &ModelHandle, excerpt_id: ExcerptId, - excerpt_visible_offset_range: Range, + excerpt_visible_offset_range: &Range, cx: &mut ViewContext<'_, '_, Editor>, -) -> Option { +) -> InlayHintQuery { let buffer = buffer.read(cx); let max_buffer_len = buffer.len(); let visible_offset_range_len = excerpt_visible_offset_range.len(); @@ -7667,12 +7694,12 @@ fn inlay_hint_query( .end .saturating_add(visible_offset_range_len), ); - Some(InlayHintQuery { + InlayHintQuery { buffer_id: buffer.remote_id(), buffer_version: buffer.version().clone(), excerpt_id, excerpt_offset_query_range: query_range_start..query_range_end, - }) + } } fn consume_contiguous_rows( diff --git a/crates/editor/src/inlay_cache.rs b/crates/editor/src/inlay_cache.rs index 7a362085147d514f7f71ea0a00e1308fec81b505..7d8dd67e78242dc8d760299106a0eef792761402 100644 --- a/crates/editor/src/inlay_cache.rs +++ b/crates/editor/src/inlay_cache.rs @@ -1,26 +1,16 @@ use std::ops::Range; -use crate::{editor_settings, scroll::ScrollAnchor, Anchor, Editor, ExcerptId, MultiBuffer}; +use crate::{ + display_map::InlayId, editor_settings, scroll::ScrollAnchor, Anchor, Editor, ExcerptId, + MultiBuffer, +}; use clock::Global; use gpui::{ModelHandle, Task, ViewContext}; +use language::Buffer; use project::{InlayHint, InlayHintKind}; use collections::{HashMap, HashSet}; -// TODO kb move to inlay_map along with the next one? -#[derive(Debug, Clone)] -pub struct Inlay { - pub id: InlayId, - pub position: Anchor, - pub text: text::Rope, -} - -#[derive(Debug, Clone)] -pub struct InlayProperties { - pub position: Anchor, - pub text: T, -} - #[derive(Debug, Copy, Clone)] pub enum InlayRefreshReason { SettingsChange(editor_settings::InlayHints), @@ -30,23 +20,21 @@ pub enum InlayRefreshReason { #[derive(Debug, Clone, Default)] pub struct InlayCache { - inlays_per_buffer: HashMap, + inlay_hints: HashMap, + inlays_in_buffers: HashMap, allowed_hint_kinds: HashSet>, } #[derive(Clone, Debug, Default)] struct BufferInlays { buffer_version: Global, - ordered_by_anchor_inlays: Vec<(Anchor, InlayId, InlayHint)>, + ordered_by_anchor_inlays: Vec<(Anchor, InlayId)>, } -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct InlayId(pub usize); - #[derive(Debug, Default)] pub struct InlaySplice { pub to_remove: Vec, - pub to_insert: Vec<(Anchor, InlayHint)>, + pub to_insert: Vec<(Option, Anchor, InlayHint)>, } pub struct InlayHintQuery { @@ -60,82 +48,99 @@ impl InlayCache { pub fn new(inlay_hint_settings: editor_settings::InlayHints) -> Self { Self { allowed_hint_kinds: allowed_inlay_hint_types(inlay_hint_settings), - inlays_per_buffer: HashMap::default(), + inlays_in_buffers: HashMap::default(), + inlay_hints: HashMap::default(), } } pub fn apply_settings( &mut self, + multi_buffer: ModelHandle, inlay_hint_settings: editor_settings::InlayHints, - ) -> InlaySplice { - let new_allowed_inlay_hint_types = allowed_inlay_hint_types(inlay_hint_settings); - - let new_allowed_hint_kinds = new_allowed_inlay_hint_types - .difference(&self.allowed_hint_kinds) - .copied() - .collect::>(); - let removed_hint_kinds = self - .allowed_hint_kinds - .difference(&new_allowed_inlay_hint_types) - .collect::>(); - let mut to_remove = Vec::new(); - let mut to_insert = Vec::new(); - for (_, inlay_id, inlay_hint) in self - .inlays_per_buffer - .iter() - .map(|(_, buffer_inlays)| buffer_inlays.ordered_by_anchor_inlays.iter()) - .flatten() - { - if removed_hint_kinds.contains(&inlay_hint.kind) { - to_remove.push(*inlay_id); - } else if new_allowed_hint_kinds.contains(&inlay_hint.kind) { - todo!("TODO kb: agree with InlayMap how splice works") - // to_insert.push((*inlay_id, *anchor, inlay_hint.to_owned())); + currently_visible_ranges: Vec<(ModelHandle, Range, ExcerptId)>, + currently_shown_inlays: Vec<(Anchor, InlayId)>, + cx: &mut ViewContext, + ) -> Option { + let new_allowed_hint_kinds = allowed_inlay_hint_types(inlay_hint_settings); + if new_allowed_hint_kinds == self.allowed_hint_kinds { + None + } else { + self.allowed_hint_kinds = new_allowed_hint_kinds; + let mut to_remove = Vec::new(); + let mut to_insert = Vec::new(); + + let mut considered_inlay_ids = HashSet::default(); + for (_, shown_inlay_id) in currently_shown_inlays { + if let Some(inlay_hint) = self.inlay_hints.get(&shown_inlay_id) { + if !self.allowed_hint_kinds.contains(&inlay_hint.kind) { + to_remove.push(shown_inlay_id); + } + considered_inlay_ids.insert(shown_inlay_id); + } } - } - self.allowed_hint_kinds = new_allowed_hint_kinds; + let multi_buffer_snapshot = multi_buffer.read(cx).snapshot(cx); + for (inlay_id, inlay_hint) in &self.inlay_hints { + if self.allowed_hint_kinds.contains(&inlay_hint.kind) + && !considered_inlay_ids.contains(inlay_id) + { + if let Some(hint_to_readd) = currently_visible_ranges.iter() + .filter(|(_, visible_range, _)| visible_range.contains(&inlay_hint.position.offset)) + .find_map(|(_, _, excerpt_id)| { + let Some(anchor) = multi_buffer_snapshot + .find_anchor_in_excerpt(*excerpt_id, inlay_hint.position) else { return None; }; + Some((Some(*inlay_id), anchor, inlay_hint.clone())) + }, + ) { + to_insert.push(hint_to_readd); + } + } + } - InlaySplice { - to_remove, - to_insert, + Some(InlaySplice { + to_remove, + to_insert, + }) } } pub fn clear(&mut self) -> Vec { - self.inlays_per_buffer - .drain() - .flat_map(|(_, buffer_inlays)| { - buffer_inlays - .ordered_by_anchor_inlays - .into_iter() - .map(|(_, id, _)| id) - }) - .collect() + let ids_to_remove = self.inlay_hints.drain().map(|(id, _)| id).collect(); + self.inlays_in_buffers.clear(); + ids_to_remove } pub fn append_inlays( &mut self, multi_buffer: ModelHandle, ranges_to_add: impl Iterator, + currently_shown_inlays: Vec<(Anchor, InlayId)>, cx: &mut ViewContext, ) -> Task> { - self.fetch_inlays(multi_buffer, ranges_to_add, false, cx) + self.fetch_inlays( + multi_buffer, + ranges_to_add, + currently_shown_inlays, + false, + cx, + ) } pub fn replace_inlays( &mut self, multi_buffer: ModelHandle, new_ranges: impl Iterator, + currently_shown_inlays: Vec<(Anchor, InlayId)>, cx: &mut ViewContext, ) -> Task> { - self.fetch_inlays(multi_buffer, new_ranges, true, cx) + self.fetch_inlays(multi_buffer, new_ranges, currently_shown_inlays, true, cx) } fn fetch_inlays( &mut self, multi_buffer: ModelHandle, inlay_fetch_ranges: impl Iterator, + currently_shown_inlays: Vec<(Anchor, InlayId)>, replace_old: bool, cx: &mut ViewContext, ) -> Task> { diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index 31af03f768ea549d04a8802623bbc364acd762a4..0a700879454931493bc16362669751163779abe8 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/crates/editor/src/multi_buffer.rs @@ -2617,6 +2617,15 @@ impl MultiBufferSnapshot { } pub fn anchor_in_excerpt(&self, excerpt_id: ExcerptId, text_anchor: text::Anchor) -> Anchor { + self.find_anchor_in_excerpt(excerpt_id, text_anchor) + .unwrap_or_else(|| panic!("excerpt not found")) + } + + pub fn find_anchor_in_excerpt( + &self, + excerpt_id: ExcerptId, + text_anchor: text::Anchor, + ) -> Option { let locator = self.excerpt_locator_for_id(excerpt_id); let mut cursor = self.excerpts.cursor::>(); cursor.seek(locator, Bias::Left, &()); @@ -2624,14 +2633,15 @@ impl MultiBufferSnapshot { if excerpt.id == excerpt_id { let text_anchor = excerpt.clip_anchor(text_anchor); drop(cursor); - return Anchor { + return Some(Anchor { buffer_id: Some(excerpt.buffer_id), excerpt_id, text_anchor, - }; + }); } } - panic!("excerpt not found"); + + None } pub fn can_resolve(&self, anchor: &Anchor) -> bool { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index fb2cef9863934d1a00ff8d81721eeba1b5aae28e..89975a57465307c86a4ae2782f8a8f356e9ce2dc 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -37,7 +37,7 @@ use language::{ deserialize_anchor, deserialize_fingerprint, deserialize_line_ending, deserialize_version, serialize_anchor, serialize_version, }, - range_from_lsp, range_to_lsp, Anchor, Bias, Buffer, CachedLspAdapter, CodeAction, CodeLabel, + range_from_lsp, range_to_lsp, Bias, Buffer, CachedLspAdapter, CodeAction, CodeLabel, Completion, Diagnostic, DiagnosticEntry, DiagnosticSet, Diff, Event as BufferEvent, File as _, Language, LanguageRegistry, LanguageServerName, LocalFile, LspAdapterDelegate, OffsetRangeExt, Operation, Patch, PendingLanguageServer, PointUtf16, TextBufferSnapshot, ToOffset, @@ -76,6 +76,7 @@ use std::{ time::{Duration, Instant}, }; use terminals::Terminals; +use text::Anchor; use util::{ debug_panic, defer, http::HttpClient, merge_json_value_into, paths::LOCAL_SETTINGS_RELATIVE_PATH, post_inc, ResultExt, TryFutureExt as _, @@ -330,7 +331,7 @@ pub struct Location { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct InlayHint { pub buffer_id: u64, - pub position: Anchor, + pub position: language::Anchor, pub label: InlayHintLabel, pub kind: Option, pub padding_left: bool,