Skip to content

Commit

Permalink
Trigger a window reload on use O# option change and others to workaro…
Browse files Browse the repository at this point in the history
…und restart bug
  • Loading branch information
dibarbet committed Aug 17, 2023
1 parent af19ffa commit f80420d
Show file tree
Hide file tree
Showing 8 changed files with 150 additions and 75 deletions.
56 changes: 56 additions & 0 deletions src/lsptoolshost/optionChanges.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { Observable } from 'rxjs';
import { CommonOptionsThatTriggerReload, LanguageServerOptionsThatTriggerReload, Options } from '../shared/options';
import { HandleOptionChanges, OptionChangeObserver, OptionChanges } from '../shared/observers/optionChangeObserver';
import ShowInformationMessage from '../shared/observers/utils/showInformationMessage';
import Disposable from '../disposable';
import { vscode } from '../vscodeAdapter';

export function registerLanguageServerOptionChanges(optionObservable: Observable<Options>, vscode: vscode): Disposable {
const optionChangeObserver: OptionChangeObserver = {
getRelevantOptions: () => {
return {
changedCommonOptions: CommonOptionsThatTriggerReload,
changedLanguageServerOptions: LanguageServerOptionsThatTriggerReload,
changedOmnisharpOptions: [],
};
},
handleOptionChanges(optionChanges) {
handleLanguageServerOptionChanges(optionChanges, vscode);
},
};

const disposable = HandleOptionChanges(optionObservable, optionChangeObserver);
return disposable;
}

function handleLanguageServerOptionChanges(changedOptions: OptionChanges, vscode: vscode): void {
if (changedOptions.changedCommonOptions.length == 0 && changedOptions.changedLanguageServerOptions.length == 0) {
// No changes to relevant options, do nothing.
return;
}

const reloadTitle = 'Reload Window';
const reloadCommand = 'workbench.action.reloadWindow';
if (changedOptions.changedCommonOptions.find((key) => key === 'useOmnisharpServer')) {
// If the user has changed the useOmnisharpServer flag we need to reload the window.
ShowInformationMessage(
vscode,
'The useOmnisharpServer option has changed. Please reload the window to apply the change',
{
title: reloadTitle,
command: reloadCommand,
}
);
return;
}

// Typically when we have a regular config change, we can just restart the server, but due to
// https://github.com/dotnet/vscode-csharp/issues/5882 we need to reload the window when using devkit.
const message = 'C# configuration has changed. Would you like to reload the window to apply your changes?';
ShowInformationMessage(vscode, message, { title: reloadTitle, command: reloadCommand });
}
5 changes: 5 additions & 0 deletions src/lsptoolshost/roslynLanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ import { RoslynLanguageClient } from './roslynLanguageClient';
import { registerUnitTestingCommands } from './unitTesting';
import SerializableSimplifyMethodParams from '../razor/src/simplify/serializableSimplifyMethodParams';
import { TextEdit } from 'vscode-html-languageservice';
import { registerLanguageServerOptionChanges } from './optionChanges';
import { Observable } from 'rxjs';

