Skip to content

Commit 443f53b

Browse files
DnreikronosZenor27
authored andcommitted
editor: Fix semantic tokens missing when opening buffer from multibuffer (zed-industries#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 zed-industries#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
1 parent b649dea commit 443f53b

3 files changed

Lines changed: 290 additions & 24 deletions

File tree

crates/editor/src/editor.rs

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,6 +1182,7 @@ pub struct Editor {
11821182
delegate_stage_and_restore: bool,
11831183
delegate_open_excerpts: bool,
11841184
enable_lsp_data: bool,
1185+
needs_initial_data_update: bool,
11851186
enable_runnables: bool,
11861187
enable_mouse_wheel_zoom: bool,
11871188
show_line_numbers: Option<bool>,
@@ -1975,6 +1976,7 @@ impl Editor {
19751976
self.buffers_with_disabled_indent_guides.clone();
19761977
clone.enable_mouse_wheel_zoom = self.enable_mouse_wheel_zoom;
19771978
clone.enable_lsp_data = self.enable_lsp_data;
1979+
clone.needs_initial_data_update = self.enable_lsp_data;
19781980
clone.enable_runnables = self.enable_runnables;
19791981
clone
19801982
}
@@ -2424,6 +2426,7 @@ impl Editor {
24242426
delegate_stage_and_restore: false,
24252427
delegate_open_excerpts: false,
24262428
enable_lsp_data: full_mode,
2429+
needs_initial_data_update: full_mode,
24272430
enable_runnables: full_mode,
24282431
enable_mouse_wheel_zoom: full_mode,
24292432
show_git_diff_gutter: None,
@@ -2652,16 +2655,7 @@ impl Editor {
26522655
);
26532656
});
26542657

2655-
editor.post_scroll_update = cx.spawn_in(window, async move |editor, cx| {
2656-
cx.background_executor()
2657-
.timer(Duration::from_millis(50))
2658-
.await;
2659-
editor
2660-
.update_in(cx, |editor, window, cx| {
2661-
editor.update_data_on_scroll(window, cx)
2662-
})
2663-
.ok();
2664-
});
2658+
editor.update_data_on_scroll(true, window, cx);
26652659
}
26662660
editor.refresh_sticky_headers(&editor.snapshot(window, cx), cx);
26672661
}
@@ -20860,7 +20854,7 @@ impl Editor {
2086020854
cx.notify();
2086120855

2086220856
self.scrollbar_marker_state.dirty = true;
20863-
self.update_data_on_scroll(window, cx);
20857+
self.update_data_on_scroll(false, window, cx);
2086420858
self.folds_did_change(cx);
2086520859
}
2086620860

@@ -26092,11 +26086,35 @@ impl Editor {
2609226086
self.enable_mouse_wheel_zoom = false;
2609326087
}
2609426088

