Skip to content

lsp: Do not drop lsp buffer handle from editor when a language change leads to buffer having a legit language#44469

Merged
osiewicz merged 1 commit intomainfrom
manifest-tree-do-not-drop-buffer-handle-when-buffer-is-newly-parsed
Dec 9, 2025
Merged

lsp: Do not drop lsp buffer handle from editor when a language change leads to buffer having a legit language#44469
osiewicz merged 1 commit intomainfrom
manifest-tree-do-not-drop-buffer-handle-when-buffer-is-newly-parsed

Conversation

@osiewicz
Copy link
Member

@osiewicz osiewicz commented Dec 9, 2025

Fixes a bug that led to us unnecessarily restarting a language server when we were looking at a single file of a given language.

Release Notes:

  • Fixed a bug that led to Zed sometimes starting an excessive amount of language servers

… leads to buffer having a legit language

Fixes a bug that led to us unnecessarily restarting a language server when we were looking at a single file of a given language.
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Dec 9, 2025
Copy link
Contributor

@SomeoneToIgnore SomeoneToIgnore left a comment

Choose a reason for hiding this comment

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

Thank you for finding this.

The only wish is to have a test to be sure we do not restart the server again in a similar situation.
Yet the problem is that our fake_lsp-related infra won't do that?

@osiewicz
Copy link
Member Author

osiewicz commented Dec 9, 2025

Yeah, not sure; I've tried the following, with no luck:

#[gpui::test]
async fn test_language_server_does_not_restart_on_initial_parse(cx: &mut TestAppContext) {
    init_test(cx, |_| {});

    let fs = FakeFs::new(cx.executor());
    fs.insert_tree(
        path!("/a"),
        json!({
            "main.rs": "fn main() { let a = 5; }",
            "other.rs": "// Test file",
        }),
    )
    .await;

    let project = Project::test(fs, [path!("/a").as_ref()], cx).await;

    let server_restarts = Arc::new(AtomicUsize::new(0));
    let closure_restarts = Arc::clone(&server_restarts);
    let language_server_name = "test language server";
    let language_name: LanguageName = "Rust".into();

    let language_registry = project.read_with(cx, |project, _| project.languages().clone());
    language_registry.add(Arc::new(Language::new(
        LanguageConfig {
            name: language_name.clone(),
            matcher: LanguageMatcher {
                path_suffixes: vec!["rs".to_string()],
                ..Default::default()
            },
            ..Default::default()
        },
        Some(tree_sitter_rust::LANGUAGE.into()),
    )));
    let mut fake_servers = language_registry.register_fake_lsp(
        "Rust",
        FakeLspAdapter {
            name: language_server_name,
            initialization_options: Some(json!({
                "testOptionValue": true
            })),
            initializer: Some(Box::new(move |fake_server| {
                let task_restarts = Arc::clone(&closure_restarts);
                fake_server.set_request_handler::<lsp::request::Shutdown, _, _>(move |_, _| {
                    task_restarts.fetch_add(1, atomic::Ordering::Release);
                    futures::future::ready(Ok(()))
                });
            })),
            ..Default::default()
        },
    );

    let buffer = project
        .update(cx, |project, cx| {
            project.open_local_buffer(path!("/a/main.rs"), cx)
        })
        .await
        .unwrap();
    let (_workspace, window) =
        cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));

    let multi_buffer = window.new(|cx| MultiBuffer::singleton(buffer.clone(), cx));
    let _editor = window.new_window_entity(|window, cx| {
        Editor::new(
            EditorMode::full(),
            multi_buffer,
            Some(project.clone()),
            window,
            cx,
        )
    });
    let _fake_server = fake_servers.next().await.unwrap();
    cx.run_until_parked();
    let servers_for_buffer = project.update(cx, |this, cx| {
        this.lsp_store().update(cx, |this, cx| {
            buffer.update(cx, |buffer, cx| {
                this.language_servers_for_local_buffer(buffer, cx)
                    .map(|server| server.1.server_id())
                    .collect::<Vec<_>>()
            })
        })
    });
    dbg!(servers_for_buffer);
    let p = buffer.update(cx, |this, cx| this.language().cloned());
    dbg!(p.unwrap().name());
    assert_eq!(
        server_restarts.load(std::sync::atomic::Ordering::Acquire),
        1
    );
}

Gonna switch to Copilot stuff in the meantime.

@osiewicz osiewicz merged commit 3180f44 into main Dec 9, 2025
24 checks passed
@osiewicz osiewicz deleted the manifest-tree-do-not-drop-buffer-handle-when-buffer-is-newly-parsed branch December 9, 2025 20:37
CherryWorm pushed a commit to CherryWorm/zed that referenced this pull request Dec 16, 2025
… leads to buffer having a legit language (zed-industries#44469)

Fixes a bug that led to us unnecessarily restarting a language server
when we were looking at a single file of a given language.

Release Notes:

- Fixed a bug that led to Zed sometimes starting an excessive amount of
language servers
someone13574 pushed a commit to someone13574/zed that referenced this pull request Dec 16, 2025
… leads to buffer having a legit language (zed-industries#44469)

Fixes a bug that led to us unnecessarily restarting a language server
when we were looking at a single file of a given language.

Release Notes:

- Fixed a bug that led to Zed sometimes starting an excessive amount of
language servers
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants