Skip to content

Commit ce2b14b

Browse files
authored
Merge pull request microsoft#261202 from microsoft/tyriar/261149
Revise run in terminal's wait for SI implementation
2 parents d7705dd + 211d70b commit ce2b14b

File tree

1 file changed

+42
-40
lines changed

1 file changed

+42
-40
lines changed

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/toolTerminalCreator.ts

Lines changed: 42 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,15 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { DeferredPromise, disposableTimeout, timeout } from '../../../../../base/common/async.js';
6+
import { DeferredPromise, disposableTimeout } from '../../../../../base/common/async.js';
77
import type { CancellationToken } from '../../../../../base/common/cancellation.js';
88
import { Codicon } from '../../../../../base/common/codicons.js';
99
import { CancellationError } from '../../../../../base/common/errors.js';
10-
import { DisposableStore } from '../../../../../base/common/lifecycle.js';
10+
import { DisposableStore, MutableDisposable } from '../../../../../base/common/lifecycle.js';
1111
import { ThemeIcon } from '../../../../../base/common/themables.js';
12+
import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
1213
import { TerminalCapability } from '../../../../../platform/terminal/common/capabilities/capabilities.js';
14+
import { TerminalSettingId } from '../../../../../platform/terminal/common/terminal.js';
1315
import { ITerminalService, type ITerminalInstance } from '../../../terminal/browser/terminal.js';
1416

1517
const enum ShellLaunchType {
@@ -37,6 +39,7 @@ export class ToolTerminalCreator {
3739
private static _lastSuccessfulShell: ShellLaunchType = ShellLaunchType.Unknown;
3840

3941
constructor(
42+
@IConfigurationService private readonly _configurationService: IConfigurationService,
4043
@ITerminalService private readonly _terminalService: ITerminalService,
4144
) {
4245
}
@@ -48,9 +51,17 @@ export class ToolTerminalCreator {
4851
shellIntegrationQuality: ShellIntegrationQuality.None,
4952
};
5053

51-
// The default profile has shell integration
52-
if (ToolTerminalCreator._lastSuccessfulShell <= ShellLaunchType.Default) {
53-
const shellIntegrationQuality = await this._waitForShellIntegration(instance, 5000);
54+
// Wait for shell integration when the fallback case has not been hit or when shell
55+
// integration injection is enabled. Note that it's possible for the fallback case to happen
56+
// and then for SI to activate again later in the session.
57+
const siInjectionEnabled = this._configurationService.getValue(TerminalSettingId.ShellIntegrationEnabled);
58+
if (
59+
ToolTerminalCreator._lastSuccessfulShell !== ShellLaunchType.Fallback ||
60+
siInjectionEnabled
61+
) {
62+
// Use a reasonable wait time depending on whether the injection setting is set
63+
const waitTime = siInjectionEnabled ? 5000 : (instance.isRemote ? 3000 : 2000);
64+
const shellIntegrationQuality = await this._waitForShellIntegration(instance, waitTime);
5465
if (token.isCancellationRequested) {
5566
instance.dispose();
5667
throw new CancellationError();
@@ -84,61 +95,52 @@ export class ToolTerminalCreator {
8495
instance: ITerminalInstance,
8596
timeoutMs: number
8697
): Promise<ShellIntegrationQuality> {
87-
const dataFinished = new DeferredPromise<void>();
98+
const store = new DisposableStore();
99+
const result = new DeferredPromise<ShellIntegrationQuality>();
88100

89-
const deferred = new DeferredPromise<ShellIntegrationQuality>();
90-
const timer = disposableTimeout(() => deferred.complete(ShellIntegrationQuality.None), timeoutMs);
101+
const siNoneTimer = store.add(new MutableDisposable());
102+
siNoneTimer.value = disposableTimeout(() => result.complete(ShellIntegrationQuality.None), timeoutMs);
91103

92104
if (instance.capabilities.get(TerminalCapability.CommandDetection)?.hasRichCommandDetection) {
93-
timer.dispose();
94-
deferred.complete(ShellIntegrationQuality.Rich);
105+
// Rich command detection is available immediately.
106+
siNoneTimer.clear();
107+
result.complete(ShellIntegrationQuality.Rich);
95108
} else {
96-
const onSetRichCommandDetection = this._terminalService.createOnInstanceCapabilityEvent(TerminalCapability.CommandDetection, e => e.onSetRichCommandDetection);
97-
98-
const richCommandDetectionListener = onSetRichCommandDetection.event((e) => {
109+
const onSetRichCommandDetection = store.add(this._terminalService.createOnInstanceCapabilityEvent(TerminalCapability.CommandDetection, e => e.onSetRichCommandDetection));
110+
store.add(onSetRichCommandDetection.event((e) => {
99111
if (e.instance !== instance) {
100112
return;
101113
}
102-
deferred.complete(ShellIntegrationQuality.Rich);
103-
});
104-
105-
const store = new DisposableStore();
114+
siNoneTimer.clear();
115+
// Rich command detection becomes available some time after the terminal is created.
116+
result.complete(ShellIntegrationQuality.Rich);
117+
}));
106118

107119
const commandDetection = instance.capabilities.get(TerminalCapability.CommandDetection);
108120
if (commandDetection) {
109-
timer.dispose();
121+
siNoneTimer.clear();
110122
// When command detection lights up, allow up to 200ms for the rich command
111123
// detection sequence to come in before declaring it as basic shell integration.
112-
// up.
113-
Promise.race([
114-
dataFinished.p,
115-
timeout(200)
116-
]).then(() => {
117-
if (!deferred.isResolved) {
118-
deferred.complete(ShellIntegrationQuality.Basic);
119-
}
120-
});
124+
store.add(disposableTimeout(() => {
125+
result.complete(ShellIntegrationQuality.Basic);
126+
}, 200));
121127
} else {
122128
store.add(instance.capabilities.onDidAddCapabilityType(e => {
123129
if (e === TerminalCapability.CommandDetection) {
124-
timer.dispose();
130+
siNoneTimer.clear();
125131
// When command detection lights up, allow up to 200ms for the rich command
126-
// detection sequence to come in before declaring it as basic shell integration.
127-
// up.
128-
Promise.race([
129-
dataFinished.p,
130-
timeout(200)
131-
]).then(() => deferred.complete(ShellIntegrationQuality.Basic));
132+
// detection sequence to come in before declaring it as basic shell
133+
// integration.
134+
store.add(disposableTimeout(() => {
135+
result.complete(ShellIntegrationQuality.Basic);
136+
}, 200));
132137
}
133138
}));
134139
}
135-
136-
deferred.p.finally(() => {
137-
store.dispose();
138-
richCommandDetectionListener.dispose();
139-
});
140140
}
141141

142-
return deferred.p;
142+
result.p.finally(() => store.dispose());
143+
144+
return result.p;
143145
}
144146
}

0 commit comments

Comments
 (0)