From e29206b56935cc2cf24423bf926b81521fd8467b Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 13 Mar 2026 18:27:01 +0200 Subject: [PATCH] Do not overly eagerly invalidate the runnables (#51500) Follow-up of https://github.com/zed-industries/zed/pull/51299 Release Notes: - N/A --- crates/editor/src/editor.rs | 19 ++-- crates/editor/src/runnables.rs | 194 +++++++++++++++++++++++++++++++-- 2 files changed, 195 insertions(+), 18 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 7536e58d2f0dbfd58f738bdb8bed3b3c2a65a25e..8c2e03722c345a0f093572c336029a0eaa355537 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2142,7 +2142,7 @@ impl Editor { editor.registered_buffers.clear(); editor.register_visible_buffers(cx); editor.invalidate_semantic_tokens(None); - editor.refresh_runnables(window, cx); + editor.refresh_runnables(None, window, cx); editor.update_lsp_data(None, window, cx); editor.refresh_inlay_hints(InlayHintRefreshReason::ServerRemoved, cx); } @@ -2172,7 +2172,7 @@ impl Editor { let buffer_id = *buffer_id; if editor.buffer().read(cx).buffer(buffer_id).is_some() { editor.register_buffer(buffer_id, cx); - editor.refresh_runnables(window, cx); + editor.refresh_runnables(Some(buffer_id), window, cx); editor.update_lsp_data(Some(buffer_id), window, cx); editor.refresh_inlay_hints(InlayHintRefreshReason::NewLinesShown, cx); refresh_linked_ranges(editor, window, cx); @@ -2251,7 +2251,7 @@ impl Editor { &task_inventory, window, |editor, _, window, cx| { - editor.refresh_runnables(window, cx); + editor.refresh_runnables(None, window, cx); }, )); }; @@ -23789,7 +23789,7 @@ impl Editor { .invalidate_buffer(&buffer.read(cx).remote_id()); self.update_lsp_data(Some(buffer_id), window, cx); self.refresh_inlay_hints(InlayHintRefreshReason::NewLinesShown, cx); - self.refresh_runnables(window, cx); + self.refresh_runnables(None, window, cx); self.colorize_brackets(false, cx); self.refresh_selected_text_highlights(&self.display_snapshot(cx), true, window, cx); cx.emit(EditorEvent::ExcerptsAdded { @@ -23850,12 +23850,11 @@ impl Editor { } self.colorize_brackets(false, cx); self.update_lsp_data(None, window, cx); - self.refresh_runnables(window, cx); + self.refresh_runnables(None, window, cx); cx.emit(EditorEvent::ExcerptsExpanded { ids: ids.clone() }) } multi_buffer::Event::Reparsed(buffer_id) => { - self.clear_runnables(Some(*buffer_id)); - self.refresh_runnables(window, cx); + self.refresh_runnables(Some(*buffer_id), window, cx); self.refresh_selected_text_highlights(&self.display_snapshot(cx), true, window, cx); self.colorize_brackets(true, cx); jsx_tag_auto_close::refresh_enabled_in_any_buffer(self, multibuffer, cx); @@ -23863,7 +23862,7 @@ impl Editor { cx.emit(EditorEvent::Reparsed(*buffer_id)); } multi_buffer::Event::DiffHunksToggled => { - self.refresh_runnables(window, cx); + self.refresh_runnables(None, window, cx); } multi_buffer::Event::LanguageChanged(buffer_id, is_fresh_language) => { if !is_fresh_language { @@ -23999,7 +23998,7 @@ impl Editor { .unwrap_or(DiagnosticSeverity::Hint); self.set_max_diagnostics_severity(new_severity, cx); } - self.refresh_runnables(window, cx); + self.refresh_runnables(None, window, cx); self.update_edit_prediction_settings(cx); self.refresh_edit_prediction(true, false, window, cx); self.refresh_inline_values(cx); @@ -25379,7 +25378,7 @@ impl Editor { self.refresh_inlay_hints(InlayHintRefreshReason::NewLinesShown, cx); if !self.buffer().read(cx).is_singleton() { self.update_lsp_data(None, window, cx); - self.refresh_runnables(window, cx); + self.refresh_runnables(None, window, cx); } } } diff --git a/crates/editor/src/runnables.rs b/crates/editor/src/runnables.rs index 9fa6b89ec130e74f388c5e82b9b346197bb13abb..e36658cf0b160dc2e340f11abe76efa5e895b4ee 100644 --- a/crates/editor/src/runnables.rs +++ b/crates/editor/src/runnables.rs @@ -1,7 +1,7 @@ use std::{collections::BTreeMap, mem, ops::Range, sync::Arc}; use clock::Global; -use collections::HashMap; +use collections::{HashMap, HashSet}; use gpui::{ App, AppContext as _, AsyncWindowContext, ClickEvent, Context, Entity, Focusable as _, MouseButton, Task, Window, @@ -30,6 +30,7 @@ use crate::{ #[derive(Debug)] pub(super) struct RunnableData { runnables: HashMap)>, + invalidate_buffer_data: HashSet, runnables_update_task: Task<()>, } @@ -37,6 +38,7 @@ impl RunnableData { pub fn new() -> Self { Self { runnables: HashMap::default(), + invalidate_buffer_data: HashSet::default(), runnables_update_task: Task::ready(()), } } @@ -108,7 +110,12 @@ pub struct ResolvedTasks { } impl Editor { - pub fn refresh_runnables(&mut self, window: &mut Window, cx: &mut Context) { + pub fn refresh_runnables( + &mut self, + invalidate_buffer_data: Option, + window: &mut Window, + cx: &mut Context, + ) { if !self.mode().is_full() || !EditorSettings::get_global(cx).gutter.runnables || !self.enable_runnables @@ -117,13 +124,18 @@ impl Editor { return; } if let Some(buffer) = self.buffer().read(cx).as_singleton() { - if self - .runnables - .has_cached(buffer.read(cx).remote_id(), &buffer.read(cx).version()) + let buffer_id = buffer.read(cx).remote_id(); + if invalidate_buffer_data != Some(buffer_id) + && self + .runnables + .has_cached(buffer_id, &buffer.read(cx).version()) { return; } } + if let Some(buffer_id) = invalidate_buffer_data { + self.runnables.invalidate_buffer_data.insert(buffer_id); + } let project = self.project().map(Entity::downgrade); let lsp_task_sources = self.lsp_task_sources(true, true, cx); @@ -249,6 +261,10 @@ impl Editor { .await; editor .update(cx, |editor, cx| { + for buffer_id in std::mem::take(&mut editor.runnables.invalidate_buffer_data) { + editor.clear_runnables(Some(buffer_id)); + } + for ((buffer_id, row), mut new_tasks) in rows { let Some(buffer) = editor.buffer().read(cx).buffer(buffer_id) else { continue; @@ -332,6 +348,7 @@ impl Editor { } else { self.runnables.runnables.clear(); } + self.runnables.invalidate_buffer_data.clear(); self.runnables.runnables_update_task = Task::ready(()); } @@ -697,12 +714,17 @@ impl Editor { mod tests { use std::{sync::Arc, time::Duration}; + use futures::StreamExt as _; use gpui::{AppContext as _, Task, TestAppContext}; use indoc::indoc; - use language::ContextProvider; + use language::{ContextProvider, FakeLspAdapter}; use languages::rust_lang; + use lsp::LanguageServerName; use multi_buffer::{MultiBuffer, PathKey}; - use project::{FakeFs, Project}; + use project::{ + FakeFs, Project, + lsp_store::lsp_ext_command::{CargoRunnableArgs, Runnable, RunnableArgs, RunnableKind}, + }; use serde_json::json; use task::{TaskTemplate, TaskTemplates}; use text::Point; @@ -710,8 +732,11 @@ mod tests { use crate::{ Editor, UPDATE_DEBOUNCE, editor_tests::init_test, scroll::scroll_amount::ScrollAmount, + test::build_editor_with_project, }; + const FAKE_LSP_NAME: &str = "the-fake-language-server"; + struct TestRustContextProvider; impl ContextProvider for TestRustContextProvider { @@ -739,6 +764,28 @@ mod tests { } } + struct TestRustContextProviderWithLsp; + + impl ContextProvider for TestRustContextProviderWithLsp { + fn associated_tasks( + &self, + _: Option>, + _: &gpui::App, + ) -> Task> { + Task::ready(Some(TaskTemplates(vec![TaskTemplate { + label: "Run test".into(), + command: "cargo".into(), + args: vec!["test".into()], + tags: vec!["rust-test".into()], + ..TaskTemplate::default() + }]))) + } + + fn lsp_task_source(&self) -> Option { + Some(LanguageServerName::new_static(FAKE_LSP_NAME)) + } + } + fn rust_lang_with_task_context() -> Arc { Arc::new( Arc::try_unwrap(rust_lang()) @@ -747,6 +794,14 @@ mod tests { ) } + fn rust_lang_with_lsp_task_context() -> Arc { + Arc::new( + Arc::try_unwrap(rust_lang()) + .unwrap() + .with_context_provider(Some(Arc::new(TestRustContextProviderWithLsp))), + ) + } + fn collect_runnable_labels( editor: &Editor, ) -> Vec<(text::BufferId, language::BufferRow, Vec)> { @@ -853,7 +908,7 @@ mod tests { editor .update(cx, |editor, window, cx| { editor.clear_runnables(None); - editor.refresh_runnables(window, cx); + editor.refresh_runnables(None, window, cx); }) .unwrap(); cx.executor().advance_clock(UPDATE_DEBOUNCE); @@ -912,4 +967,127 @@ mod tests { "first.rs runnables should survive an edit to second.rs" ); } + + #[gpui::test] + async fn test_lsp_runnables_removed_after_edit(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/project"), + json!({ + "main.rs": indoc! {" + #[test] + fn test_one() { + assert!(true); + } + + fn helper() {} + "}, + }), + ) + .await; + + let project = Project::test(fs, [path!("/project").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(rust_lang_with_lsp_task_context()); + + let mut fake_servers = language_registry.register_fake_lsp( + "Rust", + FakeLspAdapter { + name: FAKE_LSP_NAME, + ..FakeLspAdapter::default() + }, + ); + + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/project/main.rs"), cx) + }) + .await + .unwrap(); + + let buffer_id = buffer.read_with(cx, |buffer, _| buffer.remote_id()); + + let multi_buffer = cx.new(|cx| MultiBuffer::singleton(buffer.clone(), cx)); + let editor = cx.add_window(|window, cx| { + build_editor_with_project(project.clone(), multi_buffer, window, cx) + }); + + let fake_server = fake_servers.next().await.expect("fake LSP server"); + + use project::lsp_store::lsp_ext_command::Runnables; + fake_server.set_request_handler::(move |params, _| async move { + let text = params.text_document.uri.path().to_string(); + if text.contains("main.rs") { + let uri = lsp::Uri::from_file_path(path!("/project/main.rs")).expect("valid uri"); + Ok(vec![Runnable { + label: "LSP test_one".into(), + location: Some(lsp::LocationLink { + origin_selection_range: None, + target_uri: uri, + target_range: lsp::Range::new( + lsp::Position::new(0, 0), + lsp::Position::new(3, 1), + ), + target_selection_range: lsp::Range::new( + lsp::Position::new(0, 0), + lsp::Position::new(3, 1), + ), + }), + kind: RunnableKind::Cargo, + args: RunnableArgs::Cargo(CargoRunnableArgs { + environment: Default::default(), + cwd: path!("/project").into(), + override_cargo: None, + workspace_root: None, + cargo_args: vec!["test".into(), "test_one".into()], + executable_args: Vec::new(), + }), + }]) + } else { + Ok(Vec::new()) + } + }); + + // Trigger a refresh to pick up both tree-sitter and LSP runnables. + editor + .update(cx, |editor, window, cx| { + editor.refresh_runnables(None, window, cx); + }) + .expect("editor update"); + cx.executor().advance_clock(UPDATE_DEBOUNCE); + cx.executor().run_until_parked(); + + let labels = editor + .update(cx, |editor, _, _| collect_runnable_labels(editor)) + .expect("editor update"); + assert_eq!( + labels, + vec![(buffer_id, 0, vec!["LSP test_one".to_string()]),], + "LSP runnables should appear for #[test] fn" + ); + + // Remove `#[test]` attribute so the function is no longer a test. + buffer.update(cx, |buffer, cx| { + let test_attr_end = buffer.text().find("\nfn test_one").expect("find fn"); + buffer.edit([(0..test_attr_end, "")], None, cx); + }); + + // Also update the LSP handler to return no runnables. + fake_server + .set_request_handler::(move |_, _| async move { Ok(Vec::new()) }); + + cx.executor().advance_clock(UPDATE_DEBOUNCE); + cx.executor().run_until_parked(); + + let labels = editor + .update(cx, |editor, _, _| collect_runnable_labels(editor)) + .expect("editor update"); + assert_eq!( + labels, + Vec::<(text::BufferId, language::BufferRow, Vec)>::new(), + "Runnables should be removed after #[test] is deleted and LSP returns empty" + ); + } }