Merge pull request microsoft#209068 from microsoft/tyriar/208368

Don't expose cwd as a string on shellIntegration/execution

Author: Tyriar

src/vs/platform/terminal/common/capabilities/commandDetection/terminalCommand.ts

@@ -263,6 +263,7 @@ export class PartialTerminalCommand implements ICurrentPartialCommand {
 	currentContinuationMarker?: IMarker;
 	continuations?: { marker: IMarker; end: number }[];
 
+	cwd?: string;
 	command?: string;
 
 	isTrusted?: boolean;

src/vs/platform/terminal/common/capabilities/commandDetectionCapability.ts

@@ -281,6 +281,7 @@ export class CommandDetectionCapability extends Disposable implements ICommandDe
 
 	handleCommandStart(options?: IHandleCommandOptions): void {
 		this._handleCommandStartOptions = options;
+		this._currentCommand.cwd = this._cwd;
 		// Only update the column if the line has already been set
 		this._currentCommand.commandStartMarker = options?.marker || this._currentCommand.commandStartMarker;
 		if (this._currentCommand.commandStartMarker?.line === this._terminal.buffer.active.cursorY) {

src/vs/workbench/api/browser/mainThreadTerminalShellIntegration.ts

@@ -5,6 +5,7 @@
 
 import { Event } from 'vs/base/common/event';
 import { Disposable } from 'vs/base/common/lifecycle';
+import { URI } from 'vs/base/common/uri';
 import { TerminalCapability, type ITerminalCommand } from 'vs/platform/terminal/common/capabilities/capabilities';
 import { ExtHostContext, MainContext, type ExtHostTerminalShellIntegrationShape, type MainThreadTerminalShellIntegrationShape } from 'vs/workbench/api/common/extHost.protocol';
 import { ITerminalService } from 'vs/workbench/contrib/terminal/browser/terminal';
@@ -43,8 +44,9 @@ export class MainThreadTerminalShellIntegration extends Disposable implements Ma
 			if (e.data === currentCommand) {
 				return;
 			}
+			// String paths are not exposed in the extension API
 			currentCommand = e.data;
-			this._proxy.$shellExecutionStart(e.instance.instanceId, e.data.command, e.data.cwd);
+			this._proxy.$shellExecutionStart(e.instance.instanceId, e.data.command, this._convertCwdToUri(e.data.cwd));
 		}));
 
 		// onDidEndTerminalShellExecution
@@ -56,7 +58,9 @@ export class MainThreadTerminalShellIntegration extends Disposable implements Ma
 
 		// onDidChangeTerminalShellIntegration via cwd
 		const cwdChangeEvent = this._store.add(this._terminalService.createOnInstanceCapabilityEvent(TerminalCapability.CwdDetection, e => e.onDidChangeCwd));
-		this._store.add(cwdChangeEvent.event(e => this._proxy.$cwdChange(e.instance.instanceId, e.data)));
+		this._store.add(cwdChangeEvent.event(e => {
+			this._proxy.$cwdChange(e.instance.instanceId, this._convertCwdToUri(e.data));
+		}));
 
 		// Clean up after dispose
 		this._store.add(this._terminalService.onDidDisposeInstance(e => this._proxy.$closeTerminal(e.instanceId)));
@@ -71,4 +75,8 @@ export class MainThreadTerminalShellIntegration extends Disposable implements Ma
 	$executeCommand(terminalId: number, commandLine: string): void {
 		this._terminalService.getInstanceFromId(terminalId)?.runCommand(commandLine, true);
 	}
+
+	private _convertCwdToUri(cwd: string | undefined): URI | undefined {
+		return cwd ? URI.file(cwd) : undefined;
+	}
 }

src/vs/workbench/api/common/extHost.protocol.ts

@@ -2268,10 +2268,10 @@ 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;
+	$cwdChange(instanceId: number, cwd: UriComponents | undefined): void;
 	$closeTerminal(instanceId: number): void;
 }
 

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 | undefined): 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,18 +225,18 @@ 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 = {
 			get terminal(): vscode.Terminal {
-				return terminal;
+				return that.terminal;
 			},
 			get commandLine(): string | undefined {
 				return that._commandLine;
 			},
-			get cwd(): URI | string | undefined {
-				return cwd;
+			get cwd(): URI | undefined {
+				return that.cwd;
 			},
 			createDataStream(): AsyncIterable<string> {
 				return that._createDataStream();

src/vscode-dts/vscode.proposed.terminalShellIntegration.d.ts

@@ -29,11 +29,11 @@ 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.
 		 */
-		readonly cwd: Uri | string | undefined;
+		readonly cwd: Uri | undefined;
 
 		/**
 		 * Creates a stream of raw data (including escape sequences) that is written to the
@@ -67,12 +67,12 @@ declare module 'vscode' {
 	}
 
 	export interface TerminalShellIntegration {
-		// TODO: Is this fine to share the TerminalShellIntegrationChangeEvent event?
+		// TODO: Should this share TerminalShellIntegrationChangeEvent or have it's own TerminalShellIntegrationCwdChangeEvent?
 		/**
-		 * 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.