Better rpc inlay hint handling

Kirill Bulatov created

Change summary

crates/editor/src/editor.rs       | 81 +++++++++++++++---------------
crates/project/src/lsp_command.rs | 13 +++-
crates/project/src/project.rs     | 88 ++++++++++++++++++++++++++++++---
3 files changed, 129 insertions(+), 53 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -540,7 +540,7 @@ pub struct Editor {
     gutter_hovered: bool,
     link_go_to_definition_state: LinkGoToDefinitionState,
     copilot_state: CopilotState,
-    inlay_hint_cache: InlayCache,
+    inlay_cache: InlayCache,
     _subscriptions: Vec<Subscription>,
 }
 
@@ -1295,16 +1295,10 @@ impl Editor {
                     cx.emit(Event::TitleChanged);
                 }));
                 project_subscriptions.push(cx.subscribe(project, |editor, _, event, cx| {
-                    match event {
-                        project::Event::ReloadInlayHints => {
-                            editor.reload_inlay_hints(cx);
-                        }
-                        project::Event::ActiveEntryChanged(Some(_)) => {
-                            editor.reload_inlay_hints(cx);
-                        }
-                        _ => {}
+                    if let project::Event::RefreshInlays = event {
+                        editor.refresh_inlays(cx);
+                        cx.notify()
                     };
-                    cx.notify()
                 }));
             }
         }
@@ -1358,7 +1352,7 @@ impl Editor {
             hover_state: Default::default(),
             link_go_to_definition_state: Default::default(),
             copilot_state: Default::default(),
-            inlay_hint_cache: InlayCache::default(),
+            inlay_cache: InlayCache::default(),
             gutter_hovered: false,
             _subscriptions: vec![
                 cx.observe(&buffer, Self::on_buffer_changed),
@@ -2594,12 +2588,12 @@ impl Editor {
         }
     }
 
-    fn reload_inlay_hints(&self, cx: &mut ViewContext<Self>) {
+    fn refresh_inlays(&self, cx: &mut ViewContext<Self>) {
         if self.mode != EditorMode::Full {
             return;
         }
 
-        struct HintRequestKey {
+        struct InlayRequestKey {
             buffer_id: u64,
             buffer_version: Global,
             excerpt_id: ExcerptId,
@@ -2607,7 +2601,7 @@ impl Editor {
 
         let multi_buffer = self.buffer();
         let multi_buffer_snapshot = multi_buffer.read(cx).snapshot(cx);
-        let hint_fetch_tasks = multi_buffer_snapshot
+        let inlay_fetch_tasks = multi_buffer_snapshot
             .excerpts()
             .map(|(excerpt_id, buffer_snapshot, excerpt_range)| {
                 // TODO kb every time I reopen the same buffer, it's different.
@@ -2615,17 +2609,17 @@ impl Editor {
                 let buffer_id = buffer_snapshot.remote_id();
                 let buffer_version = buffer_snapshot.version().clone();
                 let buffer_handle = multi_buffer.read(cx).buffer(buffer_id);
-                let hints_up_to_date =
-                    self.inlay_hint_cache
+                let inlays_up_to_date =
+                    self.inlay_cache
                         .inlays_up_to_date(buffer_id, &buffer_version, excerpt_id);
-                let key = HintRequestKey {
+                let key = InlayRequestKey {
                     buffer_id,
                     buffer_version,
                     excerpt_id,
                 };
 
                 cx.spawn(|editor, mut cx| async move {
-                    if hints_up_to_date {
+                    if inlays_up_to_date {
                         anyhow::Ok((key, None))
                     } else {
                         let Some(buffer_handle) = buffer_handle else { return Ok((key, Some(Vec::new()))) };
@@ -2645,19 +2639,24 @@ impl Editor {
                                     })
                                 })
                             })
-                            .context("inlay hints fecth task spawn")?;
+                            .context("inlays fecth task spawn")?;
 
-                        Ok((key, Some(match task {
+                        Ok((key, match task {
                             Some(task) => {
-                                let mut new_hints = task.await.context("inlay hints for buffer task")?;
-                                new_hints.retain(|hint| {
-                                    let hint_offset = hint.position.offset;
-                                    query_start <= hint_offset && hint_offset <= query_end
-                                });
-                                new_hints
+                                match task.await.context("inlays for buffer task")? {
+                                    Some(mut new_inlays) => {
+                                        new_inlays.retain(|inlay| {
+                                            let inlay_offset = inlay.position.offset;
+                                            query_start <= inlay_offset && inlay_offset <= query_end
+                                        });
+                                        Some(new_inlays)
+                                    },
+                                    None => None,
+                                }
+
                             },
-                            None => Vec::new(),
-                        })))
+                            None => Some(Vec::new()),
+                        }))
                     }
                 })
             })
@@ -2674,35 +2673,35 @@ impl Editor {
             let multi_buffer_snapshot =
                 editor.read_with(&cx, |editor, cx| editor.buffer().read(cx).snapshot(cx))?;
 
-            for task_result in futures::future::join_all(hint_fetch_tasks).await {
+            for task_result in futures::future::join_all(inlay_fetch_tasks).await {
                 match task_result {
                     Ok((request_key, response_inlays)) => {
-                        let excerpt_hints_response = HashMap::from_iter([(
+                        let inlays_per_excerpt = HashMap::from_iter([(
                             request_key.excerpt_id,
-                            response_inlays.map(|excerpt_hints| {
-                                excerpt_hints.into_iter().fold(
+                            response_inlays.map(|excerpt_inlays| {
+                                excerpt_inlays.into_iter().fold(
                                     OrderedByAnchorOffset::default(),
-                                    |mut ordered_hints, hint| {
+                                    |mut ordered_inlays, inlay| {
                                         let anchor = multi_buffer_snapshot.anchor_in_excerpt(
                                             request_key.excerpt_id,
-                                            hint.position,
+                                            inlay.position,
                                         );
-                                        ordered_hints.add(anchor, hint);
-                                        ordered_hints
+                                        ordered_inlays.add(anchor, inlay);
+                                        ordered_inlays
                                     },
                                 )
                             }),
                         )]);
                         match inlay_updates.entry(request_key.buffer_id) {
                             hash_map::Entry::Occupied(mut o) => {
-                                o.get_mut().1.extend(excerpt_hints_response);
+                                o.get_mut().1.extend(inlays_per_excerpt);
                             }
                             hash_map::Entry::Vacant(v) => {
-                                v.insert((request_key.buffer_version, excerpt_hints_response));
+                                v.insert((request_key.buffer_version, inlays_per_excerpt));
                             }
                         }
                     }
-                    Err(e) => error!("Failed to update hints for buffer: {e:#}"),
+                    Err(e) => error!("Failed to update inlays for buffer: {e:#}"),
                 }
             }
 
@@ -2711,7 +2710,7 @@ impl Editor {
                     to_remove,
                     to_insert,
                 } = editor.update(&mut cx, |editor, _| {
-                    editor.inlay_hint_cache.update_inlays(inlay_updates)
+                    editor.inlay_cache.update_inlays(inlay_updates)
                 })?;
 
                 editor.update(&mut cx, |editor, cx| {
@@ -7306,7 +7305,7 @@ impl Editor {
         };
 
         if update_inlay_hints {
-            self.reload_inlay_hints(cx);
+            self.refresh_inlays(cx);
         }
     }
 

crates/project/src/lsp_command.rs 🔗

@@ -1834,7 +1834,7 @@ impl LspCommand for InlayHints {
                 .unwrap_or_default()
                 .into_iter()
                 .map(|lsp_hint| InlayHint {
-                    buffer_id: buffer.id(),
+                    buffer_id: origin_buffer.remote_id(),
                     position: origin_buffer.anchor_after(
                         origin_buffer
                             .clip_point_utf16(point_from_lsp(lsp_hint.position), Bias::Left),
@@ -1931,7 +1931,7 @@ impl LspCommand for InlayHints {
         _: &mut Project,
         _: PeerId,
         buffer_version: &clock::Global,
-        _: &mut AppContext,
+        cx: &mut AppContext,
     ) -> proto::InlayHintsResponse {
         proto::InlayHintsResponse {
             hints: response
@@ -1958,7 +1958,7 @@ impl LspCommand for InlayHints {
                                         location: label_part.location.map(|location| proto::Location {
                                             start: Some(serialize_anchor(&location.range.start)),
                                             end: Some(serialize_anchor(&location.range.end)),
-                                            buffer_id: location.buffer.id() as u64,
+                                            buffer_id: location.buffer.read(cx).remote_id(),
                                         }),
                                     }).collect()
                                 })
@@ -2005,8 +2005,13 @@ impl LspCommand for InlayHints {
 
         let mut hints = Vec::new();
         for message_hint in message.hints {
+            let buffer_id = message_hint
+                .position
+                .as_ref()
+                .and_then(|location| location.buffer_id)
+                .context("missing buffer id")?;
             let hint = InlayHint {
-                buffer_id: buffer.id(),
+                buffer_id,
                 position: message_hint
                     .position
                     .and_then(language::proto::deserialize_anchor)

crates/project/src/project.rs 🔗

@@ -278,7 +278,7 @@ pub enum Event {
         new_peer_id: proto::PeerId,
     },
     CollaboratorLeft(proto::PeerId),
-    ReloadInlayHints,
+    RefreshInlays,
 }
 
 pub enum LanguageServerState {
@@ -329,7 +329,7 @@ pub struct Location {
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct InlayHint {
-    pub buffer_id: usize,
+    pub buffer_id: u64,
     pub position: Anchor,
     pub label: InlayHintLabel,
     pub kind: Option<String>,
@@ -2830,7 +2830,7 @@ impl Project {
                             .upgrade(&cx)
                             .ok_or_else(|| anyhow!("project dropped"))?;
                         this.update(&mut cx, |_, cx| {
-                            cx.emit(Event::ReloadInlayHints);
+                            cx.emit(Event::RefreshInlays);
                         });
                         Ok(())
                     }
@@ -4908,10 +4908,65 @@ impl Project {
         buffer_handle: ModelHandle<Buffer>,
         range: Range<T>,
         cx: &mut ModelContext<Self>,
-    ) -> Task<Result<Vec<InlayHint>>> {
+    ) -> Task<Result<Option<Vec<InlayHint>>>> {
         let buffer = buffer_handle.read(cx);
         let range = buffer.anchor_before(range.start)..buffer.anchor_before(range.end);
-        self.request_lsp(buffer_handle, InlayHints { range }, cx)
+        let range_start = range.start;
+        let range_end = range.end;
+        let buffer_id = buffer.remote_id();
+        let buffer_version = buffer.version().clone();
+        let lsp_request = InlayHints { range };
+
+        if self.is_local() {
+            let lsp_request_task = self.request_lsp(buffer_handle.clone(), lsp_request, cx);
+            cx.spawn(|_, mut cx| async move {
+                buffer_handle
+                    .update(&mut cx, |buffer, _| {
+                        buffer.wait_for_edits(vec![range_start.timestamp, range_end.timestamp])
+                    })
+                    .await
+                    .context("waiting for inlay hint request range edits")?;
+
+                match lsp_request_task.await {
+                    Ok(hints) => Ok(Some(hints)),
+                    Err(e) if is_content_modified_error(&e) => Ok(None),
+                    Err(other_e) => Err(other_e).context("inlay hints LSP request"),
+                }
+            })
+        } else if let Some(project_id) = self.remote_id() {
+            let client = self.client.clone();
+            let request = proto::InlayHints {
+                project_id,
+                buffer_id,
+                start: Some(serialize_anchor(&range_start)),
+                end: Some(serialize_anchor(&range_end)),
+                version: serialize_version(&buffer_version),
+            };
+            cx.spawn(|project, cx| async move {
+                let response = client
+                    .request(request)
+                    .await
+                    .context("inlay hints proto request")?;
+                let hints_request_result = LspCommand::response_from_proto(
+                    lsp_request,
+                    response,
+                    project,
+                    buffer_handle,
+                    cx,
+                )
+                .await;
+
+                match hints_request_result {
+                    Ok(hints) => Ok(Some(hints)),
+                    Err(e) if is_content_modified_error(&e) => Ok(None),
+                    Err(other_err) => {
+                        Err(other_err).context("inlay hints proto response conversion")
+                    }
+                }
+            })
+        } else {
+            Task::ready(Err(anyhow!("project does not have a remote id")))
+        }
     }
 
     #[allow(clippy::type_complexity)]
@@ -6686,11 +6741,23 @@ impl Project {
 
         let buffer_hints = this
             .update(&mut cx, |project, cx| {
-                let end = buffer.read(cx).len();
-                project.inlay_hints_for_buffer(buffer, 0..end, cx)
+                let buffer_end = buffer.read(cx).len();
+                project.inlay_hints_for_buffer(
+                    buffer,
+                    envelope
+                        .payload
+                        .start
+                        .map_or(0, |anchor| anchor.offset as usize)
+                        ..envelope
+                            .payload
+                            .end
+                            .map_or(buffer_end, |anchor| anchor.offset as usize),
+                    cx,
+                )
             })
             .await
-            .context("inlay hints fetch")?;
+            .context("inlay hints fetch")?
+            .unwrap_or_default();
 
         Ok(this.update(&mut cx, |project, cx| {
             InlayHints::response_to_proto(buffer_hints, project, sender_id, &buffer_version, cx)
@@ -7765,3 +7832,8 @@ async fn wait_for_loading_buffer(
         receiver.next().await;
     }
 }
+
+// TODO kb what are better ways?
+fn is_content_modified_error(error: &anyhow::Error) -> bool {
+    format!("{error:#}").contains("content modified")
+}