diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index ce7fd8a094..657457d592 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -7953,7 +7953,8 @@ async fn test_mutual_editor_inlay_hint_cache_update( ); let inlay_cache = editor.inlay_hint_cache(); assert_eq!( - inlay_cache.version, edits_made, + inlay_cache.version(), + edits_made, "Host editor update the cache version after every cache/view change", ); }); @@ -7976,7 +7977,8 @@ async fn test_mutual_editor_inlay_hint_cache_update( ); let inlay_cache = editor.inlay_hint_cache(); assert_eq!( - inlay_cache.version, edits_made, + inlay_cache.version(), + edits_made, "Guest editor update the cache version after every cache/view change" ); }); @@ -7996,7 +7998,7 @@ async fn test_mutual_editor_inlay_hint_cache_update( "Host should get hints from the 1st edit and 1st LSP query" ); let inlay_cache = editor.inlay_hint_cache(); - assert_eq!(inlay_cache.version, edits_made); + assert_eq!(inlay_cache.version(), edits_made); }); editor_b.update(cx_b, |editor, _| { assert_eq!( @@ -8010,7 +8012,7 @@ async fn test_mutual_editor_inlay_hint_cache_update( "Guest should get hints the 1st edit and 2nd LSP query" ); let inlay_cache = editor.inlay_hint_cache(); - assert_eq!(inlay_cache.version, edits_made); + assert_eq!(inlay_cache.version(), edits_made); }); editor_a.update(cx_a, |editor, cx| { @@ -8035,7 +8037,7 @@ async fn test_mutual_editor_inlay_hint_cache_update( 4th query was made by guest (but not applied) due to cache invalidation logic" ); let inlay_cache = editor.inlay_hint_cache(); - assert_eq!(inlay_cache.version, edits_made); + assert_eq!(inlay_cache.version(), edits_made); }); editor_b.update(cx_b, |editor, _| { assert_eq!( @@ -8051,7 +8053,7 @@ async fn test_mutual_editor_inlay_hint_cache_update( "Guest should get hints from 3rd edit, 6th LSP query" ); let inlay_cache = editor.inlay_hint_cache(); - assert_eq!(inlay_cache.version, edits_made); + assert_eq!(inlay_cache.version(), edits_made); }); fake_language_server @@ -8077,7 +8079,8 @@ async fn test_mutual_editor_inlay_hint_cache_update( ); let inlay_cache = editor.inlay_hint_cache(); assert_eq!( - inlay_cache.version, edits_made, + inlay_cache.version(), + edits_made, "Host should accepted all edits and bump its cache version every time" ); }); @@ -8098,7 +8101,7 @@ async fn test_mutual_editor_inlay_hint_cache_update( ); let inlay_cache = editor.inlay_hint_cache(); assert_eq!( - inlay_cache.version, + inlay_cache.version(), edits_made, "Guest should accepted all edits and bump its cache version every time" ); @@ -8264,7 +8267,8 @@ async fn test_inlay_hint_refresh_is_forwarded( ); let inlay_cache = editor.inlay_hint_cache(); assert_eq!( - inlay_cache.version, 0, + inlay_cache.version(), + 0, "Host should not increment its cache version due to no changes", ); }); @@ -8279,7 +8283,8 @@ async fn test_inlay_hint_refresh_is_forwarded( ); let inlay_cache = editor.inlay_hint_cache(); assert_eq!( - inlay_cache.version, edits_made, + inlay_cache.version(), + edits_made, "Guest editor update the cache version after every cache/view change" ); }); @@ -8296,7 +8301,8 @@ async fn test_inlay_hint_refresh_is_forwarded( ); let inlay_cache = editor.inlay_hint_cache(); assert_eq!( - inlay_cache.version, 0, + inlay_cache.version(), + 0, "Host should not increment its cache version due to no changes", ); }); @@ -8311,7 +8317,8 @@ async fn test_inlay_hint_refresh_is_forwarded( ); let inlay_cache = editor.inlay_hint_cache(); assert_eq!( - inlay_cache.version, edits_made, + inlay_cache.version(), + edits_made, "Guest should accepted all edits and bump its cache version every time" ); }); @@ -8343,13 +8350,10 @@ fn room_participants(room: &ModelHandle, cx: &mut TestAppContext) -> RoomP fn extract_hint_labels(editor: &Editor) -> Vec { let mut labels = Vec::new(); - for (_, excerpt_hints) in &editor.inlay_hint_cache().hints { - let excerpt_hints = excerpt_hints.read(); - for (_, inlay) in excerpt_hints.hints.iter() { - match &inlay.label { - project::InlayHintLabel::String(s) => labels.push(s.to_string()), - _ => unreachable!(), - } + for hint in editor.inlay_hint_cache().hints() { + match hint.label { + project::InlayHintLabel::String(s) => labels.push(s), + _ => unreachable!(), } } labels diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 8d8b77ea95..f65f19cfec 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2723,7 +2723,7 @@ impl Editor { .collect() } - fn excerpt_visible_offsets( + pub fn excerpt_visible_offsets( &self, restrict_to_languages: Option<&HashSet>>, cx: &mut ViewContext<'_, '_, Editor>, diff --git a/crates/editor/src/inlay_hint_cache.rs b/crates/editor/src/inlay_hint_cache.rs index 2d75b4d2ce..8be72aec46 100644 --- a/crates/editor/src/inlay_hint_cache.rs +++ b/crates/editor/src/inlay_hint_cache.rs @@ -9,7 +9,7 @@ use crate::{ }; use anyhow::Context; use clock::Global; -use gpui::{ModelHandle, Task, ViewContext}; +use gpui::{ModelContext, ModelHandle, Task, ViewContext}; use language::{language_settings::InlayHintKind, Buffer, BufferSnapshot}; use log::error; use parking_lot::RwLock; @@ -17,14 +17,21 @@ use project::InlayHint; use collections::{hash_map, HashMap, HashSet}; use language::language_settings::InlayHintSettings; +use sum_tree::Bias; use util::post_inc; pub struct InlayHintCache { - pub hints: HashMap>>, - pub allowed_hint_kinds: HashSet>, - pub version: usize, - pub enabled: bool, - update_tasks: HashMap, + hints: HashMap>>, + allowed_hint_kinds: HashSet>, + version: usize, + enabled: bool, + update_tasks: HashMap, +} + +#[derive(Debug)] +struct TasksForRanges { + tasks: Vec>, + sorted_ranges: Vec>, } #[derive(Debug)] @@ -32,7 +39,7 @@ pub struct CachedExcerptHints { version: usize, buffer_version: Global, buffer_id: u64, - pub hints: Vec<(InlayId, InlayHint)>, + hints: Vec<(InlayId, InlayHint)>, } #[derive(Debug, Clone, Copy)] @@ -48,18 +55,6 @@ pub struct InlaySplice { pub to_insert: Vec, } -struct UpdateTask { - invalidate: InvalidationStrategy, - cache_version: usize, - task: RunningTask, - pending_refresh: Option, -} - -struct RunningTask { - _task: Task<()>, - is_running_rx: smol::channel::Receiver<()>, -} - #[derive(Debug)] struct ExcerptHintsUpdate { excerpt_id: ExcerptId, @@ -72,24 +67,10 @@ struct ExcerptHintsUpdate { struct ExcerptQuery { buffer_id: u64, excerpt_id: ExcerptId, - dimensions: ExcerptDimensions, cache_version: usize, invalidate: InvalidationStrategy, } -#[derive(Debug, Clone, Copy)] -struct ExcerptDimensions { - excerpt_range_start: language::Anchor, - excerpt_range_end: language::Anchor, - excerpt_visible_range_start: language::Anchor, - excerpt_visible_range_end: language::Anchor, -} - -struct HintFetchRanges { - visible_range: Range, - other_ranges: Vec>, -} - impl InvalidationStrategy { fn should_invalidate(&self) -> bool { matches!( @@ -99,35 +80,92 @@ impl InvalidationStrategy { } } -impl ExcerptQuery { - fn hints_fetch_ranges(&self, buffer: &BufferSnapshot) -> HintFetchRanges { - let visible_range = - self.dimensions.excerpt_visible_range_start..self.dimensions.excerpt_visible_range_end; - let mut other_ranges = Vec::new(); - if self - .dimensions - .excerpt_range_start - .cmp(&visible_range.start, buffer) - .is_lt() - { - let mut end = visible_range.start; - end.offset -= 1; - other_ranges.push(self.dimensions.excerpt_range_start..end); - } - if self - .dimensions - .excerpt_range_end - .cmp(&visible_range.end, buffer) - .is_gt() - { - let mut start = visible_range.end; - start.offset += 1; - other_ranges.push(start..self.dimensions.excerpt_range_end); +impl TasksForRanges { + fn new(sorted_ranges: Vec>, task: Task<()>) -> Self { + Self { + tasks: vec![task], + sorted_ranges, } + } - HintFetchRanges { - visible_range, - other_ranges: other_ranges.into_iter().map(|range| range).collect(), + fn update_cached_tasks( + &mut self, + buffer_snapshot: &BufferSnapshot, + query_range: Range, + invalidate: InvalidationStrategy, + spawn_task: impl FnOnce(Vec>) -> Task<()>, + ) { + let ranges_to_query = match invalidate { + InvalidationStrategy::None => { + let mut ranges_to_query = Vec::new(); + let mut latest_cached_range = None::<&mut Range>; + for cached_range in self + .sorted_ranges + .iter_mut() + .skip_while(|cached_range| { + cached_range + .end + .cmp(&query_range.start, buffer_snapshot) + .is_lt() + }) + .take_while(|cached_range| { + cached_range + .start + .cmp(&query_range.end, buffer_snapshot) + .is_le() + }) + { + match latest_cached_range { + Some(latest_cached_range) => { + if latest_cached_range.end.offset.saturating_add(1) + < cached_range.start.offset + { + ranges_to_query.push(latest_cached_range.end..cached_range.start); + cached_range.start = latest_cached_range.end; + } + } + None => { + if query_range + .start + .cmp(&cached_range.start, buffer_snapshot) + .is_lt() + { + ranges_to_query.push(query_range.start..cached_range.start); + cached_range.start = query_range.start; + } + } + } + latest_cached_range = Some(cached_range); + } + + match latest_cached_range { + Some(latest_cached_range) => { + if latest_cached_range.end.offset.saturating_add(1) < query_range.end.offset + { + ranges_to_query.push(latest_cached_range.end..query_range.end); + latest_cached_range.end = query_range.end; + } + } + None => { + ranges_to_query.push(query_range.clone()); + self.sorted_ranges.push(query_range); + self.sorted_ranges.sort_by(|range_a, range_b| { + range_a.start.cmp(&range_b.start, buffer_snapshot) + }); + } + } + + ranges_to_query + } + InvalidationStrategy::RefreshRequested | InvalidationStrategy::BufferEdited => { + self.tasks.clear(); + self.sorted_ranges.clear(); + vec![query_range] + } + }; + + if !ranges_to_query.is_empty() { + self.tasks.push(spawn_task(ranges_to_query)); } } } @@ -168,7 +206,6 @@ impl InlayHintCache { ); if new_splice.is_some() { self.version += 1; - self.update_tasks.clear(); self.allowed_hint_kinds = new_allowed_hint_kinds; } ControlFlow::Break(new_splice) @@ -197,7 +234,7 @@ impl InlayHintCache { pub fn spawn_hint_refresh( &mut self, - mut excerpts_to_query: HashMap, Global, Range)>, + excerpts_to_query: HashMap, Global, Range)>, invalidate: InvalidationStrategy, cx: &mut ViewContext, ) -> Option { @@ -205,43 +242,23 @@ impl InlayHintCache { return None; } - let update_tasks = &mut self.update_tasks; let mut invalidated_hints = Vec::new(); if invalidate.should_invalidate() { - let mut changed = false; - update_tasks.retain(|task_excerpt_id, _| { - let retain = excerpts_to_query.contains_key(task_excerpt_id); - changed |= !retain; - retain - }); + self.update_tasks + .retain(|task_excerpt_id, _| excerpts_to_query.contains_key(task_excerpt_id)); self.hints.retain(|cached_excerpt, cached_hints| { let retain = excerpts_to_query.contains_key(cached_excerpt); - changed |= !retain; if !retain { invalidated_hints.extend(cached_hints.read().hints.iter().map(|&(id, _)| id)); } retain }); - if changed { - self.version += 1; - } } if excerpts_to_query.is_empty() && invalidated_hints.is_empty() { return None; } - let cache_version = self.version; - excerpts_to_query.retain(|visible_excerpt_id, _| { - match update_tasks.entry(*visible_excerpt_id) { - hash_map::Entry::Occupied(o) => match o.get().cache_version.cmp(&cache_version) { - cmp::Ordering::Less => true, - cmp::Ordering::Equal => invalidate.should_invalidate(), - cmp::Ordering::Greater => false, - }, - hash_map::Entry::Vacant(_) => true, - } - }); - + let cache_version = self.version + 1; cx.spawn(|editor, mut cx| async move { editor .update(&mut cx, |editor, cx| { @@ -368,6 +385,19 @@ impl InlayHintCache { self.update_tasks.clear(); self.hints.clear(); } + + pub fn hints(&self) -> Vec { + let mut hints = Vec::new(); + for excerpt_hints in self.hints.values() { + let excerpt_hints = excerpt_hints.read(); + hints.extend(excerpt_hints.hints.iter().map(|(_, hint)| hint).cloned()); + } + hints + } + + pub fn version(&self) -> usize { + self.version + } } fn spawn_new_update_tasks( @@ -378,13 +408,14 @@ fn spawn_new_update_tasks( cx: &mut ViewContext<'_, '_, Editor>, ) { let visible_hints = Arc::new(editor.visible_inlay_hints(cx)); - for (excerpt_id, (buffer_handle, new_task_buffer_version, excerpt_visible_range)) in + for (excerpt_id, (excerpt_buffer, new_task_buffer_version, excerpt_visible_range)) in excerpts_to_query { if excerpt_visible_range.is_empty() { continue; } - let buffer = buffer_handle.read(cx); + let buffer = excerpt_buffer.read(cx); + let buffer_id = buffer.remote_id(); let buffer_snapshot = buffer.snapshot(); if buffer_snapshot .version() @@ -402,202 +433,123 @@ fn spawn_new_update_tasks( { continue; } - if !new_task_buffer_version.changed_since(&cached_buffer_version) - && !matches!(invalidate, InvalidationStrategy::RefreshRequested) - { - continue; - } }; - let buffer_id = buffer.remote_id(); - let excerpt_visible_range_start = buffer.anchor_before(excerpt_visible_range.start); - let excerpt_visible_range_end = buffer.anchor_after(excerpt_visible_range.end); - - let (multi_buffer_snapshot, full_excerpt_range) = + let (multi_buffer_snapshot, Some(query_range)) = editor.buffer.update(cx, |multi_buffer, cx| { - let multi_buffer_snapshot = multi_buffer.snapshot(cx); ( - multi_buffer_snapshot, - multi_buffer - .excerpts_for_buffer(&buffer_handle, cx) - .into_iter() - .find(|(id, _)| id == &excerpt_id) - .map(|(_, range)| range.context), + multi_buffer.snapshot(cx), + determine_query_range( + multi_buffer, + excerpt_id, + &excerpt_buffer, + excerpt_visible_range, + cx, + ), ) - }); + }) else { return; }; + let query = ExcerptQuery { + buffer_id, + excerpt_id, + cache_version: update_cache_version, + invalidate, + }; - if let Some(full_excerpt_range) = full_excerpt_range { - let query = ExcerptQuery { - buffer_id, - excerpt_id, - dimensions: ExcerptDimensions { - excerpt_range_start: full_excerpt_range.start, - excerpt_range_end: full_excerpt_range.end, - excerpt_visible_range_start, - excerpt_visible_range_end, - }, - cache_version: update_cache_version, - invalidate, - }; + let new_update_task = |fetch_ranges| { + new_update_task( + query, + fetch_ranges, + multi_buffer_snapshot, + buffer_snapshot.clone(), + Arc::clone(&visible_hints), + cached_excerpt_hints, + cx, + ) + }; - let new_update_task = |is_refresh_after_regular_task| { - new_update_task( - query, - multi_buffer_snapshot, - buffer_snapshot, - Arc::clone(&visible_hints), - cached_excerpt_hints, - is_refresh_after_regular_task, - cx, - ) - }; - match editor.inlay_hint_cache.update_tasks.entry(excerpt_id) { - hash_map::Entry::Occupied(mut o) => { - let update_task = o.get_mut(); - match (update_task.invalidate, invalidate) { - (_, InvalidationStrategy::None) => {} - ( - InvalidationStrategy::BufferEdited, - InvalidationStrategy::RefreshRequested, - ) if !update_task.task.is_running_rx.is_closed() => { - update_task.pending_refresh = Some(query); - } - _ => { - o.insert(UpdateTask { - invalidate, - cache_version: query.cache_version, - task: new_update_task(false), - pending_refresh: None, - }); - } - } - } - hash_map::Entry::Vacant(v) => { - v.insert(UpdateTask { - invalidate, - cache_version: query.cache_version, - task: new_update_task(false), - pending_refresh: None, - }); - } + match editor.inlay_hint_cache.update_tasks.entry(excerpt_id) { + hash_map::Entry::Occupied(mut o) => { + o.get_mut().update_cached_tasks( + &buffer_snapshot, + query_range, + invalidate, + new_update_task, + ); + } + hash_map::Entry::Vacant(v) => { + v.insert(TasksForRanges::new( + vec![query_range.clone()], + new_update_task(vec![query_range]), + )); } } } } +fn determine_query_range( + multi_buffer: &mut MultiBuffer, + excerpt_id: ExcerptId, + excerpt_buffer: &ModelHandle, + excerpt_visible_range: Range, + cx: &mut ModelContext<'_, MultiBuffer>, +) -> Option> { + let full_excerpt_range = multi_buffer + .excerpts_for_buffer(excerpt_buffer, cx) + .into_iter() + .find(|(id, _)| id == &excerpt_id) + .map(|(_, range)| range.context)?; + + let buffer = excerpt_buffer.read(cx); + let excerpt_visible_len = excerpt_visible_range.end - excerpt_visible_range.start; + let start_offset = excerpt_visible_range + .start + .saturating_sub(excerpt_visible_len) + .max(full_excerpt_range.start.offset); + let start = buffer.anchor_before(buffer.clip_offset(start_offset, Bias::Left)); + let end_offset = excerpt_visible_range + .end + .saturating_add(excerpt_visible_len) + .min(full_excerpt_range.end.offset) + .min(buffer.len()); + let end = buffer.anchor_after(buffer.clip_offset(end_offset, Bias::Right)); + if start.cmp(&end, buffer).is_eq() { + None + } else { + Some(start..end) + } +} + fn new_update_task( query: ExcerptQuery, + hint_fetch_ranges: Vec>, multi_buffer_snapshot: MultiBufferSnapshot, buffer_snapshot: BufferSnapshot, visible_hints: Arc>, cached_excerpt_hints: Option>>, - is_refresh_after_regular_task: bool, cx: &mut ViewContext<'_, '_, Editor>, -) -> RunningTask { - let hints_fetch_ranges = query.hints_fetch_ranges(&buffer_snapshot); - let (is_running_tx, is_running_rx) = smol::channel::bounded(1); - let _task = cx.spawn(|editor, mut cx| async move { - let _is_running_tx = is_running_tx; - let create_update_task = |range| { - fetch_and_update_hints( - editor.clone(), - multi_buffer_snapshot.clone(), - buffer_snapshot.clone(), - Arc::clone(&visible_hints), - cached_excerpt_hints.as_ref().map(Arc::clone), - query, - range, - cx.clone(), - ) - }; - - if is_refresh_after_regular_task { - let visible_range_has_updates = - match create_update_task(hints_fetch_ranges.visible_range).await { - Ok(updated) => updated, - Err(e) => { - error!("inlay hint visible range update task failed: {e:#}"); - return; - } - }; - - if visible_range_has_updates { - let other_update_results = futures::future::join_all( - hints_fetch_ranges - .other_ranges - .into_iter() - .map(create_update_task), +) -> Task<()> { + cx.spawn(|editor, cx| async move { + let task_update_results = + futures::future::join_all(hint_fetch_ranges.into_iter().map(|range| { + fetch_and_update_hints( + editor.clone(), + multi_buffer_snapshot.clone(), + buffer_snapshot.clone(), + Arc::clone(&visible_hints), + cached_excerpt_hints.as_ref().map(Arc::clone), + query, + range, + cx.clone(), ) - .await; - - for result in other_update_results { - if let Err(e) = result { - error!("inlay hint update task failed: {e:#}"); - } - } - } - } else { - let task_update_results = futures::future::join_all( - std::iter::once(hints_fetch_ranges.visible_range) - .chain(hints_fetch_ranges.other_ranges.into_iter()) - .map(create_update_task), - ) + })) .await; - for result in task_update_results { - if let Err(e) = result { - error!("inlay hint update task failed: {e:#}"); - } + for result in task_update_results { + if let Err(e) = result { + error!("inlay hint update task failed: {e:#}"); } } - - editor - .update(&mut cx, |editor, cx| { - let pending_refresh_query = editor - .inlay_hint_cache - .update_tasks - .get_mut(&query.excerpt_id) - .and_then(|task| task.pending_refresh.take()); - - if let Some(pending_refresh_query) = pending_refresh_query { - let refresh_multi_buffer = editor.buffer().read(cx); - let refresh_multi_buffer_snapshot = refresh_multi_buffer.snapshot(cx); - let refresh_visible_hints = Arc::new(editor.visible_inlay_hints(cx)); - let refresh_cached_excerpt_hints = editor - .inlay_hint_cache - .hints - .get(&pending_refresh_query.excerpt_id) - .map(Arc::clone); - if let Some(buffer) = - refresh_multi_buffer.buffer(pending_refresh_query.buffer_id) - { - editor.inlay_hint_cache.update_tasks.insert( - pending_refresh_query.excerpt_id, - UpdateTask { - invalidate: InvalidationStrategy::RefreshRequested, - cache_version: editor.inlay_hint_cache.version, - task: new_update_task( - pending_refresh_query, - refresh_multi_buffer_snapshot, - buffer.read(cx).snapshot(), - refresh_visible_hints, - refresh_cached_excerpt_hints, - true, - cx, - ), - pending_refresh: None, - }, - ); - } - } - }) - .ok(); - }); - - RunningTask { - _task, - is_running_rx, - } + }) } async fn fetch_and_update_hints( @@ -609,7 +561,7 @@ async fn fetch_and_update_hints( query: ExcerptQuery, fetch_range: Range, mut cx: gpui::AsyncAppContext, -) -> anyhow::Result { +) -> anyhow::Result<()> { let inlay_hints_fetch_task = editor .update(&mut cx, |editor, cx| { editor @@ -625,11 +577,10 @@ async fn fetch_and_update_hints( }) .ok() .flatten(); - let mut update_happened = false; - let Some(inlay_hints_fetch_task) = inlay_hints_fetch_task else { return Ok(update_happened) }; - let new_hints = inlay_hints_fetch_task - .await - .context("inlay hint fetch task")?; + let new_hints = match inlay_hints_fetch_task { + Some(task) => task.await.context("inlay hint fetch task")?, + None => return Ok(()), + }; let background_task_buffer_snapshot = buffer_snapshot.clone(); let backround_fetch_range = fetch_range.clone(); let new_update = cx @@ -645,106 +596,21 @@ async fn fetch_and_update_hints( ) }) .await; - - editor - .update(&mut cx, |editor, cx| { - if let Some(new_update) = new_update { - update_happened = !new_update.add_to_cache.is_empty() - || !new_update.remove_from_cache.is_empty() - || !new_update.remove_from_visible.is_empty(); - - let cached_excerpt_hints = editor - .inlay_hint_cache - .hints - .entry(new_update.excerpt_id) - .or_insert_with(|| { - Arc::new(RwLock::new(CachedExcerptHints { - version: query.cache_version, - buffer_version: buffer_snapshot.version().clone(), - buffer_id: query.buffer_id, - hints: Vec::new(), - })) - }); - let mut cached_excerpt_hints = cached_excerpt_hints.write(); - match query.cache_version.cmp(&cached_excerpt_hints.version) { - cmp::Ordering::Less => return, - cmp::Ordering::Greater | cmp::Ordering::Equal => { - cached_excerpt_hints.version = query.cache_version; - } - } - cached_excerpt_hints - .hints - .retain(|(hint_id, _)| !new_update.remove_from_cache.contains(hint_id)); - cached_excerpt_hints.buffer_version = buffer_snapshot.version().clone(); - editor.inlay_hint_cache.version += 1; - - let mut splice = InlaySplice { - to_remove: new_update.remove_from_visible, - to_insert: Vec::new(), - }; - - 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 = post_inc(&mut editor.next_inlay_id); - if editor - .inlay_hint_cache - .allowed_hint_kinds - .contains(&new_hint.kind) - { - splice.to_insert.push(Inlay::hint( - new_inlay_id, - new_hint_position, - &new_hint, - )); - } - - cached_excerpt_hints - .hints - .push((InlayId::Hint(new_inlay_id), new_hint)); - } - - cached_excerpt_hints - .hints - .sort_by(|(_, hint_a), (_, hint_b)| { - hint_a.position.cmp(&hint_b.position, &buffer_snapshot) - }); - drop(cached_excerpt_hints); - - if query.invalidate.should_invalidate() { - let mut outdated_excerpt_caches = HashSet::default(); - for (excerpt_id, excerpt_hints) in &editor.inlay_hint_cache().hints { - let excerpt_hints = excerpt_hints.read(); - if excerpt_hints.buffer_id == query.buffer_id - && excerpt_id != &query.excerpt_id - && buffer_snapshot - .version() - .changed_since(&excerpt_hints.buffer_version) - { - outdated_excerpt_caches.insert(*excerpt_id); - splice - .to_remove - .extend(excerpt_hints.hints.iter().map(|(id, _)| id)); - } - } - editor - .inlay_hint_cache - .hints - .retain(|excerpt_id, _| !outdated_excerpt_caches.contains(excerpt_id)); - } - - let InlaySplice { - to_remove, - to_insert, - } = splice; - if !to_remove.is_empty() || !to_insert.is_empty() { - editor.splice_inlay_hints(to_remove, to_insert, cx) - } - } - }) - .ok(); - - Ok(update_happened) + if let Some(new_update) = new_update { + editor + .update(&mut cx, |editor, cx| { + apply_hint_update( + editor, + new_update, + query, + buffer_snapshot, + multi_buffer_snapshot, + cx, + ); + }) + .ok(); + } + Ok(()) } fn calculate_hint_updates( @@ -793,19 +659,6 @@ fn calculate_hint_updates( visible_hints .iter() .filter(|hint| hint.position.excerpt_id == query.excerpt_id) - .filter(|hint| { - contains_position(&fetch_range, hint.position.text_anchor, buffer_snapshot) - }) - .filter(|hint| { - fetch_range - .start - .cmp(&hint.position.text_anchor, buffer_snapshot) - .is_le() - && fetch_range - .end - .cmp(&hint.position.text_anchor, buffer_snapshot) - .is_ge() - }) .map(|inlay_hint| inlay_hint.id) .filter(|hint_id| !excerpt_hints_to_persist.contains_key(hint_id)), ); @@ -819,16 +672,6 @@ fn calculate_hint_updates( .filter(|(cached_inlay_id, _)| { !excerpt_hints_to_persist.contains_key(cached_inlay_id) }) - .filter(|(_, cached_hint)| { - fetch_range - .start - .cmp(&cached_hint.position, buffer_snapshot) - .is_le() - && fetch_range - .end - .cmp(&cached_hint.position, buffer_snapshot) - .is_ge() - }) .map(|(cached_inlay_id, _)| *cached_inlay_id), ); } @@ -855,6 +698,113 @@ fn contains_position( && range.end.cmp(&position, buffer_snapshot).is_ge() } +fn apply_hint_update( + editor: &mut Editor, + new_update: ExcerptHintsUpdate, + query: ExcerptQuery, + buffer_snapshot: BufferSnapshot, + multi_buffer_snapshot: MultiBufferSnapshot, + cx: &mut ViewContext<'_, '_, Editor>, +) { + let cached_excerpt_hints = editor + .inlay_hint_cache + .hints + .entry(new_update.excerpt_id) + .or_insert_with(|| { + Arc::new(RwLock::new(CachedExcerptHints { + version: query.cache_version, + buffer_version: buffer_snapshot.version().clone(), + buffer_id: query.buffer_id, + hints: Vec::new(), + })) + }); + let mut cached_excerpt_hints = cached_excerpt_hints.write(); + match query.cache_version.cmp(&cached_excerpt_hints.version) { + cmp::Ordering::Less => return, + cmp::Ordering::Greater | cmp::Ordering::Equal => { + cached_excerpt_hints.version = query.cache_version; + } + } + + let mut cached_inlays_changed = !new_update.remove_from_cache.is_empty(); + cached_excerpt_hints + .hints + .retain(|(hint_id, _)| !new_update.remove_from_cache.contains(hint_id)); + let mut splice = InlaySplice { + to_remove: new_update.remove_from_visible, + to_insert: Vec::new(), + }; + for new_hint in new_update.add_to_cache { + let cached_hints = &mut cached_excerpt_hints.hints; + let insert_position = match cached_hints + .binary_search_by(|probe| probe.1.position.cmp(&new_hint.position, &buffer_snapshot)) + { + Ok(i) => { + if cached_hints[i].1.text() == new_hint.text() { + None + } else { + Some(i) + } + } + Err(i) => Some(i), + }; + + if let Some(insert_position) = insert_position { + let new_inlay_id = post_inc(&mut editor.next_inlay_id); + if editor + .inlay_hint_cache + .allowed_hint_kinds + .contains(&new_hint.kind) + { + let new_hint_position = + multi_buffer_snapshot.anchor_in_excerpt(query.excerpt_id, new_hint.position); + splice + .to_insert + .push(Inlay::hint(new_inlay_id, new_hint_position, &new_hint)); + } + cached_hints.insert(insert_position, (InlayId::Hint(new_inlay_id), new_hint)); + cached_inlays_changed = true; + } + } + cached_excerpt_hints.buffer_version = buffer_snapshot.version().clone(); + drop(cached_excerpt_hints); + + if query.invalidate.should_invalidate() { + let mut outdated_excerpt_caches = HashSet::default(); + for (excerpt_id, excerpt_hints) in &editor.inlay_hint_cache().hints { + let excerpt_hints = excerpt_hints.read(); + if excerpt_hints.buffer_id == query.buffer_id + && excerpt_id != &query.excerpt_id + && buffer_snapshot + .version() + .changed_since(&excerpt_hints.buffer_version) + { + outdated_excerpt_caches.insert(*excerpt_id); + splice + .to_remove + .extend(excerpt_hints.hints.iter().map(|(id, _)| id)); + } + } + cached_inlays_changed |= !outdated_excerpt_caches.is_empty(); + editor + .inlay_hint_cache + .hints + .retain(|excerpt_id, _| !outdated_excerpt_caches.contains(excerpt_id)); + } + + let InlaySplice { + to_remove, + to_insert, + } = splice; + let displayed_inlays_changed = !to_remove.is_empty() || !to_insert.is_empty(); + if cached_inlays_changed || displayed_inlays_changed { + editor.inlay_hint_cache.version += 1; + } + if displayed_inlays_changed { + editor.splice_inlay_hints(to_remove, to_insert, cx) + } +} + #[cfg(test)] mod tests { use std::sync::atomic::{AtomicBool, AtomicU32, Ordering}; @@ -866,6 +816,7 @@ mod tests { }; use futures::StreamExt; use gpui::{executor::Deterministic, TestAppContext, ViewHandle}; + use itertools::Itertools; use language::{ language_settings::AllLanguageSettingsContent, FakeLspAdapter, Language, LanguageConfig, }; @@ -873,7 +824,7 @@ mod tests { use parking_lot::Mutex; use project::{FakeFs, Project}; use settings::SettingsStore; - use text::Point; + use text::{Point, ToPoint}; use workspace::Workspace; use crate::editor_tests::update_test_language_settings; @@ -1879,7 +1830,7 @@ mod tests { task_lsp_request_ranges.lock().push(params.range); let query_start = params.range.start; - let i = Arc::clone(&task_lsp_request_count).fetch_add(1, Ordering::SeqCst) + 1; + let i = Arc::clone(&task_lsp_request_count).fetch_add(1, Ordering::Release) + 1; Ok(Some(vec![lsp::InlayHint { position: query_start, label: lsp::InlayHintLabel::String(i.to_string()), @@ -1894,18 +1845,44 @@ mod tests { }) .next() .await; + fn editor_visible_range( + editor: &ViewHandle, + cx: &mut gpui::TestAppContext, + ) -> Range { + let ranges = editor.update(cx, |editor, cx| editor.excerpt_visible_offsets(None, cx)); + assert_eq!( + ranges.len(), + 1, + "Single buffer should produce a single excerpt with visible range" + ); + let (_, (excerpt_buffer, _, excerpt_visible_range)) = + ranges.into_iter().next().unwrap(); + excerpt_buffer.update(cx, |buffer, _| { + let snapshot = buffer.snapshot(); + let start = buffer + .anchor_before(excerpt_visible_range.start) + .to_point(&snapshot); + let end = buffer + .anchor_after(excerpt_visible_range.end) + .to_point(&snapshot); + start..end + }) + } + + let initial_visible_range = editor_visible_range(&editor, cx); + let expected_initial_query_range_end = + lsp::Position::new(initial_visible_range.end.row * 2, 1); cx.foreground().run_until_parked(); editor.update(cx, |editor, cx| { - let mut ranges = lsp_request_ranges.lock().drain(..).collect::>(); - ranges.sort_by_key(|range| range.start); - assert_eq!(ranges.len(), 2, "When scroll is at the edge of a big document, its visible part + the rest should be queried for hints"); - assert_eq!(ranges[0].start, lsp::Position::new(0, 0), "Should query from the beginning of the document"); - assert_eq!(ranges[0].end.line, ranges[1].start.line, "Both requests should be on the same line"); - assert_eq!(ranges[0].end.character + 1, ranges[1].start.character, "Both request should be concequent"); + let ranges = lsp_request_ranges.lock().drain(..).collect::>(); + assert_eq!(ranges.len(), 1, + "When scroll is at the edge of a big document, double of its visible part range should be queried for hints in one single big request, but got: {ranges:?}"); + let query_range = &ranges[0]; + assert_eq!(query_range.start, lsp::Position::new(0, 0), "Should query initially from the beginning of the document"); + assert_eq!(query_range.end, expected_initial_query_range_end, "Should query initially for double lines of the visible part of the document"); - assert_eq!(lsp_request_count.load(Ordering::SeqCst), 2, - "When scroll is at the edge of a big document, its visible part + the rest should be queried for hints"); - let expected_layers = vec!["1".to_string(), "2".to_string()]; + assert_eq!(lsp_request_count.load(Ordering::Acquire), 1); + let expected_layers = vec!["1".to_string()]; assert_eq!( expected_layers, cached_hint_labels(editor), @@ -1913,37 +1890,114 @@ mod tests { ); assert_eq!(expected_layers, visible_hint_labels(editor, cx)); assert_eq!( - editor.inlay_hint_cache().version, 2, - "Both LSP queries should've bumped the cache version" + editor.inlay_hint_cache().version, 1, + "LSP queries should've bumped the cache version" ); }); editor.update(cx, |editor, cx| { editor.scroll_screen(&ScrollAmount::Page(1.0), cx); editor.scroll_screen(&ScrollAmount::Page(1.0), cx); - editor.change_selections(None, cx, |s| s.select_ranges([600..600])); - editor.handle_input("++++more text++++", cx); }); + let visible_range_after_scrolls = editor_visible_range(&editor, cx); + let visible_line_count = + editor.update(cx, |editor, _| editor.visible_line_count().unwrap()); + cx.foreground().run_until_parked(); + let selection_in_cached_range = editor.update(cx, |editor, cx| { + let ranges = lsp_request_ranges + .lock() + .drain(..) + .sorted_by_key(|r| r.start) + .collect::>(); + assert_eq!( + ranges.len(), + 2, + "Should query 2 ranges after both scrolls, but got: {ranges:?}" + ); + let first_scroll = &ranges[0]; + let second_scroll = &ranges[1]; + assert_eq!( + first_scroll.end, second_scroll.start, + "Should query 2 adjacent ranges after the scrolls, but got: {ranges:?}" + ); + assert_eq!( + first_scroll.start, expected_initial_query_range_end, + "First scroll should start the query right after the end of the original scroll", + ); + assert_eq!( + second_scroll.end, + lsp::Position::new( + visible_range_after_scrolls.end.row + + visible_line_count.ceil() as u32, + 0 + ), + "Second scroll should query one more screen down after the end of the visible range" + ); + + assert_eq!( + lsp_request_count.load(Ordering::Acquire), + 3, + "Should query for hints after every scroll" + ); + let expected_layers = vec!["1".to_string(), "2".to_string(), "3".to_string()]; + assert_eq!( + expected_layers, + cached_hint_labels(editor), + "Should have hints from the new LSP response after the edit" + ); + assert_eq!(expected_layers, visible_hint_labels(editor, cx)); + assert_eq!( + editor.inlay_hint_cache().version, + 3, + "Should update the cache for every LSP response with hints added" + ); + + let mut selection_in_cached_range = visible_range_after_scrolls.end; + selection_in_cached_range.row -= visible_line_count.ceil() as u32; + selection_in_cached_range + }); + + editor.update(cx, |editor, cx| { + editor.change_selections(Some(Autoscroll::center()), cx, |s| { + s.select_ranges([selection_in_cached_range..selection_in_cached_range]) + }); + }); + cx.foreground().run_until_parked(); + editor.update(cx, |_, _| { + let ranges = lsp_request_ranges + .lock() + .drain(..) + .sorted_by_key(|r| r.start) + .collect::>(); + assert!(ranges.is_empty(), "No new ranges or LSP queries should be made after returning to the selection with cached hints"); + assert_eq!(lsp_request_count.load(Ordering::Acquire), 3); + }); + + editor.update(cx, |editor, cx| { + editor.handle_input("++++more text++++", cx); + }); cx.foreground().run_until_parked(); editor.update(cx, |editor, cx| { - let mut ranges = lsp_request_ranges.lock().drain(..).collect::>(); - ranges.sort_by_key(|range| range.start); - assert_eq!(ranges.len(), 3, "When scroll is at the middle of a big document, its visible part + 2 other inbisible parts should be queried for hints"); - assert_eq!(ranges[0].start, lsp::Position::new(0, 0), "Should query from the beginning of the document"); - assert_eq!(ranges[0].end.line + 1, ranges[1].start.line, "Neighbour requests got on different lines due to the line end"); - assert_ne!(ranges[0].end.character, 0, "First query was in the end of the line, not in the beginning"); - assert_eq!(ranges[1].start.character, 0, "Second query got pushed into a new line and starts from the beginning"); - assert_eq!(ranges[1].end.line, ranges[2].start.line, "Neighbour requests should be on the same line"); - assert_eq!(ranges[1].end.character + 1, ranges[2].start.character, "Neighbour request should be concequent"); + let ranges = lsp_request_ranges.lock().drain(..).collect::>(); + assert_eq!(ranges.len(), 1, + "On edit, should scroll to selection and query a range around it. Instead, got query ranges {ranges:?}"); + let query_range = &ranges[0]; + assert!(query_range.start.line < selection_in_cached_range.row, + "Hints should be queried with the selected range after the query range start"); + assert!(query_range.end.line > selection_in_cached_range.row, + "Hints should be queried with the selected range before the query range end"); + assert!(query_range.start.line <= selection_in_cached_range.row - (visible_line_count * 3.0 / 2.0) as u32, + "Hints query range should contain one more screen before"); + assert!(query_range.end.line >= selection_in_cached_range.row + (visible_line_count * 3.0 / 2.0) as u32, + "Hints query range should contain one more screen after"); - assert_eq!(lsp_request_count.load(Ordering::SeqCst), 5, - "When scroll not at the edge of a big document, visible part + 2 other parts should be queried for hints"); - let expected_layers = vec!["3".to_string(), "4".to_string(), "5".to_string()]; + assert_eq!(lsp_request_count.load(Ordering::Acquire), 4, "Should query for hints once after the edit"); + let expected_layers = vec!["4".to_string()]; assert_eq!(expected_layers, cached_hint_labels(editor), - "Should have hints from the new LSP response after edit"); + "Should have hints from the new LSP response after the edit"); assert_eq!(expected_layers, visible_hint_labels(editor, cx)); - assert_eq!(editor.inlay_hint_cache().version, 5, "Should update the cache for every LSP response with hints added"); + assert_eq!(editor.inlay_hint_cache().version, 4, "Should update the cache for every LSP response with hints added"); }); } @@ -2177,7 +2231,7 @@ mod tests { s.select_ranges([Point::new(22, 0)..Point::new(22, 0)]) }); editor.change_selections(Some(Autoscroll::Next), cx, |s| { - s.select_ranges([Point::new(56, 0)..Point::new(56, 0)]) + s.select_ranges([Point::new(50, 0)..Point::new(50, 0)]) }); }); cx.foreground().run_until_parked(); @@ -2283,8 +2337,8 @@ all hints should be invalidated and requeried for all of its visible excerpts" assert_eq!(expected_layers, visible_hint_labels(editor, cx)); assert_eq!( editor.inlay_hint_cache().version, - last_scroll_update_version + expected_layers.len() + 1, - "Due to every excerpt having one hint, cache should update per new excerpt received + 1 for outdated hints removal" + last_scroll_update_version + expected_layers.len(), + "Due to every excerpt having one hint, cache should update per new excerpt received" ); }); } @@ -2488,8 +2542,8 @@ all hints should be invalidated and requeried for all of its visible excerpts" ); assert_eq!( editor.inlay_hint_cache().version, - 3, - "Excerpt removal should trigger cache update" + 2, + "Excerpt removal should trigger a cache update" ); }); @@ -2516,12 +2570,119 @@ all hints should be invalidated and requeried for all of its visible excerpts" ); assert_eq!( editor.inlay_hint_cache().version, - 4, - "Settings change should trigger cache update" + 3, + "Settings change should trigger a cache update" ); }); } + #[gpui::test] + async fn test_inside_char_boundary_range_hints(cx: &mut gpui::TestAppContext) { + init_test(cx, |settings| { + settings.defaults.inlay_hints = Some(InlayHintSettings { + enabled: true, + show_type_hints: true, + show_parameter_hints: true, + show_other_hints: true, + }) + }); + + let mut language = Language::new( + LanguageConfig { + name: "Rust".into(), + path_suffixes: vec!["rs".to_string()], + ..Default::default() + }, + Some(tree_sitter_rust::language()), + ); + let mut fake_servers = language + .set_fake_lsp_adapter(Arc::new(FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + inlay_hint_provider: Some(lsp::OneOf::Left(true)), + ..Default::default() + }, + ..Default::default() + })) + .await; + let fs = FakeFs::new(cx.background()); + fs.insert_tree( + "/a", + json!({ + "main.rs": format!(r#"fn main() {{\n{}\n}}"#, format!("let i = {};\n", "√".repeat(10)).repeat(500)), + "other.rs": "// Test file", + }), + ) + .await; + let project = Project::test(fs, ["/a".as_ref()], cx).await; + project.update(cx, |project, _| project.languages().add(Arc::new(language))); + let workspace = cx + .add_window(|cx| Workspace::test_new(project.clone(), cx)) + .root(cx); + let worktree_id = workspace.update(cx, |workspace, cx| { + workspace.project().read_with(cx, |project, cx| { + project.worktrees(cx).next().unwrap().read(cx).id() + }) + }); + + let _buffer = project + .update(cx, |project, cx| { + project.open_local_buffer("/a/main.rs", cx) + }) + .await + .unwrap(); + cx.foreground().run_until_parked(); + cx.foreground().start_waiting(); + let fake_server = fake_servers.next().await.unwrap(); + let editor = workspace + .update(cx, |workspace, cx| { + workspace.open_path((worktree_id, "main.rs"), None, true, cx) + }) + .await + .unwrap() + .downcast::() + .unwrap(); + let lsp_request_count = Arc::new(AtomicU32::new(0)); + let closure_lsp_request_count = Arc::clone(&lsp_request_count); + fake_server + .handle_request::(move |params, _| { + let task_lsp_request_count = Arc::clone(&closure_lsp_request_count); + async move { + assert_eq!( + params.text_document.uri, + lsp::Url::from_file_path("/a/main.rs").unwrap(), + ); + let query_start = params.range.start; + let i = Arc::clone(&task_lsp_request_count).fetch_add(1, Ordering::Release) + 1; + Ok(Some(vec![lsp::InlayHint { + position: query_start, + label: lsp::InlayHintLabel::String(i.to_string()), + kind: None, + text_edits: None, + tooltip: None, + padding_left: None, + padding_right: None, + data: None, + }])) + } + }) + .next() + .await; + + cx.foreground().run_until_parked(); + editor.update(cx, |editor, cx| { + editor.change_selections(None, cx, |s| { + s.select_ranges([Point::new(10, 0)..Point::new(10, 0)]) + }) + }); + cx.foreground().run_until_parked(); + editor.update(cx, |editor, cx| { + let expected_layers = vec!["1".to_string()]; + assert_eq!(expected_layers, cached_hint_labels(editor)); + assert_eq!(expected_layers, visible_hint_labels(editor, cx)); + assert_eq!(editor.inlay_hint_cache().version, 1); + }); + } + pub(crate) fn init_test(cx: &mut TestAppContext, f: impl Fn(&mut AllLanguageSettingsContent)) { cx.foreground().forbid_parking(); diff --git a/crates/editor/src/scroll.rs b/crates/editor/src/scroll.rs index d595337428..1f3adaf477 100644 --- a/crates/editor/src/scroll.rs +++ b/crates/editor/src/scroll.rs @@ -13,7 +13,7 @@ use gpui::{ }; use language::{Bias, Point}; use util::ResultExt; -use workspace::{item::Item, WorkspaceId}; +use workspace::WorkspaceId; use crate::{ display_map::{DisplaySnapshot, ToDisplayPoint}, @@ -333,9 +333,7 @@ impl Editor { cx, ); - if !self.is_singleton(cx) { - self.refresh_inlays(InlayRefreshReason::NewLinesShown, cx); - } + self.refresh_inlays(InlayRefreshReason::NewLinesShown, cx); } pub fn scroll_position(&self, cx: &mut ViewContext) -> Vector2F {