let _languageServer: RoslynLanguageServer;
let _channel: vscode.OutputChannel;
Expand Down Expand Up @@ -764,6 +766,7 @@ export async function activateRoslynLanguageServer(
context: vscode.ExtensionContext,
platformInfo: PlatformInformation,
optionProvider: OptionProvider,
optionObservable: Observable<Options>,
outputChannel: vscode.OutputChannel,
dotnetTestChannel: vscode.OutputChannel,
reporter: TelemetryReporter
Expand Down Expand Up @@ -799,6 +802,8 @@ export async function activateRoslynLanguageServer(
// Register any needed debugger components that need to communicate with the language server.
registerDebugger(context, _languageServer, platformInfo, optionProvider, _channel);

context.subscriptions.push(registerLanguageServerOptionChanges(optionObservable, vscode));

const options = optionProvider.GetLatestOptions();
let source = new vscode.CancellationTokenSource();
vscode.workspace.onDidChangeTextDocument(async (e) => {
Expand Down
16 changes: 3 additions & 13 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import { ErrorMessageObserver } from './observers/errorMessageObserver';
import OptionProvider from './shared/observers/optionProvider';
import DotNetTestChannelObserver from './observers/dotnetTestChannelObserver';
import DotNetTestLoggerObserver from './observers/dotnetTestLoggerObserver';
import { ShowConfigChangePrompt } from './shared/observers/optionChangeObserver';
import createOptionStream from './shared/observables/createOptionStream';
import { activateRazorExtension } from './razor/razor';
import { RazorLoggerObserver } from './observers/razorLoggerObserver';
Expand All @@ -48,7 +47,6 @@ import {
activateRoslynLanguageServer,
waitForProjectInitialization,
} from './lsptoolshost/roslynLanguageServer';
import { Options } from './shared/options';
import { MigrateOptions } from './shared/migrateOptions';
import { getBrokeredServiceContainer } from './lsptoolshost/services/brokeredServicesHosting';
import { CSharpDevKitExports } from './csharpDevKitExports';
Expand All @@ -59,6 +57,7 @@ import { csharpDevkitExtensionId, getCSharpDevKit } from './utils/getCSharpDevKi
import { BlazorDebugConfigurationProvider } from './razor/src/blazorDebug/blazorDebugConfigurationProvider';
import { RazorOmnisharpDownloader } from './razor/razorOmnisharpDownloader';
import { RoslynLanguageServerExport } from './lsptoolshost/roslynLanguageServerExportChannel';
import { registerOmnisharpOptionChanges } from './omnisharp/omnisharpOptionChanges';

export async function activate(
context: vscode.ExtensionContext
Expand Down Expand Up @@ -143,18 +142,11 @@ export async function activate(
);

context.subscriptions.push(optionProvider);
context.subscriptions.push(
ShowConfigChangePrompt(
optionStream,
'dotnet.restartServer',
Options.shouldLanguageServerOptionChangeTriggerReload,
vscode
)
);
roslynLanguageServerPromise = activateRoslynLanguageServer(
context,
platformInfo,
optionProvider,
optionStream,
csharpChannel,
dotnetTestChannel,
reporter
Expand Down Expand Up @@ -256,9 +248,7 @@ export async function activate(
);

context.subscriptions.push(optionProvider);
context.subscriptions.push(
ShowConfigChangePrompt(optionStream, 'o.restart', Options.shouldOmnisharpOptionChangeTriggerReload, vscode)
);
context.subscriptions.push(registerOmnisharpOptionChanges(vscode, optionStream));

// register JSON completion & hover providers for project.json
context.subscriptions.push(addJSONProviders());
Expand Down
33 changes: 33 additions & 0 deletions src/omnisharp/omnisharpOptionChanges.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { HandleOptionChanges, OptionChangeObserver } from '../shared/observers/optionChangeObserver';
import { CommonOptionsThatTriggerReload, OmnisharpOptionsThatTriggerReload, Options } from '../shared/options';
import ShowInformationMessage from '../shared/observers/utils/showInformationMessage';
import { vscode } from '../vscodeAdapter';
import { Observable } from 'rxjs';
import Disposable from '../disposable';

export function registerOmnisharpOptionChanges(vscode: vscode, optionObservable: Observable<Options>): Disposable {
const optionChangeObserver: OptionChangeObserver = {
getRelevantOptions: () => {
return {
changedCommonOptions: CommonOptionsThatTriggerReload,
changedLanguageServerOptions: [],
changedOmnisharpOptions: OmnisharpOptionsThatTriggerReload,
};
},
handleOptionChanges(_) {
const message =
'C# configuration has changed. Would you like to relaunch the Language Server with your changes?';
const title = 'Restart Language Server';
const commandName = 'o.restart';
ShowInformationMessage(vscode, message, { title: title, command: commandName });
},
};

const disposable = HandleOptionChanges(optionObservable, optionChangeObserver);
return disposable;
}
69 changes: 43 additions & 26 deletions src/shared/observers/optionChangeObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,55 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { vscode } from '../../vscodeAdapter';
import { Options } from '../options';
import ShowInformationMessage from './utils/showInformationMessage';
import { CommonOptions, LanguageServerOptions, OmnisharpServerOptions, Options } from '../options';
import { Observable } from 'rxjs';
import Disposable from '../../disposable';
import { filter } from 'rxjs/operators';
import { isDeepStrictEqual } from 'util';

function OptionChangeObservable(
export function HandleOptionChanges(
optionObservable: Observable<Options>,
shouldOptionChangeTriggerReload: (oldOptions: Options, newOptions: Options) => boolean
): Observable<Options> {
let options: Options;
return optionObservable.pipe(
filter((newOptions) => {
const changed = options && shouldOptionChangeTriggerReload(options, newOptions);
options = newOptions;
return changed;
})
);
}

export function ShowConfigChangePrompt(
optionObservable: Observable<Options>,
commandName: string,
shouldOptionChangeTriggerReload: (oldOptions: Options, newOptions: Options) => boolean,
vscode: vscode
optionChangeObserver: OptionChangeObserver
): Disposable {
const subscription = OptionChangeObservable(optionObservable, shouldOptionChangeTriggerReload).subscribe((_) => {
const message =
'C# configuration has changed. Would you like to relaunch the Language Server with your changes?';
ShowInformationMessage(vscode, message, { title: 'Restart Language Server', command: commandName });
let oldOptions: Options;
const subscription = optionObservable.pipe().subscribe((newOptions) => {
if (!oldOptions) {
oldOptions = newOptions;
return;
}
const relevantOptions = optionChangeObserver.getRelevantOptions();
const changedRelevantCommonOptions = relevantOptions.changedCommonOptions.filter(
(key) => !isDeepStrictEqual(oldOptions.commonOptions[key], newOptions.commonOptions[key])
);
const changedRelevantLanguageServerOptions = relevantOptions.changedLanguageServerOptions.filter(
(key) => !isDeepStrictEqual(oldOptions.languageServerOptions[key], newOptions.languageServerOptions[key])
);
const changedRelevantOmnisharpOptions = relevantOptions.changedOmnisharpOptions.filter(
(key) => !isDeepStrictEqual(oldOptions.omnisharpOptions[key], newOptions.omnisharpOptions[key])
);

if (
changedRelevantCommonOptions.length > 0 ||
changedRelevantLanguageServerOptions.length > 0 ||
changedRelevantOmnisharpOptions.length > 0
) {
optionChangeObserver.handleOptionChanges({
changedCommonOptions: changedRelevantCommonOptions,
changedLanguageServerOptions: changedRelevantLanguageServerOptions,
changedOmnisharpOptions: changedRelevantOmnisharpOptions,
});
}
});

return new Disposable(subscription);
}

export interface OptionChangeObserver {
getRelevantOptions: () => OptionChanges;
handleOptionChanges: (optionChanges: OptionChanges) => void;
}

export interface OptionChanges {
changedCommonOptions: ReadonlyArray<keyof CommonOptions>;
changedLanguageServerOptions: ReadonlyArray<keyof LanguageServerOptions>;
changedOmnisharpOptions: ReadonlyArray<keyof OmnisharpServerOptions>;
}
27 changes: 3 additions & 24 deletions src/shared/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { isDeepStrictEqual } from 'util';
import { DocumentSelector } from 'vscode-languageserver-protocol';
import { vscode, WorkspaceConfiguration } from '../vscodeAdapter';
import * as path from 'path';
Expand Down Expand Up @@ -377,26 +376,6 @@ export class Options {
);
}

public static shouldOmnisharpOptionChangeTriggerReload(oldOptions: Options, newOptions: Options): boolean {
const commonOptionsChanged = CommonOptionsThatTriggerReload.some(
(key) => !isDeepStrictEqual(oldOptions.commonOptions[key], newOptions.commonOptions[key])
);
const omnisharpOptionsChanged = OmnisharpOptionsThatTriggerReload.some(
(key) => !isDeepStrictEqual(oldOptions.omnisharpOptions[key], newOptions.omnisharpOptions[key])
);
return commonOptionsChanged || omnisharpOptionsChanged;
}

public static shouldLanguageServerOptionChangeTriggerReload(oldOptions: Options, newOptions: Options): boolean {
const commonOptionsChanged = CommonOptionsThatTriggerReload.some(
(key) => !isDeepStrictEqual(oldOptions.commonOptions[key], newOptions.commonOptions[key])
);
const languageServerOptionsChanged = LanguageServerOptionsThatTriggerReload.some(
(key) => !isDeepStrictEqual(oldOptions.languageServerOptions[key], newOptions.languageServerOptions[key])
);
return commonOptionsChanged || languageServerOptionsChanged;
}

public static getExcludedPaths(vscode: vscode, includeSearchExcludes = false): string[] {
const workspaceConfig = vscode.workspace.getConfiguration();

Expand Down Expand Up @@ -448,7 +427,7 @@ export interface CommonOptions {
unitTestDebuggingOptions: object;
}

const CommonOptionsThatTriggerReload: ReadonlyArray<keyof CommonOptions> = [
export const CommonOptionsThatTriggerReload: ReadonlyArray<keyof CommonOptions> = [
'dotnetPath',
'waitForDebugger',
'serverPath',
Expand Down Expand Up @@ -504,7 +483,7 @@ export interface OmnisharpServerOptions {
enableLspDriver?: boolean | null;
}

const OmnisharpOptionsThatTriggerReload: ReadonlyArray<keyof OmnisharpServerOptions> = [
export const OmnisharpOptionsThatTriggerReload: ReadonlyArray<keyof OmnisharpServerOptions> = [
'enableMsBuildLoadProjectsOnDemand',
'loggingLevel',
'enableEditorConfigSupport',
Expand Down Expand Up @@ -544,7 +523,7 @@ export interface LanguageServerOptions {
extensionsPaths: string[] | null;
}

const LanguageServerOptionsThatTriggerReload: ReadonlyArray<keyof LanguageServerOptions> = [
export const LanguageServerOptionsThatTriggerReload: ReadonlyArray<keyof LanguageServerOptions> = [
'logLevel',
'documentSelector',
];
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import { updateConfig, getVSCodeWithConfig } from '../testAssets/fakes';
import { timeout } from 'rxjs/operators';
import { from as observableFrom, Subject, BehaviorSubject } from 'rxjs';
import { vscode } from '../../../src/vscodeAdapter';
import { ShowConfigChangePrompt } from '../../../src/shared/observers/optionChangeObserver';
import { Options } from '../../../src/shared/options';
import { registerLanguageServerOptionChanges } from '../../../src/lsptoolshost/optionChanges';

suite('LanguageServerConfigChangeObserver', () => {
suiteSetup(() => should());
Expand All @@ -26,12 +26,7 @@ suite('LanguageServerConfigChangeObserver', () => {
setup(() => {
vscode = getVSCode();
optionObservable = new BehaviorSubject<Options>(Options.Read(vscode));
ShowConfigChangePrompt(
optionObservable,
'dotnet.restartServer',
Options.shouldLanguageServerOptionChangeTriggerReload,
vscode
);
registerLanguageServerOptionChanges(optionObservable, vscode);
commandDone = new Promise<void>((resolve) => {
signalCommandDone = () => {
resolve();
Expand All @@ -53,7 +48,7 @@ suite('LanguageServerConfigChangeObserver', () => {

test(`The information message is shown`, async () => {
expect(infoMessage).to.be.equal(
'C# configuration has changed. Would you like to relaunch the Language Server with your changes?'
'C# configuration has changed. Would you like to reload the window to apply your changes?'
);
});

Expand All @@ -63,10 +58,10 @@ suite('LanguageServerConfigChangeObserver', () => {
expect(invokedCommand).to.be.undefined;
});

test('Given an information message if the user clicks Restore, the command is executed', async () => {
test('Given an information message if the user clicks Reload, the command is executed', async () => {
doClickOk();
await commandDone;
expect(invokedCommand).to.be.equal('dotnet.restartServer');
expect(invokedCommand).to.be.equal('workbench.action.reloadWindow');
});
});
});
Expand Down
4 changes: 2 additions & 2 deletions test/unitTests/optionObserver/optionChangeObserver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import { updateConfig, getVSCodeWithConfig } from '../testAssets/fakes';
import { timeout } from 'rxjs/operators';
import { from as observableFrom, Subject, BehaviorSubject } from 'rxjs';
import { vscode } from '../../../src/vscodeAdapter';
import { ShowConfigChangePrompt } from '../../../src/shared/observers/optionChangeObserver';
import { Options } from '../../../src/shared/options';
import { registerOmnisharpOptionChanges } from '../../../src/omnisharp/omnisharpOptionChanges';

suite('OmniSharpConfigChangeObserver', () => {
suiteSetup(() => should());
Expand All @@ -26,7 +26,7 @@ suite('OmniSharpConfigChangeObserver', () => {
setup(() => {
vscode = getVSCode();
optionObservable = new BehaviorSubject<Options>(Options.Read(vscode));
ShowConfigChangePrompt(optionObservable, 'o.restart', Options.shouldOmnisharpOptionChangeTriggerReload, vscode);
registerOmnisharpOptionChanges(vscode, optionObservable);
commandDone = new Promise<void>((resolve) => {
signalCommandDone = () => {
resolve();
Expand Down

0 comments on commit f80420d

Please sign in to comment.