Skip to content

Commit 40e819f

Browse files
committed
fix: address PR review comments
- Use monotonic counter instead of Date.now() in ShellExecution.computeId() for deterministic IDs - Export UV_INSTALL_PYTHON_DONT_ASK_KEY constant and use in tests - Capitalize 'Python' in noEnvFound localized string - Re-check uv availability after installation to handle PATH issues - Add uvInstallRestartRequired localized string for when uv is not on PATH
1 parent b43e0ae commit 40e819f

File tree

4 files changed

+22
-8
lines changed

4 files changed

+22
-8
lines changed

src/common/localize.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ export namespace VenvManagerStrings {
9797
export const venvErrorNoPython3 = l10n.t('Did not find any base Python 3');
9898

9999
export const noEnvClickToCreate = l10n.t('No environment found, click to create');
100-
export const noEnvFound = l10n.t('No python environments found.');
100+
export const noEnvFound = l10n.t('No Python environments found.');
101101
export const createEnvironment = l10n.t('Create Environment');
102102

103103
export const venvName = l10n.t('Enter a name for the virtual environment');
@@ -228,8 +228,9 @@ export namespace UvInstallStrings {
228228
return l10n.t('Python installed successfully at {0}', path);
229229
}
230230
export const installFailed = l10n.t('Failed to install Python');
231-
export const uvInstallFailed = l10n.t('Failed to install uv');
232-
export const dontAskAgain = l10n.t("Don't ask again");
231+
export const uvInstallFailed = l10n.t('Failed to install uv'); export const uvInstallRestartRequired = l10n.t(
232+
'uv was installed but may not be available in the current terminal. Please restart VS Code or open a new terminal and try again.',
233+
); export const dontAskAgain = l10n.t("Don't ask again");
233234
export const clickToInstallPython = l10n.t('No Python found, click to install');
234235
export const selectPythonVersion = l10n.t('Select Python version to install');
235236
export const installed = l10n.t('installed');

src/managers/builtin/uvPythonInstaller.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { isWindows } from '../../common/utils/platformUtils';
2020
import { showErrorMessage, showInformationMessage, showQuickPick, withProgress } from '../../common/window.apis';
2121
import { isUvInstalled, resetUvInstallationCache } from './helpers';
2222

23-
const UV_INSTALL_PYTHON_DONT_ASK_KEY = 'python-envs:uv:UV_INSTALL_PYTHON_DONT_ASK';
23+
export const UV_INSTALL_PYTHON_DONT_ASK_KEY = 'python-envs:uv:UV_INSTALL_PYTHON_DONT_ASK';
2424

2525
/**
2626
* Represents a Python version from uv python list
@@ -395,6 +395,15 @@ export async function installPythonWithUv(log?: LogOutputChannel, version?: stri
395395
showErrorMessage(UvInstallStrings.uvInstallFailed);
396396
return undefined;
397397
}
398+
399+
// Verify uv is now available on PATH
400+
const uvNowInstalled = await isUvInstalled(log);
401+
if (!uvNowInstalled) {
402+
traceError('uv installed but not found on PATH - may require terminal restart');
403+
sendTelemetryEvent(EventNames.UV_PYTHON_INSTALL_FAILED, undefined, { stage: 'uvNotOnPath' });
404+
showErrorMessage(UvInstallStrings.uvInstallRestartRequired);
405+
return undefined;
406+
}
398407
}
399408

400409
// Step 2: Install Python via uv

src/test/managers/builtin/uvPythonInstaller.unit.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
clearDontAskAgain,
1212
isDontAskAgainSet,
1313
promptInstallPythonViaUv,
14+
UV_INSTALL_PYTHON_DONT_ASK_KEY,
1415
} from '../../../managers/builtin/uvPythonInstaller';
1516
import { createMockLogOutputChannel } from '../../mocks/helper';
1617

@@ -91,7 +92,7 @@ suite('uvPythonInstaller - promptInstallPythonViaUv', () => {
9192
const result = await promptInstallPythonViaUv('activation', mockLog);
9293

9394
assert.strictEqual(result, undefined);
94-
assert(mockState.set.calledWith('python-envs:uv:UV_INSTALL_PYTHON_DONT_ASK', true), 'Should set dont ask flag');
95+
assert(mockState.set.calledWith(UV_INSTALL_PYTHON_DONT_ASK_KEY, true), 'Should set dont ask flag');
9596
});
9697

9798
test('should return undefined when user dismisses the dialog', async () => {
@@ -163,7 +164,7 @@ suite('uvPythonInstaller - isDontAskAgainSet and clearDontAskAgain', () => {
163164
test('clearDontAskAgain should set flag to false', async () => {
164165
await clearDontAskAgain();
165166

166-
assert(mockState.set.calledWith('python-envs:uv:UV_INSTALL_PYTHON_DONT_ASK', false), 'Should clear the flag');
167+
assert(mockState.set.calledWith(UV_INSTALL_PYTHON_DONT_ASK_KEY, false), 'Should clear the flag');
167168
});
168169
});
169170

src/test/mocks/vsc/extHostedTypes.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,6 +1613,8 @@ export class ProcessExecution implements vscode.ProcessExecution {
16131613
}
16141614

16151615
export class ShellExecution implements vscode.ShellExecution {
1616+
private static idCounter = 0;
1617+
16161618
private _commandLine = '';
16171619

16181620
private _command: string | vscode.ShellQuotedString = '';
@@ -1706,9 +1708,10 @@ export class ShellExecution implements vscode.ShellExecution {
17061708
// }
17071709
// }
17081710
// return hash.digest('hex');
1709-
// Return a simple unique ID based on command
1711+
// Return a deterministic unique ID based on command and a monotonic counter
17101712
const cmd = typeof this._command === 'string' ? this._command : (this._command?.value ?? '');
1711-
return `shell-${cmd}-${Date.now()}`;
1713+
const argsStr = this._args?.map((a) => (typeof a === 'string' ? a : a.value)).join(',') ?? '';
1714+
return `shell-${cmd}-${argsStr}-${ShellExecution.idCounter++}`;
17121715
}
17131716
}
17141717

0 commit comments

Comments
 (0)