Simplify inlay map data (#2683)

Current logic does not need to access inlays by id in O(1), future
dynamic hints would need to know which hint they hover at, but that will
be done using binary search over the position's anchor we hover on;
nothing else seems to need this HashMap in the near future.

Because of that removal, no need to store `InlayId` apart from the
`Inlay`, hence remove the `InlayProperties` struct entirely.
This allows to eliminate a few generics along the way.

Release Notes:

- N/A
This commit is contained in:
Kirill Bulatov 2023-07-05 16:31:47 +03:00 committed by GitHub
commit 91a94d299e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 133 additions and 136 deletions

View file

@ -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<T: Into<Rope>>(
pub fn splice_inlays(
&mut self,
to_remove: Vec<InlayId>,
to_insert: Vec<(InlayId, InlayProperties<T>)>,
to_insert: Vec<Inlay>,
cx: &mut ModelContext<Self>,
) {
if to_remove.is_empty() && to_insert.is_empty() {

View file

@ -2,9 +2,9 @@ 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 language::{Chunk, Edit, Point, TextSummary};
use std::{
any::TypeId,
cmp,
@ -13,13 +13,12 @@ use std::{
vec,
};
use sum_tree::{Bias, Cursor, SumTree};
use text::Patch;
use text::{Patch, Rope};
use super::TextHighlights;
pub struct InlayMap {
snapshot: InlaySnapshot,
inlays_by_id: HashMap<InlayId, Inlay>,
inlays: Vec<Inlay>,
}
@ -43,14 +42,8 @@ pub struct Inlay {
pub text: text::Rope,
}
#[derive(Debug, Clone)]
pub struct InlayProperties<T> {
pub position: Anchor,
pub text: T,
}
impl InlayProperties<String> {
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(' ');
@ -58,7 +51,19 @@ impl InlayProperties<String> {
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<T: Into<Rope>>(id: usize, position: Anchor, text: T) -> Self {
Self {
id: InlayId::Suggestion(id),
position,
text: text.into(),
}
}
}
@ -381,7 +386,6 @@ impl InlayMap {
(
Self {
snapshot: snapshot.clone(),
inlays_by_id: HashMap::default(),
inlays: Vec::new(),
},
snapshot,
@ -523,45 +527,40 @@ impl InlayMap {
}
}
pub fn splice<T: Into<Rope>>(
pub fn splice(
&mut self,
to_remove: Vec<InlayId>,
to_insert: Vec<(InlayId, InlayProperties<T>)>,
to_insert: Vec<Inlay>,
) -> (InlaySnapshot, Vec<InlayEdit>) {
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);
}
}
for (existing_id, properties) in to_insert {
let inlay = Inlay {
id: existing_id,
position: properties.position,
text: properties.text.into(),
};
retain
});
for inlay_to_insert in to_insert {
// Avoid inserting empty inlays.
if inlay.text.is_empty() {
if inlay_to_insert.text.is_empty() {
continue;
}
self.inlays_by_id.insert(inlay.id, inlay.clone());
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);
}
@ -619,15 +618,19 @@ impl InlayMap {
} else {
InlayId::Suggestion(post_inc(next_inlay_id))
};
to_insert.push((
inlay_id,
InlayProperties {
to_insert.push(Inlay {
id: inlay_id,
position: snapshot.buffer.anchor_at(position, bias),
text,
},
));
text: text.into(),
});
} 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);
@ -1119,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()),
@ -1131,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()),
@ -1149,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()),
@ -1167,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()),
@ -1185,7 +1195,8 @@ mod tests {
kind: None,
},
)
.text,
.text
.to_string(),
" a ",
"Should not change already padded label"
);
@ -1201,13 +1212,11 @@ mod tests {
let (inlay_snapshot, _) = inlay_map.splice(
Vec::new(),
vec![(
InlayId::Hint(post_inc(&mut next_inlay_id)),
InlayProperties {
vec![Inlay {
id: InlayId::Hint(post_inc(&mut next_inlay_id)),
position: buffer.read(cx).snapshot(cx).anchor_after(3),
text: "|123|",
},
)],
text: "|123|".into(),
}],
);
assert_eq!(inlay_snapshot.text(), "abc|123|defghi");
assert_eq!(
@ -1280,20 +1289,16 @@ mod tests {
let (inlay_snapshot, _) = inlay_map.splice(
Vec::new(),
vec![
(
InlayId::Hint(post_inc(&mut next_inlay_id)),
InlayProperties {
Inlay {
id: InlayId::Hint(post_inc(&mut next_inlay_id)),
position: buffer.read(cx).snapshot(cx).anchor_before(3),
text: "|123|",
text: "|123|".into(),
},
),
(
InlayId::Suggestion(post_inc(&mut next_inlay_id)),
InlayProperties {
Inlay {
id: InlayId::Suggestion(post_inc(&mut next_inlay_id)),
position: buffer.read(cx).snapshot(cx).anchor_after(3),
text: "|456|",
text: "|456|".into(),
},
),
],
);
assert_eq!(inlay_snapshot.text(), "abx|123||456|yDzefghi");
@ -1478,8 +1483,10 @@ mod tests {
);
// The inlays can be manually removed.
let (inlay_snapshot, _) = inlay_map
.splice::<String>(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");
}
@ -1493,27 +1500,21 @@ mod tests {
let (inlay_snapshot, _) = inlay_map.splice(
Vec::new(),
vec![
(
InlayId::Hint(post_inc(&mut next_inlay_id)),
InlayProperties {
Inlay {
id: InlayId::Hint(post_inc(&mut next_inlay_id)),
position: buffer.read(cx).snapshot(cx).anchor_before(0),
text: "|123|\n",
text: "|123|\n".into(),
},
),
(
InlayId::Hint(post_inc(&mut next_inlay_id)),
InlayProperties {
Inlay {
id: InlayId::Hint(post_inc(&mut next_inlay_id)),
position: buffer.read(cx).snapshot(cx).anchor_before(4),
text: "|456|",
text: "|456|".into(),
},
),
(
InlayId::Suggestion(post_inc(&mut next_inlay_id)),
InlayProperties {
Inlay {
id: InlayId::Suggestion(post_inc(&mut next_inlay_id)),
position: buffer.read(cx).snapshot(cx).anchor_before(7),
text: "\n|567|\n",
text: "\n|567|\n".into(),
},
),
],
);
assert_eq!(inlay_snapshot.text(), "|123|\nabc\n|456|def\n|567|\n\nghi");
@ -1603,7 +1604,7 @@ mod tests {
(offset, inlay.clone())
})
.collect::<Vec<_>>();
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());
}

View file

@ -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<InlayId>,
to_insert: Vec<(Anchor, InlayId, project::InlayHint)>,
to_insert: Vec<Inlay>,
cx: &mut ViewContext<Self>,
) {
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<Self>) -> Option<Inlay> {
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 {

View file

@ -45,7 +45,7 @@ pub enum InvalidationStrategy {
#[derive(Debug, Default)]
pub struct InlaySplice {
pub to_remove: Vec<InlayId>,
pub to_insert: Vec<(Anchor, InlayId, InlayHint)>,
pub to_insert: Vec<Inlay>,
}
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