From d9e5fe654107ba3d78c41ffe13e204faebf45885 Mon Sep 17 00:00:00 2001 From: Joao Moreno Date: Mon, 17 Feb 2020 15:47:14 +0100 Subject: [PATCH] adopt LinkedText in notifications --- .../notifications/notificationsAlerts.ts | 8 ++-- .../notifications/notificationsViewer.ts | 39 +++++++------------ src/vs/workbench/common/notifications.ts | 30 +++----------- .../test/common/notifications.test.ts | 37 ++++-------------- 4 files changed, 29 insertions(+), 85 deletions(-) diff --git a/src/vs/workbench/browser/parts/notifications/notificationsAlerts.ts b/src/vs/workbench/browser/parts/notifications/notificationsAlerts.ts index fae7f87f7cb..22b683d8c68 100644 --- a/src/vs/workbench/browser/parts/notifications/notificationsAlerts.ts +++ b/src/vs/workbench/browser/parts/notifications/notificationsAlerts.ts @@ -37,7 +37,7 @@ export class NotificationsAlerts extends Disposable { if (e.item.message.original instanceof Error) { console.error(e.item.message.original); } else { - console.error(toErrorMessage(e.item.message.value, true)); + console.error(toErrorMessage(e.item.message.linkedText.toString(), true)); } } } @@ -60,11 +60,11 @@ export class NotificationsAlerts extends Disposable { private doTriggerAriaAlert(notifiation: INotificationViewItem): void { let alertText: string; if (notifiation.severity === Severity.Error) { - alertText = localize('alertErrorMessage', "Error: {0}", notifiation.message.value); + alertText = localize('alertErrorMessage', "Error: {0}", notifiation.message.linkedText.toString()); } else if (notifiation.severity === Severity.Warning) { - alertText = localize('alertWarningMessage', "Warning: {0}", notifiation.message.value); + alertText = localize('alertWarningMessage', "Warning: {0}", notifiation.message.linkedText.toString()); } else { - alertText = localize('alertInfoMessage', "Info: {0}", notifiation.message.value); + alertText = localize('alertInfoMessage', "Info: {0}", notifiation.message.linkedText.toString()); } alert(alertText); diff --git a/src/vs/workbench/browser/parts/notifications/notificationsViewer.ts b/src/vs/workbench/browser/parts/notifications/notificationsViewer.ts index 318307a8477..041863ea737 100644 --- a/src/vs/workbench/browser/parts/notifications/notificationsViewer.ts +++ b/src/vs/workbench/browser/parts/notifications/notificationsViewer.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { IListVirtualDelegate, IListRenderer } from 'vs/base/browser/ui/list/list'; -import { clearNode, addClass, removeClass, toggleClass, addDisposableListener, EventType, EventHelper } from 'vs/base/browser/dom'; +import { clearNode, addClass, removeClass, toggleClass, addDisposableListener, EventType, EventHelper, $ } from 'vs/base/browser/dom'; import { IOpenerService } from 'vs/platform/opener/common/opener'; import { URI } from 'vs/base/common/uri'; import { localize } from 'vs/nls'; @@ -23,6 +23,7 @@ import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { ProgressBar } from 'vs/base/browser/ui/progressbar/progressbar'; import { Severity } from 'vs/platform/notification/common/notification'; import { isNonEmptyArray } from 'vs/base/common/arrays'; +import { startsWith } from 'vs/base/common/strings'; export class NotificationsListDelegate implements IListVirtualDelegate { @@ -136,39 +137,25 @@ class NotificationMessageRenderer { static render(message: INotificationMessage, actionHandler?: IMessageActionHandler): HTMLElement { const messageContainer = document.createElement('span'); - // Message has no links - if (message.links.length === 0) { - messageContainer.textContent = message.value; - } + for (const node of message.linkedText.nodes) { + if (typeof node === 'string') { + messageContainer.appendChild(document.createTextNode(node)); + } else { + let title = node.title; - // Message has links - else { - let index = 0; - for (const link of message.links) { - - const textBefore = message.value.substring(index, link.offset); - if (textBefore) { - messageContainer.appendChild(document.createTextNode(textBefore)); + if (!title && startsWith(node.href, 'command:')) { + title = localize('executeCommand', "Click to execute command '{0}'", node.href.substr('command:'.length)); + } else if (!title) { + title = node.href; } - const anchor = document.createElement('a'); - anchor.textContent = link.name; - anchor.title = link.title; - anchor.href = link.href; + const anchor = $('a', { href: node.href, title: title, }, node.label); if (actionHandler) { - actionHandler.toDispose.add(addDisposableListener(anchor, EventType.CLICK, () => actionHandler.callback(link.href))); + actionHandler.toDispose.add(addDisposableListener(anchor, EventType.CLICK, () => actionHandler.callback(node.href))); } messageContainer.appendChild(anchor); - - index = link.offset + link.length; - } - - // Add text after links if any - const textAfter = message.value.substring(index); - if (textAfter) { - messageContainer.appendChild(document.createTextNode(textAfter)); } } diff --git a/src/vs/workbench/common/notifications.ts b/src/vs/workbench/common/notifications.ts index ff932b00bd8..ef22a9c992e 100644 --- a/src/vs/workbench/common/notifications.ts +++ b/src/vs/workbench/common/notifications.ts @@ -10,9 +10,8 @@ import { Disposable, IDisposable, toDisposable } from 'vs/base/common/lifecycle' import { isPromiseCanceledError } from 'vs/base/common/errors'; import { Action } from 'vs/base/common/actions'; import { isErrorWithActions } from 'vs/base/common/errorsWithActions'; -import { startsWith } from 'vs/base/common/strings'; -import { localize } from 'vs/nls'; import { find, equals } from 'vs/base/common/arrays'; +import { parseLinkedText, LinkedText } from 'vs/base/common/linkedText'; export interface INotificationsModel { @@ -393,18 +392,13 @@ export interface IMessageLink { export interface INotificationMessage { raw: string; original: NotificationMessage; - value: string; - links: IMessageLink[]; + linkedText: LinkedText; } export class NotificationViewItem extends Disposable implements INotificationViewItem { private static readonly MAX_MESSAGE_LENGTH = 1000; - // Example link: "Some message with [link text](http://link.href)." - // RegEx: [, anything not ], ], (, http://|https://|command:, no whitespace) - private static readonly LINK_REGEX = /\[([^\]]+)\]\(((?:https?:\/\/|command:)[^\)\s]+)(?: "([^"]+)")?\)/gi; - private _expanded: boolean | undefined; private _actions: INotificationActions | undefined; @@ -469,23 +463,9 @@ export class NotificationViewItem extends Disposable implements INotificationVie message = message.replace(/(\r\n|\n|\r)/gm, ' ').trim(); // Parse Links - const links: IMessageLink[] = []; - message.replace(NotificationViewItem.LINK_REGEX, (matchString: string, name: string, href: string, title: string, offset: number) => { - let massagedTitle: string; - if (title && title.length > 0) { - massagedTitle = title; - } else if (startsWith(href, 'command:')) { - massagedTitle = localize('executeCommand', "Click to execute command '{0}'", href.substr('command:'.length)); - } else { - massagedTitle = href; - } + const linkedText = parseLinkedText(message); - links.push({ name, href, title: massagedTitle, offset, length: matchString.length }); - - return matchString; - }); - - return { raw, value: message, links, original: input }; + return { raw, linkedText, original: input }; } private constructor( @@ -649,7 +629,7 @@ export class NotificationViewItem extends Disposable implements INotificationVie return false; } - if (this._message.value !== other.message.value) { + if (this._message.raw !== other.message.raw) { return false; } diff --git a/src/vs/workbench/test/common/notifications.test.ts b/src/vs/workbench/test/common/notifications.test.ts index b95b7d7b1eb..8984e0ab869 100644 --- a/src/vs/workbench/test/common/notifications.test.ts +++ b/src/vs/workbench/test/common/notifications.test.ts @@ -105,29 +105,6 @@ suite('Notifications', () => { let item6 = NotificationViewItem.create({ severity: Severity.Error, message: createErrorWithActions('Hello Error', { actions: [new Action('id', 'label')] }) })!; assert.equal(item6.actions!.primary!.length, 1); - // Links - let item7 = NotificationViewItem.create({ severity: Severity.Info, message: 'Unable to [Link 1](http://link1.com) open [Link 2](command:open.me "Open This") and [Link 3](command:without.title) and [Invalid Link4](ftp://link4.com)' })!; - - const links = item7.message.links; - assert.equal(links.length, 3); - assert.equal(links[0].name, 'Link 1'); - assert.equal(links[0].href, 'http://link1.com'); - assert.equal(links[0].title, 'http://link1.com'); - assert.equal(links[0].length, '[Link 1](http://link1.com)'.length); - assert.equal(links[0].offset, 'Unable to '.length); - - assert.equal(links[1].name, 'Link 2'); - assert.equal(links[1].href, 'command:open.me'); - assert.equal(links[1].title, 'Open This'); - assert.equal(links[1].length, '[Link 2](command:open.me "Open This")'.length); - assert.equal(links[1].offset, 'Unable to [Link 1](http://link1.com) open '.length); - - assert.equal(links[2].name, 'Link 3'); - assert.equal(links[2].href, 'command:without.title'); - assert.equal(links[2].title, 'Click to execute command \'without.title\''); - assert.equal(links[2].length, '[Link 3](command:without.title)'.length); - assert.equal(links[2].offset, 'Unable to [Link 1](http://link1.com) open [Link 2](command:open.me "Open This") and '.length); - // Filter let item8 = NotificationViewItem.create({ severity: Severity.Error, message: 'Error Message' }, NotificationsFilter.SILENT)!; assert.equal(item8.silent, true); @@ -162,19 +139,19 @@ suite('Notifications', () => { let item1Handle = model.addNotification(item1); assert.equal(lastNotificationEvent.item.severity, item1.severity); - assert.equal(lastNotificationEvent.item.message.value, item1.message); + assert.equal(lastNotificationEvent.item.message.linkedText.toString(), item1.message); assert.equal(lastNotificationEvent.index, 0); assert.equal(lastNotificationEvent.kind, NotificationChangeType.ADD); let item2Handle = model.addNotification(item2); assert.equal(lastNotificationEvent.item.severity, item2.severity); - assert.equal(lastNotificationEvent.item.message.value, item2.message); + assert.equal(lastNotificationEvent.item.message.linkedText.toString(), item2.message); assert.equal(lastNotificationEvent.index, 0); assert.equal(lastNotificationEvent.kind, NotificationChangeType.ADD); model.addNotification(item3); assert.equal(lastNotificationEvent.item.severity, item3.severity); - assert.equal(lastNotificationEvent.item.message.value, item3.message); + assert.equal(lastNotificationEvent.item.message.linkedText.toString(), item3.message); assert.equal(lastNotificationEvent.index, 0); assert.equal(lastNotificationEvent.kind, NotificationChangeType.ADD); @@ -189,27 +166,27 @@ suite('Notifications', () => { assert.equal(called, 1); assert.equal(model.notifications.length, 2); assert.equal(lastNotificationEvent.item.severity, item1.severity); - assert.equal(lastNotificationEvent.item.message.value, item1.message); + assert.equal(lastNotificationEvent.item.message.linkedText.toString(), item1.message); assert.equal(lastNotificationEvent.index, 2); assert.equal(lastNotificationEvent.kind, NotificationChangeType.REMOVE); model.addNotification(item2Duplicate); assert.equal(model.notifications.length, 2); assert.equal(lastNotificationEvent.item.severity, item2Duplicate.severity); - assert.equal(lastNotificationEvent.item.message.value, item2Duplicate.message); + assert.equal(lastNotificationEvent.item.message.linkedText.toString(), item2Duplicate.message); assert.equal(lastNotificationEvent.index, 0); assert.equal(lastNotificationEvent.kind, NotificationChangeType.ADD); item2Handle.close(); assert.equal(model.notifications.length, 1); assert.equal(lastNotificationEvent.item.severity, item2Duplicate.severity); - assert.equal(lastNotificationEvent.item.message.value, item2Duplicate.message); + assert.equal(lastNotificationEvent.item.message.linkedText.toString(), item2Duplicate.message); assert.equal(lastNotificationEvent.index, 0); assert.equal(lastNotificationEvent.kind, NotificationChangeType.REMOVE); model.notifications[0].expand(); assert.equal(lastNotificationEvent.item.severity, item3.severity); - assert.equal(lastNotificationEvent.item.message.value, item3.message); + assert.equal(lastNotificationEvent.item.message.linkedText.toString(), item3.message); assert.equal(lastNotificationEvent.index, 0); assert.equal(lastNotificationEvent.kind, NotificationChangeType.CHANGE);