Remove InlayProperties

Kirill Bulatov created

Change summary

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(-)

Detailed changes

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<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() {

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<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(' ');
@@ -57,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(),
+        }
     }
 }
 
@@ -521,10 +527,10 @@ 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();
@@ -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::<String>(
+        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::<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());
             }

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<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 {

crates/editor/src/inlay_hint_cache.rs 🔗

@@ -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