From a86de48c70e4bcc8c8da53eb5317726c74be7005 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Soares?= <37777652+Dnreikronos@users.noreply.github.com> Date: Sun, 12 Apr 2026 16:00:40 -0300 Subject: [PATCH] editor: Fix semantic tokens missing when opening buffer from multibuffer (#53712) Summary Semantic token highlighting was missing when opening a file from multibuffer search results (Ctrl+Shift+F). Which file got hit depended on window size and scroll offset. ## Root cause Two async tasks race to write `post_scroll_update`: 1. `set_visible_line_count` (scroll.rs:682) fires on first render and spawns a task that calls `register_visible_buffers` + `update_lsp_data` (requests semantic tokens). 2. `open_buffers_in_workspace` (editor.rs:25049) calls `change_selections` with autoscroll right after creating the editor. This emits `ScrollPositionChanged`, whose handler (editor.rs:2655) replaces `post_scroll_update` with a task calling `update_data_on_scroll`. 3. `update_data_on_scroll` (editor.rs:26099) has a singleton guard: `if !self.buffer().read(cx).is_singleton()` that skips `update_lsp_data` for single-file buffers. This is a scroll optimization, singleton buffers don't change their visible buffer set on scroll. 4. The initial task gets dropped, the replacement skips `update_lsp_data`, semantic tokens are never requested. ## Fix Added a `needs_initial_lsp_data` flag to the Editor struct, set to `true` on creation. `update_data_on_scroll` checks this flag alongside the singleton guard, so `update_lsp_data` runs at least once even for singletons. The flag flips to `false` right after, so subsequent scrolls behave exactly as before. No perf impact after the first render. ## Self-review checklist - [x] I've reviewed my own diff for quality, security, and reliability - [ ] Unsafe blocks (if any) have justifying comments - [ ] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #53051 ## Demo Before: https://github.com/user-attachments/assets/77d07d95-cb4a-44ff-842d-1f7a46653ca9 After: https://github.com/user-attachments/assets/2c942f52-4ec3-459f-a97b-93919e4bfb3d ## Release notes - Fixed semantic token highlighting missing when opening a buffer from multibuffer search results --- crates/editor/src/editor.rs | 44 +++-- crates/editor/src/scroll.rs | 13 +- crates/editor/src/semantic_tokens.rs | 257 +++++++++++++++++++++++++++ 3 files changed, 290 insertions(+), 24 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 77ddd5e95d159d20ac627ec1d0406823d201ff4c..bc343ca0d4c8fbba8ddd6622a20c217385c2b919 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1182,6 +1182,7 @@ pub struct Editor { delegate_stage_and_restore: bool, delegate_open_excerpts: bool, enable_lsp_data: bool, + needs_initial_data_update: bool, enable_runnables: bool, enable_mouse_wheel_zoom: bool, show_line_numbers: Option, @@ -1975,6 +1976,7 @@ impl Editor { self.buffers_with_disabled_indent_guides.clone(); clone.enable_mouse_wheel_zoom = self.enable_mouse_wheel_zoom; clone.enable_lsp_data = self.enable_lsp_data; + clone.needs_initial_data_update = self.enable_lsp_data; clone.enable_runnables = self.enable_runnables; clone } @@ -2424,6 +2426,7 @@ impl Editor { delegate_stage_and_restore: false, delegate_open_excerpts: false, enable_lsp_data: full_mode, + needs_initial_data_update: full_mode, enable_runnables: full_mode, enable_mouse_wheel_zoom: full_mode, show_git_diff_gutter: None, @@ -2652,16 +2655,7 @@ impl Editor { ); }); - editor.post_scroll_update = cx.spawn_in(window, async move |editor, cx| { - cx.background_executor() - .timer(Duration::from_millis(50)) - .await; - editor - .update_in(cx, |editor, window, cx| { - editor.update_data_on_scroll(window, cx) - }) - .ok(); - }); + editor.update_data_on_scroll(true, window, cx); } editor.refresh_sticky_headers(&editor.snapshot(window, cx), cx); } @@ -20860,7 +20854,7 @@ impl Editor { cx.notify(); self.scrollbar_marker_state.dirty = true; - self.update_data_on_scroll(window, cx); + self.update_data_on_scroll(false, window, cx); self.folds_did_change(cx); } @@ -26092,11 +26086,35 @@ impl Editor { self.enable_mouse_wheel_zoom = false; } - fn update_data_on_scroll(&mut self, window: &mut Window, cx: &mut Context<'_, Self>) { + fn update_data_on_scroll( + &mut self, + debounce: bool, + window: &mut Window, + cx: &mut Context<'_, Self>, + ) { + if debounce { + self.post_scroll_update = cx.spawn_in(window, async move |editor, cx| { + cx.background_executor() + .timer(Duration::from_millis(50)) + .await; + editor + .update_in(cx, |editor, window, cx| { + editor.do_update_data_on_scroll(window, cx); + }) + .ok(); + }); + } else { + self.post_scroll_update = Task::ready(()); + self.do_update_data_on_scroll(window, cx); + } + } + + fn do_update_data_on_scroll(&mut self, window: &mut Window, cx: &mut Context<'_, Self>) { self.register_visible_buffers(cx); self.colorize_brackets(false, cx); self.refresh_inlay_hints(InlayHintRefreshReason::NewLinesShown, cx); - if !self.buffer().read(cx).is_singleton() { + if !self.buffer().read(cx).is_singleton() || self.needs_initial_data_update { + self.needs_initial_data_update = false; self.update_lsp_data(None, window, cx); self.refresh_runnables(None, window, cx); } diff --git a/crates/editor/src/scroll.rs b/crates/editor/src/scroll.rs index 42b865b17ca4e241b8f0728488cacd42d52d257c..0735ae5170d453e8b29dd033752b1cd2c114d457 100644 --- a/crates/editor/src/scroll.rs +++ b/crates/editor/src/scroll.rs @@ -5,7 +5,7 @@ pub(crate) mod scroll_amount; use crate::editor_settings::ScrollBeyondLastLine; use crate::{ Anchor, DisplayPoint, DisplayRow, Editor, EditorEvent, EditorMode, EditorSettings, - InlayHintRefreshReason, MultiBufferSnapshot, RowExt, SizingBehavior, ToPoint, + MultiBufferSnapshot, RowExt, SizingBehavior, ToPoint, display_map::{DisplaySnapshot, ToDisplayPoint}, hover_popover::hide_hover, persistence::EditorDb, @@ -680,16 +680,7 @@ impl Editor { let opened_first_time = self.scroll_manager.visible_line_count.is_none(); self.scroll_manager.visible_line_count = Some(lines); if opened_first_time { - self.post_scroll_update = cx.spawn_in(window, async move |editor, cx| { - editor - .update_in(cx, |editor, window, cx| { - editor.register_visible_buffers(cx); - editor.colorize_brackets(false, cx); - editor.refresh_inlay_hints(InlayHintRefreshReason::NewLinesShown, cx); - editor.update_lsp_data(None, window, cx); - }) - .ok(); - }); + self.update_data_on_scroll(false, window, cx); } } diff --git a/crates/editor/src/semantic_tokens.rs b/crates/editor/src/semantic_tokens.rs index d485cfa70237fed542a240f202a8dc47b07467c4..eaadbbb0e2ee9a49e53cc645487ea489572b1241 100644 --- a/crates/editor/src/semantic_tokens.rs +++ b/crates/editor/src/semantic_tokens.rs @@ -1267,6 +1267,263 @@ mod tests { ); } + #[gpui::test] + async fn lsp_semantic_tokens_singleton_opened_from_multibuffer(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + update_test_language_settings(cx, &|language_settings| { + language_settings.languages.0.insert( + "Rust".into(), + LanguageSettingsContent { + semantic_tokens: Some(SemanticTokens::Full), + ..LanguageSettingsContent::default() + }, + ); + }); + + let rust_language = Arc::new(Language::new( + LanguageConfig { + name: "Rust".into(), + matcher: LanguageMatcher { + path_suffixes: vec!["rs".into()], + ..LanguageMatcher::default() + }, + ..LanguageConfig::default() + }, + None, + )); + + let rust_legend = lsp::SemanticTokensLegend { + token_types: vec!["function".into()], + token_modifiers: Vec::new(), + }; + + let app_state = cx.update(workspace::AppState::test); + cx.update(|cx| { + assets::Assets.load_test_fonts(cx); + crate::init(cx); + workspace::init(app_state.clone(), cx); + }); + + let project = Project::test(app_state.fs.clone(), [], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + + let mut rust_server = language_registry.register_fake_lsp( + rust_language.name(), + FakeLspAdapter { + name: "rust", + capabilities: lsp::ServerCapabilities { + semantic_tokens_provider: Some( + lsp::SemanticTokensServerCapabilities::SemanticTokensOptions( + lsp::SemanticTokensOptions { + legend: rust_legend, + full: Some(lsp::SemanticTokensFullOptions::Delta { delta: None }), + ..lsp::SemanticTokensOptions::default() + }, + ), + ), + ..lsp::ServerCapabilities::default() + }, + initializer: Some(Box::new(move |fake_server| { + fake_server + .set_request_handler::( + move |_, _| async move { + Ok(Some(lsp::SemanticTokensResult::Tokens( + lsp::SemanticTokens { + data: vec![0, 3, 4, 0, 0], + result_id: None, + }, + ))) + }, + ); + })), + ..FakeLspAdapter::default() + }, + ); + language_registry.add(rust_language.clone()); + + // foo.rs must be long enough that autoscroll triggers an actual scroll + // position change when opening from the multibuffer with cursor near + // the end. This reproduces the race: set_visible_line_count spawns a + // task, then autoscroll fires ScrollPositionChanged whose handler + // replaces post_scroll_update with a debounced task that skips + // update_lsp_data for singletons. + let mut foo_content = String::from("fn test() {}\n"); + for i in 0..100 { + foo_content.push_str(&format!("fn func_{i}() {{}}\n")); + } + + app_state + .fs + .as_fake() + .insert_tree( + EditorLspTestContext::root_path(), + json!({ + ".git": {}, + "bar.rs": "fn main() {}\n", + "foo.rs": foo_content, + }), + ) + .await; + + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + project + .update(cx, |project, cx| { + project.find_or_create_worktree(EditorLspTestContext::root_path(), true, cx) + }) + .await + .unwrap(); + cx.read(|cx| workspace.read(cx).worktree_scans_complete(cx)) + .await; + + // Open bar.rs as an editor to start the LSP server. + let bar_file = cx.read(|cx| workspace.file_project_paths(cx)[0].clone()); + let bar_item = workspace + .update_in(cx, |workspace, window, cx| { + workspace.open_path(bar_file, None, true, window, cx) + }) + .await + .expect("Could not open bar.rs"); + let bar_editor = cx.update(|_, cx| { + bar_item + .act_as::(cx) + .expect("Opened test file wasn't an editor") + }); + let bar_buffer = cx.read(|cx| { + bar_editor + .read(cx) + .buffer() + .read(cx) + .as_singleton() + .unwrap() + }); + + let _rust_server = rust_server.next().await.unwrap(); + + cx.executor().advance_clock(Duration::from_millis(200)); + let task = bar_editor.update_in(cx, |e, _, _| e.semantic_token_state.take_update_task()); + cx.run_until_parked(); + task.await; + cx.run_until_parked(); + + assert!( + !extract_semantic_highlights(&bar_editor, &cx).is_empty(), + "bar.rs should have semantic tokens after initial open" + ); + + // Get foo.rs buffer directly from the project. No editor has ever + // fetched semantic tokens for this buffer. + let foo_file = cx.read(|cx| workspace.file_project_paths(cx)[1].clone()); + let foo_buffer = project + .update(cx, |project, cx| project.open_buffer(foo_file, cx)) + .await + .expect("Could not open foo.rs buffer"); + + // Build a multibuffer with both files. The foo.rs excerpt covers a + // range near the end of the file so that opening the singleton will + // autoscroll to a position that requires changing scroll_position. + let multibuffer = cx.new(|cx| { + let mut multibuffer = MultiBuffer::new(Capability::ReadWrite); + multibuffer.set_excerpts_for_path( + PathKey::sorted(0), + bar_buffer.clone(), + [Point::new(0, 0)..Point::new(0, 12)], + 0, + cx, + ); + multibuffer.set_excerpts_for_path( + PathKey::sorted(1), + foo_buffer.clone(), + [Point::new(95, 0)..Point::new(100, 0)], + 0, + cx, + ); + multibuffer + }); + + let mb_editor = workspace.update_in(cx, |workspace, window, cx| { + let editor = + cx.new(|cx| build_editor_with_project(project.clone(), multibuffer, window, cx)); + workspace.add_item_to_active_pane(Box::new(editor.clone()), None, true, window, cx); + editor + }); + mb_editor.update_in(cx, |editor, window, cx| { + let nav_history = workspace + .read(cx) + .active_pane() + .read(cx) + .nav_history_for_item(&cx.entity()); + editor.set_nav_history(Some(nav_history)); + window.focus(&editor.focus_handle(cx), cx) + }); + + // Close bar.rs tab so only the multibuffer remains. + workspace + .update_in(cx, |workspace, window, cx| { + let pane = workspace.active_pane().clone(); + pane.update(cx, |pane, cx| { + pane.close_item_by_id( + bar_editor.entity_id(), + workspace::SaveIntent::Skip, + window, + cx, + ) + }) + }) + .await + .ok(); + + cx.run_until_parked(); + + // Position cursor in the foo.rs excerpt (near line 95+). + mb_editor.update_in(cx, |editor, window, cx| { + let snapshot = editor.display_snapshot(cx); + let end = snapshot.buffer_snapshot().len(); + editor.change_selections(None.into(), window, cx, |s| { + s.select_ranges([end..end]); + }); + }); + + // Open the singleton from the multibuffer. open_buffers_in_workspace + // creates the editor and calls change_selections with autoscroll. + // During render, set_visible_line_count fires first (spawning a task), + // then autoscroll_vertically scrolls to line ~95 which emits + // ScrollPositionChanged, whose handler replaces post_scroll_update. + mb_editor.update_in(cx, |editor, window, cx| { + editor.open_excerpts(&crate::actions::OpenExcerpts, window, cx); + }); + + cx.run_until_parked(); + cx.executor().advance_clock(Duration::from_millis(200)); + cx.run_until_parked(); + + let active_editor = workspace.read_with(cx, |workspace, cx| { + workspace + .active_item(cx) + .and_then(|item| item.act_as::(cx)) + .expect("Active item should be an editor") + }); + + assert!( + active_editor.read_with(cx, |editor, cx| editor.buffer().read(cx).is_singleton()), + "Active editor should be a singleton buffer" + ); + + // Wait for semantic tokens on the singleton. + cx.executor().advance_clock(Duration::from_millis(200)); + let task = active_editor.update_in(cx, |e, _, _| e.semantic_token_state.take_update_task()); + task.await; + cx.run_until_parked(); + + let highlights = extract_semantic_highlights(&active_editor, &cx); + assert!( + !highlights.is_empty(), + "Singleton editor opened from multibuffer should have semantic tokens" + ); + } + fn extract_semantic_highlights( editor: &Entity, cx: &TestAppContext,