fix: avoid generating multiple events for the same environment (#277)

Author: karthiknadig

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 }),
             );

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(

src/managers/builtin/venvManager.ts

@@ -40,6 +40,7 @@ export class VenvManager implements EnvironmentManager {
     private collection: PythonEnvironment[] = [];
     private readonly fsPathToEnv: Map<string, PythonEnvironment> = new Map();
     private globalEnv: PythonEnvironment | undefined;
+    private skipWatcherRefresh = false;
 
     private readonly _onDidChangeEnvironment = new EventEmitter<DidChangeEnvironmentEventArgs>();
     public readonly onDidChangeEnvironment = this._onDidChangeEnvironment.event;
@@ -86,44 +87,55 @@ export class VenvManager implements EnvironmentManager {
     }
 
     async create(scope: CreateEnvironmentScope): Promise<PythonEnvironment | undefined> {
-        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<void> {
-        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<void> {
+    async watcherRefresh(): Promise<void> {
+        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<void> {
         await withProgress(
             {
-                location: ProgressLocation.Window,
+                location,
                 title,
             },
             async () => {