[vscode-go] Restart language server automatically when its configuration changes

791 views
Skip to first unread message

Gerrit Bot (Gerrit)

unread,
May 6, 2020, 1:45:38 PM5/6/20
to Rebecca Stambler, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Gerrit Bot would like Rebecca Stambler to review this change.

View Change

Restart language server automatically when its configuration changes

This change replaces the messages that tell the user to reload with automatic calls to the restart function. This way, config changes are automatically reflected - I tested it out locally and it worked pretty nicely. Most of the code to do this was already there, it was just a matter of reordering it correctly and making sure to deregister/re-register the default providers. I also added the mtime check for the language server binary as part of the config.

The only other thing that might be still missing is automatic restarts when the language server binary changes on disk, but that might be too much - probably wouldn't be intuitive for the user.

After this is merged, it will be really simple to implement #3128.

Sorry about the huge PR! It's a lot of code shuffling, and there's one function that I moved back to its original place after #3186 - sorry about that!

/cc @hyangah

Change-Id: I258dbd3a62d21da30129c08dd840d8e7ea2848e8
GitHub-Last-Rev: 9b33e86c88277092978d5415bbc4fd8be82f882f
GitHub-Pull-Request: golang/vscode-go#24
---
M src/goCheck.ts
M src/goLanguageServer.ts
M src/goMain.ts
M src/goModules.ts
4 files changed, 176 insertions(+), 155 deletions(-)

diff --git a/src/goCheck.ts b/src/goCheck.ts
index 72146cd..ac881e5 100644
--- a/src/goCheck.ts
+++ b/src/goCheck.ts
@@ -8,7 +8,7 @@
import path = require('path');
import vscode = require('vscode');
import { goBuild } from './goBuild';
-import { parseLanguageServerConfig } from './goLanguageServer';
+import { buildLanguageServerConfig } from './goLanguageServer';
import { goLint } from './goLint';
import { buildDiagnosticCollection, lintDiagnosticCollection, vetDiagnosticCollection } from './goMain';
import { isModSupported } from './goModules';
@@ -59,7 +59,7 @@

// If a user has enabled diagnostics via a language server,
// then we disable running build or vet to avoid duplicate errors and warnings.
- const lspConfig = parseLanguageServerConfig();
+ const lspConfig = buildLanguageServerConfig();
const disableBuildAndVet = lspConfig.enabled && lspConfig.features.diagnostics;

let testPromise: Thenable<boolean>;
diff --git a/src/goLanguageServer.ts b/src/goLanguageServer.ts
index 625fd95..3911d86 100644
--- a/src/goLanguageServer.ts
+++ b/src/goLanguageServer.ts
@@ -7,28 +7,40 @@

import cp = require('child_process');
import deepEqual = require('deep-equal');
+import fs = require('fs');
import moment = require('moment');
import path = require('path');
import semver = require('semver');
import util = require('util');
import vscode = require('vscode');
import {
- Command,
- HandleDiagnosticsSignature,
- LanguageClient,
- ProvideCompletionItemsSignature,
- ProvideDocumentLinksSignature,
- RevealOutputChannelOn
+ Command, HandleDiagnosticsSignature, LanguageClient, ProvideCompletionItemsSignature,
+ ProvideDocumentLinksSignature, RevealOutputChannelOn
} from 'vscode-languageclient';
import WebRequest = require('web-request');
+import { GoDefinitionProvider } from './goDeclaration';
+import { GoHoverProvider } from './goExtraInfo';
+import { GoDocumentFormattingEditProvider } from './goFormat';
+import { GoImplementationProvider } from './goImplementations';
import { promptForMissingTool, promptForUpdatingTool } from './goInstallTools';
+import { parseLiveFile } from './goLiveErrors';
+import { restartLanguageServer } from './goMain';
+import { GO_MODE } from './goMode';
+import { GoDocumentSymbolProvider } from './goOutline';
import { getToolFromToolPath } from './goPath';
+import { GoReferenceProvider } from './goReferences';
+import { GoRenameProvider } from './goRename';
+import { GoSignatureHelpProvider } from './goSignature';
+import { GoCompletionItemProvider } from './goSuggest';
+import { GoWorkspaceSymbolProvider } from './goSymbol';
import { getTool, Tool } from './goTools';
+import { GoTypeDefinitionProvider } from './goTypeDefinition';
import { getBinPath, getCurrentGoPath, getGoConfig, getToolsEnvVars } from './util';

