Skip to content
Merged
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
19 changes: 10 additions & 9 deletions src/features/terminal/shells/cmd/cmdStartup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,14 @@ import { ShellConstants } from '../../../common/shellConstants';
import { hasStartupCode, insertStartupCode, removeStartupCode } from '../common/editUtils';
import { ShellScriptEditState, ShellSetupState, ShellStartupScriptProvider } from '../startupProvider';
import { CMD_ENV_KEY, CMD_SCRIPT_VERSION } from './cmdConstants';
import { StopWatch } from '../../../../common/stopWatch';

const exec = promisify(cp.exec);
function execCommand(command: string) {
const timer = new StopWatch();
return promisify(cp.exec)(command, { windowsHide: true }).finally(() =>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We haven't logged this at all, now we will.
I believe we've seen delays in reading registry in the past as well.

traceInfo(`Executed command: ${command} in ${timer.elapsedTime}`),
);
}

async function isCmdInstalled(): Promise<boolean> {
if (!isWindows()) {
Expand Down Expand Up @@ -94,9 +100,7 @@ async function checkRegistryAutoRun(mainBatchFile: string, regMainBatchFile: str

try {
// Check if AutoRun is set in the registry to call our batch file
const { stdout } = await exec('reg query "HKCU\\Software\\Microsoft\\Command Processor" /v AutoRun', {
windowsHide: true,
});
const { stdout } = await execCommand('reg query "HKCU\\Software\\Microsoft\\Command Processor" /v AutoRun');

// Check if the output contains our batch file path
return stdout.includes(regMainBatchFile) || stdout.includes(mainBatchFile);
Expand All @@ -112,9 +116,7 @@ async function getExistingAutoRun(): Promise<string | undefined> {
}

try {
const { stdout } = await exec('reg query "HKCU\\Software\\Microsoft\\Command Processor" /v AutoRun', {
windowsHide: true,
});
const { stdout } = await execCommand('reg query "HKCU\\Software\\Microsoft\\Command Processor" /v AutoRun');

const match = stdout.match(/AutoRun\s+REG_SZ\s+(.*)/);
if (match && match[1]) {
Expand All @@ -135,9 +137,8 @@ async function setupRegistryAutoRun(mainBatchFile: string): Promise<boolean> {

try {
// Set the registry key to call our main batch file
await exec(
await execCommand(
`reg add "HKCU\\Software\\Microsoft\\Command Processor" /v AutoRun /t REG_SZ /d "if exist \\"${mainBatchFile}\\" call \\"${mainBatchFile}\\"" /f`,
{ windowsHide: true },
);

traceInfo(
Expand Down
7 changes: 5 additions & 2 deletions src/features/terminal/shells/utils.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import * as cp from 'child_process';
import { traceVerbose } from '../../../common/logging';
import { traceError, traceInfo } from '../../../common/logging';
import { StopWatch } from '../../../common/stopWatch';

export async function runCommand(command: string): Promise<string | undefined> {
return new Promise((resolve) => {
const timer = new StopWatch();
cp.exec(command, (err, stdout) => {
if (err) {
traceVerbose(`Error running command: ${command}`, err);
traceError(`Error running command: ${command} (${timer.elapsedTime})`, err);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely if success is info, then failure must be warning/error or info, but not something less

resolve(undefined);
} else {
traceInfo(`Ran ${command} in ${timer.elapsedTime}`);
resolve(stdout?.trim());
}
});
Expand Down
2 changes: 1 addition & 1 deletion src/managers/builtin/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export async function isUvInstalled(log?: LogOutputChannel): Promise<boolean> {
if (available.completed) {
return available.promise;
}

log?.info(`Running: uv --version`);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the code is right here, as log is optional
And looking at the calling code, it isn't passed

Not sure why, I'd try to resolve this in debt, as logger shouldn't be optional. we either turn off logging or the like, but wouldn't make the logger component optional.

const proc = ch.spawn('uv', ['--version']);
proc.on('error', () => {
available.resolve(false);
Expand Down
3 changes: 3 additions & 0 deletions src/managers/conda/condaUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import { Installable } from '../common/types';
import { shortVersion, sortEnvironments } from '../common/utils';
import { CondaEnvManager } from './condaEnvManager';
import { getCondaHookPs1Path, getLocalActivationScript } from './condaSourcingUtils';
import { StopWatch } from '../../common/stopWatch';

export const CONDA_PATH_KEY = `${ENVS_EXTENSION_ID}:conda:CONDA_PATH`;
export const CONDA_PREFIXES_KEY = `${ENVS_EXTENSION_ID}:conda:CONDA_PREFIXES`;
Expand Down Expand Up @@ -202,6 +203,8 @@ async function _runConda(
): Promise<string> {
const deferred = createDeferred<string>();
args = quoteArgs(args);
const timer = new StopWatch();
deferred.promise.finally(() => traceInfo(`Ran conda in ${timer.elapsedTime}: ${conda} ${args.join(' ')}`));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We haven't logged this at all, now we will.
We know conda is slow

const proc = ch.spawn(conda, args, { shell: true });

token?.onCancellationRequested(() => {
Expand Down
Loading