Skip to content

Commit

Permalink
Better settings reload functionality
Browse files Browse the repository at this point in the history
  • Loading branch information
@TwitchBronBron
TwitchBronBron committed Oct 11, 2024
1 parent 2315387 commit b065e57
Show file tree
Hide file tree
Showing 5 changed files with 255 additions and 52 deletions.
45 changes: 45 additions & 0 deletions package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
"@types/command-line-usage": "^5.0.1",
"@types/debounce-promise": "^3.1.1",
"@types/fs-extra": "^8.0.0",
"@types/lodash.isequal": "^4.5.8",
"@types/marked": "^4.0.3",
"@types/micromatch": "^4.0.2",
"@types/mocha": "^5.2.5",
Expand Down Expand Up @@ -148,6 +149,7 @@
"fs-extra": "^8.1.0",
"ignore": "^5.3.1",
"jsonc-parser": "^2.3.0",
"lodash.isequal": "^4.5.0",
"long": "^3.2.0",
"luxon": "^2.5.2",
"micromatch": "^4.0.4",
Expand Down
111 changes: 109 additions & 2 deletions src/LanguageServer.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { createVisitor, WalkMode } from './astUtils/visitors';
import { tempDir, rootDir } from './testHelpers.spec';
import { URI } from 'vscode-uri';
import { BusyStatusTracker } from './BusyStatusTracker';
import type { BscFile } from '.';
import type { BscFile, WorkspaceConfigWithExtras } from '.';
import type { Project } from './lsp/Project';
import { LogLevel, Logger, createLogger } from './logging';
import { DiagnosticMessages } from './DiagnosticMessages';
Expand Down Expand Up @@ -122,6 +122,113 @@ describe('LanguageServer', () => {
}
}

describe('onDidChangeConfiguration', () => {
async function doTest(startingConfigs: WorkspaceConfigWithExtras[], endingConfigs: WorkspaceConfigWithExtras[]) {
(server as any)['connection'] = connection;
server['workspaceConfigsCache'] = new Map(startingConfigs.map(x => [x.workspaceFolder, x]));

const stub = sinon.stub(server as any, 'getWorkspaceConfigs').returns(Promise.resolve(endingConfigs));

await server.onDidChangeConfiguration({ settings: {} });
stub.restore();
}

it('does not reload project when: no projects are present before and after', async () => {
const stub = sinon.stub(server as any, 'syncProjects').callsFake(() => Promise.resolve());
await doTest([], []);
expect(stub.called).to.be.false;
});

it('does not reload project when: 1 project is unchanged', async () => {
const stub = sinon.stub(server as any, 'syncProjects').callsFake(() => Promise.resolve());
await doTest([{
bsconfigPath: undefined,
languageServer: {
enableThreading: true,
logLevel: 'info'
},
workspaceFolder: workspacePath,
excludePatterns: []
}], [{
bsconfigPath: undefined,
languageServer: {
enableThreading: true,
logLevel: 'info'
},
workspaceFolder: workspacePath,
excludePatterns: []
}]);
expect(stub.called).to.be.false;
});

it('reloads project when adding new project', async () => {
const stub = sinon.stub(server as any, 'syncProjects').callsFake(() => Promise.resolve());
await doTest([], [{
bsconfigPath: undefined,
languageServer: {
enableThreading: true,
logLevel: 'info'
},
workspaceFolder: workspacePath,
excludePatterns: []
}]);
expect(stub.called).to.be.true;
});

it('reloads project when deleting a project', async () => {
const stub = sinon.stub(server as any, 'syncProjects').callsFake(() => Promise.resolve());
await doTest([{
bsconfigPath: undefined,
languageServer: {
enableThreading: true,
logLevel: 'info'
},
workspaceFolder: workspacePath,
excludePatterns: []
}, {
bsconfigPath: undefined,
languageServer: {
enableThreading: true,
logLevel: 'info'
},
workspaceFolder: s`${tempDir}/project2`,
excludePatterns: []
}], [{
bsconfigPath: undefined,
languageServer: {
enableThreading: true,
logLevel: 'info'
},
workspaceFolder: workspacePath,
excludePatterns: []
}]);
expect(stub.called).to.be.true;
});

it('reloads project when changing specific settings', async () => {
const stub = sinon.stub(server as any, 'syncProjects').callsFake(() => Promise.resolve());
await doTest([{
bsconfigPath: undefined,
languageServer: {
enableThreading: true,
logLevel: 'trace'
},
workspaceFolder: workspacePath,
excludePatterns: []
}], [{
bsconfigPath: undefined,
languageServer: {
enableThreading: true,
logLevel: 'info'
},
workspaceFolder: workspacePath,
excludePatterns: []
}]);
expect(stub.called).to.be.true;
});

});

