Skip to content

Commit e91bb93

Browse files
committed
fix: address remaining API PR comments
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: de4cb63f-4a85-432d-a722-697b33e9d49a
1 parent 4edc090 commit e91bb93

6 files changed

Lines changed: 56 additions & 27 deletions

File tree

CONTRIBUTING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ The npm package under [`api/`](./api) is the public API facade other extensions
9696
- Edit the public API only in `src/api.ts` (the runtime facade: `PythonEnvironments.api()` helper and `EXTENSION_ID`), `src/types.ts` (public contracts: interfaces, types, enums), and `src/publicErrors.ts` (concrete public error classes and type guards). `api/src/*.ts` files are build artifacts — never edit or commit them.
9797
- `api/src/main.ts`, `api/src/types.ts`, and `api/src/publicErrors.ts` are produced by the publish pipeline ([`build/azure-pipeline.npm.yml`](./build/azure-pipeline.npm.yml)), which copies `src/api.ts` to `api/src/main.ts`, `src/types.ts` to `api/src/types.ts`, and `src/publicErrors.ts` to `api/src/publicErrors.ts` before compiling. The api package is therefore built in CI only; to build it locally, copy the files first (e.g. `cp src/api.ts api/src/main.ts && cp src/types.ts api/src/types.ts && cp src/publicErrors.ts api/src/publicErrors.ts`).
9898
- `src/api.ts`, `src/types.ts`, and `src/publicErrors.ts` are validated on every PR by the extension's own lint and TypeScript compile.
99-
- **Versioning:** the published package version in [`api/package.json`](./api/package.json) is maintained independently of the extension version in [`package.json`](./package.json) — the two do not need to match. Any PR that edits `src/api.ts`, `src/types.ts`, or `src/publicErrors.ts` must bump `api/package.json` (use the `skip api version` label to bypass) and add an entry to [`api/CHANGELOG.md`](./api/CHANGELOG.md) (use the `skip api changelog` label to bypass).
99+
- **Versioning and compatibility:** the published package version in [`api/package.json`](./api/package.json) is maintained independently of the extension version in [`package.json`](./package.json) — the two do not need to match. Compatibility is based on the API shape exported by the installed Python Environments extension at runtime. Package updates must preserve backwards-compatible contracts unless the API package version intentionally communicates a breaking change; consumers should treat newly added members as optional when they may run against older installed extension versions. Any PR that edits `src/api.ts`, `src/types.ts`, or `src/publicErrors.ts` must bump `api/package.json` (use the `skip api version` label to bypass) and add an entry to [`api/CHANGELOG.md`](./api/CHANGELOG.md) (use the `skip api changelog` label to bypass).
100100

101101
## Questions or Issues?
102102

