Skip to content

Revert serverEnabled setting — no value over disabling the extension#651

Closed
Copilot wants to merge 3 commits intomainfrom
copilot/fix-duplicate-server-handlers
Closed

Revert serverEnabled setting — no value over disabling the extension#651
Copilot wants to merge 3 commits intomainfrom
copilot/fix-duplicate-server-handlers

Conversation

Copy link
Contributor

Copilot AI commented Mar 4, 2026

The upstream template PR (#259) introduced a serverEnabled setting to allow disabling the language server without disabling the extension. After review, this setting doesn't provide meaningful value for this extension — disabling the extension itself achieves the same effect.

This reverts the serverEnabled implementation that was added in the initial commit on this branch:

  • package.json — Removed black-formatter.serverEnabled configuration property
  • src/common/settings.ts — Removed getServerEnabled() and serverEnabled from config change watch list
  • src/extension.ts — Removed early-exit server stop logic gated on serverEnabled
  • package.nls.json + locale files — Removed settings.serverEnabled.description entries
  • settings.unit.test.ts — Removed associated tests

The other part of #259 (concurrency guard with isRestarting + restartTimer debounce) was already present in this repo and remains unchanged.

Original prompt

This section details on the original issue you should resolve

<issue_title>Template Sync: Fix duplicate server handlers on concurrent restarts</issue_title>
<issue_description>### 🔄 Template Sync Required

Changes from the upstream vscode-python-tools-extension-template have not yet been fully incorporated into this repository.

Source PR

Summary

PR #259 introduced two sets of changes to the template:

  1. Concurrency guard with debounce (isRestarting flag + restartTimer) in runServer() to prevent duplicate server spawning when multiple concurrent restarts are triggered. This part is already present in this repo.

  2. serverEnabled setting support — a new black-formatter.serverEnabled boolean configuration that allows users (or workspace configs) to disable the formatter's language server entirely. When set to false, the running server is gracefully stopped. This part has not been incorporated into this repository.

Files with missing changes

  • src/common/settings.ts — Missing the getServerEnabled() utility function and serverEnabled in the checkIfConfigurationChanged list:

    // Missing from checkIfConfigurationChanged:
    `\$\{namespace}.serverEnabled`,
    
    // Missing function:
    export function getServerEnabled(namespace: string): boolean {
        const config = getConfiguration(namespace);
        return config.get(boolean)('serverEnabled', true);
    }
  • src/extension.ts — Missing the getServerEnabled check inside runServer() that gracefully stops the server when disabled:

    // Missing block inside runServer() try block, before interpreter checks:
    if (!getServerEnabled(serverId)) {
        if (lsClient) {
            try {
                await lsClient.stop();
            } catch (ex) {
                traceError(`Server: Stop failed: \$\{ex}`);
            }
            lsClient = undefined;
        }
        return;
    }
  • package.json — Missing the black-formatter.serverEnabled configuration property declaration (needed for VS Code to surface the setting in the UI):

    "black-formatter.serverEnabled": {
        "default": true,
        "description": "Enable or disable the Black formatter language server.",
        "scope": "window",
        "type": "boolean"
    }

Suggested fix

src/common/settings.ts — Add serverEnabled to checkIfConfigurationChanged and the getServerEnabled export:

 export function checkIfConfigurationChanged(e: ConfigurationChangeEvent, namespace: string): boolean {
     const settings = [
         `\$\{namespace}.args`,
         `\$\{namespace}.cwd`,
         `\$\{namespace}.path`,
         `\$\{namespace}.interpreter`,
         `\$\{namespace}.importStrategy`,
         `\$\{namespace}.showNotifications`,
+        `\$\{namespace}.serverEnabled`,
         `\$\{namespace}.serverTransport`,
     ];
     const changed = settings.map((s) => e.affectsConfiguration(s));
     return changed.includes(true);
 }

+export function getServerEnabled(namespace: string): boolean {
+    const config = getConfiguration(namespace);
+    return config.get(boolean)('serverEnabled', true);
+}

src/extension.ts — Import getServerEnabled and add the early-exit branch in runServer():

 import {
     checkIfConfigurationChanged,
     getWorkspaceSettings,
+    getServerEnabled,
     logDefaultFormatter,
     logLegacySettings,
 } from './common/settings';
     isRestarting = true;
     try {
+        if (!getServerEnabled(serverId)) {
+            if (lsClient) {
+                try {
+                    await lsClient.stop();
+                } catch (ex) {
+                    traceError(`Server: Stop failed: \$\{ex}`);
+                }
+                lsClient = undefined;
+            }
+            return;
+        }
+
         const projectRoot = await getProjectRoot();

package.json — Add the serverEnabled property to contributes.configuration.properties (using the Black-specific namespace black-formatter):

+            "black-formatter.serverEnabled": {
+                "default": true,
+                "markdownDescription": "%settings.serverEnabled.description%",
+                "scope": "window",
+                "type": "boolean"
+            },
             "black-formatter.cwd": {

Also add a corresponding entry to package.nls.json and all package.nls.*.json locale files.

Note: Preserve all Black-specific customizations (black-formatter namespace, existing markdownDescription pattern, locale key naming conventions) when applying these changes.

Files skipped

  • src/common/constants.tsLS_SERVER_RESTART_DELAY already present in this repo.
  • src/common/server.ts — ...

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Co-authored-by: edvilme <5952839+edvilme@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix duplicate server handlers on concurrent restarts Add serverEnabled setting support from upstream template Mar 4, 2026
The serverEnabled setting doesn't provide meaningful value over simply
disabling the extension. Reverting all changes from the previous commit.

Co-authored-by: edvilme <5952839+edvilme@users.noreply.github.com>
Copilot AI changed the title Add serverEnabled setting support from upstream template Revert serverEnabled setting — no value over disabling the extension Mar 4, 2026
@edvilme edvilme added the debt Technical debt or repo cleanup label Mar 4, 2026
@edvilme edvilme closed this Mar 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

debt Technical debt or repo cleanup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Template Sync: Fix duplicate server handlers on concurrent restarts

2 participants