diff --git a/crates/collab/src/tests/editor_tests.rs b/crates/collab/src/tests/editor_tests.rs index 0cd18c049e93c057680482950c17b070219f37b5..88dc25ac5b1f5b296571cd62b13e5b8915414b55 100644 --- a/crates/collab/src/tests/editor_tests.rs +++ b/crates/collab/src/tests/editor_tests.rs @@ -37,6 +37,7 @@ use std::{ Arc, atomic::{self, AtomicBool, AtomicUsize}, }, + time::Duration, }; use text::Point; use util::{path, rel_path::rel_path, uri}; @@ -1813,14 +1814,7 @@ async fn test_mutual_editor_inlay_hint_cache_update( settings.project.all_languages.defaults.inlay_hints = Some(InlayHintSettingsContent { enabled: Some(true), - show_value_hints: Some(true), - edit_debounce_ms: Some(0), - scroll_debounce_ms: Some(0), - show_type_hints: Some(true), - show_parameter_hints: Some(false), - show_other_hints: Some(true), - show_background: Some(false), - toggle_on_modifiers_press: None, + ..InlayHintSettingsContent::default() }) }); }); @@ -1830,15 +1824,8 @@ async fn test_mutual_editor_inlay_hint_cache_update( store.update_user_settings(cx, |settings| { settings.project.all_languages.defaults.inlay_hints = Some(InlayHintSettingsContent { - show_value_hints: Some(true), enabled: Some(true), - edit_debounce_ms: Some(0), - scroll_debounce_ms: Some(0), - show_type_hints: Some(true), - show_parameter_hints: Some(false), - show_other_hints: Some(true), - show_background: Some(false), - toggle_on_modifiers_press: None, + ..InlayHintSettingsContent::default() }) }); }); @@ -1931,6 +1918,7 @@ async fn test_mutual_editor_inlay_hint_cache_update( }); let fake_language_server = fake_language_servers.next().await.unwrap(); let editor_a = file_a.await.unwrap().downcast::().unwrap(); + executor.advance_clock(Duration::from_millis(100)); executor.run_until_parked(); let initial_edit = edits_made.load(atomic::Ordering::Acquire); @@ -1951,6 +1939,7 @@ async fn test_mutual_editor_inlay_hint_cache_update( .downcast::() .unwrap(); + executor.advance_clock(Duration::from_millis(100)); executor.run_until_parked(); editor_b.update(cx_b, |editor, cx| { assert_eq!( @@ -1969,6 +1958,7 @@ async fn test_mutual_editor_inlay_hint_cache_update( }); cx_b.focus(&editor_b); + executor.advance_clock(Duration::from_secs(1)); executor.run_until_parked(); editor_a.update(cx_a, |editor, cx| { assert_eq!( @@ -1992,6 +1982,7 @@ async fn test_mutual_editor_inlay_hint_cache_update( }); cx_a.focus(&editor_a); + executor.advance_clock(Duration::from_secs(1)); executor.run_until_parked(); editor_a.update(cx_a, |editor, cx| { assert_eq!( @@ -2013,6 +2004,7 @@ async fn test_mutual_editor_inlay_hint_cache_update( .into_response() .expect("inlay refresh request failed"); + executor.advance_clock(Duration::from_secs(1)); executor.run_until_parked(); editor_a.update(cx_a, |editor, cx| { assert_eq!( diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index a3ea03c3d72216ba6a47691ab7c91150742dbb4c..b120f672994bfb1e107de698dbc6898f6cbd980c 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1833,9 +1833,15 @@ impl Editor { project::Event::RefreshCodeLens => { // we always query lens with actions, without storing them, always refreshing them } - project::Event::RefreshInlayHints(server_id) => { + project::Event::RefreshInlayHints { + server_id, + request_id, + } => { editor.refresh_inlay_hints( - InlayHintRefreshReason::RefreshRequested(*server_id), + InlayHintRefreshReason::RefreshRequested { + server_id: *server_id, + request_id: *request_id, + }, cx, ); } diff --git a/crates/editor/src/inlays/inlay_hints.rs b/crates/editor/src/inlays/inlay_hints.rs index d0d92cbce6addf8534b37b1f7dcc0b6fec86fc42..bdfee86fb6b131d3397c33a1ef645574fca476cf 100644 --- a/crates/editor/src/inlays/inlay_hints.rs +++ b/crates/editor/src/inlays/inlay_hints.rs @@ -1,5 +1,4 @@ use std::{ - collections::hash_map, ops::{ControlFlow, Range}, time::Duration, }; @@ -49,8 +48,8 @@ pub struct LspInlayHintData { allowed_hint_kinds: HashSet>, invalidate_debounce: Option, append_debounce: Option, - hint_refresh_tasks: HashMap>, Vec>>>, - hint_chunk_fetched: HashMap>)>, + hint_refresh_tasks: HashMap>>, + hint_chunk_fetching: HashMap>)>, invalidate_hints_for_buffers: HashSet, pub added_hints: HashMap>, } @@ -63,7 +62,7 @@ impl LspInlayHintData { enabled_in_settings: settings.enabled, hint_refresh_tasks: HashMap::default(), added_hints: HashMap::default(), - hint_chunk_fetched: HashMap::default(), + hint_chunk_fetching: HashMap::default(), invalidate_hints_for_buffers: HashSet::default(), invalidate_debounce: debounce_value(settings.edit_debounce_ms), append_debounce: debounce_value(settings.scroll_debounce_ms), @@ -99,9 +98,8 @@ impl LspInlayHintData { pub fn clear(&mut self) { self.hint_refresh_tasks.clear(); - self.hint_chunk_fetched.clear(); + self.hint_chunk_fetching.clear(); self.added_hints.clear(); - self.invalidate_hints_for_buffers.clear(); } /// Checks inlay hint settings for enabled hint kinds and general enabled state. @@ -199,7 +197,7 @@ impl LspInlayHintData { ) { for buffer_id in removed_buffer_ids { self.hint_refresh_tasks.remove(buffer_id); - self.hint_chunk_fetched.remove(buffer_id); + self.hint_chunk_fetching.remove(buffer_id); } } } @@ -211,7 +209,10 @@ pub enum InlayHintRefreshReason { SettingsChange(InlayHintSettings), NewLinesShown, BufferEdited(BufferId), - RefreshRequested(LanguageServerId), + RefreshRequested { + server_id: LanguageServerId, + request_id: Option, + }, ExcerptsRemoved(Vec), } @@ -296,7 +297,7 @@ impl Editor { | InlayHintRefreshReason::Toggle(_) | InlayHintRefreshReason::SettingsChange(_) => true, InlayHintRefreshReason::NewLinesShown - | InlayHintRefreshReason::RefreshRequested(_) + | InlayHintRefreshReason::RefreshRequested { .. } | InlayHintRefreshReason::ExcerptsRemoved(_) => false, InlayHintRefreshReason::BufferEdited(buffer_id) => { let Some(affected_language) = self @@ -370,48 +371,45 @@ impl Editor { let Some(buffer) = multi_buffer.read(cx).buffer(buffer_id) else { continue; }; - let fetched_tasks = inlay_hints.hint_chunk_fetched.entry(buffer_id).or_default(); + + let (fetched_for_version, fetched_chunks) = inlay_hints + .hint_chunk_fetching + .entry(buffer_id) + .or_default(); if visible_excerpts .buffer_version - .changed_since(&fetched_tasks.0) + .changed_since(fetched_for_version) { - fetched_tasks.1.clear(); - fetched_tasks.0 = visible_excerpts.buffer_version.clone(); + *fetched_for_version = visible_excerpts.buffer_version.clone(); + fetched_chunks.clear(); inlay_hints.hint_refresh_tasks.remove(&buffer_id); } - let applicable_chunks = - semantics_provider.applicable_inlay_chunks(&buffer, &visible_excerpts.ranges, cx); + let known_chunks = if ignore_previous_fetches { + None + } else { + Some((fetched_for_version.clone(), fetched_chunks.clone())) + }; - match inlay_hints + let mut applicable_chunks = + semantics_provider.applicable_inlay_chunks(&buffer, &visible_excerpts.ranges, cx); + applicable_chunks.retain(|chunk| fetched_chunks.insert(chunk.clone())); + if applicable_chunks.is_empty() && !ignore_previous_fetches { + continue; + } + inlay_hints .hint_refresh_tasks .entry(buffer_id) .or_default() - .entry(applicable_chunks) - { - hash_map::Entry::Occupied(mut o) => { - if invalidate_cache.should_invalidate() || ignore_previous_fetches { - o.get_mut().push(spawn_editor_hints_refresh( - buffer_id, - invalidate_cache, - ignore_previous_fetches, - debounce, - visible_excerpts, - cx, - )); - } - } - hash_map::Entry::Vacant(v) => { - v.insert(Vec::new()).push(spawn_editor_hints_refresh( - buffer_id, - invalidate_cache, - ignore_previous_fetches, - debounce, - visible_excerpts, - cx, - )); - } - } + .push(spawn_editor_hints_refresh( + buffer_id, + invalidate_cache, + debounce, + visible_excerpts, + known_chunks, + applicable_chunks, + cx, + )); } } @@ -506,9 +504,13 @@ impl Editor { } InlayHintRefreshReason::NewLinesShown => InvalidationStrategy::None, InlayHintRefreshReason::BufferEdited(_) => InvalidationStrategy::BufferEdited, - InlayHintRefreshReason::RefreshRequested(server_id) => { - InvalidationStrategy::RefreshRequested(*server_id) - } + InlayHintRefreshReason::RefreshRequested { + server_id, + request_id, + } => InvalidationStrategy::RefreshRequested { + server_id: *server_id, + request_id: *request_id, + }, }; match &mut self.inlay_hints { @@ -718,44 +720,29 @@ impl Editor { fn inlay_hints_for_buffer( &mut self, invalidate_cache: InvalidationStrategy, - ignore_previous_fetches: bool, buffer_excerpts: VisibleExcerpts, + known_chunks: Option<(Global, HashSet>)>, cx: &mut Context, ) -> Option, anyhow::Result)>>> { let semantics_provider = self.semantics_provider()?; - let inlay_hints = self.inlay_hints.as_mut()?; - let buffer_id = buffer_excerpts.buffer.read(cx).remote_id(); let new_hint_tasks = semantics_provider .inlay_hints( invalidate_cache, buffer_excerpts.buffer, buffer_excerpts.ranges, - inlay_hints - .hint_chunk_fetched - .get(&buffer_id) - .filter(|_| !ignore_previous_fetches && !invalidate_cache.should_invalidate()) - .cloned(), + known_chunks, cx, ) .unwrap_or_default(); - let (known_version, known_chunks) = - inlay_hints.hint_chunk_fetched.entry(buffer_id).or_default(); - if buffer_excerpts.buffer_version.changed_since(known_version) { - known_chunks.clear(); - *known_version = buffer_excerpts.buffer_version; - } - - let mut hint_tasks = Vec::new(); + let mut hint_tasks = None; for (row_range, new_hints_task) in new_hint_tasks { - let inserted = known_chunks.insert(row_range.clone()); - if inserted || ignore_previous_fetches || invalidate_cache.should_invalidate() { - hint_tasks.push(cx.spawn(async move |_, _| (row_range, new_hints_task.await))); - } + hint_tasks + .get_or_insert_with(Vec::new) + .push(cx.spawn(async move |_, _| (row_range, new_hints_task.await))); } - - Some(hint_tasks) + hint_tasks } fn apply_fetched_hints( @@ -793,20 +780,28 @@ impl Editor { let excerpts = self.buffer.read(cx).excerpt_ids(); let hints_to_insert = new_hints .into_iter() - .filter_map(|(chunk_range, hints_result)| match hints_result { - Ok(new_hints) => Some(new_hints), - Err(e) => { - log::error!( - "Failed to query inlays for buffer row range {chunk_range:?}, {e:#}" - ); - if let Some((for_version, chunks_fetched)) = - inlay_hints.hint_chunk_fetched.get_mut(&buffer_id) - { - if for_version == &query_version { - chunks_fetched.remove(&chunk_range); + .filter_map(|(chunk_range, hints_result)| { + let chunks_fetched = inlay_hints.hint_chunk_fetching.get_mut(&buffer_id); + match hints_result { + Ok(new_hints) => { + if new_hints.is_empty() { + if let Some((_, chunks_fetched)) = chunks_fetched { + chunks_fetched.remove(&chunk_range); + } } + Some(new_hints) + } + Err(e) => { + log::error!( + "Failed to query inlays for buffer row range {chunk_range:?}, {e:#}" + ); + if let Some((for_version, chunks_fetched)) = chunks_fetched { + if for_version == &query_version { + chunks_fetched.remove(&chunk_range); + } + } + None } - None } }) .flat_map(|hints| hints.into_values()) @@ -856,9 +851,10 @@ struct VisibleExcerpts { fn spawn_editor_hints_refresh( buffer_id: BufferId, invalidate_cache: InvalidationStrategy, - ignore_previous_fetches: bool, debounce: Option, buffer_excerpts: VisibleExcerpts, + known_chunks: Option<(Global, HashSet>)>, + applicable_chunks: Vec>, cx: &mut Context<'_, Editor>, ) -> Task<()> { cx.spawn(async move |editor, cx| { @@ -869,12 +865,7 @@ fn spawn_editor_hints_refresh( let query_version = buffer_excerpts.buffer_version.clone(); let Some(hint_tasks) = editor .update(cx, |editor, cx| { - editor.inlay_hints_for_buffer( - invalidate_cache, - ignore_previous_fetches, - buffer_excerpts, - cx, - ) + editor.inlay_hints_for_buffer(invalidate_cache, buffer_excerpts, known_chunks, cx) }) .ok() else { @@ -882,6 +873,19 @@ fn spawn_editor_hints_refresh( }; let hint_tasks = hint_tasks.unwrap_or_default(); if hint_tasks.is_empty() { + editor + .update(cx, |editor, _| { + if let Some((_, hint_chunk_fetching)) = editor + .inlay_hints + .as_mut() + .and_then(|inlay_hints| inlay_hints.hint_chunk_fetching.get_mut(&buffer_id)) + { + for applicable_chunks in &applicable_chunks { + hint_chunk_fetching.remove(applicable_chunks); + } + } + }) + .ok(); return; } let new_hints = join_all(hint_tasks).await; @@ -1102,7 +1106,10 @@ pub mod tests { editor .update(cx, |editor, _window, cx| { editor.refresh_inlay_hints( - InlayHintRefreshReason::RefreshRequested(fake_server.server.server_id()), + InlayHintRefreshReason::RefreshRequested { + server_id: fake_server.server.server_id(), + request_id: Some(1), + }, cx, ); }) @@ -1958,15 +1965,8 @@ pub mod tests { async fn test_large_buffer_inlay_requests_split(cx: &mut gpui::TestAppContext) { init_test(cx, |settings| { settings.defaults.inlay_hints = Some(InlayHintSettingsContent { - show_value_hints: Some(true), enabled: Some(true), - edit_debounce_ms: Some(0), - scroll_debounce_ms: Some(0), - show_type_hints: Some(true), - show_parameter_hints: Some(true), - show_other_hints: Some(true), - show_background: Some(false), - toggle_on_modifiers_press: None, + ..InlayHintSettingsContent::default() }) }); @@ -2044,6 +2044,7 @@ pub mod tests { cx.add_window(|window, cx| Editor::for_buffer(buffer, Some(project), window, cx)); cx.executor().run_until_parked(); let _fake_server = fake_servers.next().await.unwrap(); + cx.executor().advance_clock(Duration::from_millis(100)); cx.executor().run_until_parked(); let ranges = lsp_request_ranges @@ -2129,6 +2130,7 @@ pub mod tests { ); }) .unwrap(); + cx.executor().advance_clock(Duration::from_millis(100)); cx.executor().run_until_parked(); editor.update(cx, |_, _, _| { let ranges = lsp_request_ranges @@ -2145,6 +2147,7 @@ pub mod tests { editor.handle_input("++++more text++++", window, cx); }) .unwrap(); + cx.executor().advance_clock(Duration::from_secs(1)); cx.executor().run_until_parked(); editor.update(cx, |editor, _window, cx| { let mut ranges = lsp_request_ranges.lock().drain(..).collect::>(); @@ -3887,7 +3890,10 @@ let c = 3;"# editor .update(cx, |editor, _, cx| { editor.refresh_inlay_hints( - InlayHintRefreshReason::RefreshRequested(fake_server.server.server_id()), + InlayHintRefreshReason::RefreshRequested { + server_id: fake_server.server.server_id(), + request_id: Some(1), + }, cx, ); }) @@ -4022,7 +4028,7 @@ let c = 3;"# let mut all_fetched_hints = Vec::new(); for buffer in editor.buffer.read(cx).all_buffers() { lsp_store.update(cx, |lsp_store, cx| { - let hints = &lsp_store.latest_lsp_data(&buffer, cx).inlay_hints(); + let hints = lsp_store.latest_lsp_data(&buffer, cx).inlay_hints(); all_cached_labels.extend(hints.all_cached_hints().into_iter().map(|hint| { let mut label = hint.text().to_string(); if hint.padding_left { diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 3263395f40dba231d4ec1ff49043951ab7e9b94d..25c629464ad68f04fbe9f9faa0ae1b8a9f7ff1ed 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -804,23 +804,32 @@ impl LocalLspStore { language_server .on_request::({ let lsp_store = lsp_store.clone(); + let request_id = Arc::new(AtomicUsize::new(0)); move |(), cx| { - let this = lsp_store.clone(); + let lsp_store = lsp_store.clone(); + let request_id = request_id.clone(); let mut cx = cx.clone(); async move { - this.update(&mut cx, |lsp_store, cx| { - cx.emit(LspStoreEvent::RefreshInlayHints(server_id)); - lsp_store - .downstream_client - .as_ref() - .map(|(client, project_id)| { - client.send(proto::RefreshInlayHints { - project_id: *project_id, - server_id: server_id.to_proto(), + lsp_store + .update(&mut cx, |lsp_store, cx| { + let request_id = + Some(request_id.fetch_add(1, atomic::Ordering::AcqRel)); + cx.emit(LspStoreEvent::RefreshInlayHints { + server_id, + request_id, + }); + lsp_store + .downstream_client + .as_ref() + .map(|(client, project_id)| { + client.send(proto::RefreshInlayHints { + project_id: *project_id, + server_id: server_id.to_proto(), + request_id: request_id.map(|id| id as u64), + }) }) - }) - })? - .transpose()?; + })? + .transpose()?; Ok(()) } } @@ -3610,7 +3619,10 @@ pub enum LspStoreEvent { new_language: Option>, }, Notification(String), - RefreshInlayHints(LanguageServerId), + RefreshInlayHints { + server_id: LanguageServerId, + request_id: Option, + }, RefreshCodeLens, DiagnosticsUpdated { server_id: LanguageServerId, @@ -6583,14 +6595,22 @@ impl LspStore { cx: &mut Context, ) -> HashMap, Task>> { let buffer_snapshot = buffer.read(cx).snapshot(); - let for_server = if let InvalidationStrategy::RefreshRequested(server_id) = invalidate { + let next_hint_id = self.next_hint_id.clone(); + let lsp_data = self.latest_lsp_data(&buffer, cx); + let mut lsp_refresh_requested = false; + let for_server = if let InvalidationStrategy::RefreshRequested { + server_id, + request_id, + } = invalidate + { + let invalidated = lsp_data + .inlay_hints + .invalidate_for_server_refresh(server_id, request_id); + lsp_refresh_requested = invalidated; Some(server_id) } else { None }; - let invalidate_cache = invalidate.should_invalidate(); - let next_hint_id = self.next_hint_id.clone(); - let lsp_data = self.latest_lsp_data(&buffer, cx); let existing_inlay_hints = &mut lsp_data.inlay_hints; let known_chunks = known_chunks .filter(|(known_version, _)| !lsp_data.buffer_version.changed_since(known_version)) @@ -6598,8 +6618,8 @@ impl LspStore { .unwrap_or_default(); let mut hint_fetch_tasks = Vec::new(); - let mut cached_inlay_hints = HashMap::default(); - let mut ranges_to_query = Vec::new(); + let mut cached_inlay_hints = None; + let mut ranges_to_query = None; let applicable_chunks = existing_inlay_hints .applicable_chunks(ranges.as_slice()) .filter(|chunk| !known_chunks.contains(&(chunk.start..chunk.end))) @@ -6614,12 +6634,12 @@ impl LspStore { match ( existing_inlay_hints .cached_hints(&row_chunk) - .filter(|_| !invalidate_cache) + .filter(|_| !lsp_refresh_requested) .cloned(), existing_inlay_hints .fetched_hints(&row_chunk) .as_ref() - .filter(|_| !invalidate_cache) + .filter(|_| !lsp_refresh_requested) .cloned(), ) { (None, None) => { @@ -6628,19 +6648,18 @@ impl LspStore { } else { Point::new(row_chunk.end, 0) }; - ranges_to_query.push(( + ranges_to_query.get_or_insert_with(Vec::new).push(( row_chunk, buffer_snapshot.anchor_before(Point::new(row_chunk.start, 0)) ..buffer_snapshot.anchor_after(end), )); } - (None, Some(fetched_hints)) => { - hint_fetch_tasks.push((row_chunk, fetched_hints.clone())) - } + (None, Some(fetched_hints)) => hint_fetch_tasks.push((row_chunk, fetched_hints)), (Some(cached_hints), None) => { for (server_id, cached_hints) in cached_hints { if for_server.is_none_or(|for_server| for_server == server_id) { cached_inlay_hints + .get_or_insert_with(HashMap::default) .entry(row_chunk.start..row_chunk.end) .or_insert_with(HashMap::default) .entry(server_id) @@ -6650,10 +6669,11 @@ impl LspStore { } } (Some(cached_hints), Some(fetched_hints)) => { - hint_fetch_tasks.push((row_chunk, fetched_hints.clone())); + hint_fetch_tasks.push((row_chunk, fetched_hints)); for (server_id, cached_hints) in cached_hints { if for_server.is_none_or(|for_server| for_server == server_id) { cached_inlay_hints + .get_or_insert_with(HashMap::default) .entry(row_chunk.start..row_chunk.end) .or_insert_with(HashMap::default) .entry(server_id) @@ -6665,18 +6685,18 @@ impl LspStore { } } - let cached_chunk_data = cached_inlay_hints - .into_iter() - .map(|(row_chunk, hints)| (row_chunk, Task::ready(Ok(hints)))) - .collect(); - if hint_fetch_tasks.is_empty() && ranges_to_query.is_empty() { - cached_chunk_data + if hint_fetch_tasks.is_empty() + && ranges_to_query + .as_ref() + .is_none_or(|ranges| ranges.is_empty()) + && let Some(cached_inlay_hints) = cached_inlay_hints + { + cached_inlay_hints + .into_iter() + .map(|(row_chunk, hints)| (row_chunk, Task::ready(Ok(hints)))) + .collect() } else { - if invalidate_cache { - lsp_data.inlay_hints.clear(); - } - - for (chunk, range_to_query) in ranges_to_query { + for (chunk, range_to_query) in ranges_to_query.into_iter().flatten() { let next_hint_id = next_hint_id.clone(); let buffer = buffer.clone(); let new_inlay_hints = cx @@ -6692,31 +6712,38 @@ impl LspStore { let update_cache = !lsp_data .buffer_version .changed_since(&buffer.read(cx).version()); - new_hints_by_server - .into_iter() - .map(|(server_id, new_hints)| { - let new_hints = new_hints - .into_iter() - .map(|new_hint| { - ( - InlayId::Hint(next_hint_id.fetch_add( - 1, - atomic::Ordering::AcqRel, - )), - new_hint, - ) - }) - .collect::>(); - if update_cache { - lsp_data.inlay_hints.insert_new_hints( - chunk, - server_id, - new_hints.clone(), - ); - } - (server_id, new_hints) - }) - .collect() + if new_hints_by_server.is_empty() { + if update_cache { + lsp_data.inlay_hints.invalidate_for_chunk(chunk); + } + HashMap::default() + } else { + new_hints_by_server + .into_iter() + .map(|(server_id, new_hints)| { + let new_hints = new_hints + .into_iter() + .map(|new_hint| { + ( + InlayId::Hint(next_hint_id.fetch_add( + 1, + atomic::Ordering::AcqRel, + )), + new_hint, + ) + }) + .collect::>(); + if update_cache { + lsp_data.inlay_hints.insert_new_hints( + chunk, + server_id, + new_hints.clone(), + ); + } + (server_id, new_hints) + }) + .collect() + } }) }) .map_err(Arc::new) @@ -6728,22 +6755,25 @@ impl LspStore { hint_fetch_tasks.push((chunk, new_inlay_hints)); } - let mut combined_data = cached_chunk_data; - combined_data.extend(hint_fetch_tasks.into_iter().map(|(chunk, hints_fetch)| { - ( - chunk.start..chunk.end, - cx.spawn(async move |_, _| { - hints_fetch.await.map_err(|e| { - if e.error_code() != ErrorCode::Internal { - anyhow!(e.error_code()) - } else { - anyhow!("{e:#}") - } - }) - }), - ) - })); - combined_data + cached_inlay_hints + .unwrap_or_default() + .into_iter() + .map(|(row_chunk, hints)| (row_chunk, Task::ready(Ok(hints)))) + .chain(hint_fetch_tasks.into_iter().map(|(chunk, hints_fetch)| { + ( + chunk.start..chunk.end, + cx.spawn(async move |_, _| { + hints_fetch.await.map_err(|e| { + if e.error_code() != ErrorCode::Internal { + anyhow!(e.error_code()) + } else { + anyhow!("{e:#}") + } + }) + }), + ) + })) + .collect() } } @@ -9542,7 +9572,10 @@ impl LspStore { if let Some(work) = status.pending_work.remove(&token) && !work.is_disk_based_diagnostics_progress { - cx.emit(LspStoreEvent::RefreshInlayHints(language_server_id)); + cx.emit(LspStoreEvent::RefreshInlayHints { + server_id: language_server_id, + request_id: None, + }); } cx.notify(); } @@ -9679,9 +9712,10 @@ impl LspStore { mut cx: AsyncApp, ) -> Result { lsp_store.update(&mut cx, |_, cx| { - cx.emit(LspStoreEvent::RefreshInlayHints( - LanguageServerId::from_proto(envelope.payload.server_id), - )); + cx.emit(LspStoreEvent::RefreshInlayHints { + server_id: LanguageServerId::from_proto(envelope.payload.server_id), + request_id: envelope.payload.request_id.map(|id| id as usize), + }); })?; Ok(proto::Ack {}) } @@ -10900,7 +10934,6 @@ impl LspStore { language_server.name(), Some(key.worktree_id), )); - cx.emit(LspStoreEvent::RefreshInlayHints(server_id)); let server_capabilities = language_server.capabilities(); if let Some((downstream_client, project_id)) = self.downstream_client.as_ref() { diff --git a/crates/project/src/lsp_store/inlay_hint_cache.rs b/crates/project/src/lsp_store/inlay_hint_cache.rs index 7d3ec27e5af83c4d83b269c171943d90754bd1a6..51189d8fdae788c7c12546f2c9ac1735930c3095 100644 --- a/crates/project/src/lsp_store/inlay_hint_cache.rs +++ b/crates/project/src/lsp_store/inlay_hint_cache.rs @@ -19,7 +19,10 @@ pub enum InvalidationStrategy { /// Demands to re-query all inlay hints needed and invalidate all cached entries, but does not require instant update with invalidation. /// /// Despite nothing forbids language server from sending this request on every edit, it is expected to be sent only when certain internal server state update, invisible for the editor otherwise. - RefreshRequested(LanguageServerId), + RefreshRequested { + server_id: LanguageServerId, + request_id: Option, + }, /// Multibuffer excerpt(s) and/or singleton buffer(s) were edited at least on one place. /// Neither editor nor LSP is able to tell which open file hints' are not affected, so all of them have to be invalidated, re-queried and do that fast enough to avoid being slow, but also debounce to avoid loading hints on every fast keystroke sequence. BufferEdited, @@ -36,7 +39,7 @@ impl InvalidationStrategy { pub fn should_invalidate(&self) -> bool { matches!( self, - InvalidationStrategy::RefreshRequested(_) | InvalidationStrategy::BufferEdited + InvalidationStrategy::RefreshRequested { .. } | InvalidationStrategy::BufferEdited ) } } @@ -47,6 +50,7 @@ pub struct BufferInlayHints { hints_by_chunks: Vec>, fetches_by_chunks: Vec>, hints_by_id: HashMap, + latest_invalidation_requests: HashMap>, pub(super) hint_resolves: HashMap>>, } @@ -104,6 +108,7 @@ impl BufferInlayHints { Self { hints_by_chunks: vec![None; buffer_chunks.len()], fetches_by_chunks: vec![None; buffer_chunks.len()], + latest_invalidation_requests: HashMap::default(), hints_by_id: HashMap::default(), hint_resolves: HashMap::default(), snapshot, @@ -176,6 +181,7 @@ impl BufferInlayHints { self.fetches_by_chunks = vec![None; self.buffer_chunks.len()]; self.hints_by_id.clear(); self.hint_resolves.clear(); + self.latest_invalidation_requests.clear(); } pub fn insert_new_hints( @@ -222,4 +228,48 @@ impl BufferInlayHints { pub fn buffer_chunks_len(&self) -> usize { self.buffer_chunks.len() } + + pub(crate) fn invalidate_for_server_refresh( + &mut self, + for_server: LanguageServerId, + request_id: Option, + ) -> bool { + match self.latest_invalidation_requests.entry(for_server) { + hash_map::Entry::Occupied(mut o) => { + if request_id > *o.get() { + o.insert(request_id); + } else { + return false; + } + } + hash_map::Entry::Vacant(v) => { + v.insert(request_id); + } + } + + for (chunk_id, chunk_data) in self.hints_by_chunks.iter_mut().enumerate() { + if let Some(removed_hints) = chunk_data + .as_mut() + .and_then(|chunk_data| chunk_data.remove(&for_server)) + { + for (id, _) in removed_hints { + self.hints_by_id.remove(&id); + self.hint_resolves.remove(&id); + } + self.fetches_by_chunks[chunk_id] = None; + } + } + + true + } + + pub(crate) fn invalidate_for_chunk(&mut self, chunk: BufferChunk) { + self.fetches_by_chunks[chunk.id] = None; + if let Some(hints_by_server) = self.hints_by_chunks[chunk.id].take() { + for (hint_id, _) in hints_by_server.into_values().flatten() { + self.hints_by_id.remove(&hint_id); + self.hint_resolves.remove(&hint_id); + } + } + } } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 910e217a67785249b4d83b7929b32c21b079a5d7..e228a1e353b275f400c45fe26d4ed6c94b825e96 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -337,7 +337,10 @@ pub enum Event { HostReshared, Reshared, Rejoined, - RefreshInlayHints(LanguageServerId), + RefreshInlayHints { + server_id: LanguageServerId, + request_id: Option, + }, RefreshCodeLens, RevealInProjectPanel(ProjectEntryId), SnippetEdit(BufferId, Vec<(lsp::Range, Snippet)>), @@ -3074,9 +3077,13 @@ impl Project { return; }; } - LspStoreEvent::RefreshInlayHints(server_id) => { - cx.emit(Event::RefreshInlayHints(*server_id)) - } + LspStoreEvent::RefreshInlayHints { + server_id, + request_id, + } => cx.emit(Event::RefreshInlayHints { + server_id: *server_id, + request_id: *request_id, + }), LspStoreEvent::RefreshCodeLens => cx.emit(Event::RefreshCodeLens), LspStoreEvent::LanguageServerPrompt(prompt) => { cx.emit(Event::LanguageServerPrompt(prompt.clone())) diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 1d4dbc6c86be9ba80e62c29ef32ce1161a6d1a25..6eab0136dc40bc8d52fcce29e2e94929ceb3d65d 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -1815,10 +1815,6 @@ async fn test_disk_based_diagnostics_progress(cx: &mut gpui::TestAppContext) { fake_server .start_progress(format!("{}/0", progress_token)) .await; - assert_eq!( - events.next().await.unwrap(), - Event::RefreshInlayHints(fake_server.server.server_id()) - ); assert_eq!( events.next().await.unwrap(), Event::DiskBasedDiagnosticsStarted { @@ -1957,10 +1953,6 @@ async fn test_restarting_server_with_diagnostics_running(cx: &mut gpui::TestAppC Some(worktree_id) ) ); - assert_eq!( - events.next().await.unwrap(), - Event::RefreshInlayHints(fake_server.server.server_id()) - ); fake_server.start_progress(progress_token).await; assert_eq!( events.next().await.unwrap(), diff --git a/crates/proto/proto/lsp.proto b/crates/proto/proto/lsp.proto index 7e446a915febbc03f2dd5920faf12a58a5d9b639..94da1dc80a1b9da7da53927d51ba1f4fc26844a6 100644 --- a/crates/proto/proto/lsp.proto +++ b/crates/proto/proto/lsp.proto @@ -466,6 +466,7 @@ message ResolveInlayHintResponse { message RefreshInlayHints { uint64 project_id = 1; uint64 server_id = 2; + optional uint64 request_id = 3; } message CodeLens { diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index f407a0a4dbfd00b6515a392f18572c373499d2cc..593e916a2719b95735246d068fb28426eec2bdd3 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -2338,7 +2338,15 @@ pub fn perform_project_search( #[cfg(test)] pub mod tests { - use std::{ops::Deref as _, sync::Arc, time::Duration}; + use std::{ + ops::Deref as _, + path::PathBuf, + sync::{ + Arc, + atomic::{self, AtomicUsize}, + }, + time::Duration, + }; use super::*; use editor::{DisplayPoint, display_map::DisplayRow}; @@ -4239,6 +4247,8 @@ pub mod tests { ) .await; + let requests_count = Arc::new(AtomicUsize::new(0)); + let closure_requests_count = requests_count.clone(); let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await; let language_registry = project.read_with(cx, |project, _| project.languages().clone()); let language = rust_lang(); @@ -4250,21 +4260,26 @@ pub mod tests { inlay_hint_provider: Some(lsp::OneOf::Left(true)), ..lsp::ServerCapabilities::default() }, - initializer: Some(Box::new(|fake_server| { - fake_server.set_request_handler::( - move |_, _| async move { - Ok(Some(vec![lsp::InlayHint { - position: lsp::Position::new(0, 17), - label: lsp::InlayHintLabel::String(": i32".to_owned()), - kind: Some(lsp::InlayHintKind::TYPE), - text_edits: None, - tooltip: None, - padding_left: None, - padding_right: None, - data: None, - }])) - }, - ); + initializer: Some(Box::new(move |fake_server| { + let requests_count = closure_requests_count.clone(); + fake_server.set_request_handler::({ + move |_, _| { + let requests_count = requests_count.clone(); + async move { + requests_count.fetch_add(1, atomic::Ordering::Release); + Ok(Some(vec![lsp::InlayHint { + position: lsp::Position::new(0, 17), + label: lsp::InlayHintLabel::String(": i32".to_owned()), + kind: Some(lsp::InlayHintKind::TYPE), + text_edits: None, + tooltip: None, + padding_left: None, + padding_right: None, + data: None, + }])) + } + } + }); })), ..FakeLspAdapter::default() }, @@ -4278,7 +4293,7 @@ pub mod tests { }); perform_search(search_view, "let ", cx); - let _fake_server = fake_servers.next().await.unwrap(); + let fake_server = fake_servers.next().await.unwrap(); cx.executor().advance_clock(Duration::from_secs(1)); cx.executor().run_until_parked(); search_view @@ -4291,11 +4306,127 @@ pub mod tests { ); }) .unwrap(); + assert_eq!( + requests_count.load(atomic::Ordering::Acquire), + 1, + "New hints should have been queried", + ); // Can do the 2nd search without any panics perform_search(search_view, "let ", cx); + cx.executor().advance_clock(Duration::from_secs(1)); + cx.executor().run_until_parked(); + search_view + .update(cx, |search_view, _, cx| { + assert_eq!( + search_view + .results_editor + .update(cx, |editor, cx| editor.display_text(cx)), + "\n\nfn main() { let a: i32 = 2; }\n" + ); + }) + .unwrap(); + assert_eq!( + requests_count.load(atomic::Ordering::Acquire), + 2, + "We did drop the previous buffer when cleared the old project search results, hence another query was made", + ); + + let singleton_editor = window + .update(cx, |workspace, window, cx| { + workspace.open_abs_path( + PathBuf::from(path!("/dir/main.rs")), + workspace::OpenOptions::default(), + window, + cx, + ) + }) + .unwrap() + .await + .unwrap() + .downcast::() + .unwrap(); cx.executor().advance_clock(Duration::from_millis(100)); cx.executor().run_until_parked(); + singleton_editor.update(cx, |editor, cx| { + assert_eq!( + editor.display_text(cx), + "fn main() { let a: i32 = 2; }\n", + "Newly opened editor should have the correct text with hints", + ); + }); + assert_eq!( + requests_count.load(atomic::Ordering::Acquire), + 2, + "Opening the same buffer again should reuse the cached hints", + ); + + window + .update(cx, |_, window, cx| { + singleton_editor.update(cx, |editor, cx| { + editor.handle_input("test", window, cx); + }); + }) + .unwrap(); + + cx.executor().advance_clock(Duration::from_secs(1)); + cx.executor().run_until_parked(); + singleton_editor.update(cx, |editor, cx| { + assert_eq!( + editor.display_text(cx), + "testfn main() { l: i32et a = 2; }\n", + "Newly opened editor should have the correct text with hints", + ); + }); + assert_eq!( + requests_count.load(atomic::Ordering::Acquire), + 3, + "We have edited the buffer and should send a new request", + ); + + window + .update(cx, |_, window, cx| { + singleton_editor.update(cx, |editor, cx| { + editor.undo(&editor::actions::Undo, window, cx); + }); + }) + .unwrap(); + cx.executor().advance_clock(Duration::from_secs(1)); + cx.executor().run_until_parked(); + assert_eq!( + requests_count.load(atomic::Ordering::Acquire), + 4, + "We have edited the buffer again and should send a new request again", + ); + singleton_editor.update(cx, |editor, cx| { + assert_eq!( + editor.display_text(cx), + "fn main() { let a: i32 = 2; }\n", + "Newly opened editor should have the correct text with hints", + ); + }); + project.update(cx, |_, cx| { + cx.emit(project::Event::RefreshInlayHints { + server_id: fake_server.server.server_id(), + request_id: Some(1), + }); + }); + cx.executor().advance_clock(Duration::from_secs(1)); + cx.executor().run_until_parked(); + assert_eq!( + requests_count.load(atomic::Ordering::Acquire), + 5, + "After a simulated server refresh request, we should have sent another request", + ); + + perform_search(search_view, "let ", cx); + cx.executor().advance_clock(Duration::from_secs(1)); + cx.executor().run_until_parked(); + assert_eq!( + requests_count.load(atomic::Ordering::Acquire), + 5, + "New project search should reuse the cached hints", + ); search_view .update(cx, |search_view, _, cx| { assert_eq!(