diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 64332c102a..81f073ed62 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -26,7 +26,7 @@ use aho_corasick::AhoCorasick; use anyhow::{anyhow, Result}; use blink_manager::BlinkManager; use client::{ClickhouseEvent, TelemetrySettings}; -use clock::ReplicaId; +use clock::{Global, ReplicaId}; use collections::{BTreeMap, Bound, HashMap, HashSet, VecDeque}; use copilot::Copilot; pub use display_map::DisplayPoint; @@ -1195,11 +1195,11 @@ enum GotoDefinitionKind { Type, } -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Clone)] enum InlayRefreshReason { SettingsChange(InlayHintSettings), NewLinesShown, - ExcerptEdited, + BufferEdited(HashSet>), RefreshRequested, } @@ -2617,7 +2617,7 @@ impl Editor { return; } - let invalidate_cache = match reason { + let (invalidate_cache, required_languages) = match reason { InlayRefreshReason::SettingsChange(new_settings) => { match self.inlay_hint_cache.update_settings( &self.buffer, @@ -2633,16 +2633,18 @@ impl Editor { return; } ControlFlow::Break(None) => return, - ControlFlow::Continue(()) => InvalidationStrategy::RefreshRequested, + ControlFlow::Continue(()) => (InvalidationStrategy::RefreshRequested, None), } } - InlayRefreshReason::NewLinesShown => InvalidationStrategy::None, - InlayRefreshReason::ExcerptEdited => InvalidationStrategy::ExcerptEdited, - InlayRefreshReason::RefreshRequested => InvalidationStrategy::RefreshRequested, + InlayRefreshReason::NewLinesShown => (InvalidationStrategy::None, None), + InlayRefreshReason::BufferEdited(buffer_languages) => { + (InvalidationStrategy::BufferEdited, Some(buffer_languages)) + } + InlayRefreshReason::RefreshRequested => (InvalidationStrategy::RefreshRequested, None), }; self.inlay_hint_cache.refresh_inlay_hints( - self.excerpt_visible_offsets(cx), + self.excerpt_visible_offsets(required_languages.as_ref(), cx), invalidate_cache, cx, ) @@ -2661,8 +2663,9 @@ impl Editor { fn excerpt_visible_offsets( &self, + restrict_to_languages: Option<&HashSet>>, cx: &mut ViewContext<'_, '_, Editor>, - ) -> HashMap, Range)> { + ) -> HashMap, Global, Range)> { let multi_buffer = self.buffer().read(cx); let multi_buffer_snapshot = multi_buffer.snapshot(cx); let multi_buffer_visible_start = self @@ -2680,8 +2683,22 @@ impl Editor { .range_to_buffer_ranges(multi_buffer_visible_range, cx) .into_iter() .filter(|(_, excerpt_visible_range, _)| !excerpt_visible_range.is_empty()) - .map(|(buffer, excerpt_visible_range, excerpt_id)| { - (excerpt_id, (buffer, excerpt_visible_range)) + .filter_map(|(buffer_handle, excerpt_visible_range, excerpt_id)| { + let buffer = buffer_handle.read(cx); + let language = buffer.language()?; + if let Some(restrict_to_languages) = restrict_to_languages { + if !restrict_to_languages.contains(language) { + return None; + } + } + Some(( + excerpt_id, + ( + buffer_handle, + buffer.version().clone(), + excerpt_visible_range, + ), + )) }) .collect() } @@ -7256,7 +7273,7 @@ impl Editor { fn on_buffer_event( &mut self, - _: ModelHandle, + multibuffer: ModelHandle, event: &multi_buffer::Event, cx: &mut ViewContext, ) { @@ -7268,7 +7285,33 @@ impl Editor { self.update_visible_copilot_suggestion(cx); } cx.emit(Event::BufferEdited); - self.refresh_inlays(InlayRefreshReason::ExcerptEdited, cx); + + if let Some(project) = &self.project { + let project = project.read(cx); + let languages_affected = multibuffer + .read(cx) + .all_buffers() + .into_iter() + .filter_map(|buffer| { + let buffer = buffer.read(cx); + let language = buffer.language()?; + if project.is_local() + && project.language_servers_for_buffer(buffer, cx).count() == 0 + { + None + } else { + Some(language) + } + }) + .cloned() + .collect::>(); + if !languages_affected.is_empty() { + self.refresh_inlays( + InlayRefreshReason::BufferEdited(languages_affected), + cx, + ); + } + } } multi_buffer::Event::ExcerptsAdded { buffer, diff --git a/crates/editor/src/inlay_hint_cache.rs b/crates/editor/src/inlay_hint_cache.rs index af7bf3e4c5..3f9f8e4288 100644 --- a/crates/editor/src/inlay_hint_cache.rs +++ b/crates/editor/src/inlay_hint_cache.rs @@ -38,7 +38,7 @@ pub struct CachedExcerptHints { #[derive(Debug, Clone, Copy)] pub enum InvalidationStrategy { RefreshRequested, - ExcerptEdited, + BufferEdited, None, } @@ -94,7 +94,7 @@ impl InvalidationStrategy { fn should_invalidate(&self) -> bool { matches!( self, - InvalidationStrategy::RefreshRequested | InvalidationStrategy::ExcerptEdited + InvalidationStrategy::RefreshRequested | InvalidationStrategy::BufferEdited ) } } @@ -197,7 +197,7 @@ impl InlayHintCache { pub fn refresh_inlay_hints( &mut self, - mut excerpts_to_query: HashMap, Range)>, + mut excerpts_to_query: HashMap, Global, Range)>, invalidate: InvalidationStrategy, cx: &mut ViewContext, ) { @@ -342,104 +342,113 @@ impl InlayHintCache { fn spawn_new_update_tasks( editor: &mut Editor, - excerpts_to_query: HashMap, Range)>, + excerpts_to_query: HashMap, Global, Range)>, invalidate: InvalidationStrategy, update_cache_version: usize, cx: &mut ViewContext<'_, '_, Editor>, ) { let visible_hints = Arc::new(editor.visible_inlay_hints(cx)); - for (excerpt_id, (buffer_handle, excerpt_visible_range)) in excerpts_to_query { - if !excerpt_visible_range.is_empty() { - let buffer = buffer_handle.read(cx); - let buffer_snapshot = buffer.snapshot(); - let cached_excerpt_hints = editor.inlay_hint_cache.hints.get(&excerpt_id).cloned(); - if let Some(cached_excerpt_hints) = &cached_excerpt_hints { - let new_task_buffer_version = buffer_snapshot.version(); - let cached_excerpt_hints = cached_excerpt_hints.read(); - let cached_buffer_version = &cached_excerpt_hints.buffer_version; - if cached_excerpt_hints.version > update_cache_version - || cached_buffer_version.changed_since(new_task_buffer_version) - { - return; - } - if !new_task_buffer_version.changed_since(&cached_buffer_version) - && !matches!(invalidate, InvalidationStrategy::RefreshRequested) - { - return; - } + for (excerpt_id, (buffer_handle, 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_snapshot = buffer.snapshot(); + if buffer_snapshot + .version() + .changed_since(&new_task_buffer_version) + { + continue; + } + + let cached_excerpt_hints = editor.inlay_hint_cache.hints.get(&excerpt_id).cloned(); + if let Some(cached_excerpt_hints) = &cached_excerpt_hints { + let cached_excerpt_hints = cached_excerpt_hints.read(); + let cached_buffer_version = &cached_excerpt_hints.buffer_version; + if cached_excerpt_hints.version > update_cache_version + || cached_buffer_version.changed_since(&new_task_buffer_version) + { + 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) = + 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), + ) + }); + + 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 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) = - 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), - ) - }); - - 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 = |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::ExcerptEdited, - 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, - }); - } + 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, - }); - } + } + hash_map::Entry::Vacant(v) => { + v.insert(UpdateTask { + invalidate, + cache_version: query.cache_version, + task: new_update_task(false), + pending_refresh: None, + }); } } } @@ -961,6 +970,247 @@ mod tests { }); } + #[gpui::test] + async fn test_no_hint_updates_for_unrelated_language_files(cx: &mut gpui::TestAppContext) { + let allowed_hint_kinds = HashSet::from_iter([None, Some(InlayHintKind::Type)]); + init_test(cx, |settings| { + settings.defaults.inlay_hints = Some(InlayHintSettings { + enabled: true, + show_type_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Type)), + show_parameter_hints: allowed_hint_kinds.contains(&Some(InlayHintKind::Parameter)), + show_other_hints: allowed_hint_kinds.contains(&None), + }) + }); + + let fs = FakeFs::new(cx.background()); + fs.insert_tree( + "/a", + json!({ + "main.rs": "fn main() { a } // and some long comment to ensure inlays are not trimmed out", + "other.md": "Test md file with some text", + }), + ) + .await; + let project = Project::test(fs, ["/a".as_ref()], cx).await; + let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), 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 mut rs_fake_servers = None; + let mut md_fake_servers = None; + for (name, path_suffix) in [("Rust", "rs"), ("Markdown", "md")] { + let mut language = Language::new( + LanguageConfig { + name: name.into(), + path_suffixes: vec![path_suffix.to_string()], + ..Default::default() + }, + Some(tree_sitter_rust::language()), + ); + let fake_servers = language + .set_fake_lsp_adapter(Arc::new(FakeLspAdapter { + name, + capabilities: lsp::ServerCapabilities { + inlay_hint_provider: Some(lsp::OneOf::Left(true)), + ..Default::default() + }, + ..Default::default() + })) + .await; + match name { + "Rust" => rs_fake_servers = Some(fake_servers), + "Markdown" => md_fake_servers = Some(fake_servers), + _ => unreachable!(), + } + project.update(cx, |project, _| { + project.languages().add(Arc::new(language)); + }); + } + + let _rs_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 rs_fake_server = rs_fake_servers.unwrap().next().await.unwrap(); + let rs_editor = workspace + .update(cx, |workspace, cx| { + workspace.open_path((worktree_id, "main.rs"), None, true, cx) + }) + .await + .unwrap() + .downcast::() + .unwrap(); + let rs_lsp_request_count = Arc::new(AtomicU32::new(0)); + rs_fake_server + .handle_request::(move |params, _| { + let task_lsp_request_count = Arc::clone(&rs_lsp_request_count); + async move { + assert_eq!( + params.text_document.uri, + lsp::Url::from_file_path("/a/main.rs").unwrap(), + ); + let i = Arc::clone(&task_lsp_request_count).fetch_add(1, Ordering::SeqCst); + Ok(Some(vec![lsp::InlayHint { + position: lsp::Position::new(0, i), + 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(); + rs_editor.update(cx, |editor, cx| { + let expected_layers = vec!["0".to_string()]; + assert_eq!( + expected_layers, + cached_hint_labels(editor), + "Should get its first hints when opening the editor" + ); + assert_eq!(expected_layers, visible_hint_labels(editor, cx)); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!( + inlay_cache.allowed_hint_kinds, allowed_hint_kinds, + "Cache should use editor settings to get the allowed hint kinds" + ); + assert_eq!( + inlay_cache.version, 1, + "Rust editor update the cache version after every cache/view change" + ); + }); + + cx.foreground().run_until_parked(); + let _md_buffer = project + .update(cx, |project, cx| { + project.open_local_buffer("/a/other.md", cx) + }) + .await + .unwrap(); + cx.foreground().run_until_parked(); + cx.foreground().start_waiting(); + let md_fake_server = md_fake_servers.unwrap().next().await.unwrap(); + let md_editor = workspace + .update(cx, |workspace, cx| { + workspace.open_path((worktree_id, "other.md"), None, true, cx) + }) + .await + .unwrap() + .downcast::() + .unwrap(); + let md_lsp_request_count = Arc::new(AtomicU32::new(0)); + md_fake_server + .handle_request::(move |params, _| { + let task_lsp_request_count = Arc::clone(&md_lsp_request_count); + async move { + assert_eq!( + params.text_document.uri, + lsp::Url::from_file_path("/a/other.md").unwrap(), + ); + let i = Arc::clone(&task_lsp_request_count).fetch_add(1, Ordering::SeqCst); + Ok(Some(vec![lsp::InlayHint { + position: lsp::Position::new(0, i), + 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(); + md_editor.update(cx, |editor, cx| { + let expected_layers = vec!["0".to_string()]; + assert_eq!( + expected_layers, + cached_hint_labels(editor), + "Markdown editor should have a separate verison, repeating Rust editor rules" + ); + assert_eq!(expected_layers, visible_hint_labels(editor, cx)); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds); + assert_eq!(inlay_cache.version, 1); + }); + + rs_editor.update(cx, |editor, cx| { + editor.change_selections(None, cx, |s| s.select_ranges([13..13])); + editor.handle_input("some rs change", cx); + }); + cx.foreground().run_until_parked(); + rs_editor.update(cx, |editor, cx| { + let expected_layers = vec!["1".to_string()]; + assert_eq!( + expected_layers, + cached_hint_labels(editor), + "Rust inlay cache should change after the edit" + ); + assert_eq!(expected_layers, visible_hint_labels(editor, cx)); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds); + assert_eq!( + inlay_cache.version, 2, + "Every time hint cache changes, cache version should be incremented" + ); + }); + md_editor.update(cx, |editor, cx| { + let expected_layers = vec!["0".to_string()]; + assert_eq!( + expected_layers, + cached_hint_labels(editor), + "Markdown editor should not be affected by Rust editor changes" + ); + assert_eq!(expected_layers, visible_hint_labels(editor, cx)); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds); + assert_eq!(inlay_cache.version, 1); + }); + + md_editor.update(cx, |editor, cx| { + editor.change_selections(None, cx, |s| s.select_ranges([13..13])); + editor.handle_input("some md change", cx); + }); + cx.foreground().run_until_parked(); + md_editor.update(cx, |editor, cx| { + let expected_layers = vec!["1".to_string()]; + assert_eq!( + expected_layers, + cached_hint_labels(editor), + "Rust editor should not be affected by Markdown editor changes" + ); + assert_eq!(expected_layers, visible_hint_labels(editor, cx)); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds); + assert_eq!(inlay_cache.version, 2); + }); + rs_editor.update(cx, |editor, cx| { + let expected_layers = vec!["1".to_string()]; + assert_eq!( + expected_layers, + cached_hint_labels(editor), + "Markdown editor should also change independently" + ); + assert_eq!(expected_layers, visible_hint_labels(editor, cx)); + let inlay_cache = editor.inlay_hint_cache(); + assert_eq!(inlay_cache.allowed_hint_kinds, allowed_hint_kinds); + assert_eq!(inlay_cache.version, 2); + }); + } + #[gpui::test] async fn test_hint_setting_changes(cx: &mut gpui::TestAppContext) { let allowed_hint_kinds = HashSet::from_iter([None, Some(InlayHintKind::Type)]); diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index 28c5125de2..7c94f25e1e 100644 --- a/crates/text/src/text.rs +++ b/crates/text/src/text.rs @@ -2489,7 +2489,12 @@ impl ToOffset for Point { impl ToOffset for usize { fn to_offset(&self, snapshot: &BufferSnapshot) -> usize { - assert!(*self <= snapshot.len(), "offset {self} is out of range"); + assert!( + *self <= snapshot.len(), + "offset {} is out of range, max allowed is {}", + self, + snapshot.len() + ); *self } }