From 38c1ddc1a2265e5c8c757ca92bc508f599e10339 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 28 Mar 2024 12:51:05 -0700 Subject: [PATCH] Don't expose cwd as a string on shellIntegration/execution Since we generally want to expose all paths as URIs (unless user-provided paths), we will only expose this as a URI. This means extensions are potentially missing out on important information which they may be able to handle (eg. SSHing in the terminal), but this is a small edge case and if the extension really wanted to handle it they should be able to parse the sequences themselves in the future via onDidWriteTerminalData. Part of #208368 --- .../mainThreadTerminalShellIntegration.ts | 4 ++++ .../workbench/api/common/extHost.protocol.ts | 2 +- .../common/extHostTerminalShellIntegration.ts | 20 +++++++++---------- ...ode.proposed.terminalShellIntegration.d.ts | 9 ++++----- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTerminalShellIntegration.ts b/src/vs/workbench/api/browser/mainThreadTerminalShellIntegration.ts index 044e9017d7a3a0..ab771ec17d3ec6 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalShellIntegration.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalShellIntegration.ts @@ -38,6 +38,10 @@ export class MainThreadTerminalShellIntegration extends Disposable implements Ma const commandDetectionStartEvent = this._store.add(this._terminalService.createOnInstanceCapabilityEvent(TerminalCapability.CommandDetection, e => e.onCommandExecuted)); let currentCommand: ITerminalCommand | undefined; this._store.add(commandDetectionStartEvent.event(e => { + // String paths are not exposed in the extension API + if (typeof e.data.cwd === 'string') { + return; + } // Prevent duplicate events from being sent in case command detection double fires the // event if (e.data === currentCommand) { diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index c4f5c720677e24..7fb0d5693568c8 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -2268,7 +2268,7 @@ export interface ExtHostTerminalServiceShape { export interface ExtHostTerminalShellIntegrationShape { $shellIntegrationChange(instanceId: number): void; - $shellExecutionStart(instanceId: number, commandLine: string | undefined, cwd: UriComponents | string | undefined): void; + $shellExecutionStart(instanceId: number, commandLine: string | undefined, cwd: UriComponents | undefined): void; $shellExecutionEnd(instanceId: number, commandLine: string | undefined, exitCode: number | undefined): void; $shellExecutionData(instanceId: number, data: string): void; $cwdChange(instanceId: number, cwd: UriComponents | string): void; diff --git a/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts b/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts index f4741b8b60dfbd..302404ea86f64a 100644 --- a/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts +++ b/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts @@ -103,7 +103,7 @@ export class ExtHostTerminalShellIntegration extends Disposable implements IExtH }); } - public $shellExecutionStart(instanceId: number, commandLine: string, cwd: URI | string | undefined): void { + public $shellExecutionStart(instanceId: number, commandLine: string, cwd: URI | undefined): void { // Force shellIntegration creation if it hasn't been created yet, this could when events // don't come through on startup if (!this._activeShellIntegrations.has(instanceId)) { @@ -120,7 +120,7 @@ export class ExtHostTerminalShellIntegration extends Disposable implements IExtH this._activeShellIntegrations.get(instanceId)?.emitData(data); } - public $cwdChange(instanceId: number, cwd: string): void { + public $cwdChange(instanceId: number, cwd: URI): void { this._activeShellIntegrations.get(instanceId)?.setCwd(cwd); } @@ -136,7 +136,7 @@ class InternalTerminalShellIntegration extends Disposable { get currentExecution(): InternalTerminalShellExecution | undefined { return this._currentExecution; } private _ignoreNextExecution: boolean = false; - private _cwd: URI | string | undefined; + private _cwd: URI | undefined; readonly store: DisposableStore = this._register(new DisposableStore()); @@ -157,7 +157,7 @@ class InternalTerminalShellIntegration extends Disposable { const that = this; this.value = { - get cwd(): URI | string | undefined { + get cwd(): URI | undefined { return that._cwd; }, executeCommand(commandLine): vscode.TerminalShellExecution { @@ -171,7 +171,7 @@ class InternalTerminalShellIntegration extends Disposable { }; } - startShellExecution(commandLine: string, cwd: URI | string | undefined, fireEventInMicrotask?: boolean): InternalTerminalShellExecution { + startShellExecution(commandLine: string, cwd: URI | undefined, fireEventInMicrotask?: boolean): InternalTerminalShellExecution { if (this._ignoreNextExecution && this._currentExecution) { this._ignoreNextExecution = false; } else { @@ -201,12 +201,10 @@ class InternalTerminalShellIntegration extends Disposable { } } - setCwd(cwd: URI | string): void { + setCwd(cwd: URI | undefined): void { let wasChanged = false; if (URI.isUri(this._cwd)) { - if (this._cwd.toString() !== cwd.toString()) { - wasChanged = true; - } + wasChanged = !URI.isUri(cwd) || this._cwd.toString() !== cwd.toString(); } else if (this._cwd !== cwd) { wasChanged = true; } @@ -227,7 +225,7 @@ class InternalTerminalShellExecution { constructor( readonly terminal: vscode.Terminal, private _commandLine: string | undefined, - readonly cwd: URI | string | undefined, + readonly cwd: URI | undefined, ) { const that = this; this.value = { @@ -237,7 +235,7 @@ class InternalTerminalShellExecution { get commandLine(): string | undefined { return that._commandLine; }, - get cwd(): URI | string | undefined { + get cwd(): URI | undefined { return cwd; }, createDataStream(): AsyncIterable { diff --git a/src/vscode-dts/vscode.proposed.terminalShellIntegration.d.ts b/src/vscode-dts/vscode.proposed.terminalShellIntegration.d.ts index cc2f4f5eb959e0..15382de1e40ae2 100644 --- a/src/vscode-dts/vscode.proposed.terminalShellIntegration.d.ts +++ b/src/vscode-dts/vscode.proposed.terminalShellIntegration.d.ts @@ -30,12 +30,12 @@ declare module 'vscode' { /** * The working directory that was reported by the shell when this command executed. This - * will be a {@link Uri} if the string reported by the shell can reliably be mapped to the + * will be a {@link Uri} if the path reported by the shell can reliably be mapped to the * connected machine. This requires the shell integration to support working directory * reporting via the [`OSC 633 ; P`](https://code.visualstudio.com/docs/terminal/shell-integration#_vs-code-custom-sequences-osc-633-st) * or `OSC 1337 ; CurrentDir= ST` sequences. */ - readonly cwd: Uri | string | undefined; + readonly cwd: Uri | undefined; /** * Creates a stream of raw data (including escape sequences) that is written to the @@ -69,12 +69,11 @@ declare module 'vscode' { } export interface TerminalShellIntegration { - // TODO: Is this fine to share the TerminalShellIntegrationChangeEvent event? /** - * The current working directory of the terminal. This will be a {@link Uri} if the string + * The current working directory of the terminal. This will be a {@link Uri} if the path * reported by the shell can reliably be mapped to the connected machine. */ - readonly cwd: Uri | string | undefined; + readonly cwd: Uri | undefined; /** * Execute a command, sending ^C as necessary to interrupt any running command if needed.