interface LanguageServerConfig {
serverName: string;
path: string;
+ modtime: Date;
enabled: boolean;
flags: string[];
env: any;
@@ -47,38 +59,39 @@
let latestConfig: LanguageServerConfig;
let serverOutputChannel: vscode.OutputChannel;

-// startLanguageServer starts the language server (if enabled), returning
-// true on success.
-export async function registerLanguageFeatures(ctx: vscode.ExtensionContext): Promise<boolean> {
- // Subscribe to notifications for changes to the configuration of the language server.
- ctx.subscriptions.push(vscode.workspace.onDidChangeConfiguration((e) => watchLanguageServerConfiguration(e)));
+// defaultLanguageProviders is the list of providers currently registered.
+let defaultLanguageProviders: vscode.Disposable[] = [];

- const config = parseLanguageServerConfig();
- if (!config.enabled) {
- return false;
- }
+// restartCommand is the command used by the user to restart the language
+// server.
+let restartCommand: vscode.Disposable;

- // Support a command to restart the language server, if it's enabled.
- ctx.subscriptions.push(vscode.commands.registerCommand('go.languageserver.restart', () => {
- return startLanguageServer(ctx, parseLanguageServerConfig());
- }));
+// startLanguageServerWithFallback starts the language server, if enabled,
+// or falls back to the default language providers.
+export async function startLanguageServerWithFallback(ctx: vscode.ExtensionContext, activation: boolean) {
+ const cfg = buildLanguageServerConfig();

// If the language server is gopls, we can check if the user needs to
- // update their gopls version.
- if (config.serverName === 'gopls') {
- const tool = getTool(config.serverName);
- if (!tool) {
- return false;
- }
- const versionToUpdate = await shouldUpdateLanguageServer(tool, config.path, config.checkForUpdates);
- if (versionToUpdate) {
- promptForUpdatingTool(tool.name);
+ // update their gopls version. We do this only once per VS Code
+ // activation to avoid inundating the user.
+ if (activation && cfg.enabled && cfg.serverName === 'gopls') {
+ const tool = getTool(cfg.serverName);
+ if (tool) {
+ const versionToUpdate = await shouldUpdateLanguageServer(tool, cfg.path, cfg.checkForUpdates);
+ if (versionToUpdate) {
+ promptForUpdatingTool(tool.name);
+ }
}
}

- // This function handles the case when the server isn't started yet,
- // so we can call it to start the language server.
- return startLanguageServer(ctx, config);
+ const started = await startLanguageServer(ctx, cfg);
+
+ // If the server has been disabled, or failed to start,
+ // fall back to the default providers, while making sure not to
+ // re-register any providers.
+ if (!started && defaultLanguageProviders.length === 0) {
+ registerDefaultProviders(ctx);
+ }
}

async function startLanguageServer(ctx: vscode.ExtensionContext, config: LanguageServerConfig): Promise<boolean> {
@@ -97,28 +110,39 @@
// Check if we should recreate the language client. This may be necessary
// if the user has changed settings in their config.
if (!deepEqual(latestConfig, config)) {
- // Track the latest config used to start the language server.
+ // Track the latest config used to start the language server,
+ // and rebuild the language client.
latestConfig = config;
-
- // If the user has not enabled or installed the language server, return.
- if (!config.enabled || !config.path) {
- return false;
- }
- buildLanguageClient(config);
+ languageClient = await buildLanguageClient(config);
}

+ // If the user has not enabled the language server, return early.
+ if (!config.enabled) {
+ return false;
+ }
+
+ // Set up the command to allow the user to manually restart the
+ // language server.
+ if (!restartCommand) {
+ restartCommand = vscode.commands.registerCommand('go.languageserver.restart', restartLanguageServer);
+ ctx.subscriptions.push(restartCommand);
+ }
+
+ // Before starting the language server, make sure to deregister any
+ // currently registered language providers.
+ disposeDefaultProviders();
+
languageServerDisposable = languageClient.start();
ctx.subscriptions.push(languageServerDisposable);
-
return true;
}

-function buildLanguageClient(config: LanguageServerConfig) {
+async function buildLanguageClient(config: LanguageServerConfig): Promise<LanguageClient> {
// Reuse the same output channel for each instance of the server.
- if (!serverOutputChannel) {
+ if (config.enabled && !serverOutputChannel) {
serverOutputChannel = vscode.window.createOutputChannel(config.serverName);
}
- languageClient = new LanguageClient(
+ const c = new LanguageClient(
config.serverName,
{
command: config.path,
@@ -212,7 +236,7 @@
}
}
);
- languageClient.onReady().then(() => {
+ c.onReady().then(() => {
const capabilities = languageClient.initializeResult && languageClient.initializeResult.capabilities;
if (!capabilities) {
return vscode.window.showErrorMessage(
@@ -220,52 +244,70 @@
);
}
});
+ return c;
}

-function watchLanguageServerConfiguration(e: vscode.ConfigurationChangeEvent) {
+// registerUsualProviders registers the language feature providers if the language server is not enabled.
+function registerDefaultProviders(ctx: vscode.ExtensionContext) {
+ const completionProvider = new GoCompletionItemProvider(ctx.globalState);
+ defaultLanguageProviders.push(completionProvider);
+ defaultLanguageProviders.push(vscode.languages.registerCompletionItemProvider(GO_MODE, completionProvider, '.', '"'));
+ defaultLanguageProviders.push(vscode.languages.registerHoverProvider(GO_MODE, new GoHoverProvider()));
+ defaultLanguageProviders.push(vscode.languages.registerDefinitionProvider(GO_MODE, new GoDefinitionProvider()));
+ defaultLanguageProviders.push(vscode.languages.registerReferenceProvider(GO_MODE, new GoReferenceProvider()));
+ defaultLanguageProviders.push(
+ vscode.languages.registerDocumentSymbolProvider(GO_MODE, new GoDocumentSymbolProvider())
+ );
+ defaultLanguageProviders.push(vscode.languages.registerWorkspaceSymbolProvider(new GoWorkspaceSymbolProvider()));
+ defaultLanguageProviders.push(
+ vscode.languages.registerSignatureHelpProvider(GO_MODE, new GoSignatureHelpProvider(), '(', ',')
+ );
+ defaultLanguageProviders.push(
+ vscode.languages.registerImplementationProvider(GO_MODE, new GoImplementationProvider())
+ );
+ defaultLanguageProviders.push(
+ vscode.languages.registerDocumentFormattingEditProvider(GO_MODE, new GoDocumentFormattingEditProvider())
+ );
+ defaultLanguageProviders.push(
+ vscode.languages.registerTypeDefinitionProvider(GO_MODE, new GoTypeDefinitionProvider())
+ );
+ defaultLanguageProviders.push(vscode.languages.registerRenameProvider(GO_MODE, new GoRenameProvider()));
+ defaultLanguageProviders.push(vscode.workspace.onDidChangeTextDocument(parseLiveFile, null, ctx.subscriptions));
+
+ for (const provider of defaultLanguageProviders) {
+ ctx.subscriptions.push(provider);
+ }
+}
+
+function disposeDefaultProviders() {
+ for (const disposable of defaultLanguageProviders) {
+ disposable.dispose();
+ }
+ defaultLanguageProviders = [];
+}
+
+export function watchLanguageServerConfiguration(e: vscode.ConfigurationChangeEvent) {
if (!e.affectsConfiguration('go')) {
return;
}

- const config = parseLanguageServerConfig();
- let reloadMessage: string;
-
- // If the user has disabled or enabled the language server.
- if (e.affectsConfiguration('go.useLanguageServer')) {
- if (config.enabled) {
- reloadMessage = 'Reload VS Code window to enable the use of language server';
- } else {
- reloadMessage = 'Reload VS Code window to disable the use of language server';
- }
- }
-
if (
+ e.affectsConfiguration('go.useLanguageServer') ||
e.affectsConfiguration('go.languageServerFlags') ||
e.affectsConfiguration('go.languageServerExperimentalFeatures')
) {
- reloadMessage = 'Reload VS Code window for the changes in language server settings to take effect';
- }
-
- // If there was a change in the configuration of the language server,
- // then ask the user to reload VS Code.
- if (reloadMessage) {
- vscode.window.showInformationMessage(reloadMessage, 'Reload').then((selected) => {
- if (selected === 'Reload') {
- vscode.commands.executeCommand('workbench.action.reloadWindow');
- }
- });
+ restartLanguageServer();
}
}

-export function parseLanguageServerConfig(): LanguageServerConfig {
+export function buildLanguageServerConfig(): LanguageServerConfig {
const goConfig = getGoConfig();
const toolsEnv = getToolsEnvVars();
- const languageServerPath = getLanguageServerToolPath();
- const languageServerName = getToolFromToolPath(languageServerPath);
- return {
- serverName: languageServerName,
- path: languageServerPath,
- enabled: goConfig['useLanguageServer'],
+ const cfg: LanguageServerConfig = {
+ serverName: '',
+ path: '',
+ modtime: null,
+ enabled: goConfig['useLanguageServer'] === true,
flags: goConfig['languageServerFlags'] || [],
features: {
// TODO: We should have configs that match these names.
@@ -276,21 +318,45 @@
env: toolsEnv,
checkForUpdates: goConfig['useGoProxyToCheckForToolUpdates']
};
+ // Don't look for the path if the server is not enabled.
+ if (!cfg.enabled) {
+ return cfg;
+ }
+ const languageServerPath = getLanguageServerToolPath();
+ if (!languageServerPath) {
+ // Assume the getLanguageServerToolPath will show the relevant
+ // errors to the user. Disable the language server.
+ cfg.enabled = false;
+ return cfg;
+ }
+ cfg.path = languageServerPath;
+ cfg.serverName = getToolFromToolPath(cfg.path);
+
+ // Get the mtime of the language server binary so that we always pick up
+ // the right version.
+ const stats = fs.statSync(languageServerPath);
+ if (!stats) {
+ vscode.window.showErrorMessage(`Unable to stat path to language server binary: ${languageServerPath}.
+Please try reinstalling it.`);
+ // Disable the language server.
+ cfg.enabled = false;
+ return cfg;
+ }
+ cfg.modtime = stats.mtime;
+
+ return cfg;
}

/**
*
- * If the user has enabled the language server, return the absolute path to the
- * correct binary. If the required tool is not available, prompt the user to
- * install it. Only gopls is officially supported.
+ * Return the absolute path to the correct binary. If the required tool is not available,
+ * prompt the user to install it. Only gopls is officially supported.
*/
export function getLanguageServerToolPath(): string {
- // If language server is not enabled, return
const goConfig = getGoConfig();
if (!goConfig['useLanguageServer']) {
return;
}
-
// Check that all workspace folders are configured with the same GOPATH.
if (!allFoldersHaveSameGopath()) {
vscode.window.showInformationMessage(
diff --git a/src/goMain.ts b/src/goMain.ts
index f9684d4..c604b49 100644
--- a/src/goMain.ts
+++ b/src/goMain.ts
@@ -13,82 +13,54 @@
import { check, notifyIfGeneratedFile, removeTestStatus } from './goCheck';
import { GoCodeActionProvider } from './goCodeAction';
import {
- applyCodeCoverage,
- applyCodeCoverageToAllEditors,
- initCoverageDecorators,
- removeCodeCoverageOnFileSave,
- toggleCoverageCurrentPackage,
- trackCodeCoverageRemovalOnFileChange,
- updateCodeCoverageDecorators
+ applyCodeCoverage, applyCodeCoverageToAllEditors, initCoverageDecorators, removeCodeCoverageOnFileSave,
+ toggleCoverageCurrentPackage, trackCodeCoverageRemovalOnFileChange, updateCodeCoverageDecorators
} from './goCover';
import { GoDebugConfigurationProvider } from './goDebugConfiguration';
-import { GoDefinitionProvider } from './goDeclaration';
import { extractFunction, extractVariable } from './goDoctor';
-import { GoHoverProvider } from './goExtraInfo';
import { runFillStruct } from './goFillStruct';
-import { GoDocumentFormattingEditProvider } from './goFormat';
import * as goGenerateTests from './goGenerateTests';
import { goGetPackage } from './goGetPackage';
import { implCursor } from './goImpl';
-import { GoImplementationProvider } from './goImplementations';
import { addImport, addImportToWorkspace } from './goImport';
import { installCurrentPackage } from './goInstall';
import {
- installAllTools,
- installTools,
- offerToInstallTools,
- promptForMissingTool,
+ installAllTools, installTools, offerToInstallTools, promptForMissingTool,
updateGoPathGoRootFromConfig
} from './goInstallTools';
-import { registerLanguageFeatures } from './goLanguageServer';
+import { startLanguageServerWithFallback, watchLanguageServerConfiguration } from './goLanguageServer';
import { lintCode } from './goLint';
-import { parseLiveFile } from './goLiveErrors';
import { GO_MODE } from './goMode';
import { addTags, removeTags } from './goModifytags';
import { GO111MODULE, isModSupported } from './goModules';
-import { GoDocumentSymbolProvider } from './goOutline';
import { clearCacheForTools, fileExists } from './goPath';
import { playgroundCommand } from './goPlayground';
-import { GoReferenceProvider } from './goReferences';
import { GoReferencesCodeLensProvider } from './goReferencesCodelens';
-import { GoRenameProvider } from './goRename';
import { GoRunTestCodeLensProvider } from './goRunTestCodelens';
-import { GoSignatureHelpProvider } from './goSignature';
import { outputChannel, showHideStatus } from './goStatus';
-import { GoCompletionItemProvider } from './goSuggest';
-import { GoWorkspaceSymbolProvider } from './goSymbol';
import { testAtCursor, testCurrentFile, testCurrentPackage, testPrevious, testWorkspace } from './goTest';
import { getConfiguredTools } from './goTools';
-import { GoTypeDefinitionProvider } from './goTypeDefinition';
import { vetCode } from './goVet';
import {
- getFromGlobalState,
- getFromWorkspaceState,
- setGlobalState,
- setWorkspaceState,
- updateGlobalState,
+ getFromGlobalState, getFromWorkspaceState, setGlobalState, setWorkspaceState, updateGlobalState,
updateWorkspaceState
} from './stateUtils';
import { disposeTelemetryReporter, sendTelemetryEventForConfig } from './telemetry';
import { cancelRunningTests, showTestOutput } from './testUtils';
import {
- cleanupTempDir,
- getBinPath,
- getCurrentGoPath,
- getExtensionCommands,
- getGoConfig,
- getGoVersion,
- getToolsEnvVars,
- getToolsGopath,
- getWorkspaceFolderPath,
- handleDiagnosticErrors,
- isGoPathSet
+ cleanupTempDir, getBinPath, getCurrentGoPath, getExtensionCommands, getGoConfig,
+ getGoVersion, getToolsEnvVars, getToolsGopath, getWorkspaceFolderPath, handleDiagnosticErrors, isGoPathSet
} from './util';

export let buildDiagnosticCollection: vscode.DiagnosticCollection;
export let lintDiagnosticCollection: vscode.DiagnosticCollection;
export let vetDiagnosticCollection: vscode.DiagnosticCollection;

+// restartLanguageServer wraps all of the logic needed to restart the
+// language server. It can be used to enable, disable, or otherwise change
+// the configuration of the server.
+export let restartLanguageServer: () => {};
+
export function activate(ctx: vscode.ExtensionContext): void {
setGlobalState(ctx.globalState);
setWorkspaceState(ctx.workspaceState);
@@ -145,12 +117,21 @@

offerToInstallTools();

- // This handles all of the configurations and registrations for the language server.
- // It also registers the necessary language feature providers that the language server may not support.
- const ok = await registerLanguageFeatures(ctx);
- if (!ok) {
- registerUsualProviders(ctx);
- }
+ // Subscribe to notifications for changes to the configuration
+ // of the language server, even if it's not currently in use.
+ ctx.subscriptions.push(vscode.workspace.onDidChangeConfiguration(
+ (e) => watchLanguageServerConfiguration(e)
+ ));
+
+ // Set the function that is used to restart the language server.
+ // This is necessary, even if the language server is not currently
+ // in use.
+ restartLanguageServer = async () => {
+ startLanguageServerWithFallback(ctx, false);
+ };
+
+ // Start the language server, or fallback to the default language providers.
+ startLanguageServerWithFallback(ctx, true);

if (
vscode.window.activeTextEditor &&
@@ -623,28 +604,6 @@
);
}

-// registerUsualProviders registers the language feature providers if the language server is not enabled.
-function registerUsualProviders(ctx: vscode.ExtensionContext) {
- const provider = new GoCompletionItemProvider(ctx.globalState);
- ctx.subscriptions.push(provider);
- ctx.subscriptions.push(vscode.languages.registerCompletionItemProvider(GO_MODE, provider, '.', '"'));
- ctx.subscriptions.push(vscode.languages.registerHoverProvider(GO_MODE, new GoHoverProvider()));
- ctx.subscriptions.push(vscode.languages.registerDefinitionProvider(GO_MODE, new GoDefinitionProvider()));
- ctx.subscriptions.push(vscode.languages.registerReferenceProvider(GO_MODE, new GoReferenceProvider()));
- ctx.subscriptions.push(vscode.languages.registerDocumentSymbolProvider(GO_MODE, new GoDocumentSymbolProvider()));
- ctx.subscriptions.push(vscode.languages.registerWorkspaceSymbolProvider(new GoWorkspaceSymbolProvider()));
- ctx.subscriptions.push(
- vscode.languages.registerSignatureHelpProvider(GO_MODE, new GoSignatureHelpProvider(), '(', ',')
- );
- ctx.subscriptions.push(vscode.languages.registerImplementationProvider(GO_MODE, new GoImplementationProvider()));
- ctx.subscriptions.push(
- vscode.languages.registerDocumentFormattingEditProvider(GO_MODE, new GoDocumentFormattingEditProvider())
- );
- ctx.subscriptions.push(vscode.languages.registerTypeDefinitionProvider(GO_MODE, new GoTypeDefinitionProvider()));
- ctx.subscriptions.push(vscode.languages.registerRenameProvider(GO_MODE, new GoRenameProvider()));
- vscode.workspace.onDidChangeTextDocument(parseLiveFile, null, ctx.subscriptions);
-}
-
function addOnChangeTextDocumentListeners(ctx: vscode.ExtensionContext) {
vscode.workspace.onDidChangeTextDocument(trackCodeCoverageRemovalOnFileChange, null, ctx.subscriptions);
vscode.workspace.onDidChangeTextDocument(removeTestStatus, null, ctx.subscriptions);
diff --git a/src/goModules.ts b/src/goModules.ts
index d50e75a..f203960 100644
--- a/src/goModules.ts
+++ b/src/goModules.ts
@@ -7,6 +7,7 @@
import path = require('path');
import vscode = require('vscode');
import { installTools } from './goInstallTools';
+import { restartLanguageServer } from './goMain';
import { envPath, fixDriveCasingInWindows } from './goPath';
import { getTool } from './goTools';
import { getFromGlobalState, updateGlobalState } from './stateUtils';
@@ -135,12 +136,7 @@
if (goConfig.inspect('useLanguageServer').workspaceFolderValue === false) {
goConfig.update('useLanguageServer', true, vscode.ConfigurationTarget.WorkspaceFolder);
}
- const reloadMsg = 'Reload VS Code window to enable the use of Go language server';
- vscode.window.showInformationMessage(reloadMsg, 'Reload').then((selectedForReload) => {
- if (selectedForReload === 'Reload') {
- vscode.commands.executeCommand('workbench.action.reloadWindow');
- }
- });
+ restartLanguageServer();
}
});
}

To view, visit change 232598. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: vscode-go
Gerrit-Branch: master
Gerrit-Change-Id: I258dbd3a62d21da30129c08dd840d8e7ea2848e8
Gerrit-Change-Number: 232598
Gerrit-PatchSet: 1
Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
Gerrit-Reviewer: Rebecca Stambler <rsta...@golang.org>
Gerrit-MessageType: newchange

Gerrit Bot (Gerrit)

unread,
May 6, 2020, 1:48:20 PM5/6/20
to Rebecca Stambler, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Gerrit Bot uploaded patch set #2 to this change.

View Change

src: restart language server automatically when its configuration changes


This change replaces the messages that tell the user to reload with automatic calls to the restart function. This way, config changes are automatically reflected - I tested it out locally and it worked pretty nicely. Most of the code to do this was already there, it was just a matter of reordering it correctly and making sure to deregister/re-register the default providers. I also added the mtime check for the language server binary as part of the config.

The only other thing that might be still missing is automatic restarts when the language server binary changes on disk, but that might be too much - probably wouldn't be intuitive for the user.

After this is merged, it will be really simple to implement #3128.

Sorry about the huge PR! It's a lot of code shuffling, and there's one function that I moved back to its original place after #3186 - sorry about that!

/cc @hyangah

Change-Id: I258dbd3a62d21da30129c08dd840d8e7ea2848e8
GitHub-Last-Rev: 9b33e86c88277092978d5415bbc4fd8be82f882f
GitHub-Pull-Request: golang/vscode-go#24
---
M src/goCheck.ts
M src/goLanguageServer.ts
M src/goMain.ts
M src/goModules.ts
4 files changed, 176 insertions(+), 155 deletions(-)

To view, visit change 232598. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: vscode-go
Gerrit-Branch: master
Gerrit-Change-Id: I258dbd3a62d21da30129c08dd840d8e7ea2848e8
Gerrit-Change-Number: 232598
Gerrit-PatchSet: 2
Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
Gerrit-Reviewer: Rebecca Stambler <rsta...@golang.org>
Gerrit-MessageType: newpatchset

Hyang-Ah Hana Kim (Gerrit)

unread,
May 6, 2020, 2:20:46 PM5/6/20
to Rebecca Stambler, Gerrit Bot, goph...@pubsubhelper.golang.org, Hyang-Ah Hana Kim, golang-co...@googlegroups.com

Already reviewed and approved in https://github.com/microsoft/vscode-go/pull/3211

Patch set 2:Code-Review +2

View Change

    To view, visit change 232598. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: vscode-go
    Gerrit-Branch: master
    Gerrit-Change-Id: I258dbd3a62d21da30129c08dd840d8e7ea2848e8
    Gerrit-Change-Number: 232598
    Gerrit-PatchSet: 2
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Comment-Date: Wed, 06 May 2020 18:20:42 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Gerrit Bot (Gerrit)

    unread,
    May 6, 2020, 2:51:51 PM5/6/20
    to Rebecca Stambler, Hyang-Ah Hana Kim, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

    Gerrit Bot uploaded patch set #3 to this change.

    View Change

    src: restart language server automatically when its configuration changes


    This change replaces the messages that tell the user to reload with automatic calls to the restart function. This way, config changes are automatically reflected - I tested it out locally and it worked pretty nicely. Most of the code to do this was already there, it was just a matter of reordering it correctly and making sure to deregister/re-register the default providers. I also added the mtime check for the language server binary as part of the config.

    The only other thing that might be still missing is automatic restarts when the language server binary changes on disk, but that might be too much - probably wouldn't be intuitive for the user.

    After this is merged, it will be really simple to implement #3128.

    Sorry about the huge PR! It's a lot of code shuffling, and there's one function that I moved back to its original place after #3186 - sorry about that!

    /cc @hyangah

    Change-Id: I258dbd3a62d21da30129c08dd840d8e7ea2848e8
    GitHub-Last-Rev: 29fd4d68c860e55b64eb2a41676f9dcd299f30ea

    GitHub-Pull-Request: golang/vscode-go#24
    ---
    M src/goCheck.ts
    M src/goLanguageServer.ts
    M src/goMain.ts
    M src/goModules.ts
    4 files changed, 176 insertions(+), 155 deletions(-)

    To view, visit change 232598. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: vscode-go
    Gerrit-Branch: master
    Gerrit-Change-Id: I258dbd3a62d21da30129c08dd840d8e7ea2848e8
    Gerrit-Change-Number: 232598
    Gerrit-PatchSet: 3
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Rebecca Stambler <rsta...@golang.org>
    Gerrit-MessageType: newpatchset

    Rebecca Stambler (Gerrit)

    unread,
    May 6, 2020, 2:54:33 PM5/6/20
    to Gerrit Bot, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Hyang-Ah Hana Kim, golang-co...@googlegroups.com

    Rebecca Stambler submitted this change.

    View Change

    Approvals: Hyang-Ah Hana Kim: Looks good to me, approved
    src: restart language server automatically when its configuration changes


    This change replaces the messages that tell the user to reload with automatic calls to the restart function. This way, config changes are automatically reflected - I tested it out locally and it worked pretty nicely. Most of the code to do this was already there, it was just a matter of reordering it correctly and making sure to deregister/re-register the default providers. I also added the mtime check for the language server binary as part of the config.

    The only other thing that might be still missing is automatic restarts when the language server binary changes on disk, but that might be too much - probably wouldn't be intuitive for the user.

    After this is merged, it will be really simple to implement #3128.

    Sorry about the huge PR! It's a lot of code shuffling, and there's one function that I moved back to its original place after #3186 - sorry about that!

    /cc @hyangah

    Change-Id: I258dbd3a62d21da30129c08dd840d8e7ea2848e8
    GitHub-Last-Rev: 29fd4d68c860e55b64eb2a41676f9dcd299f30ea
    GitHub-Pull-Request: golang/vscode-go#24
    Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/232598
    Reviewed-by: Hyang-Ah Hana Kim <hya...@gmail.com>
    index c3efb43..3ebc650 100644
     		'go',  // id
    config.serverName, // name
    {
    @@ -213,7 +237,7 @@

    }
    }
    );
    - languageClient.onReady().then(() => {
    + c.onReady().then(() => {
    const capabilities = languageClient.initializeResult && languageClient.initializeResult.capabilities;
    if (!capabilities) {
    return vscode.window.showErrorMessage(
    @@ -221,52 +245,70 @@
    @@ -277,21 +319,45 @@
    index 7f34b11..5003eff 100644
    --- a/src/goMain.ts
    +++ b/src/goMain.ts
    @@ -12,82 +12,54 @@
    @@ -144,12 +116,21 @@


    offerToInstallTools();

    - // This handles all of the configurations and registrations for the language server.
    - // It also registers the necessary language feature providers that the language server may not support.
    - const ok = await registerLanguageFeatures(ctx);
    - if (!ok) {
    - registerUsualProviders(ctx);
    - }
    + // Subscribe to notifications for changes to the configuration
    + // of the language server, even if it's not currently in use.
    + ctx.subscriptions.push(vscode.workspace.onDidChangeConfiguration(
    + (e) => watchLanguageServerConfiguration(e)
    + ));
    +
    + // Set the function that is used to restart the language server.
    + // This is necessary, even if the language server is not currently
    + // in use.
    + restartLanguageServer = async () => {
    + startLanguageServerWithFallback(ctx, false);
    + };
    +
    + // Start the language server, or fallback to the default language providers.
    + startLanguageServerWithFallback(ctx, true);

    if (
    vscode.window.activeTextEditor &&
    @@ -622,28 +603,6 @@
    Gerrit-PatchSet: 4
    Gerrit-Owner: Gerrit Bot <letsus...@gmail.com>
    Gerrit-Reviewer: Hyang-Ah Hana Kim <hya...@gmail.com>
    Gerrit-Reviewer: Rebecca Stambler <rsta...@golang.org>
    Gerrit-MessageType: merged
    Reply all
    Reply to author
    Forward
    0 new messages