From 1affd37392f5dbd92ad3cef07303e32b50fd42e0 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 10 Jan 2025 02:52:52 -0500 Subject: [PATCH 1/5] Try to start on shell type API --- src/vs/workbench/api/browser/mainThreadTerminalService.ts | 8 ++++++++ src/vs/workbench/api/common/extHost.protocol.ts | 3 ++- src/vs/workbench/api/common/extHostTerminalService.ts | 7 ++++++- src/vs/workbench/contrib/terminal/browser/terminal.ts | 4 ++-- .../contrib/terminal/browser/terminalInstance.ts | 2 ++ 5 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index 57fe7dc25abf7..1379c0aca51f4 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -87,6 +87,7 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape this._store.add(_terminalService.onAnyInstanceTitleChange(instance => instance && this._onTitleChanged(instance.instanceId, instance.title))); this._store.add(_terminalService.onAnyInstanceDataInput(instance => this._proxy.$acceptTerminalInteraction(instance.instanceId))); this._store.add(_terminalService.onAnyInstanceSelectionChange(instance => this._proxy.$acceptTerminalSelection(instance.instanceId, instance.selection))); + this._store.add(_terminalService.onAnyInstanceShellTypeChange(instance => this._proxy.$acceptTerminalShellType(instance.instanceId, instance.shellType))); // Set initial ext host state for (const instance of this._terminalService.instances) { @@ -353,6 +354,13 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape this._proxy.$acceptTerminalTitleChange(terminalId, name); } + private _onShellTypeChanged(terminalId: number, shellType: string): void { + const terminalInstance = this._terminalService.getInstanceFromId(terminalId)?.shellType; + if (terminalInstance) { + this._proxy.$provideTerminalShellType(terminalId, shellType); + } + } + private _onTerminalDisposed(terminalInstance: ITerminalInstance): void { this._proxy.$acceptTerminalClosed(terminalInstance.instanceId, terminalInstance.exitCode, terminalInstance.exitReason ?? TerminalExitReason.Unknown); } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 9b7b0424e7a87..8b2fe5266e39c 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -43,7 +43,7 @@ import { AuthInfo, Credentials } from '../../../platform/request/common/request. import { ClassifiedEvent, IGDPRProperty, OmitMetadata, StrictPropertyCheck } from '../../../platform/telemetry/common/gdprTypings.js'; import { TelemetryLevel } from '../../../platform/telemetry/common/telemetry.js'; import { ISerializableEnvironmentDescriptionMap, ISerializableEnvironmentVariableCollection } from '../../../platform/terminal/common/environmentVariable.js'; -import { ICreateContributedTerminalProfileOptions, IProcessProperty, IProcessReadyWindowsPty, IShellLaunchConfigDto, ITerminalEnvironment, ITerminalLaunchError, ITerminalProfile, TerminalExitReason, TerminalLocation } from '../../../platform/terminal/common/terminal.js'; +import { ICreateContributedTerminalProfileOptions, IProcessProperty, IProcessReadyWindowsPty, IShellLaunchConfigDto, ITerminalEnvironment, ITerminalLaunchError, ITerminalProfile, TerminalExitReason, TerminalLocation, TerminalShellType } from '../../../platform/terminal/common/terminal.js'; import { ProvidedPortAttributes, TunnelCreationOptions, TunnelOptions, TunnelPrivacyId, TunnelProviderFeatures } from '../../../platform/tunnel/common/tunnel.js'; import { EditSessionIdentityMatch } from '../../../platform/workspace/common/editSessions.js'; import { WorkspaceTrustRequestOptions } from '../../../platform/workspace/common/workspaceTrust.js'; @@ -2417,6 +2417,7 @@ export interface ExtHostTerminalServiceShape { $acceptTerminalMaximumDimensions(id: number, cols: number, rows: number): void; $acceptTerminalInteraction(id: number): void; $acceptTerminalSelection(id: number, selection: string | undefined): void; + $acceptTerminalShellType(id: number, shellType: string): Promise; $startExtensionTerminal(id: number, initialDimensions: ITerminalDimensionsDto | undefined): Promise; $acceptProcessAckDataEvent(id: number, charCount: number): void; $acceptProcessInput(id: number, data: string): void; diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index 907fb7125e533..85db4c09e0ec0 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -18,7 +18,7 @@ import { serializeEnvironmentDescriptionMap, serializeEnvironmentVariableCollect import { CancellationTokenSource } from '../../../base/common/cancellation.js'; import { generateUuid } from '../../../base/common/uuid.js'; import { IEnvironmentVariableCollectionDescription, IEnvironmentVariableMutator, ISerializableEnvironmentVariableCollection } from '../../../platform/terminal/common/environmentVariable.js'; -import { ICreateContributedTerminalProfileOptions, IProcessReadyEvent, IShellLaunchConfigDto, ITerminalChildProcess, ITerminalLaunchError, ITerminalProfile, TerminalIcon, TerminalLocation, IProcessProperty, ProcessPropertyType, IProcessPropertyMap } from '../../../platform/terminal/common/terminal.js'; +import { ICreateContributedTerminalProfileOptions, IProcessReadyEvent, IShellLaunchConfigDto, ITerminalChildProcess, ITerminalLaunchError, ITerminalProfile, TerminalIcon, TerminalLocation, IProcessProperty, ProcessPropertyType, IProcessPropertyMap, TerminalShellType, GeneralShellType } from '../../../platform/terminal/common/terminal.js'; import { TerminalDataBufferer } from '../../../platform/terminal/common/terminalDataBuffering.js'; import { ThemeColor } from '../../../base/common/themables.js'; import { Promises } from '../../../base/common/async.js'; @@ -764,6 +764,11 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I return completions; } + // Take in shellType as a string and return VSCode Terminal Shell Type? + public async $acceptTerminalShellType(id: number, shellType: string): Promise { + // TODO: Implement + return GeneralShellType.Python; + } public registerTerminalQuickFixProvider(id: string, extensionId: string, provider: vscode.TerminalQuickFixProvider): vscode.Disposable { if (this._quickFixProviders.has(id)) { diff --git a/src/vs/workbench/contrib/terminal/browser/terminal.ts b/src/vs/workbench/contrib/terminal/browser/terminal.ts index ead75598c0ec3..72d5522afd4f9 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminal.ts @@ -262,7 +262,7 @@ export interface ITerminalService extends ITerminalInstanceHost { readonly onDidRequestStartExtensionTerminal: Event; readonly onDidRegisterProcessSupport: Event; readonly onDidChangeConnectionState: Event; - + readonly onAnyInstanceShellTypeChanged: Event; // Here or on proposed api? - also there is onDidChangeShellType that already exists.. // Group events readonly onDidChangeActiveGroup: Event; @@ -705,7 +705,7 @@ export interface ITerminalInstance extends IBaseTerminalInstance { onDidExecuteText: Event; onDidChangeTarget: Event; onDidSendText: Event; - onDidChangeShellType: Event; + onDidChangeShellType: Event; // how should this be used in correlation to my api which should expose shell type onDidChangeVisibility: Event; /** diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index 589b1deaf9cac..4fd29f09e5cb5 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -1910,6 +1910,8 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { this._shellType = shellType; this._terminalShellTypeContextKey.set(shellType?.toString()); this._onDidChangeShellType.fire(shellType); + // Can I fire here to let my new API know? but how should I do that + } } From 63a931c495dde1303a987885931cb969bb8903bc Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 10 Jan 2025 02:57:34 -0500 Subject: [PATCH 2/5] fix myself --- src/vs/workbench/api/browser/mainThreadTerminalService.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index 1379c0aca51f4..2b9182c259817 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -87,7 +87,7 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape this._store.add(_terminalService.onAnyInstanceTitleChange(instance => instance && this._onTitleChanged(instance.instanceId, instance.title))); this._store.add(_terminalService.onAnyInstanceDataInput(instance => this._proxy.$acceptTerminalInteraction(instance.instanceId))); this._store.add(_terminalService.onAnyInstanceSelectionChange(instance => this._proxy.$acceptTerminalSelection(instance.instanceId, instance.selection))); - this._store.add(_terminalService.onAnyInstanceShellTypeChange(instance => this._proxy.$acceptTerminalShellType(instance.instanceId, instance.shellType))); + this._store.add(_terminalService.onAnyInstanceShellTypeChange(instance => this._onShellTypeChanged(instance.instanceId, instance.shellType))); // Do I need this? I don't think so // Set initial ext host state for (const instance of this._terminalService.instances) { @@ -357,7 +357,7 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape private _onShellTypeChanged(terminalId: number, shellType: string): void { const terminalInstance = this._terminalService.getInstanceFromId(terminalId)?.shellType; if (terminalInstance) { - this._proxy.$provideTerminalShellType(terminalId, shellType); + this._proxy.$acceptTerminalShellType(terminalId, shellType); } } From 26b6986e85fd10ea74329a3de9c2bfa6d787a9d9 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 10 Jan 2025 03:03:06 -0500 Subject: [PATCH 3/5] leave some questions --- src/vs/workbench/api/common/extHostTerminalService.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index 85db4c09e0ec0..57a995c09af09 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -419,6 +419,8 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I readonly onDidChangeTerminalState = this._onDidChangeTerminalState.event; protected readonly _onDidChangeShell = new Emitter(); readonly onDidChangeShell = this._onDidChangeShell.event; + // Should onDidChange/AcceptShellType go here? + protected readonly _onDidWriteTerminalData = new Emitter({ onWillAddFirstListener: () => this._proxy.$startSendingDataEvents(), From 2020869d6b6f55e0292eda4b297aaa3b46ead21b Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 10 Jan 2025 15:52:28 -0500 Subject: [PATCH 4/5] return void for $acceptTerminalShellType --- src/vs/workbench/api/common/extHost.protocol.ts | 2 +- src/vs/workbench/api/common/extHostTerminalService.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 8b2fe5266e39c..c2d006d10a874 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -2417,7 +2417,7 @@ export interface ExtHostTerminalServiceShape { $acceptTerminalMaximumDimensions(id: number, cols: number, rows: number): void; $acceptTerminalInteraction(id: number): void; $acceptTerminalSelection(id: number, selection: string | undefined): void; - $acceptTerminalShellType(id: number, shellType: string): Promise; + $acceptTerminalShellType(id: number, shellType: string): void; $startExtensionTerminal(id: number, initialDimensions: ITerminalDimensionsDto | undefined): Promise; $acceptProcessAckDataEvent(id: number, charCount: number): void; $acceptProcessInput(id: number, data: string): void; diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index 57a995c09af09..540c2617e67f8 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -767,7 +767,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I return completions; } // Take in shellType as a string and return VSCode Terminal Shell Type? - public async $acceptTerminalShellType(id: number, shellType: string): Promise { + public $acceptTerminalShellType(id: number, shellType: string): void { // TODO: Implement return GeneralShellType.Python; } From 73a79dde410e88305fda658981f4dadc0e97e932 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 10 Jan 2025 16:21:57 -0500 Subject: [PATCH 5/5] partial changes after feedback! --- .../api/browser/mainThreadTerminalService.ts | 10 +++++----- src/vs/workbench/api/common/extHost.protocol.ts | 2 +- .../api/common/extHostTerminalService.ts | 17 ++++++++++++----- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index 2b9182c259817..9bc981b6435fa 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -87,7 +87,7 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape this._store.add(_terminalService.onAnyInstanceTitleChange(instance => instance && this._onTitleChanged(instance.instanceId, instance.title))); this._store.add(_terminalService.onAnyInstanceDataInput(instance => this._proxy.$acceptTerminalInteraction(instance.instanceId))); this._store.add(_terminalService.onAnyInstanceSelectionChange(instance => this._proxy.$acceptTerminalSelection(instance.instanceId, instance.selection))); - this._store.add(_terminalService.onAnyInstanceShellTypeChange(instance => this._onShellTypeChanged(instance.instanceId, instance.shellType))); // Do I need this? I don't think so + this._store.add(_terminalService.onAnyInstanceShellTypeChanged(instance => this._onShellTypeChanged(instance.instanceId))); // Set initial ext host state for (const instance of this._terminalService.instances) { @@ -354,10 +354,10 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape this._proxy.$acceptTerminalTitleChange(terminalId, name); } - private _onShellTypeChanged(terminalId: number, shellType: string): void { - const terminalInstance = this._terminalService.getInstanceFromId(terminalId)?.shellType; - if (terminalInstance) { - this._proxy.$acceptTerminalShellType(terminalId, shellType); + private _onShellTypeChanged(terminalId: number): void { + const terminalInstanceShellType = this._terminalService.getInstanceFromId(terminalId)?.shellType; + if (terminalInstanceShellType) { + this._proxy.$acceptTerminalShellType(terminalId, terminalInstanceShellType); } } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index c2d006d10a874..c76ebd8fde729 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -2417,7 +2417,7 @@ export interface ExtHostTerminalServiceShape { $acceptTerminalMaximumDimensions(id: number, cols: number, rows: number): void; $acceptTerminalInteraction(id: number): void; $acceptTerminalSelection(id: number, selection: string | undefined): void; - $acceptTerminalShellType(id: number, shellType: string): void; + $acceptTerminalShellType(id: number, shellType: TerminalShellType | undefined): void; $startExtensionTerminal(id: number, initialDimensions: ITerminalDimensionsDto | undefined): Promise; $acceptProcessAckDataEvent(id: number, charCount: number): void; $acceptProcessInput(id: number, data: string): void; diff --git a/src/vs/workbench/api/common/extHostTerminalService.ts b/src/vs/workbench/api/common/extHostTerminalService.ts index 540c2617e67f8..c8204f7d32dff 100644 --- a/src/vs/workbench/api/common/extHostTerminalService.ts +++ b/src/vs/workbench/api/common/extHostTerminalService.ts @@ -85,7 +85,8 @@ export class ExtHostTerminal extends Disposable { private _pidPromiseComplete: ((value: number | undefined) => any) | undefined; private _rows: number | undefined; private _exitStatus: vscode.TerminalExitStatus | undefined; - private _state: vscode.TerminalState = { isInteractedWith: false }; + // TODO: add shellType as TerminalState on in newly created proposed API. + private _state: vscode.TerminalState = { isInteractedWith: false, shellType: undefined }; private _selection: string | undefined; shellIntegration: vscode.TerminalShellIntegration | undefined; @@ -264,6 +265,10 @@ export class ExtHostTerminal extends Disposable { return false; } + public setShellType(shellType: TerminalShellType): void { + this._state = shellType; + } + public setSelection(selection: string | undefined): void { this._selection = selection; } @@ -766,10 +771,12 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I return completions; } - // Take in shellType as a string and return VSCode Terminal Shell Type? - public $acceptTerminalShellType(id: number, shellType: string): void { - // TODO: Implement - return GeneralShellType.Python; + + public $acceptTerminalShellType(id: number, shellType: TerminalShellType | undefined): void { + const terminal = this.getTerminalById(id); + terminal?.setShellType(shellType); + + this._onDidChangeTerminalState.fire(terminal?.shellType); } public registerTerminalQuickFixProvider(id: string, extensionId: string, provider: vscode.TerminalQuickFixProvider): vscode.Disposable {