api/scripts/test-package.cjs

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,17 @@ try {
8383
fs.writeFileSync(path.join(vscodeStubRoot, 'package.json'), JSON.stringify({ main: 'index.js' }));
8484
fs.writeFileSync(
8585
path.join(vscodeStubRoot, 'index.js'),
86-
"exports.extensions = { getExtension: () => undefined };",
86+
[
87+
"const runtimeApi = { getEnvironments: async () => [] };",
88+
"const extension = {",
89+
" isActive: false,",
90+
" exports: undefined,",
91+
" packageJSON: { version: '1.37.0' },",
92+
" activate: async () => { extension.isActive = true; extension.exports = runtimeApi; return runtimeApi; },",
93+
"};",
94+
'exports.__runtimeApi = runtimeApi;',
95+
'exports.extensions = { getExtension: () => extension };',
96+
].join('\n'),
8797
);
8898
const installedPackageJson = JSON.parse(fs.readFileSync(path.join(installedPackageRoot, 'package.json'), 'utf8'));
8999
assert.strictEqual(installedPackageJson.main, './out/cjs/main.cjs');
@@ -117,6 +127,24 @@ try {
117127
'function',
118128
'CommonJS consumers should load the package runtime facade',
119129
);
130+
execFileSync(
131+
process.execPath,
132+
[
133+
'--eval',
134+
[
135+
"const packageModule = require('@vscode/python-environments');",
136+
"const vscode = require('vscode');",
137+
"(async () => {",
138+
' const api = await packageModule.PythonEnvironments.api();',
139+
' if (api !== vscode.__runtimeApi) process.exit(1);',
140+
'})().catch(() => process.exit(1));',
141+
].join('\n'),
142+
],
143+
{
144+
cwd: path.join(testRoot, 'legacy'),
145+
encoding: 'utf8',
146+
},
147+
);
120148
assert.strictEqual(
121149
canonicalPath(requireFromConsumer.resolve('@vscode/python-environments')),
122150
canonicalPath(path.join(installedPackageRoot, installedPackageJson.exports.require.default)),
@@ -128,7 +156,7 @@ try {
128156
[
129157
'--input-type=module',
130158
'--eval',
131-
"const packageModule = await import('@vscode/python-environments'); if (typeof packageModule.PythonEnvironments.api !== 'function') process.exit(1); console.log(import.meta.resolve('@vscode/python-environments'));",
159+
"const packageModule = await import('@vscode/python-environments'); const vscode = await import('vscode'); if (typeof packageModule.PythonEnvironments.api !== 'function') process.exit(1); const api = await packageModule.PythonEnvironments.api(); if (api !== vscode.default.__runtimeApi) process.exit(1); console.log(import.meta.resolve('@vscode/python-environments'));",
132160
],
133161
{
134162
cwd: path.join(testRoot, 'modern'),

api/test/consumer.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ import type {
22
PackageManager,
33
Pep440Version,
44
PythonEnvironment,
5+
PythonEnvironmentApi,
56
PythonPackageGetterApi,
67
} from '@vscode/python-environments';
78
import {
89
isPackageVersionLookupNotSupportedError,
910
PackageVersionLookupNotSupportedError,
11+
PythonEnvironments,
1012
} from '@vscode/python-environments';
1113

1214
type Equal<Left, Right> =
@@ -32,6 +34,7 @@ const explicitLegacyAvailableVersions: Promise<Pep440Version[] | undefined> = ap
3234
const throwingAvailableVersions: Promise<Pep440Version[]> = api.getPackageAvailableVersions(environment, 'example', {
3335
errorMode: 'throw',
3436
});
37+
const runtimeApi: Promise<PythonEnvironmentApi> = PythonEnvironments.api();
3538

3639
// The unsupported-capability error is part of the public contract: it is constructible, extends
3740
// Error, and exposes a stable string-literal `code` discriminator.
@@ -50,6 +53,7 @@ void refreshReturnIsExact;
5053
void legacyAvailableVersions;
5154
void explicitLegacyAvailableVersions;
5255
void throwingAvailableVersions;
56+
void runtimeApi;
5357
void lookupErrorIsError;
5458
void lookupErrorCodeIsExact;
5559
void guardNarrows;

src/features/projectManager.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as path from 'path';
2-
import { Disposable, Event, EventEmitter, MarkdownString, Uri, workspace } from 'vscode';
2+
import { Disposable, Event, EventEmitter, MarkdownString, Uri } from 'vscode';
33
import type { IconPath, PythonProject } from '../api';
44
import { DEFAULT_ENV_MANAGER_ID, DEFAULT_PACKAGE_MANAGER_ID } from '../common/constants';
55
import { createSimpleDebounce } from '../common/utils/debounce';
@@ -50,11 +50,11 @@ export interface InlineScriptProjectRegistrationMarker {
5050
}
5151

5252
export class PythonProjectsImpl implements PythonProject {
53-
name: string;
54-
uri: Uri;
55-
description?: string;
56-
tooltip?: string | MarkdownString;
57-
iconPath?: IconPath;
53+
readonly name: string;
54+
readonly uri: Uri;
55+
readonly description?: string;
56+
readonly tooltip?: string | MarkdownString;
57+
readonly iconPath?: IconPath;
5858

5959
constructor(
6060
name: string,
@@ -257,7 +257,7 @@ export class PythonProjectManagerImpl implements PythonProjectManager {
257257
const envManagerId = getDefaultEnvManagerSetting(this);
258258
const pkgManagerId = getDefaultPkgManagerSetting(this);
259259

260-
const globalConfig = workspace.getConfiguration('python-envs', undefined);
260+
const globalConfig = getConfiguration('python-envs', undefined);
261261
const defaultEnvManager = globalConfig.get<string>('defaultEnvManager', DEFAULT_ENV_MANAGER_ID);
262262
const defaultPkgManager = globalConfig.get<string>('defaultPackageManager', DEFAULT_PACKAGE_MANAGER_ID);
263263

src/test/features/envCommands.unit.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ suite('Environment removal command ownership', () => {
4545
envPath: path.join(process.cwd(), 'removal-env'),
4646
});
4747
const remove = sinon.stub().resolves();
48-
const owner = new InternalEnvironmentManager(managerId, {
48+
const owner = new InternalEnvironmentManager(managerId, {
4949
name: 'test',
5050
preferredPackageManagerId: 'ms-python.python:pip',
5151
get: async () => environment,

src/types.ts

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -503,24 +503,21 @@ export interface EnvironmentManager {
503503
onDidChangeEnvironment?: Event<DidChangeEnvironmentEventArgs>;
504504

505505
/**
506-
* Resolves the specified Python environment. The environment can be either a {@link PythonEnvironment} or a {@link Uri} context.
506+
* Resolves the Python environment associated with the specified URI context.
507507
*
508-
* This method is used to obtain a fully detailed {@link PythonEnvironment} object. The input can be:
509-
* - A {@link PythonEnvironment} object, which might be missing key details such as {@link PythonEnvironment.execInfo}.
510-
* - A {@link Uri} object, which typically represents either:
511-
* - A folder that contains the Python environment.
512-
* - The path to a Python executable.
508+
* This method is used to obtain a fully detailed {@link PythonEnvironment} object. The input
509+
* URI typically represents either a folder that contains the Python environment or the path to
510+
* a Python executable.
513511
*
514-
* @param context - The context for resolving the environment, which can be a {@link PythonEnvironment} or a {@link Uri}.
512+
* @param context - The URI context for resolving the environment.
515513
* @returns A promise that resolves to the fully detailed {@link PythonEnvironment}, or `undefined` if the environment cannot be resolved.
516514
*
517515
* @remarks
518-
* Called to turn a lightly-populated {@link PythonEnvironment} or a {@link Uri}
519-
* pointing at an interpreter or environment folder into a fully-populated
520-
* {@link PythonEnvironment} with complete {@link PythonEnvironment.execInfo}. Typical
521-
* triggers include the user manually selecting an interpreter path, resolving
522-
* `python.defaultInterpreterPath` at startup, and populating execution details before
523-
* launching Python.
516+
* Called to turn a {@link Uri} pointing at an interpreter or environment folder into a
517+
* fully-populated {@link PythonEnvironment} with complete {@link PythonEnvironment.execInfo}.
518+
* Typical triggers include the user manually selecting an interpreter path, resolving
519+
* `python.defaultInterpreterPath` at startup, and populating execution details before launching
520+
* Python.
524521
*/
525522
resolve(context: ResolveEnvironmentContext): Promise<PythonEnvironment | undefined>;
526523

@@ -1080,10 +1077,10 @@ export interface PythonEnvironmentsApi {
10801077
onDidChangeEnvironments: Event<DidChangeEnvironmentsEventArgs>;
10811078

10821079
/**
1083-
* This method is used to get the details missing from a PythonEnvironment. Like
1084-
* {@link PythonEnvironment.execInfo} and other details.
1080+
* This method is used to get the details for the Python environment associated with a URI
1081+
* context, such as an interpreter path or environment folder.
10851082
*
1086-
* @param context : The PythonEnvironment or Uri for which details are required.
1083+
* @param context - The URI context for which environment details are required.
10871084
*/
10881085
resolveEnvironment(context: ResolveEnvironmentContext): Promise<PythonEnvironment | undefined>;
10891086
}

0 commit comments

Comments
 (0)