Skip to content

Rename EnvironmentId to EnvironmentPath #19887

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 27, 2022
Merged

Rename EnvironmentId to EnvironmentPath #19887

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4396943
Rename `EnvironmentId` to `EnvironmentPath`
Sep 27, 2022
bd7c5bd
Fix tests
Sep 27, 2022
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
8 changes: 0 additions & 8 deletions src/client/deprecatedProposedApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,6 @@ export function buildDeprecatedProposedApi(
const env = await interpreterService.getActiveInterpreter(resource);
return env ? { execCommand: [env.path] } : { execCommand: undefined };
},
async getActiveEnvironmentPath(resource?: Resource) {
sendApiTelemetry('getActiveEnvironmentPath');
const env = await interpreterService.getActiveInterpreter(resource);
if (!env) {
return undefined;
}
return getEnvPath(env.path, env.envPath);
},
async getEnvironmentDetails(
path: string,
options?: EnvironmentDetailsOptions,
Expand Down
6 changes: 1 addition & 5 deletions src/client/deprecatedProposedApiTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,6 @@ export interface DeprecatedProposedAPI {
*/
execCommand: string[] | undefined;
}>;
/**
* @deprecated Use {@link getActiveEnvironmentId} instead. This will soon be removed.
*/
getActiveEnvironmentPath(resource?: Resource): Promise<EnvPathType | undefined>;
/**
* Returns details for the given interpreter. Details such as absolute interpreter path,
* version, type (conda, pyenv, etc). Metadata such as `sysPrefix` can be found under
Expand Down Expand Up @@ -135,7 +131,7 @@ export interface DeprecatedProposedAPI {
*/
onDidEnvironmentsChanged: Event<EnvironmentsChangedParams[]>;
/**
* @deprecated Use {@link ProposedExtensionAPI.environment} `onDidChangeActiveEnvironmentId` instead. This will soon be removed.
* @deprecated Use {@link ProposedExtensionAPI.environment} `onDidChangeActiveEnvironmentPath` instead. This will soon be removed.
*/
onDidActiveEnvironmentChanged: Event<ActiveEnvironmentChangedParams>;
};
Expand Down
32 changes: 21 additions & 11 deletions src/client/proposedApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { IConfigurationService, IDisposableRegistry, IExtensions, IInterpreterPa
import { Architecture } from './common/utils/platform';
import { IServiceContainer } from './ioc/types';
import {
ActiveEnvironmentIdChangeEvent,
ActiveEnvironmentPathChangeEvent,
Environment,
EnvironmentsChangeEvent,
ProposedExtensionAPI,
Expand All @@ -17,7 +17,7 @@ import {
Resource,
EnvironmentType,
EnvironmentTools,
EnvironmentId,
EnvironmentPath,
} from './proposedApiTypes';
import { PythonEnvInfo, PythonEnvKind, PythonEnvType } from './pythonEnvironments/base/info';
import { getEnvPath } from './pythonEnvironments/base/info/env';
Expand All @@ -38,7 +38,7 @@ type ActiveEnvironmentChangeEvent = {
path: string;
};

const onDidActiveInterpreterChangedEvent = new EventEmitter<ActiveEnvironmentIdChangeEvent>();
const onDidActiveInterpreterChangedEvent = new EventEmitter<ActiveEnvironmentPathChangeEvent>();
export function reportActiveInterpreterChanged(e: ActiveEnvironmentChangeEvent): void {
onDidActiveInterpreterChangedEvent.fire({ id: getEnvID(e.path), path: e.path, resource: e.resource });
reportActiveInterpreterChangedDeprecated({ path: e.path, resource: e.resource?.uri });
Expand Down Expand Up @@ -164,24 +164,34 @@ export function buildProposedApi(

const proposed: ProposedExtensionAPI = {
environment: {
getActiveEnvironmentId(resource?: Resource) {
sendApiTelemetry('getActiveEnvironmentId');
getActiveEnvironmentPath(resource?: Resource) {
sendApiTelemetry('getActiveEnvironmentPath');
resource = resource && 'uri' in resource ? resource.uri : resource;
const path = configService.getSettings(resource).pythonPath;
const id = path === 'python' ? 'DEFAULT_PYTHON' : getEnvID(path);
return { id, path };
return {
id,
path,
/**
* @deprecated Only provided for backwards compatibility and will soon be removed.
*/
pathType: 'interpreterPath',
};
},
updateActiveEnvironmentId(env: Environment | EnvironmentId | string, resource?: Resource): Promise<void> {
sendApiTelemetry('updateActiveEnvironmentId');
updateActiveEnvironmentPath(
env: Environment | EnvironmentPath | string,
resource?: Resource,
): Promise<void> {
sendApiTelemetry('updateActiveEnvironmentPath');
const path = typeof env !== 'string' ? env.path : env;
resource = resource && 'uri' in resource ? resource.uri : resource;
return interpreterPathService.update(resource, ConfigurationTarget.WorkspaceFolder, path);
},
get onDidChangeActiveEnvironmentId() {
sendApiTelemetry('onDidChangeActiveEnvironmentId');
get onDidChangeActiveEnvironmentPath() {
sendApiTelemetry('onDidChangeActiveEnvironmentPath');
return onDidActiveInterpreterChangedEvent.event;
},
resolveEnvironment: async (env: Environment | EnvironmentId | string) => {
resolveEnvironment: async (env: Environment | EnvironmentPath | string) => {
let path = typeof env !== 'string' ? env.path : env;
if (pathUtils.basename(path) === path) {
// Value can be `python`, `python3`, `python3.9` etc.
Expand Down
18 changes: 10 additions & 8 deletions src/client/proposedApiTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,22 @@ export interface ProposedExtensionAPI {
* @param resource : Uri of a file or workspace folder. This is used to determine the env in a multi-root
* scenario. If `undefined`, then the API returns what ever is set for the workspace.
*/
getActiveEnvironmentId(resource?: Resource): EnvironmentId;
getActiveEnvironmentPath(resource?: Resource): EnvironmentPath;
/**
* Sets the active environment path for the python extension for the resource. Configuration target will always
* be the workspace folder.
* @param environment : Full path to environment folder or python executable for the environment. Can also pass
* the environment itself.
* @param resource : [optional] File or workspace to scope to a particular workspace folder.
*/
updateActiveEnvironmentId(
environment: Environment | EnvironmentId | string,
updateActiveEnvironmentPath(
environment: string | EnvironmentPath | Environment,
resource?: Resource,
): Promise<void>;
/**
* This event is triggered when the active environment setting changes.
*/
readonly onDidChangeActiveEnvironmentId: Event<ActiveEnvironmentIdChangeEvent>;
readonly onDidChangeActiveEnvironmentPath: Event<ActiveEnvironmentPathChangeEvent>;
/**
* Carries environments found by the extension at the time of fetching the property. Note this may not
* contain all environments in the system as a refresh might be going on.
Expand Down Expand Up @@ -55,7 +55,9 @@ export interface ProposedExtensionAPI {
* @param environment : Full path to environment folder or python executable for the environment. Can also pass
* the environment id or the environment itself.
*/
resolveEnvironment(environment: Environment | EnvironmentId | string): Promise<ResolvedEnvironment | undefined>;
resolveEnvironment(
environment: Environment | EnvironmentPath | string,
): Promise<ResolvedEnvironment | undefined>;
};
}

Expand All @@ -70,7 +72,7 @@ export type RefreshOptions = {
/**
* Details about the environment. Note the environment folder, type and name never changes over time.
*/
export type Environment = EnvironmentId & {
export type Environment = EnvironmentPath & {
/**
* Carries details about python executable.
*/
Expand Down Expand Up @@ -175,7 +177,7 @@ export type EnvironmentsChangeEvent = {
readonly type: 'add' | 'remove' | 'update';
};

export type ActiveEnvironmentIdChangeEvent = EnvironmentId & {
export type ActiveEnvironmentPathChangeEvent = EnvironmentPath & {
/**
* Workspace folder the environment changed for.
*/
Expand All @@ -187,7 +189,7 @@ export type ActiveEnvironmentIdChangeEvent = EnvironmentId & {
*/
export type Resource = Uri | WorkspaceFolder;

export type EnvironmentId = {
export type EnvironmentPath = {
/**
* The ID of the environment.
*/
Expand Down
57 changes: 35 additions & 22 deletions src/test/proposedApi.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@ import { sleep } from './core';
import { PythonEnvKind, PythonEnvSource } from '../client/pythonEnvironments/base/info';
import { Architecture } from '../client/common/utils/platform';
import { PythonEnvCollectionChangedEvent } from '../client/pythonEnvironments/base/watcher';
import { normCasePath } from '../client/common/platform/fs-paths';
import {
ProposedExtensionAPI,
ActiveEnvironmentIdChangeEvent,
ActiveEnvironmentPathChangeEvent,
EnvironmentPath,
EnvironmentsChangeEvent,
ProposedExtensionAPI,
} from '../client/proposedApiTypes';
import { normCasePath } from '../client/common/platform/fs-paths';

suite('Proposed Extension API', () => {
let serviceContainer: typemoq.IMock<IServiceContainer>;
Expand Down Expand Up @@ -74,8 +75,8 @@ suite('Proposed Extension API', () => {
});

test('Provide an event to track when active environment details change', async () => {
const events: ActiveEnvironmentIdChangeEvent[] = [];
proposed.environment.onDidChangeActiveEnvironmentId((e) => {
const events: ActiveEnvironmentPathChangeEvent[] = [];
proposed.environment.onDidChangeActiveEnvironmentPath((e) => {
events.push(e);
});
reportActiveInterpreterChanged({ path: 'path/to/environment', resource: undefined });
Expand All @@ -85,32 +86,44 @@ suite('Proposed Extension API', () => {
]);
});

test('getActiveEnvironmentId: No resource', () => {
test('getActiveEnvironmentPath: No resource', () => {
const pythonPath = 'this/is/a/test/path';
configService
.setup((c) => c.getSettings(undefined))
.returns(() => (({ pythonPath } as unknown) as IPythonSettings));
const actual = proposed.environment.getActiveEnvironmentId();
assert.deepEqual(actual, { id: normCasePath(pythonPath), path: pythonPath });
const actual = proposed.environment.getActiveEnvironmentPath();
assert.deepEqual(actual, ({
id: normCasePath(pythonPath),
path: pythonPath,
pathType: 'interpreterPath',
} as unknown) as EnvironmentPath);
});

test('getActiveEnvironmentId: default python', () => {
test('getActiveEnvironmentPath: default python', () => {
const pythonPath = 'python';
configService
.setup((c) => c.getSettings(undefined))
.returns(() => (({ pythonPath } as unknown) as IPythonSettings));
const actual = proposed.environment.getActiveEnvironmentId();
assert.deepEqual(actual, { id: 'DEFAULT_PYTHON', path: pythonPath });
const actual = proposed.environment.getActiveEnvironmentPath();
assert.deepEqual(actual, ({
id: 'DEFAULT_PYTHON',
path: pythonPath,
pathType: 'interpreterPath',
} as unknown) as EnvironmentPath);
});

test('getActiveEnvironmentId: With resource', () => {
test('getActiveEnvironmentPath: With resource', () => {
const pythonPath = 'this/is/a/test/path';
const resource = Uri.file(__filename);
configService
.setup((c) => c.getSettings(resource))
.returns(() => (({ pythonPath } as unknown) as IPythonSettings));
const actual = proposed.environment.getActiveEnvironmentId(resource);
assert.deepEqual(actual, { id: normCasePath(pythonPath), path: pythonPath });
const actual = proposed.environment.getActiveEnvironmentPath(resource);
assert.deepEqual(actual, ({
id: normCasePath(pythonPath),
path: pythonPath,
pathType: 'interpreterPath',
} as unknown) as EnvironmentPath);
});

test('resolveEnvironment: invalid environment (when passed as string)', async () => {
Expand Down Expand Up @@ -317,44 +330,44 @@ suite('Proposed Extension API', () => {
assert.deepEqual(eventValues, expectedEvents);
});

test('updateActiveEnvironmentId: no resource', async () => {
test('updateActiveEnvironmentPath: no resource', async () => {
interpreterPathService
.setup((i) => i.update(undefined, ConfigurationTarget.WorkspaceFolder, 'this/is/a/test/python/path'))
.returns(() => Promise.resolve())
.verifiable(typemoq.Times.once());

await proposed.environment.updateActiveEnvironmentId('this/is/a/test/python/path');
await proposed.environment.updateActiveEnvironmentPath('this/is/a/test/python/path');

interpreterPathService.verifyAll();
});

test('updateActiveEnvironmentId: passed as Environment', async () => {
test('updateActiveEnvironmentPath: passed as Environment', async () => {
interpreterPathService
.setup((i) => i.update(undefined, ConfigurationTarget.WorkspaceFolder, 'this/is/a/test/python/path'))
.returns(() => Promise.resolve())
.verifiable(typemoq.Times.once());

await proposed.environment.updateActiveEnvironmentId({
await proposed.environment.updateActiveEnvironmentPath({
id: normCasePath('this/is/a/test/python/path'),
path: 'this/is/a/test/python/path',
});

interpreterPathService.verifyAll();
});

test('updateActiveEnvironmentId: with uri', async () => {
test('updateActiveEnvironmentPath: with uri', async () => {
const uri = Uri.parse('a');
interpreterPathService
.setup((i) => i.update(uri, ConfigurationTarget.WorkspaceFolder, 'this/is/a/test/python/path'))
.returns(() => Promise.resolve())
.verifiable(typemoq.Times.once());

await proposed.environment.updateActiveEnvironmentId('this/is/a/test/python/path', uri);
await proposed.environment.updateActiveEnvironmentPath('this/is/a/test/python/path', uri);

interpreterPathService.verifyAll();
});

test('updateActiveEnvironmentId: with workspace folder', async () => {
test('updateActiveEnvironmentPath: with workspace folder', async () => {
const uri = Uri.parse('a');
interpreterPathService
.setup((i) => i.update(uri, ConfigurationTarget.WorkspaceFolder, 'this/is/a/test/python/path'))
Expand All @@ -366,7 +379,7 @@ suite('Proposed Extension API', () => {
index: 0,
};

await proposed.environment.updateActiveEnvironmentId('this/is/a/test/python/path', workspace);
await proposed.environment.updateActiveEnvironmentPath('this/is/a/test/python/path', workspace);

interpreterPathService.verifyAll();
});
Expand Down