From bcacd2fea3ef82171270b7b96ccc9dc2a78f1e94 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 26 Feb 2018 07:41:27 +0100 Subject: [PATCH] fix #34367 --- src/vs/base/browser/htmlContentRenderer.ts | 11 +-- .../notification/common/notification.ts | 10 +-- .../notifications/notificationsViewer.ts | 82 +++++++++++-------- src/vs/workbench/common/notifications.ts | 65 +++++++++++---- .../test/common/notifications.test.ts | 16 +++- 5 files changed, 117 insertions(+), 67 deletions(-) diff --git a/src/vs/base/browser/htmlContentRenderer.ts b/src/vs/base/browser/htmlContentRenderer.ts index fb9f3c6cf1a..d5757848fc9 100644 --- a/src/vs/base/browser/htmlContentRenderer.ts +++ b/src/vs/base/browser/htmlContentRenderer.ts @@ -9,9 +9,8 @@ import * as DOM from 'vs/base/browser/dom'; import { defaultGenerator } from 'vs/base/common/idGenerator'; import { escape } from 'vs/base/common/strings'; import { removeMarkdownEscapes, IMarkdownString } from 'vs/base/common/htmlContent'; -import { marked, MarkedRenderer, MarkedOptions } from 'vs/base/common/marked/marked'; +import { marked, MarkedOptions } from 'vs/base/common/marked/marked'; import { IMouseEvent } from 'vs/base/browser/mouseEvent'; -import { assign } from 'vs/base/common/objects'; import { IDisposable } from 'vs/base/common/lifecycle'; export interface IContentActionHandler { @@ -25,7 +24,6 @@ export interface RenderOptions { actionHandler?: IContentActionHandler; codeBlockRenderer?: (modeId: string, value: string) => Thenable; codeBlockRenderCallback?: () => void; - joinRendererConfiguration?: (renderer: MarkedRenderer) => MarkedOptions; } function createElement(options: RenderOptions): HTMLElement { @@ -166,13 +164,6 @@ export function renderMarkdown(markdown: IMarkdownString, options: RenderOptions renderer }; - if (options.joinRendererConfiguration) { - const additionalMarkedOptions = options.joinRendererConfiguration(renderer); - if (additionalMarkedOptions) { - assign(markedOptions, additionalMarkedOptions); - } - } - element.innerHTML = marked(markdown.value, markedOptions); signalInnerHTML(); diff --git a/src/vs/platform/notification/common/notification.ts b/src/vs/platform/notification/common/notification.ts index a50aa94d9f0..93fa74ef128 100644 --- a/src/vs/platform/notification/common/notification.ts +++ b/src/vs/platform/notification/common/notification.ts @@ -8,7 +8,6 @@ import Severity from 'vs/base/common/severity'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { IDisposable } from 'vs/base/common/lifecycle'; -import { IMarkdownString } from 'vs/base/common/htmlContent'; import { IAction } from 'vs/base/common/actions'; import Event, { Emitter } from 'vs/base/common/event'; @@ -16,7 +15,7 @@ export import Severity = Severity; export const INotificationService = createDecorator('notificationService'); -export type NotificationMessage = string | IMarkdownString | Error; +export type NotificationMessage = string | Error; export interface INotification { @@ -26,11 +25,8 @@ export interface INotification { severity: Severity; /** - * The message of the notification. This can either be a `string`, `Error` - * or `IMarkdownString`. - * - * **Note:** Currently only links are supported in notifications. Links to commands can - * be embedded provided that the `IMarkdownString` is trusted. + * The message of the notification. This can either be a `string` or `Error`. Messages + * can optionally include links in the format: `[text](link)` */ message: NotificationMessage; diff --git a/src/vs/workbench/browser/parts/notifications/notificationsViewer.ts b/src/vs/workbench/browser/parts/notifications/notificationsViewer.ts index f8177525340..0338036c31a 100644 --- a/src/vs/workbench/browser/parts/notifications/notificationsViewer.ts +++ b/src/vs/workbench/browser/parts/notifications/notificationsViewer.ts @@ -6,8 +6,7 @@ 'use strict'; import { IDelegate, IRenderer } from 'vs/base/browser/ui/list/list'; -import { renderMarkdown, IContentActionHandler } from 'vs/base/browser/htmlContentRenderer'; -import { clearNode, addClass, removeClass, toggleClass } from 'vs/base/browser/dom'; +import { clearNode, addClass, removeClass, toggleClass, addDisposableListener } from 'vs/base/browser/dom'; import { IOpenerService } from 'vs/platform/opener/common/opener'; import URI from 'vs/base/common/uri'; import { onUnexpectedError } from 'vs/base/common/errors'; @@ -15,17 +14,15 @@ import { localize } from 'vs/nls'; import { ButtonGroup } from 'vs/base/browser/ui/button/button'; import { attachButtonStyler, attachProgressBarStyler } from 'vs/platform/theme/common/styler'; import { IThemeService } from 'vs/platform/theme/common/themeService'; -import { IMarkdownString } from 'vs/base/common/htmlContent'; import { ActionBar } from 'vs/base/browser/ui/actionbar/actionbar'; import { IAction, IActionRunner } from 'vs/base/common/actions'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IDisposable, dispose } from 'vs/base/common/lifecycle'; import { IContextMenuService } from 'vs/platform/contextview/browser/contextView'; import { DropdownMenuActionItem } from 'vs/base/browser/ui/dropdown/dropdown'; -import { INotificationViewItem, NotificationViewItem, NotificationViewItemLabelKind } from 'vs/workbench/common/notifications'; +import { INotificationViewItem, NotificationViewItem, NotificationViewItemLabelKind, INotificationMessage } from 'vs/workbench/common/notifications'; import { ClearNotificationAction, ExpandNotificationAction, CollapseNotificationAction, ConfigureNotificationAction } from 'vs/workbench/browser/parts/notifications/notificationsActions'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; -import { MarkedOptions } from 'vs/base/common/marked/marked'; import { ProgressBar } from 'vs/base/browser/ui/progressbar/progressbar'; import { Severity } from 'vs/platform/notification/common/notification'; @@ -92,8 +89,8 @@ export class NotificationsListDelegate implements IDelegate text || ''; - private static readonly MARKED_NOOP_TARGETS = [ - 'blockquote', 'br', 'code', 'codespan', 'del', 'em', 'heading', 'hr', 'html', - 'image', 'list', 'listitem', 'paragraph', 'strong', 'table', 'tablecell', - 'tablerow' - ]; - - public static render(markdown: IMarkdownString, actionHandler?: IContentActionHandler): HTMLElement { - return renderMarkdown(markdown, { - inline: true, - joinRendererConfiguration: renderer => { - - // Overwrite markdown render functions as no-ops - NotificationMessageMarkdownRenderer.MARKED_NOOP_TARGETS.forEach(fn => renderer[fn] = NotificationMessageMarkdownRenderer.MARKED_NOOP); - - return { - gfm: false, // disable GitHub style markdown, - smartypants: false // disable some text transformations - } as MarkedOptions; - }, - actionHandler - }); +interface IMessageActionHandler { + callback: (href: string) => void; + disposeables: IDisposable[]; +} + +class NotificationMessageRenderer { + + public 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; + } + + // Message has links + else { + let index = 0; + let textBefore: string; + for (let i = 0; i < message.links.length; i++) { + const link = message.links[i]; + + textBefore = message.value.substring(index, link.offset); + if (textBefore) { + messageContainer.appendChild(document.createTextNode(textBefore)); + } + + const anchor = document.createElement('a'); + anchor.textContent = link.name; + anchor.title = link.href; + anchor.href = link.href; + + if (actionHandler) { + actionHandler.disposeables.push(addDisposableListener(anchor, 'click', () => actionHandler.callback(link.href))); + } + + messageContainer.appendChild(anchor); + + index = link.offset + link.length; + } + } + + return messageContainer; } } @@ -340,8 +356,8 @@ export class NotificationTemplateRenderer { private renderMessage(notification: INotificationViewItem): boolean { clearNode(this.template.message); - this.template.message.appendChild(NotificationMessageMarkdownRenderer.render(notification.message, { - callback: (content: string) => this.openerService.open(URI.parse(content)).then(void 0, onUnexpectedError), + this.template.message.appendChild(NotificationMessageRenderer.render(notification.message, { + callback: link => this.openerService.open(URI.parse(link)).then(void 0, onUnexpectedError), disposeables: this.inputDisposeables })); diff --git a/src/vs/workbench/common/notifications.ts b/src/vs/workbench/common/notifications.ts index ea4e3732fe7..ad8a9c223ac 100644 --- a/src/vs/workbench/common/notifications.ts +++ b/src/vs/workbench/common/notifications.ts @@ -5,7 +5,6 @@ 'use strict'; -import { IMarkdownString } from 'vs/base/common/htmlContent'; import { INotification, INotificationHandle, INotificationActions, INotificationProgress, NoOpNotification, Severity, NotificationMessage } from 'vs/platform/notification/common/notification'; import { toErrorMessage } from 'vs/base/common/errorMessage'; import Event, { Emitter, once } from 'vs/base/common/event'; @@ -184,7 +183,7 @@ export class NotificationsModel implements INotificationsModel { export interface INotificationViewItem { readonly severity: Severity; - readonly message: IMarkdownString; + readonly message: INotificationMessage; readonly source: string; readonly actions: INotificationActions; readonly progress: INotificationViewItemProgress; @@ -320,10 +319,27 @@ export class NotificationViewItemProgress implements INotificationViewItemProgre } } +export interface IMessageLink { + name: string; + href: string; + offset: number; + length: number; +} + +export interface INotificationMessage { + raw: string; + value: string; + links: IMessageLink[]; +} + export class NotificationViewItem implements INotificationViewItem { private static MAX_MESSAGE_LENGTH = 1000; + // Example link: "Some message with [link text](http://link.href)." + // RegEx: [, anything not ], ], (, http:|https:, //, no whitespace) + private static LINK_REGEX = /\[([^\]]+)\]\((https?:\/\/[^\)\s]+)\)/gi; + private _expanded: boolean; private toDispose: IDisposable[]; @@ -346,15 +362,11 @@ export class NotificationViewItem implements INotificationViewItem { severity = Severity.Info; } - const message = NotificationViewItem.toMarkdownString(notification.message); + const message = NotificationViewItem.parseNotificationMessage(notification.message); if (!message) { return null; // we need a message to show } - if (message.value.length > NotificationViewItem.MAX_MESSAGE_LENGTH) { - message.value = `${message.value.substr(0, NotificationViewItem.MAX_MESSAGE_LENGTH)}...`; - } - let actions: INotificationActions; if (notification.actions) { actions = notification.actions; @@ -365,21 +377,42 @@ export class NotificationViewItem implements INotificationViewItem { return new NotificationViewItem(severity, message, notification.source, actions); } - private static toMarkdownString(input: NotificationMessage): IMarkdownString { - let message: IMarkdownString; + private static parseNotificationMessage(input: NotificationMessage): INotificationMessage { + let message: string; if (input instanceof Error) { - message = { value: toErrorMessage(input, false), isTrusted: false }; + message = toErrorMessage(input, false); } else if (typeof input === 'string') { - message = { value: input, isTrusted: false }; - } else if (input.value && typeof input.value === 'string') { message = input; } - return message; + if (!message) { + return null; // we need a message to show + } + + const raw = message; + + // Make sure message is in the limits + if (message.length > NotificationViewItem.MAX_MESSAGE_LENGTH) { + message = `${message.substr(0, NotificationViewItem.MAX_MESSAGE_LENGTH)}...`; + } + + // Remove newlines from messages as we do not support that and it makes link parsing hard + 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, offset: number) => { + links.push({ name, href, offset, length: matchString.length }); + + return matchString; + }); + + + return { raw, value: message, links }; } - private constructor(private _severity: Severity, private _message: IMarkdownString, private _source: string, actions?: INotificationActions) { + private constructor(private _severity: Severity, private _message: INotificationMessage, private _source: string, actions?: INotificationActions) { this.toDispose = []; this.setActions(actions); @@ -452,7 +485,7 @@ export class NotificationViewItem implements INotificationViewItem { return this._progress; } - public get message(): IMarkdownString { + public get message(): INotificationMessage { return this._message; } @@ -470,7 +503,7 @@ export class NotificationViewItem implements INotificationViewItem { } public updateMessage(input: NotificationMessage): void { - const message = NotificationViewItem.toMarkdownString(input); + const message = NotificationViewItem.parseNotificationMessage(input); if (!message) { return; } diff --git a/src/vs/workbench/test/common/notifications.test.ts b/src/vs/workbench/test/common/notifications.test.ts index c14367ab9b6..533cc9ae266 100644 --- a/src/vs/workbench/test/common/notifications.test.ts +++ b/src/vs/workbench/test/common/notifications.test.ts @@ -18,7 +18,6 @@ suite('Notifications', () => { // Invalid assert.ok(!NotificationViewItem.create({ severity: Severity.Error, message: '' })); assert.ok(!NotificationViewItem.create({ severity: Severity.Error, message: null })); - assert.ok(!NotificationViewItem.create({ severity: Severity.Error, message: { value: '', isTrusted: true } })); // Duplicates let item1 = NotificationViewItem.create({ severity: Severity.Error, message: 'Error Message' }); @@ -107,6 +106,21 @@ suite('Notifications', () => { // Error with Action let item6 = NotificationViewItem.create({ severity: Severity.Error, message: create('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](https://link2.com) and [Invalid Link3](ftp://link3.com)' }); + + const links = item7.message.links; + assert.equal(links.length, 2); + assert.equal(links[0].name, 'Link 1'); + assert.equal(links[0].href, '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, 'https://link2.com'); + assert.equal(links[1].length, '[Link 2](https://link2.com)'.length); + assert.equal(links[1].offset, 'Unable to [Link 1](http://link1.com) open '.length); }); test('Model', () => { -- GitLab