From 6e0db6af2368472cdb9e03751f1988bda2449acb Mon Sep 17 00:00:00 2001 From: George Cook Date: Mon, 15 Feb 2021 21:04:56 +0100 Subject: [PATCH] do not revalidate on changes that are outside of any workspace (#315) --- src/LanguageServer.spec.ts | 39 ++++++++++++++++++++++++++++++++++++++ src/LanguageServer.ts | 34 +++++++++++++++++++++++---------- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/LanguageServer.spec.ts b/src/LanguageServer.spec.ts index 79f84d584..2178d1159 100644 --- a/src/LanguageServer.spec.ts +++ b/src/LanguageServer.spec.ts @@ -250,6 +250,7 @@ describe('LanguageServer', () => { let libPath = s`${workspacePath}/source/lib.brs`; writeToFs(libPath, 'sub lib(): return : end sub'); + server.workspaces[0].configFilePath = `${workspacePath}/bsconfig.json`; await svr.onDidChangeWatchedFiles({ changes: [{ uri: getFileProtocolPath(libPath), @@ -351,6 +352,44 @@ describe('LanguageServer', () => { pathAbsolute: s`${rootDir}/source/lib.brs` }]); }); + + it('does not trigger revalidates when changes are in files which are not tracked', async () => { + svr.connection = { + sendNotification: () => { } + }; + svr.workspaces.push({ + builder: { + getDiagnostics: () => [], + program: { + validate: () => { } + } + } + }); + + sinon.stub(util, 'isDirectorySync').returns(true); + sinon.stub(glob, 'sync').returns([ + s`${rootDir}/source/main.brs`, + s`${rootDir}/source/lib.brs` + ]); + const stub = sinon.stub(server, 'handleFileChanges').returns(Promise.resolve()); + + await (server as any).onDidChangeWatchedFiles({ + changes: [{ + type: FileChangeType.Created, + uri: getFileProtocolPath('some/other/folder/maybe/some/vscode/settings') + }] + } as DidChangeWatchedFilesParams); + + expect(stub.callCount).to.equal(1); + + expect(stub.getCalls()[0].args[1]).to.eql([{ + type: FileChangeType.Created, + pathAbsolute: s`${rootDir}/source/main.brs` + }, { + type: FileChangeType.Created, + pathAbsolute: s`${rootDir}/source/lib.brs` + }]); + }); }); describe('onSignatureHelp', () => { diff --git a/src/LanguageServer.ts b/src/LanguageServer.ts index 6f6d5e470..c591e2c78 100644 --- a/src/LanguageServer.ts +++ b/src/LanguageServer.ts @@ -576,11 +576,13 @@ export class LanguageServer { } }) ); - //wait for all of the programs to finish starting up - await this.waitAllProgramFirstRuns(); + if (workspaces.length > 0) { + //wait for all of the programs to finish starting up + await this.waitAllProgramFirstRuns(); - // valdiate all workspaces - this.validateAllThrottled(); //eslint-disable-line + // valdiate all workspaces + this.validateAllThrottled(); //eslint-disable-line + } } private getRootDir(workspace: Workspace) { @@ -691,8 +693,11 @@ export class LanguageServer { workspacesToReload.push(workspace); } } - //reload any workspaces that need to be reloaded - await this.reloadWorkspaces(workspacesToReload); + if (workspacesToReload.length > 0) { + //vsc can generate a ton of these changes, for vsc system files, so we need to bail if there's no work to do on any of our actual workspace files + //reload any workspaces that need to be reloaded + await this.reloadWorkspaces(workspacesToReload); + } //set the list of workspaces to non-reloaded workspaces workspaces = workspaces.filter(x => !workspacesToReload.includes(x)); @@ -736,9 +741,6 @@ export class LanguageServer { await Promise.all( workspaces.map((workspace) => this.handleFileChanges(workspace, changes)) ); - - //validate all workspaces - await this.validateAllThrottled(); } this.connection.sendNotification('build-status', 'success'); } @@ -751,11 +753,16 @@ export class LanguageServer { public async handleFileChanges(workspace: Workspace, changes: { type: FileChangeType; pathAbsolute: string }[]) { //this loop assumes paths are both file paths and folder paths, which eliminates the need to detect. //All functions below can handle being given a file path AND a folder path, and will only operate on the one they are looking for + let consumeCount = 0; await Promise.all(changes.map(async (change) => { await this.keyedThrottler.run(change.pathAbsolute, async () => { - await this.handleFileChange(workspace, change); + consumeCount += await this.handleFileChange(workspace, change) ? 1 : 0; }); })); + + if (consumeCount > 0) { + await this.validateAllThrottled(); + } } /** @@ -767,6 +774,7 @@ export class LanguageServer { const program = workspace.builder.program; const options = workspace.builder.options; const rootDir = workspace.builder.rootDir; + //deleted if (change.type === FileChangeType.Deleted) { //try to act on this path as a directory @@ -775,6 +783,9 @@ export class LanguageServer { //if this is a file loaded in the program, remove it if (program.hasFile(change.pathAbsolute)) { program.removeFile(change.pathAbsolute); + return true; + } else { + return false; } //created @@ -793,8 +804,10 @@ export class LanguageServer { }, await workspace.builder.getFileContents(change.pathAbsolute) ); + return true; } else { //no dest path means the program doesn't want this file + return false; } //changed @@ -812,6 +825,7 @@ export class LanguageServer { } else { program.removeFile(change.pathAbsolute); } + return true; } }