26095-
fn update_data_on_scroll(&mut self, window: &mut Window, cx: &mut Context<'_, Self>) {
26089+
fn update_data_on_scroll(
26090+
&mut self,
26091+
debounce: bool,
26092+
window: &mut Window,
26093+
cx: &mut Context<'_, Self>,
26094+
) {
26095+
if debounce {
26096+
self.post_scroll_update = cx.spawn_in(window, async move |editor, cx| {
26097+
cx.background_executor()
26098+
.timer(Duration::from_millis(50))
26099+
.await;
26100+
editor
26101+
.update_in(cx, |editor, window, cx| {
26102+
editor.do_update_data_on_scroll(window, cx);
26103+
})
26104+
.ok();
26105+
});
26106+
} else {
26107+
self.post_scroll_update = Task::ready(());
26108+
self.do_update_data_on_scroll(window, cx);
26109+
}
26110+
}
26111+
26112+
fn do_update_data_on_scroll(&mut self, window: &mut Window, cx: &mut Context<'_, Self>) {
2609626113
self.register_visible_buffers(cx);
2609726114
self.colorize_brackets(false, cx);
2609826115
self.refresh_inlay_hints(InlayHintRefreshReason::NewLinesShown, cx);
26099-
if !self.buffer().read(cx).is_singleton() {
26116+
if !self.buffer().read(cx).is_singleton() || self.needs_initial_data_update {
26117+
self.needs_initial_data_update = false;
2610026118
self.update_lsp_data(None, window, cx);
2610126119
self.refresh_runnables(None, window, cx);
2610226120
}

crates/editor/src/scroll.rs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ pub(crate) mod scroll_amount;
55
use crate::editor_settings::ScrollBeyondLastLine;
66
use crate::{
77
Anchor, DisplayPoint, DisplayRow, Editor, EditorEvent, EditorMode, EditorSettings,
8-
InlayHintRefreshReason, MultiBufferSnapshot, RowExt, SizingBehavior, ToPoint,
8+
MultiBufferSnapshot, RowExt, SizingBehavior, ToPoint,
99
display_map::{DisplaySnapshot, ToDisplayPoint},
1010
hover_popover::hide_hover,
1111
persistence::EditorDb,
@@ -680,16 +680,7 @@ impl Editor {
680680
let opened_first_time = self.scroll_manager.visible_line_count.is_none();
681681
self.scroll_manager.visible_line_count = Some(lines);
682682
if opened_first_time {
683-
self.post_scroll_update = cx.spawn_in(window, async move |editor, cx| {
684-
editor
685-
.update_in(cx, |editor, window, cx| {
686-
editor.register_visible_buffers(cx);
687-
editor.colorize_brackets(false, cx);
688-
editor.refresh_inlay_hints(InlayHintRefreshReason::NewLinesShown, cx);
689-
editor.update_lsp_data(None, window, cx);
690-
})
691-
.ok();
692-
});
683+
self.update_data_on_scroll(false, window, cx);
693684
}
694685
}
695686

crates/editor/src/semantic_tokens.rs

Lines changed: 257 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1267,6 +1267,263 @@ mod tests {
12671267
);
12681268
}
12691269

1270+
#[gpui::test]
1271+
async fn lsp_semantic_tokens_singleton_opened_from_multibuffer(cx: &mut TestAppContext) {
1272+
init_test(cx, |_| {});
1273+
1274+
update_test_language_settings(cx, &|language_settings| {
1275+
language_settings.languages.0.insert(
1276+
"Rust".into(),
1277+
LanguageSettingsContent {
1278+
semantic_tokens: Some(SemanticTokens::Full),
1279+
..LanguageSettingsContent::default()
1280+
},
1281+
);
1282+
});
1283+
1284+
let rust_language = Arc::new(Language::new(
1285+
LanguageConfig {
1286+
name: "Rust".into(),
1287+
matcher: LanguageMatcher {
1288+
path_suffixes: vec!["rs".into()],
1289+
..LanguageMatcher::default()
1290+
},
1291+
..LanguageConfig::default()
1292+
},
1293+
None,
1294+
));
1295+
1296+
let rust_legend = lsp::SemanticTokensLegend {
1297+
token_types: vec!["function".into()],
1298+
token_modifiers: Vec::new(),
1299+
};
1300+
1301+
let app_state = cx.update(workspace::AppState::test);
1302+
cx.update(|cx| {
1303+
assets::Assets.load_test_fonts(cx);
1304+
crate::init(cx);
1305+
workspace::init(app_state.clone(), cx);
1306+
});
1307+
1308+
let project = Project::test(app_state.fs.clone(), [], cx).await;
1309+
let language_registry = project.read_with(cx, |project, _| project.languages().clone());
1310+
1311+
let mut rust_server = language_registry.register_fake_lsp(
1312+
rust_language.name(),
1313+
FakeLspAdapter {
1314+
name: "rust",
1315+
capabilities: lsp::ServerCapabilities {
1316+
semantic_tokens_provider: Some(
1317+
lsp::SemanticTokensServerCapabilities::SemanticTokensOptions(
1318+
lsp::SemanticTokensOptions {
1319+
legend: rust_legend,
1320+
full: Some(lsp::SemanticTokensFullOptions::Delta { delta: None }),
1321+
..lsp::SemanticTokensOptions::default()
1322+
},
1323+
),
1324+
),
1325+
..lsp::ServerCapabilities::default()
1326+
},
1327+
initializer: Some(Box::new(move |fake_server| {
1328+
fake_server
1329+
.set_request_handler::<lsp::request::SemanticTokensFullRequest, _, _>(
1330+
move |_, _| async move {
1331+
Ok(Some(lsp::SemanticTokensResult::Tokens(
1332+
lsp::SemanticTokens {
1333+
data: vec![0, 3, 4, 0, 0],
1334+
result_id: None,
1335+
},
1336+
)))
1337+
},
1338+
);
1339+
})),
1340+
..FakeLspAdapter::default()
1341+
},
1342+
);
1343+
language_registry.add(rust_language.clone());
1344+
1345+
// foo.rs must be long enough that autoscroll triggers an actual scroll
1346+
// position change when opening from the multibuffer with cursor near
1347+
// the end. This reproduces the race: set_visible_line_count spawns a
1348+
// task, then autoscroll fires ScrollPositionChanged whose handler
1349+
// replaces post_scroll_update with a debounced task that skips
1350+
// update_lsp_data for singletons.
1351+
let mut foo_content = String::from("fn test() {}\n");
1352+
for i in 0..100 {
1353+
foo_content.push_str(&format!("fn func_{i}() {{}}\n"));
1354+
}
1355+
1356+
app_state
1357+
.fs
1358+
.as_fake()
1359+
.insert_tree(
1360+
EditorLspTestContext::root_path(),
1361+
json!({
1362+
".git": {},
1363+
"bar.rs": "fn main() {}\n",
1364+
"foo.rs": foo_content,
1365+
}),
1366+
)
1367+
.await;
1368+
1369+
let (multi_workspace, cx) =
1370+
cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx));
1371+
let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone());
1372+
project
1373+
.update(cx, |project, cx| {
1374+
project.find_or_create_worktree(EditorLspTestContext::root_path(), true, cx)
1375+
})
1376+
.await
1377+
.unwrap();
1378+
cx.read(|cx| workspace.read(cx).worktree_scans_complete(cx))
1379+
.await;
1380+
1381+
// Open bar.rs as an editor to start the LSP server.
1382+
let bar_file = cx.read(|cx| workspace.file_project_paths(cx)[0].clone());
1383+
let bar_item = workspace
1384+
.update_in(cx, |workspace, window, cx| {
1385+
workspace.open_path(bar_file, None, true, window, cx)
1386+
})
1387+
.await
1388+
.expect("Could not open bar.rs");
1389+
let bar_editor = cx.update(|_, cx| {
1390+
bar_item
1391+
.act_as::<Editor>(cx)
1392+
.expect("Opened test file wasn't an editor")
1393+
});
1394+
let bar_buffer = cx.read(|cx| {
1395+
bar_editor
1396+
.read(cx)
1397+
.buffer()
1398+
.read(cx)
1399+
.as_singleton()
1400+
.unwrap()
1401+
});
1402+
1403+
let _rust_server = rust_server.next().await.unwrap();
1404+
1405+
cx.executor().advance_clock(Duration::from_millis(200));
1406+
let task = bar_editor.update_in(cx, |e, _, _| e.semantic_token_state.take_update_task());
1407+
cx.run_until_parked();
1408+
task.await;
1409+
cx.run_until_parked();
1410+
1411+
assert!(
1412+
!extract_semantic_highlights(&bar_editor, &cx).is_empty(),
1413+
"bar.rs should have semantic tokens after initial open"
1414+
);
1415+
1416+
// Get foo.rs buffer directly from the project. No editor has ever
1417+
// fetched semantic tokens for this buffer.
1418+
let foo_file = cx.read(|cx| workspace.file_project_paths(cx)[1].clone());
1419+
let foo_buffer = project
1420+
.update(cx, |project, cx| project.open_buffer(foo_file, cx))
1421+
.await
1422+
.expect("Could not open foo.rs buffer");
1423+
1424+
// Build a multibuffer with both files. The foo.rs excerpt covers a
1425+
// range near the end of the file so that opening the singleton will
1426+
// autoscroll to a position that requires changing scroll_position.
1427+
let multibuffer = cx.new(|cx| {
1428+
let mut multibuffer = MultiBuffer::new(Capability::ReadWrite);
1429+
multibuffer.set_excerpts_for_path(
1430+
PathKey::sorted(0),
1431+
bar_buffer.clone(),
1432+
[Point::new(0, 0)..Point::new(0, 12)],
1433+
0,
1434+
cx,
1435+
);
1436+
multibuffer.set_excerpts_for_path(
1437+
PathKey::sorted(1),
1438+
foo_buffer.clone(),
1439+
[Point::new(95, 0)..Point::new(100, 0)],
1440+
0,
1441+
cx,
1442+
);
1443+
multibuffer
1444+
});
1445+
1446+
let mb_editor = workspace.update_in(cx, |workspace, window, cx| {
1447+
let editor =
1448+
cx.new(|cx| build_editor_with_project(project.clone(), multibuffer, window, cx));
1449+
workspace.add_item_to_active_pane(Box::new(editor.clone()), None, true, window, cx);
1450+
editor
1451+
});
1452+
mb_editor.update_in(cx, |editor, window, cx| {
1453+
let nav_history = workspace
1454+
.read(cx)
1455+
.active_pane()
1456+
.read(cx)
1457+
.nav_history_for_item(&cx.entity());
1458+
editor.set_nav_history(Some(nav_history));
1459+
window.focus(&editor.focus_handle(cx), cx)
1460+
});
1461+
1462+
// Close bar.rs tab so only the multibuffer remains.
1463+
workspace
1464+
.update_in(cx, |workspace, window, cx| {
1465+
let pane = workspace.active_pane().clone();
1466+
pane.update(cx, |pane, cx| {
1467+
pane.close_item_by_id(
1468+
bar_editor.entity_id(),
1469+
workspace::SaveIntent::Skip,
1470+
window,
1471+
cx,
1472+
)
1473+
})
1474+
})
1475+
.await
1476+
.ok();
1477+
1478+
cx.run_until_parked();
1479+
1480+
// Position cursor in the foo.rs excerpt (near line 95+).
1481+
mb_editor.update_in(cx, |editor, window, cx| {
1482+
let snapshot = editor.display_snapshot(cx);
1483+
let end = snapshot.buffer_snapshot().len();
1484+
editor.change_selections(None.into(), window, cx, |s| {
1485+
s.select_ranges([end..end]);
1486+
});
1487+
});
1488+
1489+
// Open the singleton from the multibuffer. open_buffers_in_workspace
1490+
// creates the editor and calls change_selections with autoscroll.
1491+
// During render, set_visible_line_count fires first (spawning a task),
1492+
// then autoscroll_vertically scrolls to line ~95 which emits
1493+
// ScrollPositionChanged, whose handler replaces post_scroll_update.
1494+
mb_editor.update_in(cx, |editor, window, cx| {
1495+
editor.open_excerpts(&crate::actions::OpenExcerpts, window, cx);
1496+
});
1497+
1498+
cx.run_until_parked();
1499+
cx.executor().advance_clock(Duration::from_millis(200));
1500+
cx.run_until_parked();
1501+
1502+
let active_editor = workspace.read_with(cx, |workspace, cx| {
1503+
workspace
1504+
.active_item(cx)
1505+
.and_then(|item| item.act_as::<Editor>(cx))
1506+
.expect("Active item should be an editor")
1507+
});
1508+
1509+
assert!(
1510+
active_editor.read_with(cx, |editor, cx| editor.buffer().read(cx).is_singleton()),
1511+
"Active editor should be a singleton buffer"
1512+
);
1513+
1514+
// Wait for semantic tokens on the singleton.
1515+
cx.executor().advance_clock(Duration::from_millis(200));
1516+
let task = active_editor.update_in(cx, |e, _, _| e.semantic_token_state.take_update_task());
1517+
task.await;
1518+
cx.run_until_parked();
1519+
1520+
let highlights = extract_semantic_highlights(&active_editor, &cx);
1521+
assert!(
1522+
!highlights.is_empty(),
1523+
"Singleton editor opened from multibuffer should have semantic tokens"
1524+
);
1525+
}
1526+
12701527
fn extract_semantic_highlights(
12711528
editor: &Entity<Editor>,
12721529
cx: &TestAppContext,

0 commit comments

Comments
 (0)