Skip to content

Commit

Permalink
Don't expose cwd as a string on shellIntegration/execution
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Tyriar committed Mar 28, 2024
1 parent e857eec commit 38c1ddc
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 17 deletions.
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/api/common/extHost.protocol.ts
Expand Up @@ -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;
Expand Down
20 changes: 9 additions & 11 deletions src/vs/workbench/api/common/extHostTerminalShellIntegration.ts
Expand Up @@ -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)) {
Expand All @@ -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);
}

Expand All @@ -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());

Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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 = {
Expand All @@ -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<string> {
Expand Down
9 changes: 4 additions & 5 deletions src/vscode-dts/vscode.proposed.terminalShellIntegration.d.ts
Expand Up @@ -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=<Cwd> 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
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 38c1ddc

Please sign in to comment.