Skip to content

language: Return early if no grammars are added#48685

Merged
MrSubidubi merged 1 commit intozed-industries:mainfrom
marcocondrache:zymfoxyu
Feb 8, 2026
Merged

language: Return early if no grammars are added#48685
MrSubidubi merged 1 commit intozed-industries:mainfrom
marcocondrache:zymfoxyu

Conversation

@marcocondrache
Copy link
Contributor

@marcocondrache marcocondrache commented Feb 7, 2026

Helps #48601

Whenever an extension is installed, we call register_grammars even when the grammar list is empty. This unnecessarily increments reload_count and notifies the LSP store, which clears all languages and triggers a full reparse.

Clearing languages also emits LanguageChanged events for buffers, causing the editor to perform expensive recomputations (like #48622) which can block the main thread for large multibuffers.

This PR addresses the empty-grammar case. If an extension actually adds a grammar, the underlying issue still exists and will require additional fixes to fully resolve.

  • Tests or screenshots needed?
  • Code Reviewed
  • Manual QA

Release Notes:

  • Fixed an issue where installing theme extensions could block the main thread

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Feb 7, 2026
@github-actions github-actions bot added the community champion Issues filed by our amazing community champions! 🫶 label Feb 7, 2026
Copy link
Member

@MrSubidubi MrSubidubi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As so often, nice catch and nice digging!

I just did a quick check and it seems that

fn register_grammars(&self, grammars: Vec<(Arc<str>, PathBuf)>) {
self.language_registry.register_wasm_grammars(grammars)
}
is the only caller of this currently. As I see it, we could perhaps even get rid of acquiring the lock if we instead check upfront if the Vec is empty by either handling that at the call site or chaning the argument type, can we not?

@marcocondrache
Copy link
Contributor Author

marcocondrache commented Feb 8, 2026

@MrSubidubi thanks for checking! Yes, that’s right. The check shouldn’t live in the proxy implementations. It makes more sense to handle it at the extension_host level, since that’s responsible for making the proxy calls. If no new grammars are needed, we can simply skip calling the proxy.

I initially added the check there because register_wasm_grammars has implicit side effects and that isn’t very obvious to the caller, so putting the check there felt like a safer choice at the time

Copy link
Member

@MrSubidubi MrSubidubi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just checked again, sorry for the back and forth and the nit here. I think it would be better if we were to add this check at the level of the language registry -

fn remove_languages(
&mut self,
languages_to_remove: &[LanguageName],
grammars_to_remove: &[Arc<str>],
) {
if languages_to_remove.is_empty() && grammars_to_remove.is_empty() {
return;
}
does this also within the registry state and I think we should not handle checking this differently for something that is similar here. So let this just take a Vec or alternatively a Peekable so that we handle this the same for all cases.

@marcocondrache
Copy link
Contributor Author

marcocondrache commented Feb 8, 2026

@MrSubidubi thank you and no worries! I've added the check back into register_wasm_grammar 😊.

I totally missed remove_languages so it makes sense to have those checks there.

Copy link
Member

@MrSubidubi MrSubidubi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, nice find, thank you so much and thanks for the follow-ups!

@MrSubidubi MrSubidubi enabled auto-merge (squash) February 8, 2026 17:45
@MrSubidubi MrSubidubi merged commit f1f8c55 into zed-industries:main Feb 8, 2026
27 checks passed
baldwindavid added a commit to baldwindavid/zed that referenced this pull request Feb 9, 2026
* main: (57 commits)
  agent: Fix disabled MCP servers disappearing from UI after restart (zed-industries#47758)
  Update Rust crate git2 to v0.20.4 [SECURITY] (zed-industries#48400)
  Update Rust crate time to v0.3.47 [SECURITY] (zed-industries#48514)
  gpui: Reset `external_files_dragged` after successful drag-drop on macOS (zed-industries#48727)
  language: Return early if no grammars are added (zed-industries#48685)
  Properly handle multi-char folds (zed-industries#48721)
  collab: Proxy `GET /extensions` to Cloud (zed-industries#48717)
  git: Fix a potential misalignment in the side-by-side diff (zed-industries#48690)
  Move extension API DTOs into `cloud_api_types` (zed-industries#48689)
  git: Add a setting for the default view mode of `SplittableEditor` (zed-industries#48440)
  Use proper settings name for semantic tokens' settings UI (zed-industries#48686)
  gpui: Fix restarting panicking due to double borrows on windows (zed-industries#48667)
  Strip broken thinking blocks from Anthropic requests (zed-industries#48548)
  keymap_editor: Add `alt-l` keybinding for cycling favorite models (zed-industries#48390)
  Only raise Windows timer resolution while blocking with timeout (zed-industries#48379)
  editor: Propagate `buffer_font_features` to signatureHelp popover (zed-industries#48653)
  Add configurable LSP timeout setting (zed-industries#44745)
  editor: Use buffer_font for folds and change foreground color (zed-industries#48652)
  lsp: Update root_path for compatibility with language servers (zed-industries#48587)
  Fix panic with LSP folds on disappearing excerpts (zed-industries#48649)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement community champion Issues filed by our amazing community champions! 🫶

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants