chore: refactoring and cleanup (#337)

karthiknadig · eleanorjboyd · commit 489961aae179 · 2025-04-25T11:58:52.000-07:00
diff --git a/src/api.ts b/src/api.ts
@@ -2,16 +2,16 @@
 // Licensed under the MIT License.
 
 import {
-    Uri,
     Disposable,
-    MarkdownString,
     Event,
+    FileChangeType,
     LogOutputChannel,
-    ThemeIcon,
-    Terminal,
+    MarkdownString,
     TaskExecution,
+    Terminal,
     TerminalOptions,
-    FileChangeType,
+    ThemeIcon,
+    Uri,
 } from 'vscode';
 
 /**
diff --git a/src/common/localize.ts b/src/common/localize.ts
@@ -169,10 +169,11 @@ export namespace EnvViewStrings {
     export const selectedWorkspaceTooltip = l10n.t('This environment is selected for workspace files');
 }
 
-export namespace ShellStartupActivationStrings {
+export namespace ActivationStrings {
     export const envCollectionDescription = l10n.t('Environment variables for shell activation');
     export const revertedShellStartupScripts = l10n.t(
         'Removed shell startup profile code for Python environment activation. See [logs](command:{0})',
         Commands.viewLogs,
     );
+    export const activatingEnvironment = l10n.t('Activating environment');
 }
diff --git a/src/features/terminal/shellStartupActivationVariablesManager.ts b/src/features/terminal/shellStartupActivationVariablesManager.ts
@@ -1,10 +1,9 @@
 import { ConfigurationChangeEvent, Disposable, GlobalEnvironmentVariableCollection } from 'vscode';
-import { DidChangeEnvironmentEventArgs } from '../../api';
-import { ShellStartupActivationStrings } from '../../common/localize';
+import { DidChangeEnvironmentEventArgs, PythonProjectEnvironmentApi } from '../../api';
+import { ActivationStrings } from '../../common/localize';
 import { getWorkspaceFolder, getWorkspaceFolders, onDidChangeConfiguration } from '../../common/workspace.apis';
-import { EnvironmentManagers } from '../../internal.api';
 import { ShellEnvsProvider } from './shells/startupProvider';
-import { getAutoActivationType } from './utils';
+import { ACT_TYPE_SHELL, getAutoActivationType } from './utils';
 
 export interface ShellStartupActivationVariablesManager extends Disposable {
     initialize(): Promise<void>;
@@ -15,14 +14,14 @@ export class ShellStartupActivationVariablesManagerImpl implements ShellStartupA
     constructor(
         private readonly envCollection: GlobalEnvironmentVariableCollection,
         private readonly shellEnvsProviders: ShellEnvsProvider[],
-        private readonly em: EnvironmentManagers,
+        private readonly api: PythonProjectEnvironmentApi,
     ) {
-        this.envCollection.description = ShellStartupActivationStrings.envCollectionDescription;
+        this.envCollection.description = ActivationStrings.envCollectionDescription;
         this.disposables.push(
             onDidChangeConfiguration(async (e: ConfigurationChangeEvent) => {
                 await this.handleConfigurationChange(e);
             }),
-            this.em.onDidChangeEnvironmentFiltered(async (e: DidChangeEnvironmentEventArgs) => {
+            this.api.onDidChangeEnvironment(async (e: DidChangeEnvironmentEventArgs) => {
                 await this.handleEnvironmentChange(e);
             }),
         );
@@ -31,7 +30,7 @@ export class ShellStartupActivationVariablesManagerImpl implements ShellStartupA
     private async handleConfigurationChange(e: ConfigurationChangeEvent) {
         if (e.affectsConfiguration('python-envs.terminal.autoActivationType')) {
             const autoActType = getAutoActivationType();
-            if (autoActType === 'shellStartup') {
+            if (autoActType === ACT_TYPE_SHELL) {
                 await this.initializeInternal();
             } else {
                 const workspaces = getWorkspaceFolders() ?? [];
@@ -49,7 +48,7 @@ export class ShellStartupActivationVariablesManagerImpl implements ShellStartupA
 
     private async handleEnvironmentChange(e: DidChangeEnvironmentEventArgs) {
         const autoActType = getAutoActivationType();
-        if (autoActType === 'shellStartup' && e.uri) {
+        if (autoActType === ACT_TYPE_SHELL && e.uri) {
             const wf = getWorkspaceFolder(e.uri);
             if (wf) {
                 const envVars = this.envCollection.getScoped({ workspaceFolder: wf });
@@ -75,7 +74,7 @@ export class ShellStartupActivationVariablesManagerImpl implements ShellStartupA
                 const collection = this.envCollection.getScoped({ workspaceFolder: workspace });
                 promises.push(
                     ...this.shellEnvsProviders.map(async (provider) => {
-                        const env = await this.em.getEnvironment(workspace.uri);
+                        const env = await this.api.getEnvironment(workspace.uri);
                         if (env) {
                             provider.updateEnvVariables(collection, env);
                         } else {
@@ -86,7 +85,7 @@ export class ShellStartupActivationVariablesManagerImpl implements ShellStartupA
             });
             await Promise.all(promises);
         } else {
-            const env = await this.em.getEnvironment(undefined);
+            const env = await this.api.getEnvironment(undefined);
             await Promise.all(
                 this.shellEnvsProviders.map(async (provider) => {
                     if (env) {
@@ -101,7 +100,7 @@ export class ShellStartupActivationVariablesManagerImpl implements ShellStartupA
 
     public async initialize(): Promise<void> {
         const autoActType = getAutoActivationType();
-        if (autoActType === 'shellStartup') {
+        if (autoActType === ACT_TYPE_SHELL) {
             await this.initializeInternal();
         }
     }
diff --git a/src/features/terminal/shellStartupSetupHandlers.ts b/src/features/terminal/shellStartupSetupHandlers.ts
@@ -1,10 +1,10 @@
 import { l10n, ProgressLocation } from 'vscode';
 import { executeCommand } from '../../common/command.api';
-import { Common, ShellStartupActivationStrings } from '../../common/localize';
+import { ActivationStrings, Common } from '../../common/localize';
 import { traceInfo, traceVerbose } from '../../common/logging';
 import { showErrorMessage, showInformationMessage, withProgress } from '../../common/window.apis';
 import { ShellScriptEditState, ShellStartupScriptProvider } from './shells/startupProvider';
-import { getAutoActivationType, setAutoActivationType } from './utils';
+import { ACT_TYPE_COMMAND, ACT_TYPE_SHELL, getAutoActivationType, setAutoActivationType } from './utils';
 
 export async function handleSettingUpShellProfile(
     providers: ShellStartupScriptProvider[],
@@ -14,7 +14,7 @@ export async function handleSettingUpShellProfile(
     const response = await showInformationMessage(
         l10n.t(
             'To use "{0}" activation, the shell profiles need to be set up. Do you want to set it up now?',
-            'shellStartup',
+            ACT_TYPE_SHELL,
         ),
         { modal: true, detail: l10n.t('Shells: {0}', shells) },
         Common.yes,
@@ -59,11 +59,11 @@ export async function handleSettingUpShellProfile(
 
 export async function cleanupStartupScripts(allProviders: ShellStartupScriptProvider[]): Promise<void> {
     await Promise.all(allProviders.map((provider) => provider.teardownScripts()));
-    if (getAutoActivationType() === 'shellStartup') {
-        setAutoActivationType('command');
+    if (getAutoActivationType() === ACT_TYPE_SHELL) {
+        setAutoActivationType(ACT_TYPE_COMMAND);
         traceInfo(
             'Setting `python-envs.terminal.autoActivationType` to `command`, after removing shell startup scripts.',
         );
     }
-    setImmediate(async () => await showInformationMessage(ShellStartupActivationStrings.revertedShellStartupScripts));
+    setImmediate(async () => await showInformationMessage(ActivationStrings.revertedShellStartupScripts));
 }
diff --git a/src/features/terminal/terminalManager.ts b/src/features/terminal/terminalManager.ts
@@ -2,6 +2,7 @@ import * as fsapi from 'fs-extra';
 import * as path from 'path';
 import { Disposable, EventEmitter, ProgressLocation, Terminal, TerminalOptions, Uri } from 'vscode';
 import { PythonEnvironment, PythonEnvironmentApi, PythonProject, PythonTerminalCreateOptions } from '../../api';
+import { ActivationStrings } from '../../common/localize';
 import { traceInfo, traceVerbose } from '../../common/logging';
 import {
     createTerminal,
@@ -23,7 +24,15 @@ import {
     TerminalActivationInternal,
     TerminalEnvironment,
 } from './terminalActivationState';
-import { AutoActivationType, getAutoActivationType, getEnvironmentForTerminal, waitForShellIntegration } from './utils';
+import {
+    ACT_TYPE_COMMAND,
+    ACT_TYPE_OFF,
+    ACT_TYPE_SHELL,
+    AutoActivationType,
+    getAutoActivationType,
+    getEnvironmentForTerminal,
+    waitForShellIntegration,
+} from './utils';
 
 export interface TerminalCreation {
     create(environment: PythonEnvironment, options: PythonTerminalCreateOptions): Promise<Terminal>;
@@ -108,7 +117,7 @@ export class TerminalManagerImpl implements TerminalManager {
             onDidChangeConfiguration(async (e) => {
                 if (e.affectsConfiguration('python-envs.terminal.autoActivationType')) {
                     const actType = getAutoActivationType();
-                    if (actType === 'shellStartup') {
+                    if (actType === ACT_TYPE_SHELL) {
                         traceInfo(`Auto activation type changed to ${actType}`);
                         const shells = new Set(
                             terminals()
@@ -178,10 +187,10 @@ export class TerminalManagerImpl implements TerminalManager {
         let isSetup = this.shellSetup.get(shellType);
         if (isSetup === true) {
             traceVerbose(`Shell profile for ${shellType} is already setup.`);
-            return 'shellStartup';
+            return ACT_TYPE_SHELL;
         } else if (isSetup === false) {
             traceVerbose(`Shell profile for ${shellType} is not set up, using command fallback.`);
-            return 'command';
+            return ACT_TYPE_COMMAND;
         }
     }
 
@@ -197,25 +206,25 @@ export class TerminalManagerImpl implements TerminalManager {
             await this.handleSetupCheck(shellType);
 
             // Check again after the setup check.
-            return this.getShellActivationType(shellType) ?? 'command';
+            return this.getShellActivationType(shellType) ?? ACT_TYPE_COMMAND;
         }
         traceInfo(`Shell startup not supported for ${shellType}, using command activation as fallback`);
-        return 'command';
+        return ACT_TYPE_COMMAND;
     }
 
     private async autoActivateOnTerminalOpen(terminal: Terminal, environment: PythonEnvironment): Promise<void> {
         let actType = getAutoActivationType();
         const shellType = identifyTerminalShell(terminal);
-        if (actType === 'shellStartup') {
+        if (actType === ACT_TYPE_SHELL) {
             actType = await this.getEffectiveActivationType(shellType);
         }
 
-        if (actType === 'command') {
+        if (actType === ACT_TYPE_COMMAND) {
             if (isActivatableEnvironment(environment)) {
                 await withProgress(
                     {
                         location: ProgressLocation.Window,
-                        title: `Activating environment: ${environment.environmentPath.fsPath}`,
+                        title: `${ActivationStrings.activatingEnvironment}: ${environment.environmentPath.fsPath}`,
                     },
                     async () => {
                         await waitForShellIntegration(terminal);
@@ -225,9 +234,9 @@ export class TerminalManagerImpl implements TerminalManager {
             } else {
                 traceVerbose(`Environment ${environment.environmentPath.fsPath} is not activatable`);
             }
-        } else if (actType === 'off') {
+        } else if (actType === ACT_TYPE_OFF) {
             traceInfo(`"python-envs.terminal.autoActivationType" is set to "${actType}", skipping auto activation`);
-        } else if (actType === 'shellStartup') {
+        } else if (actType === ACT_TYPE_SHELL) {
             traceInfo(
                 `"python-envs.terminal.autoActivationType" is set to "${actType}", terminal should be activated by shell startup script`,
             );
@@ -237,7 +246,7 @@ export class TerminalManagerImpl implements TerminalManager {
     public async create(environment: PythonEnvironment, options: PythonTerminalCreateOptions): Promise<Terminal> {
         const autoActType = getAutoActivationType();
         let envVars = options.env;
-        if (autoActType === 'shellStartup') {
+        if (autoActType === ACT_TYPE_SHELL) {
             const vars = await Promise.all(this.startupEnvProviders.map((p) => p.getEnvVariables(environment)));
 
             vars.forEach((varMap) => {
@@ -249,6 +258,7 @@ export class TerminalManagerImpl implements TerminalManager {
             });
         }
 
+        // Uncomment the code line below after the issue is resolved:
         // https://github.com/microsoft/vscode-python-environments/issues/172
         // const name = options.name ?? `Python: ${environment.displayName}`;
         const newTerminal = createTerminal({
@@ -266,7 +276,7 @@ export class TerminalManagerImpl implements TerminalManager {
             isTransient: options.isTransient,
         });
 
-        if (autoActType === 'command') {
+        if (autoActType === ACT_TYPE_COMMAND) {
             if (options.disableActivation) {
                 this.skipActivationOnOpen.add(newTerminal);
                 return newTerminal;
@@ -360,9 +370,9 @@ export class TerminalManagerImpl implements TerminalManager {
 
     public async initialize(api: PythonEnvironmentApi): Promise<void> {
         const actType = getAutoActivationType();
-        if (actType === 'command') {
+        if (actType === ACT_TYPE_COMMAND) {
             await Promise.all(terminals().map(async (t) => this.activateUsingCommand(api, t)));
-        } else if (actType === 'shellStartup') {
+        } else if (actType === ACT_TYPE_SHELL) {
             const shells = new Set(
                 terminals()
                     .map((t) => identifyTerminalShell(t))
diff --git a/src/features/terminal/utils.ts b/src/features/terminal/utils.ts
@@ -90,6 +90,9 @@ export async function getEnvironmentForTerminal(
     return env;
 }
 
+export const ACT_TYPE_SHELL = 'shellStartup';
+export const ACT_TYPE_COMMAND = 'command';
+export const ACT_TYPE_OFF = 'off';
 export type AutoActivationType = 'off' | 'command' | 'shellStartup';
 export function getAutoActivationType(): AutoActivationType {
     // 'startup' auto-activation means terminal is activated via shell startup scripts.

Original file line number	Diff line number	Diff line change
`@@ -169,10 +169,11 @@ export namespace EnvViewStrings {`
`169`	`169`	`export const selectedWorkspaceTooltip = l10n.t('This environment is selected for workspace files');`
`170`	`170`	`}`
`171`	`171`
`172`		`-export namespace ShellStartupActivationStrings {`
	`172`	`+export namespace ActivationStrings {`
`173`	`173`	`export const envCollectionDescription = l10n.t('Environment variables for shell activation');`
`174`	`174`	`export const revertedShellStartupScripts = l10n.t(`
`175`	`175`	`'Removed shell startup profile code for Python environment activation. See [logs](command:{0})',`
`176`	`176`	`Commands.viewLogs,`
`177`	`177`	`);`
	`178`	`+ export const activatingEnvironment = l10n.t('Activating environment');`
`178`	`179`	`}`
Original file line number	Diff line number	Diff line change
`@@ -90,6 +90,9 @@ export async function getEnvironmentForTerminal(`
`90`	`90`	`return env;`
`91`	`91`	`}`
`92`	`92`
	`93`	`+export const ACT_TYPE_SHELL = 'shellStartup';`
	`94`	`+export const ACT_TYPE_COMMAND = 'command';`
	`95`	`+export const ACT_TYPE_OFF = 'off';`
`93`	`96`	`export type AutoActivationType = 'off' \| 'command' \| 'shellStartup';`
`94`	`97`	`export function getAutoActivationType(): AutoActivationType {`
`95`	`98`	`// 'startup' auto-activation means terminal is activated via shell startup scripts.`