Use jsdoc to mark APIs that don't work in the web extension host

[jrieken]

We have a few APIs that don't make sense in the web extension host and we should mark then clear, one sample is ExtensionContent#globalStoragePath which is spec'd to be a file path. We should find graceful fallbacks, using default values, or throw

symbol	today	future		done
ExtensionContext.environmentVariableCollection	💥	could be graceful/empty? @Tyriar		✅
env.shell	💥	better use empty string		✅
env.appRoot	🙅	use empty string, add doc		✅
window.createTerminal	💥	only pseudo terminal, doc the rest	@Tyriar for adding doc	✅
task.executeTask	💥	only custom exec, doc the rest	@alexr00 for adding doc	✅
workspace.openTunnel	💥	@alexr00 for adding doc		✅
_workbench.downloadResource	🙅	missing, could be supported	we are leaving this, the real thing would be #12588	🙅

[jrieken]

use xyzUri for: vscode.ExtensionContext.globalStoragePath, vscode.ExtensionContext.storagePath, and vscode.ExtensionContext.logPath

[jrieken]

@rebornix @mjbvz fyi - I have made the storage paths use URI, enabled them for web, and adopted them for notebooks and webview. Please double check.

[jrieken]

Proposal for extending vscode.ExtensionContext

export interface ExtensionContext {

		/**
		 * The uri of a directory in which the extension can create log files.
		 * The directory might not exist on disk and creation is up to the extension. However,
		 * the parent directory is guaranteed to be existent.
		 *
		 * @see vscode.workspace.fs
		 */
		readonly logUri: Uri;

		/**
		 * The uri of a workspace specific directory in which the extension
		 * can store private state. The directory might not exist on disk and creation is
		 * up to the extension. However, the parent directory is guaranteed to be existent.
		 *
		 * Use [`workspaceState`](#ExtensionContext.workspaceState) or
		 * [`globalState`](#ExtensionContext.globalState) to store key value data.
		 */
		readonly storageUri: Uri | undefined;

		/**
		 * The uri of a directory in which the extension can store global state.
		 * The directory might not exist on disk and creation is
		 * up to the extension. However, the parent directory is guaranteed to be existent.
		 *
		 * Use [`globalState`](#ExtensionContext.globalState) to store key value data.
		 */
		readonly globalStorageUri: Uri;
}

[mjbvz]

Looks good to me. Should we mark the string version of these properties (including extensionPath) as deprecated?

[jrieken]

Yeah, somewhat a soft deprecation. All currently existing use-cases work still fine but future use-cases should use the URI variant. For extensionPath we went with saying that it is a shorthand notation for extensionUri.fsPath but I don't really like that

[alexr00]

Proposal for executeTask:

		/**
		 * Executes a task that is managed by VS Code. The returned
		 * task execution can be used to terminate the task.
		 *
		 * *Note* that `executeTask` cannot execute ShellExecution and ProcessExecution
		 * tasks when running in an environment where a new process can't be started.
		 * In such an environment, only CustomExecution tasks can be run.
		 *
		 * @param task the task to execute
		 */
		export function executeTask(task: Task): Thenable<TaskExecution>;

Proposal for openTunnel:

		/**
		 * Forwards a port. If the current resolver implements RemoteAuthorityResolver:forwardPort then that will be used to make the tunnel.
		 * By default, openTunnel only support localhost; however, RemoteAuthorityResolver:tunnelFactory can be used to support other ips.
		 *
		 * *Note* that `openTunnel` will throw an error when running in an environment without a remote.
		 *
		 * @param tunnelOptions The `localPort` is a suggestion only. If that port is not available another will be chosen.
		 */
		export function openTunnel(tunnelOptions: TunnelOptions): Thenable<Tunnel>;

[Tyriar]

ExtensionContext.environmentVariableCollection | 💥 | could be graceful/empty? @Tyriar

An extension would explicitly request this on the web worker extension before it tries to call createTerminal, wouldn't it be better to fail fast? It makes no sense to call this without following it up with the createTerminal.

@Tyriar for adding doc

Should we use @throws instead of *Note*?

[jrieken]

An extension would explicitly request this on the web worker extension before it tries to call createTerminal, wouldn't it be better to fail fast? It makes no sense to call this without following it up with the createTerminal

Isn't the terminal just a consumer of the env-collection and not the prereq? Looking at the API of EnvironmentVariableCollection there is nothing that strongly hints towards terminal so I wonder why this is so tightly coupled, despite living in env and not terminal

[Tyriar]

@jrieken you're right that was the intent behind the design, I'll change it to be functional but currently it won't actually do anything.

[jrieken]

currently it won't actually do anything.

Yeah, I think it's good if it's read/write only, e.g being consumable by extensions that wanna do something with it

[jrieken]

Should we use @throws instead of Note?

I like 👍

[Tyriar]

@jrieken done my bits

[jrieken]

👏 ideas what to do with env.shell instead of throwing? empty string?

[jrieken]

I have pushed another change that turned the errors into NotSupportedError errors (instead of NotImplemented)