TerminalShellExecution#cwd should not be an URI or string

[jrieken]

Testing #208462

The cwd: Uri | string | undefined; doesn't seem helpful because string basically means "VS Code couldn't figure this out, good luck to you". In the API we do the hard part and extensions should have easy times, meaning it should be URI | undefined

[jrieken]

same for TerminalShellIntegration#cwd

[Tyriar]

It will actually never be a URI currently but it was added for future compatibility.

VS Code couldn't figure this out, good luck to you

This is a very valid scenario, consider SSHing some remote that VS Code knows nothing about, or some shell reports via a virtual file system that we're not aware of, etc.

[jrieken]

How should an extension use the cwd when delivered as string? All of our APIs are uri-based and strings are usually assumed as file-uris.

This is a very valid scenario, consider SSHing some remote that VS Code knows nothing about, or some shell reports via a virtual file system that we're not aware of, etc.

Make it undefined in such cases until we have a better fix. If we cannot get this right, don't offer such an API

[Tyriar]

We've touched on this in a past API sync a few months ago and URI | string | undefined was fine back then if I remember right.

It's worth mentioning that there are several shell/process-related cwds as strings currently, where these are the strings directly passed to spawn:

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

Lines 8134 to 8139 in 738438c

	export interface ProcessExecutionOptions {
	/**
	* The current working directory of the executed program or shell.
	* If omitted the tools current workspace root is used.
	*/
	cwd?: string;

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

Lines 8243 to 8247 in 738438c

	/**
	* The current working directory of the executed shell.
	* If omitted the tools current workspace root is used.
	*/
	cwd?: string;

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

Lines 11720 to 11723 in 738438c

	/**
	* A path or Uri for the current working directory to be used for the terminal.
	*/
	cwd?: string | Uri;

Make it undefined in such cases until we have a better fix. If we cannot get this right, don't offer such an API

I don't think it's possible to get this as a URI, so we would be hiding information that may be useful - for example we find it useful and display the cwd in the terminal tab, exactly as it's reported by the shell. Shells can give us whatever they want for the cwd. If we went with URI | undefined it would be a breaking change to switch to URI | string | undefined, in addition to being inconsistent with TerminalOptions.cwd.

[jrieken]

Aren't those samples all about extensions giving data to us, not us giving data to extensions?

[Tyriar]

Yep. Thanks for the feedback, I'll have a deeper think about it next month