From 130f3fea074a2ce4a67c73ca82af64889535594c Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Thu, 7 Nov 2024 17:41:31 -0500 Subject: [PATCH] Eagerly compute SHA's for watched files --- vscode/src/test/suite/workspace.test.ts | 44 +++++++++++-- vscode/src/workspace.ts | 84 +++++++++++++++---------- 2 files changed, 90 insertions(+), 38 deletions(-) diff --git a/vscode/src/test/suite/workspace.test.ts b/vscode/src/test/suite/workspace.test.ts index 69b550019..ed91e2619 100644 --- a/vscode/src/test/suite/workspace.test.ts +++ b/vscode/src/test/suite/workspace.test.ts @@ -59,15 +59,15 @@ suite("Workspace", () => { await workspace.activate(); - for (let i = 0; i < 5; i++) { - await new Promise((resolve) => setTimeout(resolve, 200)); + for (let i = 0; i < 4; i++) { + await new Promise((resolve) => setTimeout(resolve, 50)); fs.writeFileSync(path.join(gitDir, "rebase-apply"), "1"); - await new Promise((resolve) => setTimeout(resolve, 200)); + await new Promise((resolve) => setTimeout(resolve, 50)); fs.rmSync(path.join(gitDir, "rebase-apply")); } // Give enough time for all watchers to fire and all debounces to run off - await new Promise((resolve) => setTimeout(resolve, 10000)); + await new Promise((resolve) => setTimeout(resolve, 6000)); startStub.restore(); restartSpy.restore(); @@ -77,4 +77,40 @@ suite("Workspace", () => { // The restart call only happens once because of the debounce assert.strictEqual(restartSpy.callCount, 1); }).timeout(60000); + + test("does not restart when watched files are touched without modifying contents", async () => { + const lockfileUri = vscode.Uri.file( + path.join(workspacePath, "Gemfile.lock"), + ); + const contents = Buffer.from("hello"); + await vscode.workspace.fs.writeFile(lockfileUri, contents); + + const workspace = new Workspace( + context, + workspaceFolder, + FAKE_TELEMETRY, + () => {}, + new Map(), + ); + + await workspace.activate(); + + const startStub = sinon.stub(workspace, "start"); + const restartSpy = sinon.spy(workspace, "restart"); + + for (let i = 0; i < 4; i++) { + const updateDate = new Date(); + fs.utimesSync(lockfileUri.fsPath, updateDate, updateDate); + await new Promise((resolve) => setTimeout(resolve, 50)); + } + + // Give enough time for all watchers to fire and all debounces to run off + await new Promise((resolve) => setTimeout(resolve, 6000)); + + startStub.restore(); + restartSpy.restore(); + + assert.strictEqual(startStub.callCount, 0); + assert.strictEqual(restartSpy.callCount, 0); + }).timeout(10000); }); diff --git a/vscode/src/workspace.ts b/vscode/src/workspace.ts index dc054a6b1..ae00a4958 100644 --- a/vscode/src/workspace.ts +++ b/vscode/src/workspace.ts @@ -14,6 +14,13 @@ import { } from "./common"; import { WorkspaceChannel } from "./workspaceChannel"; +const WATCHED_FILES = [ + "Gemfile.lock", + "gems.locked", + ".rubocop.yml", + ".rubocop", +]; + export class Workspace implements WorkspaceInterface { public lspClient?: Client; public readonly ruby: Ruby; @@ -58,8 +65,6 @@ export class Workspace implements WorkspaceInterface { this.createTestItems = createTestItems; this.isMainWorkspace = isMainWorkspace; this.virtualDocuments = virtualDocuments; - - this.registerRestarts(context); } // Activate this workspace. This method is intended to be invoked only once, unlikely `start` which may be invoked @@ -82,6 +87,20 @@ export class Workspace implements WorkspaceInterface { rootGitUri, ".git/{rebase-merge,rebase-apply,BISECT_START,CHERRY_PICK_HEAD}", ); + + this.registerRestarts(); + + // Eagerly calculate SHA's for the watched files to avoid unnecessary restarts + for (const file of WATCHED_FILES) { + const uri = vscode.Uri.joinPath(this.workspaceFolder.uri, file); + const currentSha = await this.fileContentsSha(uri); + + if (!currentSha) { + continue; + } + + this.restartDocumentShas.set(uri.fsPath, currentSha); + } } async start(debugMode?: boolean) { @@ -342,26 +361,14 @@ export class Workspace implements WorkspaceInterface { return result; } - private registerRestarts(context: vscode.ExtensionContext) { - this.createRestartWatcher(context, "Gemfile.lock"); - this.createRestartWatcher(context, "gems.locked"); - this.createRestartWatcher(context, "**/.rubocop.yml"); - this.createRestartWatcher(context, ".rubocop"); + private registerRestarts() { + this.createRestartWatcher(`{${WATCHED_FILES.join(",")}}`); // If a configuration that affects the Ruby LSP has changed, update the client options using the latest // configuration and restart the server - context.subscriptions.push( + this.context.subscriptions.push( vscode.workspace.onDidChangeConfiguration(async (event) => { if (event.affectsConfiguration("rubyLsp")) { - // Re-activate Ruby if the version manager changed - if ( - event.affectsConfiguration("rubyLsp.rubyVersionManager") || - event.affectsConfiguration("rubyLsp.bundleGemfile") || - event.affectsConfiguration("rubyLsp.customRubyCommand") - ) { - await this.ruby.activateRuby(); - } - this.outputChannel.info( "Restarting the Ruby LSP because configuration changed", ); @@ -371,10 +378,7 @@ export class Workspace implements WorkspaceInterface { ); } - private createRestartWatcher( - context: vscode.ExtensionContext, - pattern: string, - ) { + private createRestartWatcher(pattern: string) { const watcher = vscode.workspace.createFileSystemWatcher( new vscode.RelativePattern(this.workspaceFolder, pattern), ); @@ -382,27 +386,27 @@ export class Workspace implements WorkspaceInterface { // Handler for only triggering restart if the contents of the file have been modified. If the file was just touched, // but the contents are the same, we don't want to restart const debouncedRestartWithHashCheck = async (uri: vscode.Uri) => { - const fileContents = await vscode.workspace.fs.readFile(uri); const fsPath = uri.fsPath; + const currentSha = await this.fileContentsSha(uri); - const hash = createHash("sha256"); - hash.update(fileContents.toString()); - const currentSha = hash.digest("hex"); - - if (this.restartDocumentShas.get(fsPath) !== currentSha) { + if (currentSha && this.restartDocumentShas.get(fsPath) !== currentSha) { this.restartDocumentShas.set(fsPath, currentSha); - await this.debouncedRestart(`${pattern} changed`); + await this.debouncedRestart(`${fsPath} changed, matching ${pattern}`); } }; - const debouncedRestart = async () => { - await this.debouncedRestart(`${pattern} changed`); + const debouncedRestart = async (uri: vscode.Uri) => { + this.restartDocumentShas.delete(uri.fsPath); + await this.debouncedRestart(`${uri.fsPath} changed, matching ${pattern}`); }; - context.subscriptions.push( + this.context.subscriptions.push( watcher, watcher.onDidChange(debouncedRestartWithHashCheck), - watcher.onDidCreate(debouncedRestart), + // Interestingly, we are seeing create events being fired even when the file already exists. If a create event is + // fired for an update to an existing file, then we need to check if the contents still match to prevent unwanted + // restarts + watcher.onDidCreate(debouncedRestartWithHashCheck), watcher.onDidDelete(debouncedRestart), ); } @@ -416,9 +420,9 @@ export class Workspace implements WorkspaceInterface { this.#inhibitRestart = true; }; - const stop = async () => { + const stop = async (uri: vscode.Uri) => { this.#inhibitRestart = false; - await this.debouncedRestart(`${glob} changed`); + await this.debouncedRestart(`${uri.fsPath} changed, matching ${glob}`); }; this.context.subscriptions.push( @@ -429,4 +433,16 @@ export class Workspace implements WorkspaceInterface { workspaceWatcher.onDidDelete(stop), ); } + + private async fileContentsSha(uri: vscode.Uri): Promise { + try { + const fileContents = await vscode.workspace.fs.readFile(uri); + + const hash = createHash("sha256"); + hash.update(fileContents.toString()); + return hash.digest("hex"); + } catch (error: any) { + return undefined; + } + } }