Skip to content

Add EnvironmentVariableScope parameter to getEnvironmentVariables for targeted variable retrieval #689

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1211,6 +1211,31 @@ export interface PythonExecutionApi
PythonTaskRunApi,
PythonBackgroundRunApi {}

/**
* Enum representing the scope for environment variables.
*/
export enum EnvironmentVariableScope {
/**
* Core environment variables required for Python process execution
* (e.g., PATH, PYTHONPATH, PYTHONHOME). These are essential for
* running Python subprocesses and scripts.
*/
Process = 1,

/**
* User-defined environment variables from .env files that should be
* injected into terminals. These are additional variables meant for
* development environment customization.
*/
Terminal = 2,

/**
* Combines both Process and Terminal scopes to get all environment
* variables.
*/
All = Process | Terminal
}

/**
* Event arguments for when the monitored `.env` files or any other sources change.
*/
Expand Down Expand Up @@ -1240,11 +1265,13 @@ export interface PythonEnvironmentVariablesApi {
* @param uri The URI of the project, workspace or a file in a for which environment variables are required.
* @param overrides Additional environment variables to override the defaults.
* @param baseEnvVar The base environment variables that should be used as a starting point.
* @param scope The scope of environment variables to return. Defaults to All (both Process and Terminal variables).
*/
getEnvironmentVariables(
uri: Uri,
overrides?: ({ [key: string]: string | undefined } | Uri)[],
baseEnvVar?: { [key: string]: string | undefined },
scope?: EnvironmentVariableScope,
): Promise<{ [key: string]: string | undefined }>;

/**
Expand Down
49 changes: 29 additions & 20 deletions src/features/execution/envVariableManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as fsapi from 'fs-extra';
import * as path from 'path';
import { Event, EventEmitter, FileChangeType, Uri } from 'vscode';
import { Disposable } from 'vscode-jsonrpc';
import { DidChangeEnvironmentVariablesEventArgs, PythonEnvironmentVariablesApi } from '../../api';
import { DidChangeEnvironmentVariablesEventArgs, EnvironmentVariableScope, PythonEnvironmentVariablesApi } from '../../api';
import { resolveVariables } from '../../common/utils/internalVariables';
import { createFileSystemWatcher, getConfiguration } from '../../common/workspace.apis';
import { PythonProjectManager } from '../../internal.api';
Expand Down Expand Up @@ -40,36 +40,45 @@ export class PythonEnvVariableManager implements EnvVarManager {
uri: Uri,
overrides?: ({ [key: string]: string | undefined } | Uri)[],
baseEnvVar?: { [key: string]: string | undefined },
scope: EnvironmentVariableScope = EnvironmentVariableScope.All,
): Promise<{ [key: string]: string | undefined }> {
const project = this.pm.get(uri);

const base = baseEnvVar || { ...process.env };
let env = base;

const config = getConfiguration('python', project?.uri ?? uri);
let envFilePath = config.get<string>('envFile');
envFilePath = envFilePath ? path.normalize(resolveVariables(envFilePath, uri)) : undefined;

if (envFilePath && (await fsapi.pathExists(envFilePath))) {
const other = await parseEnvFile(Uri.file(envFilePath));
env = mergeEnvVariables(env, other);
// Handle Process scope - return only the base environment variables
if (scope === EnvironmentVariableScope.Process) {
return env;
}

let projectEnvFilePath = project ? path.normalize(path.join(project.uri.fsPath, '.env')) : undefined;
if (
projectEnvFilePath &&
projectEnvFilePath?.toLowerCase() !== envFilePath?.toLowerCase() &&
(await fsapi.pathExists(projectEnvFilePath))
) {
const other = await parseEnvFile(Uri.file(projectEnvFilePath));
env = mergeEnvVariables(env, other);
}
// Handle Terminal scope or All scope - include .env files
if (scope & EnvironmentVariableScope.Terminal) {
const config = getConfiguration('python', project?.uri ?? uri);
let envFilePath = config.get<string>('envFile');
envFilePath = envFilePath ? path.normalize(resolveVariables(envFilePath, uri)) : undefined;

if (overrides) {
for (const override of overrides) {
const other = override instanceof Uri ? await parseEnvFile(override) : override;
if (envFilePath && (await fsapi.pathExists(envFilePath))) {
const other = await parseEnvFile(Uri.file(envFilePath));
env = mergeEnvVariables(env, other);
}

let projectEnvFilePath = project ? path.normalize(path.join(project.uri.fsPath, '.env')) : undefined;
if (
projectEnvFilePath &&
projectEnvFilePath?.toLowerCase() !== envFilePath?.toLowerCase() &&
(await fsapi.pathExists(projectEnvFilePath))
) {
const other = await parseEnvFile(Uri.file(projectEnvFilePath));
env = mergeEnvVariables(env, other);
}

if (overrides) {
for (const override of overrides) {
const other = override instanceof Uri ? await parseEnvFile(override) : override;
env = mergeEnvVariables(env, other);
}
}
}

return env;
Expand Down
4 changes: 3 additions & 1 deletion src/features/pythonApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
PythonTerminalCreateOptions,
DidChangeEnvironmentVariablesEventArgs,
CreateEnvironmentOptions,
EnvironmentVariableScope,
} from '../api';
import {
EnvironmentManagers,
Expand Down Expand Up @@ -333,8 +334,9 @@ class PythonEnvironmentApiImpl implements PythonEnvironmentApi {
uri: Uri,
overrides?: ({ [key: string]: string | undefined } | Uri)[],
baseEnvVar?: { [key: string]: string | undefined },
scope?: EnvironmentVariableScope,
): Promise<{ [key: string]: string | undefined }> {
return this.envVarManager.getEnvironmentVariables(checkUri(uri) as Uri, overrides, baseEnvVar);
return this.envVarManager.getEnvironmentVariables(checkUri(uri) as Uri, overrides, baseEnvVar, scope);
}
}

Expand Down
12 changes: 9 additions & 3 deletions src/features/terminal/terminalEnvVarInjector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import * as fse from 'fs-extra';
import * as path from 'path';
import {
Disposable,
EnvironmentVariableScope,
EnvironmentVariableScope as VSCodeEnvironmentVariableScope,
GlobalEnvironmentVariableCollection,
workspace,
WorkspaceFolder,
} from 'vscode';
import { EnvironmentVariableScope } from '../../api';
import { traceError, traceVerbose } from '../../common/logging';
import { resolveVariables } from '../../common/utils/internalVariables';
import { getConfiguration, getWorkspaceFolder } from '../../common/workspace.apis';
Expand Down Expand Up @@ -109,7 +110,12 @@ export class TerminalEnvVarInjector implements Disposable {
private async injectEnvironmentVariablesForWorkspace(workspaceFolder: WorkspaceFolder): Promise<void> {
const workspaceUri = workspaceFolder.uri;
try {
const envVars = await this.envVarManager.getEnvironmentVariables(workspaceUri);
const envVars = await this.envVarManager.getEnvironmentVariables(
workspaceUri,
undefined,
undefined,
EnvironmentVariableScope.Terminal
);

// use scoped environment variable collection
const envVarScope = this.getEnvironmentVariableCollectionScoped({ workspaceFolder });
Expand Down Expand Up @@ -161,7 +167,7 @@ export class TerminalEnvVarInjector implements Disposable {
this.envVarCollection.clear();
}

private getEnvironmentVariableCollectionScoped(scope: EnvironmentVariableScope = {}) {
private getEnvironmentVariableCollectionScoped(scope: VSCodeEnvironmentVariableScope = {}) {
const envVarCollection = this.envVarCollection as GlobalEnvironmentVariableCollection;
return envVarCollection.getScoped(scope);
}
Expand Down
28 changes: 28 additions & 0 deletions src/test/features/envVariableManager.unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import * as sinon from 'sinon';
import { EnvironmentVariableScope } from '../../api';

suite('Environment Variable Scope Tests', () => {
test('should have correct enum values', () => {
// Test that the enum values are what we expect
sinon.assert.match(EnvironmentVariableScope.Process, 1);
sinon.assert.match(EnvironmentVariableScope.Terminal, 2);
sinon.assert.match(EnvironmentVariableScope.All, 3); // Process | Terminal = 1 | 2 = 3
});

test('should support bitwise operations', () => {
// Test that All combines Process and Terminal
const all = EnvironmentVariableScope.Process | EnvironmentVariableScope.Terminal;
sinon.assert.match(all, EnvironmentVariableScope.All);

// Test that we can check if All includes Process
const includesProcess = EnvironmentVariableScope.All & EnvironmentVariableScope.Process;
sinon.assert.match(includesProcess, EnvironmentVariableScope.Process);

// Test that we can check if All includes Terminal
const includesTerminal = EnvironmentVariableScope.All & EnvironmentVariableScope.Terminal;
sinon.assert.match(includesTerminal, EnvironmentVariableScope.Terminal);
});
});