Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ export class PartialTerminalCommand implements ICurrentPartialCommand {
currentContinuationMarker?: IMarker;
continuations?: { marker: IMarker; end: number }[];

cwd?: string;
command?: string;

isTrusted?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
12 changes: 10 additions & 2 deletions src/vs/workbench/api/browser/mainThreadTerminalShellIntegration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand All @@ -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)));
Expand All @@ -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;
}
}
4 changes: 2 additions & 2 deletions src/vs/workbench/api/common/extHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
24 changes: 11 additions & 13 deletions src/vs/workbench/api/common/extHostTerminalShellIntegration.ts
Original file line number Diff line number Diff line change
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 | undefined): 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,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();
Expand Down
10 changes: 5 additions & 5 deletions src/vscode-dts/vscode.proposed.terminalShellIntegration.d.ts
Original file line number Diff line number Diff line change
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,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.
Expand Down
Loading