editor: Fix semantic tokens missing when opening buffer from multibuffer

[Dnreikronos]

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

• 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
• Tests cover the new/changed behavior
• Performance impact has been considered and is acceptable

Closes #53051

Demo

Before:
https://github.com/user-attachments/assets/77d07d95-cb4a-44ff-842d-1f7a46653ca9
After:

semantic.mp4

Release notes

• Fixed semantic token highlighting missing when opening a buffer from multibuffer search results

[SomeoneToIgnore]

Thank you for this very nice find.

Before it gets too late, a few notes:

• the new test passes on main hence does not test anything related really
• we'd better rework the code to use update_data_on_scroll in both cases then
• fold_creases better re-set the post_scroll_update task then, as it's now important to make all of them sequential

[Dnreikronos]

Thank you for this very nice find.

Before it gets too late, a few notes:

• the new test passes on main hence does not test anything related really
• we'd better rework the code to use update_data_on_scroll in both cases then
• fold_creases better re-set the post_scroll_update task then, as it's now important to make all of them sequential

Thanks for the review! Here's what I changed:

1. set_visible_line_count now calls update_data_on_scroll instead of duplicating the logic.
2. fold_creases resets post_scroll_update before calling update_data_on_scroll synchronously.
3. Removed the now-unused InlayHintRefreshReason import from scroll.rs.

On the test: it passes on main because set_visible_line_count used to call update_lsp_data directly, bypassing the singleton guard. After the refactor it goes through update_data_on_scroll, so the flag is what keeps the test green. That said, it's a heavy test, happy to drop it if you prefer.

[SomeoneToIgnore]

Just to mention, I do not see those changes described.
Nonetheless, a few comments:

fold_creases resets post_scroll_update before calling update_data_on_scroll synchronously.

The reset seems somewhat odd? I would expect all code paths to replace the old task with the new task, and, depending on the conditions, use the timeout or not.
Either way, all places where we update the data would be to call one method without any extra code around.
May be good to refactor the update_data_on_scroll method to actually set the task itself?

On the test: it passes on main because

That does not make any sense to me: I have taken the test only and applied to main, no other code changes — it passes on the old code which means the test is wrong and exposes no bugs.

[Dnreikronos]

Just to mention, I do not see those changes described. Nonetheless, a few comments:

fold_creases resets post_scroll_update before calling update_data_on_scroll synchronously.

The reset seems somewhat odd? I would expect all code paths to replace the old task with the new task, and, depending on the conditions, use the timeout or not. Either way, all places where we update the data would be to call one method without any extra code around. May be good to refactor the update_data_on_scroll method to actually set the task itself?

On the test: it passes on main because

That does not make any sense to me: I have taken the test only and applied to main, no other code changes — it passes on the old code which means the test is wrong and exposes no bugs.

You're right, the test can't fail on main.
On main, set_visible_line_count calls update_lsp_data unconditionally for every editor, so singletons always get tokens on first render through that path. The test can't fail there because that unconditional call covers it.

This was wrong, and I apologize for the confusion. I rewrote the test and it now fails on main with no other code changes.

The previous test used a 1-line file. Autoscroll had nothing to scroll, so ScrollPositionChanged never fired, and the initial set_visible_line_count task ran fine. The race never triggered.

The new test uses a 100-line file with the multibuffer excerpt at lines 95-100. When the singleton opens and autoscrolls to that position, it actually changes the scroll position. That fires ScrollPositionChanged, whose handler replaces post_scroll_update with a debounced task that goes through update_data_on_scroll, which skips update_lsp_data for singletons. The initial task from set_visible_line_count gets dropped, tokens are never requested.

Also pushed the other changes you asked for:

• update_data_on_scroll renamed to update_visible_data, now sets the task itself
• Non-debounce path calls the method directly instead of spawning a task
• needs_initial_lsp_data renamed to needs_initial_data_update

If you find any other thing that you think it's not good, please let me know

[Dnreikronos]

Once everything is validated I'll squash everything into a single commit

[SomeoneToIgnore]

Thank you, this looks quite good now — will wait until it turns into a ready PR, not a draft.

Much better now.

[zed-industries-bot]

	Warnings
⚠️	This PR is missing release notes. Please add a "Release Notes" section that describes the change: Release Notes: - Added/Fixed/Improved ... If your change is not user-facing, you can use "N/A" for the entry: Release Notes: - N/A

Generated by 🚫 dangerJS against cc00d98

[SomeoneToIgnore]

Thank you.