describe('sendDiagnostics', () => {
it('dedupes diagnostics found at same location from multiple projects', async () => {
fsExtra.outputFileSync(s`${rootDir}/common/lib.brs`, `
Expand Down Expand Up @@ -497,7 +604,7 @@ describe('LanguageServer', () => {
it('rebuilds the path filterer when certain files are changed', async () => {

sinon.stub(server['projectManager'], 'handleFileChanges').callsFake(() => Promise.resolve());

(server as any)['connection'] = connection;
async function test(filePath: string, expected = true) {
const stub = sinon.stub(server as any, 'rebuildPathFilterer');

Expand Down
96 changes: 64 additions & 32 deletions src/LanguageServer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ import { ProjectManager } from './lsp/ProjectManager';
import * as fsExtra from 'fs-extra';
import type { MaybePromise } from './interfaces';
import { workerPool } from './lsp/worker/WorkerThreadProject';
// eslint-disable-next-line @typescript-eslint/no-require-imports
import isEqual = require('lodash.isequal');

export class LanguageServer {
/**
Expand Down Expand Up @@ -230,6 +232,11 @@ export class LanguageServer {
public async onInitialized() {
this.logger.log('onInitialized');

//cache a copy of all workspace configurations to use for comparison later
this.workspaceConfigsCache = new Map(
(await this.getWorkspaceConfigs()).map(x => [x.workspaceFolder, x])
);

//set our logger to the most verbose logLevel found across any project
await this.syncLogLevel();

Expand Down Expand Up @@ -309,15 +316,12 @@ export class LanguageServer {
}
};

let workspaceResult = await getLogLevel(
await this.connection.workspace.getWorkspaceFolders(),
async (workspace) => {
const config = (await this.getClientConfiguration<BrightScriptClientConfiguration>(workspace.uri, 'brightscript'));
return config?.languageServer?.logLevel;
}
);
const workspaces = await this.getWorkspaceConfigs();

let workspaceResult = await getLogLevel(workspaces, workspace => workspace?.languageServer?.logLevel);

if (workspaceResult) {
this.logger.info(`Setting global logLevel to '${workspaceResult.logLevelText}' based on configuration from workspace '${workspaceResult?.item?.uri}'`);
this.logger.info(`Setting global logLevel to '${workspaceResult.logLevelText}' based on configuration from workspace '${workspaceResult?.item?.workspaceFolder}'`);
this.logger.logLevel = workspaceResult.logLevel;
return;
}
Expand Down Expand Up @@ -409,17 +413,55 @@ export class LanguageServer {
return completions;
}

/**
* Get a list of workspaces, and their configurations.
* Get only the settings for the workspace that are relevant to the language server. We do this so we can cache this object for use in change detection in the future.
*/
private async getWorkspaceConfigs(): Promise<WorkspaceConfigWithExtras[]> {
//get all workspace folders (we'll use these to get settings)
let workspaces = await Promise.all(
(await this.connection.workspace.getWorkspaceFolders() ?? []).map(async (x) => {
const workspaceFolder = util.uriToPath(x.uri);
const brightscriptConfig = await this.getClientConfiguration<BrightScriptClientConfiguration>(x.uri, 'brightscript');
return {
workspaceFolder: workspaceFolder,
excludePatterns: await this.getWorkspaceExcludeGlobs(workspaceFolder),
bsconfigPath: brightscriptConfig.configFile,
languageServer: {
enableThreading: brightscriptConfig.languageServer?.enableThreading ?? LanguageServer.enableThreadingDefault,
logLevel: brightscriptConfig?.languageServer?.logLevel
}

} as WorkspaceConfigWithExtras;
})
);
return workspaces;
}

private workspaceConfigsCache = new Map<string, WorkspaceConfigWithExtras>();

@AddStackToErrorMessage
public async onDidChangeConfiguration(args: DidChangeConfigurationParams) {
this.logger.log('onDidChangeConfiguration', 'Reloading all projects');

//if configuration changed, rebuild the path filterer
await this.rebuildPathFilterer();
const configs = new Map(
(await this.getWorkspaceConfigs()).map(x => [x.workspaceFolder, x])
);
//find any changed configs. This includes newly created workspaces, deleted workspaces, etc.
//TODO: enhance this to only reload specific projects, depending on the change
if (!isEqual(configs, this.workspaceConfigsCache)) {
//now that we've processed any config diffs, update the cached copy of them
this.workspaceConfigsCache = configs;

//if configuration changed, rebuild the path filterer
await this.rebuildPathFilterer();

//if the user changes any user/workspace config settings, just mass-reload all projects
await this.syncProjects(true);
//if the user changes any user/workspace config settings, just mass-reload all projects
await this.syncProjects(true);
}
}


@AddStackToErrorMessage
public async onHover(params: TextDocumentPositionParams) {
this.logger.debug('onHover', params);
Expand Down Expand Up @@ -608,20 +650,7 @@ export class LanguageServer {
* @param forceReload if true, all projects are discarded and recreated from scratch
*/
private async syncProjects(forceReload = false) {
// get all workspace paths from the client
let workspaces = await Promise.all(
(await this.connection.workspace.getWorkspaceFolders() ?? []).map(async (x) => {
const workspaceFolder = util.uriToPath(x.uri);
const config = await this.getClientConfiguration<BrightScriptClientConfiguration>(x.uri, 'brightscript');
return {
workspaceFolder: workspaceFolder,
excludePatterns: await this.getWorkspaceExcludeGlobs(workspaceFolder),
bsconfigPath: config.configFile,
enableThreading: config.languageServer?.enableThreading ?? LanguageServer.enableThreadingDefault

} as WorkspaceConfig;
})
);
const workspaces = await this.getWorkspaceConfigs();

await this.projectManager.syncProjects(workspaces, forceReload);

Expand All @@ -635,12 +664,7 @@ export class LanguageServer {
* @param workspaceFolder the folder for the workspace in the client
*/
private async getClientConfiguration<T extends Record<string, any>>(workspaceFolder: string, section: string): Promise<T> {
let scopeUri: string;
if (workspaceFolder.startsWith('file:')) {
scopeUri = URI.parse(workspaceFolder).toString();
} else {
scopeUri = URI.file(workspaceFolder).toString();
}
const scopeUri = util.pathToUri(workspaceFolder);
let config = {};

//if the client supports configuration, look for config group called "brightscript"
Expand Down Expand Up @@ -746,3 +770,11 @@ function logAndIgnoreError(error: Error) {
}
console.error(error);
}

export type WorkspaceConfigWithExtras = WorkspaceConfig & {
bsconfigPath: string;
languageServer: {
enableThreading: boolean;
logLevel: LogLevel | string | undefined;
};
};
Loading

0 comments on commit b065e57

Please sign in to comment.