From 47aa3ad09ab147001d73bd150e90eeb9dbcbac8d Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Tue, 12 Jan 2021 23:12:10 -0800 Subject: [PATCH] Continue work on opener service - Add error notification if opener throws an exception - Add public facing id to openers. This is used in settings - Add intellisense for the opener id setting --- extensions/simple-browser/src/extension.ts | 26 ++++----- src/vs/vscode.proposed.d.ts | 19 ++++++- .../api/browser/mainThreadUriOpeners.ts | 44 ++++++++++----- .../workbench/api/common/extHost.protocol.ts | 8 +-- .../workbench/api/common/extHostUriOpener.ts | 56 ++++++++++--------- .../externalUriOpener/common/configuration.ts | 15 ++++- 6 files changed, 106 insertions(+), 62 deletions(-) diff --git a/extensions/simple-browser/src/extension.ts b/extensions/simple-browser/src/extension.ts index 568552c684e..129b3bf6fcc 100644 --- a/extensions/simple-browser/src/extension.ts +++ b/extensions/simple-browser/src/extension.ts @@ -13,6 +13,13 @@ const localize = nls.loadMessageBundle(); const openApiCommand = 'simpleBrowser.api.open'; const showCommand = 'simpleBrowser.show'; +const enabledHosts = new Set([ + 'localhost', + '127.0.0.1' +]); + +const openerId = 'simpleBrowser.open'; + export function activate(context: vscode.ExtensionContext) { const manager = new SimpleBrowserManager(context.extensionUri); @@ -42,25 +49,18 @@ export function activate(context: vscode.ExtensionContext) { return false; } - const enabledHosts = configuration.get('opener.enabledHosts', [ - 'localhost', - '127.0.0.1' - ]); - try { - const originalUri = new URL(uri.toString()); - if (!enabledHosts.includes(originalUri.hostname)) { - return false; - } - } catch { - return false; + const originalUri = new URL(uri.toString()); + if (enabledHosts.has(originalUri.hostname)) { + return true; } - return true; + return false; }, - openExternalUri(_opener, resolveUri) { + openExternalUri(resolveUri: vscode.Uri) { return manager.show(resolveUri.toString()); } }, { + id: openerId, label: localize('openTitle', "Open in simple browser"), })); } diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 113d4621efe..a9849d1eae4 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -2292,7 +2292,7 @@ declare module 'vscode' { * Handles opening uris to external resources, such as http(s) links. * * Extensions can implement an `ExternalUriOpener` to open `http` links to a webserver - * inside of VS Code instead of having the link be opened by the webbrowser. + * inside of VS Code instead of having the link be opened by the web browser. * * Currently openers may only be registered for `http` and `https` uris. */ @@ -2312,6 +2312,12 @@ declare module 'vscode' { /** * Open the given uri. * + * This is invoked when: + * + * - The user clicks a link which does not have an assigned opener. In this case, first `canOpenExternalUri` + * is called and if the user selects this opener, then `openExternalUri` is called. + * - The user sets the default opener for a link in their settings and then visits a link. + * * @param resolvedUri The uri to open. This uri may have been transformed by port forwarding, so it * may not match the original uri passed to `canOpenExternalUri`. Use `ctx.originalUri` to check the * original uri. @@ -2335,8 +2341,17 @@ declare module 'vscode' { readonly sourceUri: Uri; } - + /** + * Additional metadata about the registered opener. + */ interface ExternalUriOpenerMetadata { + /** + * Unique id of the opener, such as `myExtension.browserPreview` + * + * This is used in settings and commands to identifier the opener. + */ + readonly id: string; + /** * Text displayed to the user that explains what the opener does. * diff --git a/src/vs/workbench/api/browser/mainThreadUriOpeners.ts b/src/vs/workbench/api/browser/mainThreadUriOpeners.ts index c3e8176a6f5..f521610aa0d 100644 --- a/src/vs/workbench/api/browser/mainThreadUriOpeners.ts +++ b/src/vs/workbench/api/browser/mainThreadUriOpeners.ts @@ -4,11 +4,15 @@ *--------------------------------------------------------------------------------------------*/ import { CancellationToken } from 'vs/base/common/cancellation'; +import { isPromiseCanceledError } from 'vs/base/common/errors'; import { Disposable } from 'vs/base/common/lifecycle'; import { Schemas } from 'vs/base/common/network'; import { URI } from 'vs/base/common/uri'; +import { localize } from 'vs/nls'; import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; +import { INotificationService } from 'vs/platform/notification/common/notification'; import { ExtHostContext, ExtHostUriOpenersShape, IExtHostContext, MainContext, MainThreadUriOpenersShape } from 'vs/workbench/api/common/extHost.protocol'; +import { externalUriOpenerIdSchemaAddition } from 'vs/workbench/contrib/externalUriOpener/common/configuration'; import { ExternalOpenerEntry, IExternalOpenerProvider, IExternalUriOpenerService } from 'vs/workbench/contrib/externalUriOpener/common/externalUriOpenerService'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; import { extHostNamedCustomer } from '../common/extHostCustomers'; @@ -23,12 +27,13 @@ interface RegisteredOpenerMetadata { export class MainThreadUriOpeners extends Disposable implements MainThreadUriOpenersShape, IExternalOpenerProvider { private readonly proxy: ExtHostUriOpenersShape; - private readonly registeredOpeners = new Map(); + private readonly _registeredOpeners = new Map(); constructor( context: IExtHostContext, @IExternalUriOpenerService private readonly externalUriOpenerService: IExternalUriOpenerService, @IExtensionService private readonly extensionService: IExtensionService, + @INotificationService private readonly notificationService: INotificationService, ) { super(); this.proxy = context.getProxy(ExtHostContext.ExtHostUriOpeners); @@ -47,48 +52,59 @@ export class MainThreadUriOpeners extends Disposable implements MainThreadUriOpe await this.extensionService.activateByEvent(`onUriOpen:${targetUri.scheme}`); // If there are no handlers there is no point in making a round trip - const hasHandler = Array.from(this.registeredOpeners.values()).some(x => x.schemes.has(targetUri.scheme)); + const hasHandler = Array.from(this._registeredOpeners.values()).some(x => x.schemes.has(targetUri.scheme)); if (!hasHandler) { return []; } - const openerHandles = await this.proxy.$getOpenersForUri(targetUri, CancellationToken.None); - - return openerHandles.map(handle => this.openerForCommand(handle, targetUri)); + const openerIds = await this.proxy.$getOpenersForUri(targetUri, CancellationToken.None); + return openerIds.map(id => this.createOpener(id, targetUri)); } - private openerForCommand(openerHandle: number, sourceUri: URI): ExternalOpenerEntry { - const metadata = this.registeredOpeners.get(openerHandle)!; + private createOpener(openerId: string, sourceUri: URI): ExternalOpenerEntry { + const metadata = this._registeredOpeners.get(openerId)!; return { - id: metadata.extensionId.value, + id: openerId, label: metadata.label, openExternal: async (href) => { const resolveUri = URI.parse(href); - await this.proxy.$openUri(openerHandle, { resolveUri, sourceUri }, CancellationToken.None); + try { + await this.proxy.$openUri(openerId, { resolveUri, sourceUri }, CancellationToken.None); + } catch (e) { + if (!isPromiseCanceledError(e)) { + this.notificationService.error(localize('openerFailedMessage', "Could not open uri: {0}", e.toString())); + } + } return true; }, }; } async $registerUriOpener( - handle: number, + id: string, schemes: readonly string[], extensionId: ExtensionIdentifier, label: string, ): Promise { - this.registeredOpeners.set(handle, { + if (this._registeredOpeners.has(id)) { + throw new Error(`Opener with id already registered: '${id}'`); + } + + this._registeredOpeners.set(id, { schemes: new Set(schemes), label, extensionId, }); + + externalUriOpenerIdSchemaAddition.enum?.push(id); } - async $unregisterUriOpener(handle: number): Promise { - this.registeredOpeners.delete(handle); + async $unregisterUriOpener(id: string): Promise { + this._registeredOpeners.delete(id); } dispose(): void { super.dispose(); - this.registeredOpeners.clear(); + this._registeredOpeners.clear(); } } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index afca7cb2b68..d997cd5f951 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -802,13 +802,13 @@ export interface ExtHostUrlsShape { } export interface MainThreadUriOpenersShape extends IDisposable { - $registerUriOpener(handle: number, schemes: readonly string[], extensionId: ExtensionIdentifier, label: string): Promise; - $unregisterUriOpener(handle: number): Promise; + $registerUriOpener(id: string, schemes: readonly string[], extensionId: ExtensionIdentifier, label: string): Promise; + $unregisterUriOpener(id: string): Promise; } export interface ExtHostUriOpenersShape { - $getOpenersForUri(uri: UriComponents, token: CancellationToken): Promise; - $openUri(handle: number, context: { resolveUri: UriComponents, sourceUri: UriComponents }, token: CancellationToken): Promise; + $getOpenersForUri(uri: UriComponents, token: CancellationToken): Promise; + $openUri(id: string, context: { resolveUri: UriComponents, sourceUri: UriComponents }, token: CancellationToken): Promise; } export interface ITextSearchComplete { diff --git a/src/vs/workbench/api/common/extHostUriOpener.ts b/src/vs/workbench/api/common/extHostUriOpener.ts index 81b761bb3ca..7d8115d41e2 100644 --- a/src/vs/workbench/api/common/extHostUriOpener.ts +++ b/src/vs/workbench/api/common/extHostUriOpener.ts @@ -19,11 +19,9 @@ interface OpenerEntry { export class ExtHostUriOpeners implements ExtHostUriOpenersShape { - private static HandlePool = 0; - private readonly _proxy: MainThreadUriOpenersShape; - private readonly _openers = new Map(); + private readonly _openers = new Map(); constructor( mainContext: IMainContext, @@ -37,48 +35,52 @@ export class ExtHostUriOpeners implements ExtHostUriOpenersShape { opener: vscode.ExternalUriOpener, metadata: vscode.ExternalUriOpenerMetadata, ): vscode.Disposable { - const handle = ExtHostUriOpeners.HandlePool++; + const id = metadata.id; + if (this._openers.has(id)) { + throw new Error(`Opener with id already registered: '${id}'`); + } - this._openers.set(handle, { + this._openers.set(id, { opener, extension: extensionId, schemes: new Set(schemes), metadata }); - this._proxy.$registerUriOpener(handle, schemes, extensionId, metadata.label); + this._proxy.$registerUriOpener(id, schemes, extensionId, metadata.label); return toDisposable(() => { - this._openers.delete(handle); - this._proxy.$unregisterUriOpener(handle); + this._openers.delete(id); + this._proxy.$unregisterUriOpener(id); }); } - async $getOpenersForUri(uriComponents: UriComponents, token: CancellationToken): Promise { + async $getOpenersForUri(uriComponents: UriComponents, token: CancellationToken): Promise { const uri = URI.revive(uriComponents); - const promises = Array.from(this._openers.entries()).map(async ([handle, { schemes, opener, }]): Promise => { - if (!schemes.has(uri.scheme)) { - return undefined; - } - - try { - if (await opener.canOpenExternalUri(uri, token)) { - return handle; + const promises = Array.from(this._openers.entries()) + .map(async ([id, { schemes, opener, }]): Promise => { + if (!schemes.has(uri.scheme)) { + return undefined; } - } catch (e) { - console.log(e); - // noop - } - return undefined; - }); - return (await Promise.all(promises)).filter(handle => typeof handle === 'number') as number[]; + try { + if (await opener.canOpenExternalUri(uri, token)) { + return id; + } + } catch (e) { + console.log(e); + // noop + } + return undefined; + }); + + return (await Promise.all(promises)).filter(handle => typeof handle === 'string') as string[]; } - async $openUri(handle: number, context: { resolveUri: UriComponents, sourceUri: UriComponents }, token: CancellationToken): Promise { - const entry = this._openers.get(handle); + async $openUri(id: string, context: { resolveUri: UriComponents, sourceUri: UriComponents }, token: CancellationToken): Promise { + const entry = this._openers.get(id); if (!entry) { - throw new Error(`Unknown opener handle: '${handle}'`); + throw new Error(`Unknown opener id: '${id}'`); } return entry.opener.openExternalUri(URI.revive(context.resolveUri), { sourceUri: URI.revive(context.sourceUri) diff --git a/src/vs/workbench/contrib/externalUriOpener/common/configuration.ts b/src/vs/workbench/contrib/externalUriOpener/common/configuration.ts index 62a1acab41e..b61f29d6937 100644 --- a/src/vs/workbench/contrib/externalUriOpener/common/configuration.ts +++ b/src/vs/workbench/contrib/externalUriOpener/common/configuration.ts @@ -6,6 +6,7 @@ import { IConfigurationNode } from 'vs/platform/configuration/common/configurationRegistry'; import { workbenchConfigurationNodeBase } from 'vs/workbench/common/configuration'; import * as nls from 'vs/nls'; +import { IJSONSchema } from 'vs/base/common/jsonSchema'; export const externalUriOpenersSettingId = 'workbench.externalUriOpeners'; @@ -14,6 +15,11 @@ export interface ExternalUriOpenerConfiguration { readonly id: string; } +export const externalUriOpenerIdSchemaAddition: IJSONSchema = { + type: 'string', + enum: [] +}; + export const externalUriOpenersConfigurationNode: IConfigurationNode = { ...workbenchConfigurationNodeBase, properties: { @@ -35,8 +41,13 @@ export const externalUriOpenersConfigurationNode: IConfigurationNode = { description: nls.localize('externalUriOpeners.hostname', "The hostname of sites the opener applies to."), }, 'id': { - type: 'string', - description: nls.localize('externalUriOpeners.id', "The id of the opener."), + anyOf: [ + { + type: 'string', + description: nls.localize('externalUriOpeners.id', "The id of the opener."), + }, + externalUriOpenerIdSchemaAddition + ] } } }