From 5e30a2bcaeea052d20854009362e7053a0fd74ee Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Tue, 8 Apr 2025 16:35:39 -0700 Subject: [PATCH] fix: avoid generating multiple events for the same environment --- src/features/envManagers.ts | 20 ++++--- src/managers/builtin/main.ts | 2 +- src/managers/builtin/venvManager.ts | 86 ++++++++++++++++++----------- 3 files changed, 68 insertions(+), 40 deletions(-) diff --git a/src/features/envManagers.ts b/src/features/envManagers.ts index 405b1cd0..6cfda52b 100644 --- a/src/features/envManagers.ts +++ b/src/features/envManagers.ts @@ -283,9 +283,10 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } } - const oldEnv = this._previousEnvironments.get(project?.uri.toString() ?? 'global'); + const key = project ? project.uri.toString() : 'global'; + const oldEnv = this._previousEnvironments.get(key); if (oldEnv?.envId.id !== environment?.envId.id) { - this._previousEnvironments.set(project?.uri.toString() ?? 'global', environment); + this._previousEnvironments.set(key, environment); setImmediate(() => this._onDidChangeEnvironmentFiltered.fire({ uri: project?.uri, new: environment, old: oldEnv }), ); @@ -320,9 +321,10 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { } const project = this.pm.get(uri); - const oldEnv = this._previousEnvironments.get(project?.uri.toString() ?? 'global'); + const key = project ? project.uri.toString() : 'global'; + const oldEnv = this._previousEnvironments.get(key); if (oldEnv?.envId.id !== environment?.envId.id) { - this._previousEnvironments.set(project?.uri.toString() ?? 'global', environment); + this._previousEnvironments.set(key, environment); events.push({ uri: project?.uri, new: environment, old: oldEnv }); } }); @@ -361,9 +363,10 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { // Always get the new first, then compare with the old. This has minor impact on the ordering of // events. But it ensures that we always get the latest environment at the time of this call. const newEnv = await manager.get(uri); - const oldEnv = this._previousEnvironments.get(project?.uri.toString() ?? 'global'); + const key = project ? project.uri.toString() : 'global'; + const oldEnv = this._previousEnvironments.get(key); if (oldEnv?.envId.id !== newEnv?.envId.id) { - this._previousEnvironments.set(project?.uri.toString() ?? 'global', newEnv); + this._previousEnvironments.set(key, newEnv); events.push({ uri: project?.uri, new: newEnv, old: oldEnv }); } }; @@ -404,9 +407,10 @@ export class PythonEnvironmentManagers implements EnvironmentManagers { // Always get the new first, then compare with the old. This has minor impact on the ordering of // events. But it ensures that we always get the latest environment at the time of this call. const newEnv = await manager.get(scope); - const oldEnv = this._previousEnvironments.get(project?.uri.toString() ?? 'global'); + const key = project ? project.uri.toString() : 'global'; + const oldEnv = this._previousEnvironments.get(key); if (oldEnv?.envId.id !== newEnv?.envId.id) { - this._previousEnvironments.set(project?.uri.toString() ?? 'global', newEnv); + this._previousEnvironments.set(key, newEnv); setImmediate(() => this._onDidChangeEnvironmentFiltered.fire({ uri: project?.uri, new: newEnv, old: oldEnv }), ); diff --git a/src/managers/builtin/main.ts b/src/managers/builtin/main.ts index f2b0832b..7171b78f 100644 --- a/src/managers/builtin/main.ts +++ b/src/managers/builtin/main.ts @@ -27,7 +27,7 @@ export async function registerSystemPythonFeatures( ); const venvDebouncedRefresh = createSimpleDebounce(500, () => { - venvManager.refresh(undefined); + venvManager.watcherRefresh(); }); const watcher = createFileSystemWatcher('{**/pyenv.cfg,**/bin/python,**/python.exe}', false, true, false); disposables.push( diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index 08fa90c0..86860c26 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -40,6 +40,7 @@ export class VenvManager implements EnvironmentManager { private collection: PythonEnvironment[] = []; private readonly fsPathToEnv: Map = new Map(); private globalEnv: PythonEnvironment | undefined; + private skipWatcherRefresh = false; private readonly _onDidChangeEnvironment = new EventEmitter(); public readonly onDidChangeEnvironment = this._onDidChangeEnvironment.event; @@ -86,44 +87,55 @@ export class VenvManager implements EnvironmentManager { } async create(scope: CreateEnvironmentScope): Promise { - let isGlobal = scope === 'global'; - if (Array.isArray(scope) && scope.length > 1) { - isGlobal = true; - } - let uri: Uri | undefined = undefined; - if (isGlobal) { - uri = await getGlobalVenvLocation(); - } else { - uri = scope instanceof Uri ? scope : (scope as Uri[])[0]; - } + try { + this.skipWatcherRefresh = true; + let isGlobal = scope === 'global'; + if (Array.isArray(scope) && scope.length > 1) { + isGlobal = true; + } + let uri: Uri | undefined = undefined; + if (isGlobal) { + uri = await getGlobalVenvLocation(); + } else { + uri = scope instanceof Uri ? scope : (scope as Uri[])[0]; + } - if (!uri) { - return; - } + if (!uri) { + return; + } - const venvRoot: Uri = uri; - const globals = await this.baseManager.getEnvironments('global'); - const environment = await createPythonVenv(this.nativeFinder, this.api, this.log, this, globals, venvRoot); - if (environment) { - this.addEnvironment(environment, true); + const venvRoot: Uri = uri; + const globals = await this.baseManager.getEnvironments('global'); + const environment = await createPythonVenv(this.nativeFinder, this.api, this.log, this, globals, venvRoot); + if (environment) { + this.addEnvironment(environment, true); + } + return environment; + } finally { + this.skipWatcherRefresh = false; } - return environment; } async remove(environment: PythonEnvironment): Promise { - await removeVenv(environment, this.log); - this.updateCollection(environment); - this._onDidChangeEnvironments.fire([{ environment, kind: EnvironmentChangeKind.remove }]); + try { + this.skipWatcherRefresh = true; - const changedUris = this.updateFsPathToEnv(environment); + await removeVenv(environment, this.log); + this.updateCollection(environment); + this._onDidChangeEnvironments.fire([{ environment, kind: EnvironmentChangeKind.remove }]); - for (const uri of changedUris) { - const newEnv = await this.get(uri); - this._onDidChangeEnvironment.fire({ uri, old: environment, new: newEnv }); - } + const changedUris = this.updateFsPathToEnv(environment); + + for (const uri of changedUris) { + const newEnv = await this.get(uri); + this._onDidChangeEnvironment.fire({ uri, old: environment, new: newEnv }); + } - if (this.globalEnv?.envId.id === environment.envId.id) { - await this.set(undefined, undefined); + if (this.globalEnv?.envId.id === environment.envId.id) { + await this.set(undefined, undefined); + } + } finally { + this.skipWatcherRefresh = false; } } @@ -148,10 +160,22 @@ export class VenvManager implements EnvironmentManager { return this.internalRefresh(scope, true, VenvManagerStrings.venvRefreshing); } - private async internalRefresh(scope: RefreshEnvironmentsScope, hardRefresh: boolean, title: string): Promise { + async watcherRefresh(): Promise { + if (this.skipWatcherRefresh) { + return; + } + return this.internalRefresh(undefined, false, VenvManagerStrings.venvRefreshing); + } + + private async internalRefresh( + scope: RefreshEnvironmentsScope, + hardRefresh: boolean, + title: string, + location: ProgressLocation = ProgressLocation.Window, + ): Promise { await withProgress( { - location: ProgressLocation.Window, + location, title, }, async () => {