From 6ba1c3071a53e566e9c0fdfd75326589e0fc6cb1 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 5 Jul 2023 15:23:56 +0300 Subject: [PATCH 1/2] Simplify inlay map data --- crates/editor/src/display_map/inlay_map.rs | 28 +++++++++++++--------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/crates/editor/src/display_map/inlay_map.rs b/crates/editor/src/display_map/inlay_map.rs index 99b22dcbb6..90e5c8a9ce 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -2,7 +2,7 @@ use crate::{ multi_buffer::{MultiBufferChunks, MultiBufferRows}, Anchor, InlayId, MultiBufferSnapshot, ToOffset, }; -use collections::{BTreeMap, BTreeSet, HashMap}; +use collections::{BTreeMap, BTreeSet}; use gpui::fonts::HighlightStyle; use language::{Chunk, Edit, Point, Rope, TextSummary}; use std::{ @@ -19,7 +19,6 @@ use super::TextHighlights; pub struct InlayMap { snapshot: InlaySnapshot, - inlays_by_id: HashMap, inlays: Vec, } @@ -381,7 +380,6 @@ impl InlayMap { ( Self { snapshot: snapshot.clone(), - inlays_by_id: HashMap::default(), inlays: Vec::new(), }, snapshot, @@ -531,13 +529,14 @@ impl InlayMap { let snapshot = &mut self.snapshot; let mut edits = BTreeSet::new(); - self.inlays.retain(|inlay| !to_remove.contains(&inlay.id)); - for inlay_id in to_remove { - if let Some(inlay) = self.inlays_by_id.remove(&inlay_id) { + self.inlays.retain(|inlay| { + let retain = !to_remove.contains(&inlay.id); + if !retain { let offset = inlay.position.to_offset(&snapshot.buffer); edits.insert(offset); } - } + retain + }); for (existing_id, properties) in to_insert { let inlay = Inlay { @@ -551,7 +550,6 @@ impl InlayMap { continue; } - self.inlays_by_id.insert(inlay.id, inlay.clone()); match self .inlays .binary_search_by(|probe| probe.position.cmp(&inlay.position, &snapshot.buffer)) @@ -627,7 +625,13 @@ impl InlayMap { }, )); } else { - to_remove.push(*self.inlays_by_id.keys().choose(rng).unwrap()); + to_remove.push( + self.inlays + .iter() + .choose(rng) + .map(|inlay| inlay.id) + .unwrap(), + ); } } log::info!("removing inlays: {:?}", to_remove); @@ -1478,8 +1482,10 @@ mod tests { ); // The inlays can be manually removed. - let (inlay_snapshot, _) = inlay_map - .splice::(inlay_map.inlays_by_id.keys().copied().collect(), Vec::new()); + let (inlay_snapshot, _) = inlay_map.splice::( + inlay_map.inlays.iter().map(|inlay| inlay.id).collect(), + Vec::new(), + ); assert_eq!(inlay_snapshot.text(), "abxJKLyDzefghi"); } From d7f6b5e1a07f645a22160df2f39f424d02d69998 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 5 Jul 2023 15:58:31 +0300 Subject: [PATCH 2/2] Remove InlayProperties --- crates/editor/src/display_map.rs | 7 +- crates/editor/src/display_map/inlay_map.rs | 167 ++++++++++----------- crates/editor/src/editor.rs | 41 +++-- crates/editor/src/inlay_hint_cache.rs | 28 ++-- 4 files changed, 117 insertions(+), 126 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index 714dc74509..6e04833f17 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -20,7 +20,6 @@ use language::{ use std::{any::TypeId, fmt::Debug, num::NonZeroU32, ops::Range, sync::Arc}; use sum_tree::{Bias, TreeMap}; use tab_map::TabMap; -use text::Rope; use wrap_map::WrapMap; pub use block_map::{ @@ -28,7 +27,7 @@ pub use block_map::{ BlockDisposition, BlockId, BlockProperties, BlockStyle, RenderBlock, TransformBlock, }; -pub use self::inlay_map::{Inlay, InlayProperties}; +pub use self::inlay_map::Inlay; #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum FoldStatus { @@ -246,10 +245,10 @@ impl DisplayMap { self.inlay_map.current_inlays() } - pub fn splice_inlays>( + pub fn splice_inlays( &mut self, to_remove: Vec, - to_insert: Vec<(InlayId, InlayProperties)>, + to_insert: Vec, cx: &mut ModelContext, ) { 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 90e5c8a9ce..6a59cecae8 100644 --- a/crates/editor/src/display_map/inlay_map.rs +++ b/crates/editor/src/display_map/inlay_map.rs @@ -4,7 +4,7 @@ use crate::{ }; use collections::{BTreeMap, BTreeSet}; use gpui::fonts::HighlightStyle; -use language::{Chunk, Edit, Point, Rope, TextSummary}; +use language::{Chunk, Edit, Point, TextSummary}; use std::{ any::TypeId, cmp, @@ -13,7 +13,7 @@ use std::{ vec, }; use sum_tree::{Bias, Cursor, SumTree}; -use text::Patch; +use text::{Patch, Rope}; use super::TextHighlights; @@ -42,14 +42,8 @@ pub struct Inlay { pub text: text::Rope, } -#[derive(Debug, Clone)] -pub struct InlayProperties { - pub position: Anchor, - pub text: T, -} - -impl InlayProperties { - pub fn new(position: Anchor, hint: &project::InlayHint) -> Self { +impl Inlay { + pub fn hint(id: usize, position: Anchor, hint: &project::InlayHint) -> Self { let mut text = hint.text(); if hint.padding_right && !text.ends_with(' ') { text.push(' '); @@ -57,7 +51,19 @@ impl InlayProperties { if hint.padding_left && !text.starts_with(' ') { text.insert(0, ' '); } - Self { position, text } + Self { + id: InlayId::Hint(id), + position, + text: text.into(), + } + } + + pub fn suggestion>(id: usize, position: Anchor, text: T) -> Self { + Self { + id: InlayId::Suggestion(id), + position, + text: text.into(), + } } } @@ -521,10 +527,10 @@ impl InlayMap { } } - pub fn splice>( + pub fn splice( &mut self, to_remove: Vec, - to_insert: Vec<(InlayId, InlayProperties)>, + to_insert: Vec, ) -> (InlaySnapshot, Vec) { let snapshot = &mut self.snapshot; let mut edits = BTreeSet::new(); @@ -538,28 +544,23 @@ impl InlayMap { retain }); - for (existing_id, properties) in to_insert { - let inlay = Inlay { - id: existing_id, - position: properties.position, - text: properties.text.into(), - }; - + for inlay_to_insert in to_insert { // Avoid inserting empty inlays. - if inlay.text.is_empty() { + if inlay_to_insert.text.is_empty() { continue; } - match self - .inlays - .binary_search_by(|probe| probe.position.cmp(&inlay.position, &snapshot.buffer)) - { + let offset = inlay_to_insert.position.to_offset(&snapshot.buffer); + match self.inlays.binary_search_by(|probe| { + probe + .position + .cmp(&inlay_to_insert.position, &snapshot.buffer) + }) { Ok(ix) | Err(ix) => { - self.inlays.insert(ix, inlay.clone()); + self.inlays.insert(ix, inlay_to_insert); } } - let offset = inlay.position.to_offset(&snapshot.buffer); edits.insert(offset); } @@ -617,13 +618,11 @@ impl InlayMap { } else { InlayId::Suggestion(post_inc(next_inlay_id)) }; - to_insert.push(( - inlay_id, - InlayProperties { - position: snapshot.buffer.anchor_at(position, bias), - text, - }, - )); + to_insert.push(Inlay { + id: inlay_id, + position: snapshot.buffer.anchor_at(position, bias), + text: text.into(), + }); } else { to_remove.push( self.inlays @@ -1123,7 +1122,8 @@ mod tests { #[test] fn test_inlay_properties_label_padding() { assert_eq!( - InlayProperties::new( + Inlay::hint( + 0, Anchor::min(), &InlayHint { label: InlayHintLabel::String("a".to_string()), @@ -1135,13 +1135,15 @@ mod tests { kind: None, }, ) - .text, + .text + .to_string(), "a", "Should not pad label if not requested" ); assert_eq!( - InlayProperties::new( + Inlay::hint( + 0, Anchor::min(), &InlayHint { label: InlayHintLabel::String("a".to_string()), @@ -1153,13 +1155,15 @@ mod tests { kind: None, }, ) - .text, + .text + .to_string(), " a ", "Should pad label for every side requested" ); assert_eq!( - InlayProperties::new( + Inlay::hint( + 0, Anchor::min(), &InlayHint { label: InlayHintLabel::String(" a ".to_string()), @@ -1171,13 +1175,15 @@ mod tests { kind: None, }, ) - .text, + .text + .to_string(), " a ", "Should not change already padded label" ); assert_eq!( - InlayProperties::new( + Inlay::hint( + 0, Anchor::min(), &InlayHint { label: InlayHintLabel::String(" a ".to_string()), @@ -1189,7 +1195,8 @@ mod tests { kind: None, }, ) - .text, + .text + .to_string(), " a ", "Should not change already padded label" ); @@ -1205,13 +1212,11 @@ mod tests { let (inlay_snapshot, _) = inlay_map.splice( Vec::new(), - vec![( - InlayId::Hint(post_inc(&mut next_inlay_id)), - InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_after(3), - text: "|123|", - }, - )], + vec![Inlay { + id: InlayId::Hint(post_inc(&mut next_inlay_id)), + position: buffer.read(cx).snapshot(cx).anchor_after(3), + text: "|123|".into(), + }], ); assert_eq!(inlay_snapshot.text(), "abc|123|defghi"); assert_eq!( @@ -1284,20 +1289,16 @@ mod tests { let (inlay_snapshot, _) = inlay_map.splice( Vec::new(), vec![ - ( - InlayId::Hint(post_inc(&mut next_inlay_id)), - InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_before(3), - text: "|123|", - }, - ), - ( - InlayId::Suggestion(post_inc(&mut next_inlay_id)), - InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_after(3), - text: "|456|", - }, - ), + Inlay { + id: InlayId::Hint(post_inc(&mut next_inlay_id)), + position: buffer.read(cx).snapshot(cx).anchor_before(3), + text: "|123|".into(), + }, + Inlay { + id: InlayId::Suggestion(post_inc(&mut next_inlay_id)), + position: buffer.read(cx).snapshot(cx).anchor_after(3), + text: "|456|".into(), + }, ], ); assert_eq!(inlay_snapshot.text(), "abx|123||456|yDzefghi"); @@ -1482,7 +1483,7 @@ mod tests { ); // The inlays can be manually removed. - let (inlay_snapshot, _) = inlay_map.splice::( + let (inlay_snapshot, _) = inlay_map.splice( inlay_map.inlays.iter().map(|inlay| inlay.id).collect(), Vec::new(), ); @@ -1499,27 +1500,21 @@ mod tests { let (inlay_snapshot, _) = inlay_map.splice( Vec::new(), vec![ - ( - InlayId::Hint(post_inc(&mut next_inlay_id)), - InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_before(0), - text: "|123|\n", - }, - ), - ( - InlayId::Hint(post_inc(&mut next_inlay_id)), - InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_before(4), - text: "|456|", - }, - ), - ( - InlayId::Suggestion(post_inc(&mut next_inlay_id)), - InlayProperties { - position: buffer.read(cx).snapshot(cx).anchor_before(7), - text: "\n|567|\n", - }, - ), + Inlay { + id: InlayId::Hint(post_inc(&mut next_inlay_id)), + position: buffer.read(cx).snapshot(cx).anchor_before(0), + text: "|123|\n".into(), + }, + Inlay { + id: InlayId::Hint(post_inc(&mut next_inlay_id)), + position: buffer.read(cx).snapshot(cx).anchor_before(4), + text: "|456|".into(), + }, + Inlay { + id: InlayId::Suggestion(post_inc(&mut next_inlay_id)), + position: buffer.read(cx).snapshot(cx).anchor_before(7), + text: "\n|567|\n".into(), + }, ], ); assert_eq!(inlay_snapshot.text(), "|123|\nabc\n|456|def\n|567|\n\nghi"); @@ -1609,7 +1604,7 @@ mod tests { (offset, inlay.clone()) }) .collect::>(); - let mut expected_text = Rope::from(buffer_snapshot.text().as_str()); + 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()); } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 992ecd7459..e979bd9c1e 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -190,6 +190,15 @@ pub enum InlayId { Hint(usize), } +impl InlayId { + fn id(&self) -> usize { + match self { + Self::Suggestion(id) => *id, + Self::Hint(id) => *id, + } + } +} + actions!( editor, [ @@ -2708,17 +2717,11 @@ impl Editor { fn splice_inlay_hints( &self, to_remove: Vec, - to_insert: Vec<(Anchor, InlayId, project::InlayHint)>, + to_insert: Vec, cx: &mut ViewContext, ) { - let buffer = self.buffer.read(cx).read(cx); - let new_inlays = to_insert - .into_iter() - .map(|(position, id, hint)| (id, InlayProperties::new(position, &hint))) - .collect(); - drop(buffer); self.display_map.update(cx, |display_map, cx| { - display_map.splice_inlays(to_remove, new_inlays, cx); + display_map.splice_inlays(to_remove, to_insert, cx); }); } @@ -3403,7 +3406,7 @@ impl Editor { } self.display_map.update(cx, |map, cx| { - map.splice_inlays::<&str>(vec![suggestion.id], Vec::new(), cx) + map.splice_inlays(vec![suggestion.id], Vec::new(), cx) }); cx.notify(); true @@ -3436,7 +3439,7 @@ impl Editor { fn take_active_copilot_suggestion(&mut self, cx: &mut ViewContext) -> Option { let suggestion = self.copilot_state.suggestion.take()?; self.display_map.update(cx, |map, cx| { - map.splice_inlays::<&str>(vec![suggestion.id], Default::default(), cx); + map.splice_inlays(vec![suggestion.id], Default::default(), cx); }); let buffer = self.buffer.read(cx).read(cx); @@ -3467,21 +3470,11 @@ impl Editor { to_remove.push(suggestion.id); } - let suggestion_inlay_id = InlayId::Suggestion(post_inc(&mut self.next_inlay_id)); - let to_insert = vec![( - suggestion_inlay_id, - InlayProperties { - position: cursor, - text: text.clone(), - }, - )]; + let suggestion_inlay = + Inlay::suggestion(post_inc(&mut self.next_inlay_id), cursor, text); + self.copilot_state.suggestion = Some(suggestion_inlay.clone()); self.display_map.update(cx, move |map, cx| { - map.splice_inlays(to_remove, to_insert, cx) - }); - self.copilot_state.suggestion = Some(Inlay { - id: suggestion_inlay_id, - position: cursor, - text, + map.splice_inlays(to_remove, vec![suggestion_inlay], cx) }); cx.notify(); } else { diff --git a/crates/editor/src/inlay_hint_cache.rs b/crates/editor/src/inlay_hint_cache.rs index 3f9f8e4288..a6ea3962d2 100644 --- a/crates/editor/src/inlay_hint_cache.rs +++ b/crates/editor/src/inlay_hint_cache.rs @@ -45,7 +45,7 @@ pub enum InvalidationStrategy { #[derive(Debug, Default)] pub struct InlaySplice { pub to_remove: Vec, - pub to_insert: Vec<(Anchor, InlayId, InlayHint)>, + pub to_insert: Vec, } struct UpdateTask { @@ -285,13 +285,13 @@ impl InlayHintCache { if !old_kinds.contains(&cached_hint.kind) && new_kinds.contains(&cached_hint.kind) { - to_insert.push(( + to_insert.push(Inlay::hint( + cached_hint_id.id(), multi_buffer_snapshot.anchor_in_excerpt( *excerpt_id, cached_hint.position, ), - *cached_hint_id, - cached_hint.clone(), + &cached_hint, )); } excerpt_cache.next(); @@ -307,11 +307,11 @@ impl InlayHintCache { for (cached_hint_id, maybe_missed_cached_hint) in excerpt_cache { 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(( + to_insert.push(Inlay::hint( + cached_hint_id.id(), multi_buffer_snapshot .anchor_in_excerpt(*excerpt_id, maybe_missed_cached_hint.position), - *cached_hint_id, - maybe_missed_cached_hint.clone(), + &maybe_missed_cached_hint, )); } } @@ -657,18 +657,22 @@ async fn fetch_and_update_hints( for new_hint in new_update.add_to_cache { let new_hint_position = multi_buffer_snapshot .anchor_in_excerpt(query.excerpt_id, new_hint.position); - let new_inlay_id = InlayId::Hint(post_inc(&mut editor.next_inlay_id)); + let new_inlay_id = post_inc(&mut editor.next_inlay_id); if editor .inlay_hint_cache .allowed_hint_kinds .contains(&new_hint.kind) { - splice - .to_insert - .push((new_hint_position, new_inlay_id, new_hint.clone())); + splice.to_insert.push(Inlay::hint( + new_inlay_id, + new_hint_position, + &new_hint, + )); } - cached_excerpt_hints.hints.push((new_inlay_id, new_hint)); + cached_excerpt_hints + .hints + .push((InlayId::Hint(new_inlay_id), new_hint)); } cached_excerpt_hints