From 0b8568bc2167cd0606d42b589d3a79d41cd11011 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 12:27:48 -0700 Subject: [PATCH 01/14] Move links files into own folder, start regex link provider --- .../browser/links/terminalLinkHelpers.ts | 97 +++++++++ .../terminalLinkManager.ts} | 21 +- .../links/terminalRegexLocalLinkProvider.ts | 146 ++++++++++++++ .../{ => links}/terminalWebLinkProvider.ts | 95 +-------- .../terminal/browser/terminalInstance.ts | 24 +-- .../{ => links}/terminalLinkHandler.test.ts | 6 +- .../browser/links/terminalLinkHelpers.test.ts | 186 ++++++++++++++++++ .../terminalWebLinkProvider.test.ts | 179 +---------------- 8 files changed, 464 insertions(+), 290 deletions(-) create mode 100644 src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts rename src/vs/workbench/contrib/terminal/browser/{terminalLinkHandler.ts => links/terminalLinkManager.ts} (95%) create mode 100644 src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts rename src/vs/workbench/contrib/terminal/browser/{ => links}/terminalWebLinkProvider.ts (51%) rename src/vs/workbench/contrib/terminal/test/browser/{ => links}/terminalLinkHandler.test.ts (98%) create mode 100644 src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHelpers.test.ts rename src/vs/workbench/contrib/terminal/test/browser/{ => links}/terminalWebLinkProvider.test.ts (52%) diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts new file mode 100644 index 0000000000000..e25db6781e18c --- /dev/null +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts @@ -0,0 +1,97 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { IViewportRange, IBufferRange, IBufferLine, IBuffer } from 'xterm'; +import { IRange } from 'vs/editor/common/core/range'; + +export function convertLinkRangeToBuffer(lines: IBufferLine[], bufferWidth: number, range: IRange, startLine: number) { + const bufferRange: IBufferRange = { + start: { + x: range.startColumn, + y: range.startLineNumber + startLine + }, + end: { + x: range.endColumn - 1, + y: range.endLineNumber + startLine + } + }; + + // Shift start range right for each wide character before the link + let startOffset = 0; + const startWrappedLineCount = Math.ceil(range.startColumn / bufferWidth); + for (let y = 0; y < startWrappedLineCount; y++) { + const lineLength = Math.min(bufferWidth, range.startColumn - y * bufferWidth); + let lineOffset = 0; + const line = lines[y]; + for (let x = 0; x < Math.min(bufferWidth, lineLength + lineOffset); x++) { + const width = line.getCell(x)?.getWidth(); + if (width === 2) { + lineOffset++; + } + } + startOffset += lineOffset; + } + + // Shift end range right for each wide character inside the link + let endOffset = 0; + const endWrappedLineCount = Math.ceil(range.endColumn / bufferWidth); + for (let y = startWrappedLineCount - 1; y < endWrappedLineCount; y++) { + const start = (y === startWrappedLineCount - 1 ? (range.startColumn + startOffset) % bufferWidth : 0); + const lineLength = Math.min(bufferWidth, range.endColumn + startOffset - y * bufferWidth); + const startLineOffset = (y === startWrappedLineCount - 1 ? startOffset : 0); + let lineOffset = 0; + const line = lines[y]; + for (let x = start; x < Math.min(bufferWidth, lineLength + lineOffset + startLineOffset); x++) { + const cell = line.getCell(x)!; + const width = cell.getWidth(); + // Offset for 0 cells following wide characters + if (width === 2) { + lineOffset++; + } + // Offset for early wrapping when the last cell in row is a wide character + if (x === bufferWidth - 1 && cell.getChars() === '') { + lineOffset++; + } + } + endOffset += lineOffset; + } + + // Apply the width character offsets to the result + bufferRange.start.x += startOffset; + bufferRange.end.x += startOffset + endOffset; + + // Convert back to wrapped lines + while (bufferRange.start.x > bufferWidth) { + bufferRange.start.x -= bufferWidth; + bufferRange.start.y++; + } + while (bufferRange.end.x > bufferWidth) { + bufferRange.end.x -= bufferWidth; + bufferRange.end.y++; + } + + return bufferRange; +} + +export function convertBufferRangeToViewport(bufferRange: IBufferRange, viewportY: number): IViewportRange { + return { + start: { + x: bufferRange.start.x - 1, + y: bufferRange.start.y - viewportY - 1 + }, + end: { + x: bufferRange.end.x - 1, + y: bufferRange.end.y - viewportY - 1 + } + }; +} + +export function getXtermLineContent(buffer: IBuffer, lineStart: number, lineEnd: number): string { + let line = ''; + for (let i = lineStart; i <= lineEnd; i++) { + line += buffer.getLine(i)?.translateToString(true); + } + return line; +} diff --git a/src/vs/workbench/contrib/terminal/browser/terminalLinkHandler.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts similarity index 95% rename from src/vs/workbench/contrib/terminal/browser/terminalLinkHandler.ts rename to src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts index 095d513717308..377edd908da31 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalLinkHandler.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts @@ -21,7 +21,8 @@ import { OperatingSystem, isMacintosh } from 'vs/base/common/platform'; import { IMarkdownString, MarkdownString } from 'vs/base/common/htmlContent'; import { Emitter, Event } from 'vs/base/common/event'; import { ILogService } from 'vs/platform/log/common/log'; -import { TerminalWebLinkProvider } from 'vs/workbench/contrib/terminal/browser/terminalWebLinkProvider'; +import { TerminalWebLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider'; +import { TerminalRegexLocalLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider'; const pathPrefix = '(\\.\\.?|\\~)'; const pathSeparatorClause = '\\/'; @@ -69,7 +70,10 @@ interface IPath { normalize(path: string): string; } -export class TerminalLinkHandler extends DisposableStore { +/** + * An object responsible for managing registration of link matchers and link providers. + */ +export class TerminalLinkManager extends DisposableStore { private _widgetManager: TerminalWidgetManager | undefined; private _processCwd: string | undefined; private _gitDiffPreImagePattern: RegExp; @@ -82,7 +86,7 @@ export class TerminalLinkHandler extends DisposableStore { private _hasBeforeHandleLinkListeners = false; protected static _LINK_INTERCEPT_THRESHOLD = LINK_INTERCEPT_THRESHOLD; - public static readonly LINK_INTERCEPT_THRESHOLD = TerminalLinkHandler._LINK_INTERCEPT_THRESHOLD; + public static readonly LINK_INTERCEPT_THRESHOLD = TerminalLinkManager._LINK_INTERCEPT_THRESHOLD; private readonly _onBeforeHandleLink = this.add(new Emitter({ onFirstListenerAdd: () => this._hasBeforeHandleLinkListeners = true, @@ -282,6 +286,13 @@ export class TerminalLinkHandler extends DisposableStore { }; const wrappedActivateCallback = this._wrapLinkHandler(this._handleHypertextLink.bind(this)); this._linkProvider = this._xterm.registerLinkProvider(new TerminalWebLinkProvider(this._xterm, wrappedActivateCallback, tooltipCallback, this._leaveCallback)); + + // Local links + const tooltipLinkCallback = (event: MouseEvent, link: string, location: IViewportRange) => { + this._tooltipCallback(event, link, location, this._handleLocalLink.bind(this, link)); + }; + const wrappedLinkActivateCallback = this._wrapLinkHandler(this._handleLocalLink.bind(this)); + this._linkProvider = this._xterm.registerLinkProvider(new TerminalRegexLocalLinkProvider(this._xterm, this._processManager, wrappedLinkActivateCallback, tooltipLinkCallback, this._leaveCallback)); } protected _wrapLinkHandler(handler: (link: string) => void): XtermLinkMatcherHandler { @@ -298,9 +309,9 @@ export class TerminalLinkHandler extends DisposableStore { const wasHandled = await new Promise(r => { const timeoutId = setTimeout(() => { canceled = true; - this._logService.error(`An extension intecepted a terminal link but it timed out after ${TerminalLinkHandler.LINK_INTERCEPT_THRESHOLD / 1000} seconds`); + this._logService.error(`An extension intecepted a terminal link but it timed out after ${TerminalLinkManager.LINK_INTERCEPT_THRESHOLD / 1000} seconds`); r(false); - }, TerminalLinkHandler.LINK_INTERCEPT_THRESHOLD); + }, TerminalLinkManager.LINK_INTERCEPT_THRESHOLD); let canceled = false; const resolve = (handled: boolean) => { if (!canceled) { diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts new file mode 100644 index 0000000000000..db8dc68f7ef25 --- /dev/null +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts @@ -0,0 +1,146 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Terminal, ILinkProvider, IViewportRange, IBufferCellPosition, ILink } from 'xterm'; +import { ITerminalProcessManager } from 'vs/workbench/contrib/terminal/common/terminal'; + +export class TerminalRegexLocalLinkProvider implements ILinkProvider { + constructor( + private readonly _xterm: Terminal, + private readonly _processManager: ITerminalProcessManager | undefined, + private readonly _activateCallback: (event: MouseEvent, uri: string) => void, + private readonly _tooltipCallback: (event: MouseEvent, uri: string, location: IViewportRange) => boolean | void, + private readonly _leaveCallback: () => void + ) { + } + + public provideLink(position: IBufferCellPosition, callback: (link: ILink | undefined) => void): void { + let startLine = position.y - 1; + let endLine = startLine; + + while (this._xterm.buffer.active.getLine(startLine)?.isWrapped) { + startLine--; + } + + while (this._xterm.buffer.active.getLine(endLine + 1)?.isWrapped) { + endLine++; + } + + const text = getXtermLineContent(this._xterm.buffer.active, startLine, endLine); + + // clone regex to do a global search on text + const rex = new RegExp(this._localLinkRegex, 'g'); + let match; + let stringIndex = -1; + while ((match = rex.exec(text)) !== null) { + // const uri = match[typeof matcher.matchIndex !== 'number' ? 0 : matcher.matchIndex]; + const uri = match[0]; + if (!uri) { + // something matched but does not comply with the given matchIndex + // since this is most likely a bug the regex itself we simply do nothing here + // this._logService.debug('match found without corresponding matchIndex', match, matcher); + break; + } + + // Get index, match.index is for the outer match which includes negated chars + // therefore we cannot use match.index directly, instead we search the position + // of the match group in text again + // also correct regex and string search offsets for the next loop run + stringIndex = text.indexOf(uri, stringIndex + 1); + rex.lastIndex = stringIndex + uri.length; + if (stringIndex < 0) { + // invalid stringIndex (should not have happened) + break; + } + + console.log('found link!', stringIndex, uri); + + // get the buffer index as [absolute row, col] for the match + // const bufferIndex = this._xterm.buffer.active.stringIndexToBufferIndex(rowIndex, stringIndex); + // if (bufferIndex[0] < 0) { + // // invalid bufferIndex (should not have happened) + // break; + // } + + // const line = this._xterm.buffer.active.getLine(bufferIndex[0]); + // if (!line) { + // break; + // } + + // const attr = line.getFg(bufferIndex[1]); + // const fg = attr ? (attr >> 9) & 0x1ff : undefined; + + + // if (matcher.validationCallback) { + // matcher.validationCallback(uri, isValid => { + // // Discard link if the line has already changed + // if (this._rowsTimeoutId) { + // return; + // } + // if (isValid) { + // this._addLink(bufferIndex[1], bufferIndex[0] - this._bufferService.buffer.ydisp, uri, matcher, fg); + // } + // }); + // } else { + // this._addLink(bufferIndex[1], bufferIndex[0] - this._bufferService.buffer.ydisp, uri, matcher, fg); + // } + } + + // this._linkComputerTarget = new TerminalLinkAdapter(this._xterm, startLine, endLine); + // const links = LinkComputer.computeLinks(this._linkComputerTarget); + + // let found = false; + // links.forEach(link => { + // const range = this._convertLinkRangeToBuffer(link.range, startLine); + + // // Check if the link if within the mouse position + // if (this._positionIsInRange(position, range)) { + // found = true; + + // callback({ + // text: link.url?.toString() || '', + // range, + // activate: (event: MouseEvent, text: string) => { + // this._activateCallback(event, text); + // }, + // hover: (event: MouseEvent, text: string) => { + // setTimeout(() => { + // this._tooltipCallback(event, text, convertBufferRangeToViewport(range, this._xterm.buffer.active.viewportY)); + // }, 200); + // }, + // leave: () => { + // this._leaveCallback(); + // } + // }); + // } + // }); + + // if (!found) { + callback(undefined); + // } + } + + protected get _localLinkRegex(): RegExp { + if (!this._processManager) { + throw new Error('Process manager is required'); + } + const baseLocalLinkClause = this._processManager.os === OperatingSystem.Windows ? winLocalLinkClause : unixLocalLinkClause; + // Append line and column number regex + return new RegExp(`${baseLocalLinkClause}(${lineAndColumnClause})`); + } + + // private _positionIsInRange(position: IBufferCellPosition, range: IBufferRange): boolean { + // if (position.y < range.start.y || position.y > range.end.y) { + // return false; + // } + // if (position.y === range.start.y && position.x < range.start.x) { + // return false; + // } + // if (position.y === range.end.y && position.x > range.end.x) { + // return false; + // } + // return true; + // } +} diff --git a/src/vs/workbench/contrib/terminal/browser/terminalWebLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts similarity index 51% rename from src/vs/workbench/contrib/terminal/browser/terminalWebLinkProvider.ts rename to src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts index b15ac750d0c46..cef244e8d2104 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalWebLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts @@ -3,10 +3,9 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Terminal, IViewportRange, ILinkProvider, IBufferCellPosition, ILink, IBufferRange, IBuffer, IBufferLine } from 'xterm'; +import { Terminal, IViewportRange, ILinkProvider, IBufferCellPosition, ILink, IBufferRange, IBufferLine } from 'xterm'; import { ILinkComputerTarget, LinkComputer } from 'vs/editor/common/modes/linkComputer'; -import { IRange } from 'vs/editor/common/core/range'; - +import { getXtermLineContent, convertLinkRangeToBuffer, convertBufferRangeToViewport } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers'; export class TerminalWebLinkProvider implements ILinkProvider { private _linkComputerTarget: ILinkComputerTarget | undefined; @@ -85,88 +84,6 @@ export class TerminalWebLinkProvider implements ILinkProvider { } } -export function convertLinkRangeToBuffer(lines: IBufferLine[], bufferWidth: number, range: IRange, startLine: number) { - const bufferRange: IBufferRange = { - start: { - x: range.startColumn, - y: range.startLineNumber + startLine - }, - end: { - x: range.endColumn - 1, - y: range.endLineNumber + startLine - } - }; - - // Shift start range right for each wide character before the link - let startOffset = 0; - const startWrappedLineCount = Math.ceil(range.startColumn / bufferWidth); - for (let y = 0; y < startWrappedLineCount; y++) { - const lineLength = Math.min(bufferWidth, range.startColumn - y * bufferWidth); - let lineOffset = 0; - const line = lines[y]; - for (let x = 0; x < Math.min(bufferWidth, lineLength + lineOffset); x++) { - const width = line.getCell(x)?.getWidth(); - if (width === 2) { - lineOffset++; - } - } - startOffset += lineOffset; - } - - // Shift end range right for each wide character inside the link - let endOffset = 0; - const endWrappedLineCount = Math.ceil(range.endColumn / bufferWidth); - for (let y = startWrappedLineCount - 1; y < endWrappedLineCount; y++) { - const start = (y === startWrappedLineCount - 1 ? (range.startColumn + startOffset) % bufferWidth : 0); - const lineLength = Math.min(bufferWidth, range.endColumn + startOffset - y * bufferWidth); - const startLineOffset = (y === startWrappedLineCount - 1 ? startOffset : 0); - let lineOffset = 0; - const line = lines[y]; - for (let x = start; x < Math.min(bufferWidth, lineLength + lineOffset + startLineOffset); x++) { - const cell = line.getCell(x)!; - const width = cell.getWidth(); - // Offset for 0 cells following wide characters - if (width === 2) { - lineOffset++; - } - // Offset for early wrapping when the last cell in row is a wide character - if (x === bufferWidth - 1 && cell.getChars() === '') { - lineOffset++; - } - } - endOffset += lineOffset; - } - - // Apply the width character offsets to the result - bufferRange.start.x += startOffset; - bufferRange.end.x += startOffset + endOffset; - - // Convert back to wrapped lines - while (bufferRange.start.x > bufferWidth) { - bufferRange.start.x -= bufferWidth; - bufferRange.start.y++; - } - while (bufferRange.end.x > bufferWidth) { - bufferRange.end.x -= bufferWidth; - bufferRange.end.y++; - } - - return bufferRange; -} - -function convertBufferRangeToViewport(bufferRange: IBufferRange, viewportY: number): IViewportRange { - return { - start: { - x: bufferRange.start.x - 1, - y: bufferRange.start.y - viewportY - 1 - }, - end: { - x: bufferRange.end.x - 1, - y: bufferRange.end.y - viewportY - 1 - } - }; -} - class TerminalLinkAdapter implements ILinkComputerTarget { constructor( private _xterm: Terminal, @@ -182,11 +99,3 @@ class TerminalLinkAdapter implements ILinkComputerTarget { return getXtermLineContent(this._xterm.buffer.active, this._lineStart, this._lineEnd); } } - -function getXtermLineContent(buffer: IBuffer, lineStart: number, lineEnd: number): string { - let line = ''; - for (let i = lineStart; i <= lineEnd; i++) { - line += buffer.getLine(i)?.translateToString(true); - } - return line; -} diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index 5e047de7b2c8f..3909aef1d50fc 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -28,7 +28,7 @@ import { TerminalWidgetManager } from 'vs/workbench/contrib/terminal/browser/ter import { IShellLaunchConfig, ITerminalDimensions, ITerminalProcessManager, KEYBINDING_CONTEXT_TERMINAL_TEXT_SELECTED, NEVER_MEASURE_RENDER_TIME_STORAGE_KEY, ProcessState, TERMINAL_VIEW_ID, IWindowsShellHelper, SHELL_PATH_INVALID_EXIT_CODE, SHELL_PATH_DIRECTORY_EXIT_CODE, SHELL_CWD_INVALID_EXIT_CODE, KEYBINDING_CONTEXT_TERMINAL_A11Y_TREE_FOCUS, INavigationMode, TitleEventSource, LEGACY_CONSOLE_MODE_EXIT_CODE, DEFAULT_COMMANDS_TO_SKIP_SHELL } from 'vs/workbench/contrib/terminal/common/terminal'; import { ansiColorIdentifiers, TERMINAL_BACKGROUND_COLOR, TERMINAL_CURSOR_BACKGROUND_COLOR, TERMINAL_CURSOR_FOREGROUND_COLOR, TERMINAL_FOREGROUND_COLOR, TERMINAL_SELECTION_BACKGROUND_COLOR } from 'vs/workbench/contrib/terminal/common/terminalColorRegistry'; import { TerminalConfigHelper } from 'vs/workbench/contrib/terminal/browser/terminalConfigHelper'; -import { TerminalLinkHandler } from 'vs/workbench/contrib/terminal/browser/terminalLinkHandler'; +import { TerminalLinkManager } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkManager'; import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility'; import { ITerminalInstanceService, ITerminalInstance, TerminalShellType, ITerminalBeforeHandleLinkEvent } from 'vs/workbench/contrib/terminal/browser/terminal'; import { TerminalProcessManager } from 'vs/workbench/contrib/terminal/browser/terminalProcessManager'; @@ -96,7 +96,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { private _messageTitleDisposable: IDisposable | undefined; private _widgetManager: TerminalWidgetManager | undefined; - private _linkHandler: TerminalLinkHandler | undefined; + private _linkManager: TerminalLinkManager | undefined; private _commandTrackerAddon: CommandTrackerAddon | undefined; private _navigationModeAddon: INavigationMode & ITerminalAddon | undefined; @@ -388,8 +388,8 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { this._processManager.onProcessData(data => this._onProcessData(data)); this._xterm.onData(data => this._processManager.write(data)); this.processReady.then(async () => { - if (this._linkHandler) { - this._linkHandler.processCwd = await this._processManager.getInitialCwd(); + if (this._linkManager) { + this._linkManager.processCwd = await this._processManager.getInitialCwd(); } }); // Init winpty compat and link handler after process creation as they rely on the @@ -405,8 +405,8 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { return false; }); } - this._linkHandler = this._instantiationService.createInstance(TerminalLinkHandler, xterm, this._processManager, this._configHelper); - this._linkHandler.onBeforeHandleLink(e => { + this._linkManager = this._instantiationService.createInstance(TerminalLinkManager, xterm, this._processManager, this._configHelper); + this._linkManager.onBeforeHandleLink(e => { e.terminal = this; this._onBeforeHandleLink.fire(e); }); @@ -584,7 +584,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { const widgetManager = new TerminalWidgetManager(this._wrapperElement); this._widgetManager = widgetManager; - this._processManager.onProcessReady(() => this._linkHandler?.setWidgetManager(widgetManager)); + this._processManager.onProcessReady(() => this._linkManager?.setWidgetManager(widgetManager)); const computedStyle = window.getComputedStyle(this._container); const width = parseInt(computedStyle.getPropertyValue('width').replace('px', ''), 10); @@ -653,7 +653,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { } public registerLinkMatcher(regex: RegExp, handler: (url: string) => void, matchIndex?: number, validationCallback?: (uri: string, callback: (isValid: boolean) => void) => void): number { - return this._linkHandler!.registerCustomLinkHandler(regex, handler, matchIndex, validationCallback); + return this._linkManager!.registerCustomLinkHandler(regex, handler, matchIndex, validationCallback); } public deregisterLinkMatcher(linkMatcherId: number): void { @@ -715,8 +715,8 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { dispose(this._windowsShellHelper); this._windowsShellHelper = undefined; - dispose(this._linkHandler); - this._linkHandler = undefined; + dispose(this._linkManager); + this._linkManager = undefined; dispose(this._commandTrackerAddon); this._commandTrackerAddon = undefined; dispose(this._widgetManager); @@ -1118,8 +1118,8 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { private async _updateProcessCwd(): Promise { // reset cwd if it has changed, so file based url paths can be resolved const cwd = await this.getCwd(); - if (cwd && this._linkHandler) { - this._linkHandler.processCwd = cwd; + if (cwd && this._linkManager) { + this._linkManager.processCwd = cwd; } return cwd; } diff --git a/src/vs/workbench/contrib/terminal/test/browser/terminalLinkHandler.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHandler.test.ts similarity index 98% rename from src/vs/workbench/contrib/terminal/test/browser/terminalLinkHandler.test.ts rename to src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHandler.test.ts index f817d6bb400a7..801ebd423c1d1 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/terminalLinkHandler.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHandler.test.ts @@ -5,14 +5,14 @@ import * as assert from 'assert'; import { OperatingSystem } from 'vs/base/common/platform'; -import { TerminalLinkHandler, LineColumnInfo, XtermLinkMatcherHandler } from 'vs/workbench/contrib/terminal/browser/terminalLinkHandler'; +import { TerminalLinkManager, LineColumnInfo, XtermLinkMatcherHandler } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkManager'; import * as strings from 'vs/base/common/strings'; import { ITerminalInstanceService } from 'vs/workbench/contrib/terminal/browser/terminal'; import { Event } from 'vs/base/common/event'; import { ITerminalConfigHelper } from 'vs/workbench/contrib/terminal/common/terminal'; import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; -class TestTerminalLinkHandler extends TerminalLinkHandler { +class TestTerminalLinkHandler extends TerminalLinkManager { public get localLinkRegex(): RegExp { return this._localLinkRegex; } @@ -29,7 +29,7 @@ class TestTerminalLinkHandler extends TerminalLinkHandler { return true; } public wrapLinkHandler(handler: (link: string) => void): XtermLinkMatcherHandler { - TerminalLinkHandler._LINK_INTERCEPT_THRESHOLD = 0; + TerminalLinkManager._LINK_INTERCEPT_THRESHOLD = 0; return this._wrapLinkHandler(handler); } } diff --git a/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHelpers.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHelpers.test.ts new file mode 100644 index 0000000000000..dcad739385fb8 --- /dev/null +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHelpers.test.ts @@ -0,0 +1,186 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { IBufferLine, IBufferCell } from 'xterm'; +import { convertLinkRangeToBuffer } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers'; + +suite('Workbench - Terminal Link Helpers', () => { + suite('convertLinkRangeToBuffer', () => { + test('should convert ranges for ascii characters', () => { + const lines = createBufferLineArray([ + { text: 'AA http://t', width: 11 }, + { text: '.com/f/', width: 8 } + ]); + const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 4, startLineNumber: 1, endColumn: 19, endLineNumber: 1 }, 0); + assert.deepEqual(bufferRange, { + start: { x: 4, y: 1 }, + end: { x: 7, y: 2 } + }); + }); + test('should convert ranges for wide characters before the link', () => { + const lines = createBufferLineArray([ + { text: 'A文 http://', width: 11 }, + { text: 't.com/f/', width: 9 } + ]); + const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 4, startLineNumber: 1, endColumn: 19, endLineNumber: 1 }, 0); + assert.deepEqual(bufferRange, { + start: { x: 4 + 1, y: 1 }, + end: { x: 7 + 1, y: 2 } + }); + }); + test('should convert ranges for wide characters inside the link', () => { + const lines = createBufferLineArray([ + { text: 'AA http://t', width: 11 }, + { text: '.com/文/', width: 8 } + ]); + const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 4, startLineNumber: 1, endColumn: 19, endLineNumber: 1 }, 0); + assert.deepEqual(bufferRange, { + start: { x: 4, y: 1 }, + end: { x: 7 + 1, y: 2 } + }); + }); + test('should convert ranges for wide characters before and inside the link', () => { + const lines = createBufferLineArray([ + { text: 'A文 http://', width: 11 }, + { text: 't.com/文/', width: 9 } + ]); + const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 4, startLineNumber: 1, endColumn: 19, endLineNumber: 1 }, 0); + assert.deepEqual(bufferRange, { + start: { x: 4 + 1, y: 1 }, + end: { x: 7 + 2, y: 2 } + }); + }); + test('should convert ranges for ascii characters (link starts on wrapped)', () => { + const lines = createBufferLineArray([ + { text: 'AAAAAAAAAAA', width: 11 }, + { text: 'AA http://t', width: 11 }, + { text: '.com/f/', width: 8 } + ]); + const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 15, startLineNumber: 1, endColumn: 30, endLineNumber: 1 }, 0); + assert.deepEqual(bufferRange, { + start: { x: 4, y: 2 }, + end: { x: 7, y: 3 } + }); + }); + test('should convert ranges for wide characters before the link (link starts on wrapped)', () => { + const lines = createBufferLineArray([ + { text: 'AAAAAAAAAAA', width: 11 }, + { text: 'A文 http://', width: 11 }, + { text: 't.com/f/', width: 9 } + ]); + const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 15, startLineNumber: 1, endColumn: 30, endLineNumber: 1 }, 0); + assert.deepEqual(bufferRange, { + start: { x: 4 + 1, y: 2 }, + end: { x: 7 + 1, y: 3 } + }); + }); + test('should convert ranges for wide characters inside the link (link starts on wrapped)', () => { + const lines = createBufferLineArray([ + { text: 'AAAAAAAAAAA', width: 11 }, + { text: 'AA http://t', width: 11 }, + { text: '.com/文/', width: 8 } + ]); + const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 15, startLineNumber: 1, endColumn: 30, endLineNumber: 1 }, 0); + assert.deepEqual(bufferRange, { + start: { x: 4, y: 2 }, + end: { x: 7 + 1, y: 3 } + }); + }); + test('should convert ranges for wide characters before and inside the link', () => { + const lines = createBufferLineArray([ + { text: 'AAAAAAAAAAA', width: 11 }, + { text: 'A文 http://', width: 11 }, + { text: 't.com/文/', width: 9 } + ]); + const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 15, startLineNumber: 1, endColumn: 30, endLineNumber: 1 }, 0); + assert.deepEqual(bufferRange, { + start: { x: 4 + 1, y: 2 }, + end: { x: 7 + 2, y: 3 } + }); + }); + test('should convert ranges for several wide characters before the link', () => { + const lines = createBufferLineArray([ + { text: 'A文文AAAAAA', width: 11 }, + { text: 'AA文文 http', width: 11 }, + { text: '://t.com/f/', width: 11 } + ]); + const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 15, startLineNumber: 1, endColumn: 30, endLineNumber: 1 }, 0); + // This test ensures that the start offset is applies to the end before it's counted + assert.deepEqual(bufferRange, { + start: { x: 4 + 4, y: 2 }, + end: { x: 7 + 4, y: 3 } + }); + }); + test('should convert ranges for several wide characters before and inside the link', () => { + const lines = createBufferLineArray([ + { text: 'A文文AAAAAA', width: 11 }, + { text: 'AA文文 http', width: 11 }, + { text: '://t.com/文', width: 11 }, + { text: '文/', width: 3 } + ]); + const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 15, startLineNumber: 1, endColumn: 31, endLineNumber: 1 }, 0); + // This test ensures that the start offset is applies to the end before it's counted + assert.deepEqual(bufferRange, { + start: { x: 4 + 4, y: 2 }, + end: { x: 2, y: 4 } + }); + }); + }); +}); + +const TEST_WIDE_CHAR = '文'; +const TEST_NULL_CHAR = 'C'; + +function createBufferLineArray(lines: { text: string, width: number }[]): IBufferLine[] { + let result: IBufferLine[] = []; + lines.forEach((l, i) => { + result.push(new TestBufferLine( + l.text, + l.width, + i + 1 !== lines.length + )); + }); + return result; +} + +class TestBufferLine implements IBufferLine { + constructor( + private _text: string, + public length: number, + public isWrapped: boolean + ) { + + } + getCell(x: number): IBufferCell | undefined { + // Create a fake line of cells and use that to resolve the width + let cells: string = ''; + let offset = 0; + for (let i = 0; i <= x - offset; i++) { + const char = this._text.charAt(i); + cells += char; + if (this._text.charAt(i) === TEST_WIDE_CHAR) { + // Skip the next character as it's width is 0 + cells += TEST_NULL_CHAR; + offset++; + } + } + return { + getChars: () => { + return x >= cells.length ? '' : cells.charAt(x); + }, + getWidth: () => { + switch (cells.charAt(x)) { + case TEST_WIDE_CHAR: return 2; + case TEST_NULL_CHAR: return 0; + default: return 1; + } + } + } as any; + } + translateToString(): string { + throw new Error('Method not implemented.'); + } +} diff --git a/src/vs/workbench/contrib/terminal/test/browser/terminalWebLinkProvider.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts similarity index 52% rename from src/vs/workbench/contrib/terminal/test/browser/terminalWebLinkProvider.test.ts rename to src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts index 88acc527444aa..3c51ca1fb51a8 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/terminalWebLinkProvider.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts @@ -4,8 +4,8 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; -import { convertLinkRangeToBuffer, TerminalWebLinkProvider } from 'vs/workbench/contrib/terminal/browser/terminalWebLinkProvider'; -import { IBufferLine, IBufferCell, Terminal, ILink, IBufferRange, IBufferCellPosition } from 'xterm'; +import { TerminalWebLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider'; +import { Terminal, ILink, IBufferRange, IBufferCellPosition } from 'xterm'; suite('Workbench - TerminalWebLinkProvider', () => { suite('TerminalWebLinkProvider', () => { @@ -85,179 +85,4 @@ suite('Workbench - TerminalWebLinkProvider', () => { await assertLink('aa https://foo.bar/[this is foo site] aa', { range: [[5, 1], [38, 1]], text: 'https://foo.bar/[this is foo site]' }); }); }); - suite('convertLinkRangeToBuffer', () => { - test('should convert ranges for ascii characters', () => { - const lines = createBufferLineArray([ - { text: 'AA http://t', width: 11 }, - { text: '.com/f/', width: 8 } - ]); - const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 4, startLineNumber: 1, endColumn: 19, endLineNumber: 1 }, 0); - assert.deepEqual(bufferRange, { - start: { x: 4, y: 1 }, - end: { x: 7, y: 2 } - }); - }); - test('should convert ranges for wide characters before the link', () => { - const lines = createBufferLineArray([ - { text: 'A文 http://', width: 11 }, - { text: 't.com/f/', width: 9 } - ]); - const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 4, startLineNumber: 1, endColumn: 19, endLineNumber: 1 }, 0); - assert.deepEqual(bufferRange, { - start: { x: 4 + 1, y: 1 }, - end: { x: 7 + 1, y: 2 } - }); - }); - test('should convert ranges for wide characters inside the link', () => { - const lines = createBufferLineArray([ - { text: 'AA http://t', width: 11 }, - { text: '.com/文/', width: 8 } - ]); - const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 4, startLineNumber: 1, endColumn: 19, endLineNumber: 1 }, 0); - assert.deepEqual(bufferRange, { - start: { x: 4, y: 1 }, - end: { x: 7 + 1, y: 2 } - }); - }); - test('should convert ranges for wide characters before and inside the link', () => { - const lines = createBufferLineArray([ - { text: 'A文 http://', width: 11 }, - { text: 't.com/文/', width: 9 } - ]); - const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 4, startLineNumber: 1, endColumn: 19, endLineNumber: 1 }, 0); - assert.deepEqual(bufferRange, { - start: { x: 4 + 1, y: 1 }, - end: { x: 7 + 2, y: 2 } - }); - }); - test('should convert ranges for ascii characters (link starts on wrapped)', () => { - const lines = createBufferLineArray([ - { text: 'AAAAAAAAAAA', width: 11 }, - { text: 'AA http://t', width: 11 }, - { text: '.com/f/', width: 8 } - ]); - const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 15, startLineNumber: 1, endColumn: 30, endLineNumber: 1 }, 0); - assert.deepEqual(bufferRange, { - start: { x: 4, y: 2 }, - end: { x: 7, y: 3 } - }); - }); - test('should convert ranges for wide characters before the link (link starts on wrapped)', () => { - const lines = createBufferLineArray([ - { text: 'AAAAAAAAAAA', width: 11 }, - { text: 'A文 http://', width: 11 }, - { text: 't.com/f/', width: 9 } - ]); - const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 15, startLineNumber: 1, endColumn: 30, endLineNumber: 1 }, 0); - assert.deepEqual(bufferRange, { - start: { x: 4 + 1, y: 2 }, - end: { x: 7 + 1, y: 3 } - }); - }); - test('should convert ranges for wide characters inside the link (link starts on wrapped)', () => { - const lines = createBufferLineArray([ - { text: 'AAAAAAAAAAA', width: 11 }, - { text: 'AA http://t', width: 11 }, - { text: '.com/文/', width: 8 } - ]); - const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 15, startLineNumber: 1, endColumn: 30, endLineNumber: 1 }, 0); - assert.deepEqual(bufferRange, { - start: { x: 4, y: 2 }, - end: { x: 7 + 1, y: 3 } - }); - }); - test('should convert ranges for wide characters before and inside the link', () => { - const lines = createBufferLineArray([ - { text: 'AAAAAAAAAAA', width: 11 }, - { text: 'A文 http://', width: 11 }, - { text: 't.com/文/', width: 9 } - ]); - const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 15, startLineNumber: 1, endColumn: 30, endLineNumber: 1 }, 0); - assert.deepEqual(bufferRange, { - start: { x: 4 + 1, y: 2 }, - end: { x: 7 + 2, y: 3 } - }); - }); - test('should convert ranges for several wide characters before the link', () => { - const lines = createBufferLineArray([ - { text: 'A文文AAAAAA', width: 11 }, - { text: 'AA文文 http', width: 11 }, - { text: '://t.com/f/', width: 11 } - ]); - const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 15, startLineNumber: 1, endColumn: 30, endLineNumber: 1 }, 0); - // This test ensures that the start offset is applies to the end before it's counted - assert.deepEqual(bufferRange, { - start: { x: 4 + 4, y: 2 }, - end: { x: 7 + 4, y: 3 } - }); - }); - test('should convert ranges for several wide characters before and inside the link', () => { - const lines = createBufferLineArray([ - { text: 'A文文AAAAAA', width: 11 }, - { text: 'AA文文 http', width: 11 }, - { text: '://t.com/文', width: 11 }, - { text: '文/', width: 3 } - ]); - const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 15, startLineNumber: 1, endColumn: 31, endLineNumber: 1 }, 0); - // This test ensures that the start offset is applies to the end before it's counted - assert.deepEqual(bufferRange, { - start: { x: 4 + 4, y: 2 }, - end: { x: 2, y: 4 } - }); - }); - }); }); - -const TEST_WIDE_CHAR = '文'; -const TEST_NULL_CHAR = 'C'; - -function createBufferLineArray(lines: { text: string, width: number }[]): IBufferLine[] { - let result: IBufferLine[] = []; - lines.forEach((l, i) => { - result.push(new TestBufferLine( - l.text, - l.width, - i + 1 !== lines.length - )); - }); - return result; -} - -class TestBufferLine implements IBufferLine { - constructor( - private _text: string, - public length: number, - public isWrapped: boolean - ) { - - } - getCell(x: number): IBufferCell | undefined { - // Create a fake line of cells and use that to resolve the width - let cells: string = ''; - let offset = 0; - for (let i = 0; i <= x - offset; i++) { - const char = this._text.charAt(i); - cells += char; - if (this._text.charAt(i) === TEST_WIDE_CHAR) { - // Skip the next character as it's width is 0 - cells += TEST_NULL_CHAR; - offset++; - } - } - return { - getChars: () => { - return x >= cells.length ? '' : cells.charAt(x); - }, - getWidth: () => { - switch (cells.charAt(x)) { - case TEST_WIDE_CHAR: return 2; - case TEST_NULL_CHAR: return 0; - default: return 1; - } - } - } as any; - } - translateToString(): string { - throw new Error('Method not implemented.'); - } -} From fa3a5569bae1ebac9f556d6699d3a22d6f16c1d7 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 12:48:02 -0700 Subject: [PATCH 02/14] Basic regex link detection --- .../browser/links/terminalLinkHelpers.ts | 2 +- .../links/terminalRegexLocalLinkProvider.ts | 70 +++++++++++++++++-- ...er.test.ts => terminalLinkManager.test.ts} | 18 ++--- 3 files changed, 76 insertions(+), 14 deletions(-) rename src/vs/workbench/contrib/terminal/test/browser/links/{terminalLinkHandler.test.ts => terminalLinkManager.test.ts} (95%) diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts index e25db6781e18c..79ae05f53bb94 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts @@ -37,7 +37,7 @@ export function convertLinkRangeToBuffer(lines: IBufferLine[], bufferWidth: numb // Shift end range right for each wide character inside the link let endOffset = 0; const endWrappedLineCount = Math.ceil(range.endColumn / bufferWidth); - for (let y = startWrappedLineCount - 1; y < endWrappedLineCount; y++) { + for (let y = Math.max(0, startWrappedLineCount - 1); y < endWrappedLineCount; y++) { const start = (y === startWrappedLineCount - 1 ? (range.startColumn + startOffset) % bufferWidth : 0); const lineLength = Math.min(bufferWidth, range.endColumn + startOffset - y * bufferWidth); const startLineOffset = (y === startWrappedLineCount - 1 ? startOffset : 0); diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts index db8dc68f7ef25..033c164ea43ff 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts @@ -3,8 +3,43 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Terminal, ILinkProvider, IViewportRange, IBufferCellPosition, ILink } from 'xterm'; +import { Terminal, ILinkProvider, IViewportRange, IBufferCellPosition, ILink, IBufferLine } from 'xterm'; import { ITerminalProcessManager } from 'vs/workbench/contrib/terminal/common/terminal'; +import { getXtermLineContent, convertLinkRangeToBuffer, convertBufferRangeToViewport } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers'; +import { OperatingSystem } from 'vs/base/common/platform'; + +const pathPrefix = '(\\.\\.?|\\~)'; +const pathSeparatorClause = '\\/'; +// '":; are allowed in paths but they are often separators so ignore them +// Also disallow \\ to prevent a catastropic backtracking case #24798 +const excludedPathCharactersClause = '[^\\0\\s!$`&*()\\[\\]+\'":;\\\\]'; +/** A regex that matches paths in the form /foo, ~/foo, ./foo, ../foo, foo/bar */ +const unixLocalLinkClause = '((' + pathPrefix + '|(' + excludedPathCharactersClause + ')+)?(' + pathSeparatorClause + '(' + excludedPathCharactersClause + ')+)+)'; + +const winDrivePrefix = '[a-zA-Z]:'; +const winPathPrefix = '(' + winDrivePrefix + '|\\.\\.?|\\~)'; +const winPathSeparatorClause = '(\\\\|\\/)'; +const winExcludedPathCharactersClause = '[^\\0<>\\?\\|\\/\\s!$`&*()\\[\\]+\'":;]'; +/** A regex that matches paths in the form c:\foo, ~\foo, .\foo, ..\foo, foo\bar */ +const winLocalLinkClause = '((' + winPathPrefix + '|(' + winExcludedPathCharactersClause + ')+)?(' + winPathSeparatorClause + '(' + winExcludedPathCharactersClause + ')+)+)'; + +/** As xterm reads from DOM, space in that case is nonbreaking char ASCII code - 160, +replacing space with nonBreakningSpace or space ASCII code - 32. */ +const lineAndColumnClause = [ + '((\\S*)", line ((\\d+)( column (\\d+))?))', // "(file path)", line 45 [see #40468] + '((\\S*)",((\\d+)(:(\\d+))?))', // "(file path)",45 [see #78205] + '((\\S*) on line ((\\d+)(, column (\\d+))?))', // (file path) on line 8, column 13 + '((\\S*):line ((\\d+)(, column (\\d+))?))', // (file path):line 8, column 13 + '(([^\\s\\(\\)]*)(\\s?[\\(\\[](\\d+)(,\\s?(\\d+))?)[\\)\\]])', // (file path)(45), (file path) (45), (file path)(45,18), (file path) (45,18), (file path)(45, 18), (file path) (45, 18), also with [] + '(([^:\\s\\(\\)<>\'\"\\[\\]]*)(:(\\d+))?(:(\\d+))?)' // (file path):336, (file path):336:9 +].join('|').replace(/ /g, `[${'\u00A0'} ]`); + +// Changing any regex may effect this value, hence changes this as well if required. +// const winLineAndColumnMatchIndex = 12; +// const unixLineAndColumnMatchIndex = 11; + +// Each line and column clause have 6 groups (ie no. of expressions in round brackets) +// const lineAndColumnClauseGroupCount = 6; export class TerminalRegexLocalLinkProvider implements ILinkProvider { constructor( @@ -20,11 +55,17 @@ export class TerminalRegexLocalLinkProvider implements ILinkProvider { let startLine = position.y - 1; let endLine = startLine; + const lines: IBufferLine[] = [ + this._xterm.buffer.active.getLine(startLine)! + ]; + while (this._xterm.buffer.active.getLine(startLine)?.isWrapped) { + lines.unshift(this._xterm.buffer.active.getLine(startLine - 1)!); startLine--; } while (this._xterm.buffer.active.getLine(endLine + 1)?.isWrapped) { + lines.push(this._xterm.buffer.active.getLine(endLine + 1)!); endLine++; } @@ -55,7 +96,30 @@ export class TerminalRegexLocalLinkProvider implements ILinkProvider { break; } - console.log('found link!', stringIndex, uri); + // Convert the link text's string index into a wrapped buffer range + const bufferRange = convertLinkRangeToBuffer(lines, this._xterm.cols, { + startColumn: stringIndex + 1, + startLineNumber: 1, + endColumn: stringIndex + uri.length + 1, + endLineNumber: 1 + }, startLine); + + callback({ + text: uri, + range: bufferRange, + activate: (event: MouseEvent, text: string) => { + this._activateCallback(event, text); + }, + hover: (event: MouseEvent, text: string) => { + setTimeout(() => { + this._tooltipCallback(event, text, convertBufferRangeToViewport(bufferRange, this._xterm.buffer.active.viewportY)); + }, 200); + }, + leave: () => { + this._leaveCallback(); + } + }); + return; // get the buffer index as [absolute row, col] for the match // const bufferIndex = this._xterm.buffer.active.stringIndexToBufferIndex(rowIndex, stringIndex); @@ -117,9 +181,7 @@ export class TerminalRegexLocalLinkProvider implements ILinkProvider { // } // }); - // if (!found) { callback(undefined); - // } } protected get _localLinkRegex(): RegExp { diff --git a/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHandler.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkManager.test.ts similarity index 95% rename from src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHandler.test.ts rename to src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkManager.test.ts index 801ebd423c1d1..1ef7431d9f0bd 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHandler.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkManager.test.ts @@ -12,7 +12,7 @@ import { Event } from 'vs/base/common/event'; import { ITerminalConfigHelper } from 'vs/workbench/contrib/terminal/common/terminal'; import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; -class TestTerminalLinkHandler extends TerminalLinkManager { +class TestTerminalLinkManager extends TerminalLinkManager { public get localLinkRegex(): RegExp { return this._localLinkRegex; } @@ -86,7 +86,7 @@ const testConfigHelper: ITerminalConfigHelper = { suite('Workbench - TerminalLinkHandler', () => { suite('localLinkRegex', () => { test('Windows', () => { - const terminalLinkHandler = new TestTerminalLinkHandler(new TestXterm() as any, { + const terminalLinkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Windows, userHome: '' } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); @@ -162,7 +162,7 @@ suite('Workbench - TerminalLinkHandler', () => { }); test('Linux', () => { - const terminalLinkHandler = new TestTerminalLinkHandler(new TestXterm() as any, { + const terminalLinkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Linux, userHome: '' } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); @@ -230,7 +230,7 @@ suite('Workbench - TerminalLinkHandler', () => { suite('preprocessPath', () => { test('Windows', () => { - const linkHandler = new TestTerminalLinkHandler(new TestXterm() as any, { + const linkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Windows, userHome: 'C:\\Users\\Me' } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); @@ -243,7 +243,7 @@ suite('Workbench - TerminalLinkHandler', () => { assert.equal(linkHandler.preprocessPath('C:\\absolute\\path\\file5'), 'C:\\absolute\\path\\file5'); }); test('Windows - spaces', () => { - const linkHandler = new TestTerminalLinkHandler(new TestXterm() as any, { + const linkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Windows, userHome: 'C:\\Users\\M e' } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); @@ -257,7 +257,7 @@ suite('Workbench - TerminalLinkHandler', () => { }); test('Linux', () => { - const linkHandler = new TestTerminalLinkHandler(new TestXterm() as any, { + const linkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Linux, userHome: '/home/me' } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); @@ -270,7 +270,7 @@ suite('Workbench - TerminalLinkHandler', () => { }); test('No Workspace', () => { - const linkHandler = new TestTerminalLinkHandler(new TestXterm() as any, { + const linkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Linux, userHome: '/home/me' } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); @@ -284,7 +284,7 @@ suite('Workbench - TerminalLinkHandler', () => { test('gitDiffLinkRegex', () => { // The platform is irrelevant because the links generated by Git are the same format regardless of platform - const linkHandler = new TestTerminalLinkHandler(new TestXterm() as any, { + const linkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Linux, userHome: '' } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); @@ -315,7 +315,7 @@ suite('Workbench - TerminalLinkHandler', () => { const nullMouseEvent: any = Object.freeze({ preventDefault: () => { } }); test('should allow intercepting of links with onBeforeHandleLink', async () => { - const linkHandler = new TestTerminalLinkHandler(new TestXterm() as any, { + const linkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Linux, userHome: '' } as any, testConfigHelper, null!, null!, new TestConfigurationService(), new MockTerminalInstanceService(), null!, null!); From 183c832391d67dd8cced1b1407c729dc170518e2 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 12:57:13 -0700 Subject: [PATCH 03/14] Pass in validation callback --- .../browser/links/terminalLinkManager.ts | 2 +- .../links/terminalRegexLocalLinkProvider.ts | 93 ++++--------------- 2 files changed, 21 insertions(+), 74 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts index 377edd908da31..32d7911aaf51a 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts @@ -292,7 +292,7 @@ export class TerminalLinkManager extends DisposableStore { this._tooltipCallback(event, link, location, this._handleLocalLink.bind(this, link)); }; const wrappedLinkActivateCallback = this._wrapLinkHandler(this._handleLocalLink.bind(this)); - this._linkProvider = this._xterm.registerLinkProvider(new TerminalRegexLocalLinkProvider(this._xterm, this._processManager, wrappedLinkActivateCallback, tooltipLinkCallback, this._leaveCallback)); + this._linkProvider = this._xterm.registerLinkProvider(new TerminalRegexLocalLinkProvider(this._xterm, this._processManager, wrappedLinkActivateCallback, tooltipLinkCallback, this._leaveCallback, this._validateLocalLink.bind(this))); } protected _wrapLinkHandler(handler: (link: string) => void): XtermLinkMatcherHandler { diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts index 033c164ea43ff..f24f80da3786d 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts @@ -47,7 +47,8 @@ export class TerminalRegexLocalLinkProvider implements ILinkProvider { private readonly _processManager: ITerminalProcessManager | undefined, private readonly _activateCallback: (event: MouseEvent, uri: string) => void, private readonly _tooltipCallback: (event: MouseEvent, uri: string, location: IViewportRange) => boolean | void, - private readonly _leaveCallback: () => void + private readonly _leaveCallback: () => void, + private readonly _validationCallback: (uri: string, callback: (isValid: boolean) => void) => void ) { } @@ -104,83 +105,29 @@ export class TerminalRegexLocalLinkProvider implements ILinkProvider { endLineNumber: 1 }, startLine); - callback({ - text: uri, - range: bufferRange, - activate: (event: MouseEvent, text: string) => { - this._activateCallback(event, text); - }, - hover: (event: MouseEvent, text: string) => { - setTimeout(() => { - this._tooltipCallback(event, text, convertBufferRangeToViewport(bufferRange, this._xterm.buffer.active.viewportY)); - }, 200); - }, - leave: () => { - this._leaveCallback(); + this._validationCallback(uri, isValid => { + // TODO: Discard if buffers have changes or if another link was added for this line + if (isValid) { + callback({ + text: uri, + range: bufferRange, + activate: (event: MouseEvent, text: string) => { + this._activateCallback(event, text); + }, + hover: (event: MouseEvent, text: string) => { + setTimeout(() => { + this._tooltipCallback(event, text, convertBufferRangeToViewport(bufferRange, this._xterm.buffer.active.viewportY)); + }, 200); + }, + leave: () => { + this._leaveCallback(); + } + }); } }); return; - - // get the buffer index as [absolute row, col] for the match - // const bufferIndex = this._xterm.buffer.active.stringIndexToBufferIndex(rowIndex, stringIndex); - // if (bufferIndex[0] < 0) { - // // invalid bufferIndex (should not have happened) - // break; - // } - - // const line = this._xterm.buffer.active.getLine(bufferIndex[0]); - // if (!line) { - // break; - // } - - // const attr = line.getFg(bufferIndex[1]); - // const fg = attr ? (attr >> 9) & 0x1ff : undefined; - - - // if (matcher.validationCallback) { - // matcher.validationCallback(uri, isValid => { - // // Discard link if the line has already changed - // if (this._rowsTimeoutId) { - // return; - // } - // if (isValid) { - // this._addLink(bufferIndex[1], bufferIndex[0] - this._bufferService.buffer.ydisp, uri, matcher, fg); - // } - // }); - // } else { - // this._addLink(bufferIndex[1], bufferIndex[0] - this._bufferService.buffer.ydisp, uri, matcher, fg); - // } } - // this._linkComputerTarget = new TerminalLinkAdapter(this._xterm, startLine, endLine); - // const links = LinkComputer.computeLinks(this._linkComputerTarget); - - // let found = false; - // links.forEach(link => { - // const range = this._convertLinkRangeToBuffer(link.range, startLine); - - // // Check if the link if within the mouse position - // if (this._positionIsInRange(position, range)) { - // found = true; - - // callback({ - // text: link.url?.toString() || '', - // range, - // activate: (event: MouseEvent, text: string) => { - // this._activateCallback(event, text); - // }, - // hover: (event: MouseEvent, text: string) => { - // setTimeout(() => { - // this._tooltipCallback(event, text, convertBufferRangeToViewport(range, this._xterm.buffer.active.viewportY)); - // }, 200); - // }, - // leave: () => { - // this._leaveCallback(); - // } - // }); - // } - // }); - callback(undefined); } From d2ad3c0477d6a9622a575319607fcaf5eeae00fe Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 13:12:33 -0700 Subject: [PATCH 04/14] Add first test for regex local link provider --- .../browser/links/terminalLinkHelpers.ts | 15 +- .../browser/links/terminalLinkManager.ts | 6 +- .../links/terminalRegexLocalLinkProvider.ts | 54 +++---- .../browser/links/terminalWebLinkProvider.ts | 26 +--- .../terminal/browser/terminalInstance.ts | 2 +- .../terminalRegexLocalLinkProvider.test.ts | 58 ++++++++ .../links/terminalWebLinkProvider.test.ts | 132 +++++++++--------- 7 files changed, 173 insertions(+), 120 deletions(-) create mode 100644 src/vs/workbench/contrib/terminal/test/browser/links/terminalRegexLocalLinkProvider.test.ts diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts index 79ae05f53bb94..d04bcf16344ef 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { IViewportRange, IBufferRange, IBufferLine, IBuffer } from 'xterm'; +import { IViewportRange, IBufferRange, IBufferLine, IBuffer, IBufferCellPosition } from 'xterm'; import { IRange } from 'vs/editor/common/core/range'; export function convertLinkRangeToBuffer(lines: IBufferLine[], bufferWidth: number, range: IRange, startLine: number) { @@ -95,3 +95,16 @@ export function getXtermLineContent(buffer: IBuffer, lineStart: number, lineEnd: } return line; } + +export function positionIsInRange(position: IBufferCellPosition, range: IBufferRange): boolean { + if (position.y < range.start.y || position.y > range.end.y) { + return false; + } + if (position.y === range.start.y && position.x < range.start.x) { + return false; + } + if (position.y === range.end.y && position.x > range.end.x) { + return false; + } + return true; +} diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts index 32d7911aaf51a..2e16150ca3435 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts @@ -17,7 +17,7 @@ import { Terminal, ILinkMatcherOptions, IViewportRange, ITerminalAddon } from 'x import { REMOTE_HOST_SCHEME } from 'vs/platform/remote/common/remoteHosts'; import { posix, win32 } from 'vs/base/common/path'; import { ITerminalInstanceService, ITerminalBeforeHandleLinkEvent, LINK_INTERCEPT_THRESHOLD } from 'vs/workbench/contrib/terminal/browser/terminal'; -import { OperatingSystem, isMacintosh } from 'vs/base/common/platform'; +import { OperatingSystem, isMacintosh, OS } from 'vs/base/common/platform'; import { IMarkdownString, MarkdownString } from 'vs/base/common/htmlContent'; import { Emitter, Event } from 'vs/base/common/event'; import { ILogService } from 'vs/platform/log/common/log'; @@ -101,7 +101,7 @@ export class TerminalLinkManager extends DisposableStore { constructor( private _xterm: Terminal, - private readonly _processManager: ITerminalProcessManager | undefined, + private readonly _processManager: ITerminalProcessManager, private readonly _configHelper: ITerminalConfigHelper, @IOpenerService private readonly _openerService: IOpenerService, @IEditorService private readonly _editorService: IEditorService, @@ -292,7 +292,7 @@ export class TerminalLinkManager extends DisposableStore { this._tooltipCallback(event, link, location, this._handleLocalLink.bind(this, link)); }; const wrappedLinkActivateCallback = this._wrapLinkHandler(this._handleLocalLink.bind(this)); - this._linkProvider = this._xterm.registerLinkProvider(new TerminalRegexLocalLinkProvider(this._xterm, this._processManager, wrappedLinkActivateCallback, tooltipLinkCallback, this._leaveCallback, this._validateLocalLink.bind(this))); + this._linkProvider = this._xterm.registerLinkProvider(new TerminalRegexLocalLinkProvider(this._xterm, this._processManager.os || OS, wrappedLinkActivateCallback, tooltipLinkCallback, this._leaveCallback, this._validateLocalLink.bind(this))); } protected _wrapLinkHandler(handler: (link: string) => void): XtermLinkMatcherHandler { diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts index f24f80da3786d..37305aa6b8983 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts @@ -4,8 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Terminal, ILinkProvider, IViewportRange, IBufferCellPosition, ILink, IBufferLine } from 'xterm'; -import { ITerminalProcessManager } from 'vs/workbench/contrib/terminal/common/terminal'; -import { getXtermLineContent, convertLinkRangeToBuffer, convertBufferRangeToViewport } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers'; +import { getXtermLineContent, convertLinkRangeToBuffer, convertBufferRangeToViewport, positionIsInRange } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers'; import { OperatingSystem } from 'vs/base/common/platform'; const pathPrefix = '(\\.\\.?|\\~)'; @@ -44,7 +43,7 @@ const lineAndColumnClause = [ export class TerminalRegexLocalLinkProvider implements ILinkProvider { constructor( private readonly _xterm: Terminal, - private readonly _processManager: ITerminalProcessManager | undefined, + private readonly _processOperatingSystem: OperatingSystem, private readonly _activateCallback: (event: MouseEvent, uri: string) => void, private readonly _tooltipCallback: (event: MouseEvent, uri: string, location: IViewportRange) => boolean | void, private readonly _leaveCallback: () => void, @@ -105,26 +104,30 @@ export class TerminalRegexLocalLinkProvider implements ILinkProvider { endLineNumber: 1 }, startLine); - this._validationCallback(uri, isValid => { - // TODO: Discard if buffers have changes or if another link was added for this line - if (isValid) { - callback({ - text: uri, - range: bufferRange, - activate: (event: MouseEvent, text: string) => { - this._activateCallback(event, text); - }, - hover: (event: MouseEvent, text: string) => { - setTimeout(() => { - this._tooltipCallback(event, text, convertBufferRangeToViewport(bufferRange, this._xterm.buffer.active.viewportY)); - }, 200); - }, - leave: () => { - this._leaveCallback(); - } - }); - } - }); + if (positionIsInRange(position, bufferRange)) { + this._validationCallback(uri, isValid => { + // TODO: Discard if buffers have changes or if another link was added for this line + if (isValid) { + callback({ + text: uri, + range: bufferRange, + activate: (event: MouseEvent, text: string) => this._activateCallback(event, text), + hover: (event: MouseEvent, text: string) => { + // TODO: This tooltip timer is currently not totally reliable + setTimeout(() => { + this._tooltipCallback(event, text, convertBufferRangeToViewport(bufferRange, this._xterm.buffer.active.viewportY)); + }, 200); + }, + leave: () => this._leaveCallback() + }); + } else { + // TODO: Support multiple matches from the regexes + callback(undefined); + } + }); + } else { + callback(undefined); + } return; } @@ -132,10 +135,7 @@ export class TerminalRegexLocalLinkProvider implements ILinkProvider { } protected get _localLinkRegex(): RegExp { - if (!this._processManager) { - throw new Error('Process manager is required'); - } - const baseLocalLinkClause = this._processManager.os === OperatingSystem.Windows ? winLocalLinkClause : unixLocalLinkClause; + const baseLocalLinkClause = this._processOperatingSystem === OperatingSystem.Windows ? winLocalLinkClause : unixLocalLinkClause; // Append line and column number regex return new RegExp(`${baseLocalLinkClause}(${lineAndColumnClause})`); } diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts index cef244e8d2104..cd0cb7a32daae 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts @@ -5,7 +5,7 @@ import { Terminal, IViewportRange, ILinkProvider, IBufferCellPosition, ILink, IBufferRange, IBufferLine } from 'xterm'; import { ILinkComputerTarget, LinkComputer } from 'vs/editor/common/modes/linkComputer'; -import { getXtermLineContent, convertLinkRangeToBuffer, convertBufferRangeToViewport } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers'; +import { getXtermLineContent, convertLinkRangeToBuffer, convertBufferRangeToViewport, positionIsInRange } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers'; export class TerminalWebLinkProvider implements ILinkProvider { private _linkComputerTarget: ILinkComputerTarget | undefined; @@ -44,23 +44,20 @@ export class TerminalWebLinkProvider implements ILinkProvider { const range = convertLinkRangeToBuffer(lines, this._xterm.cols, link.range, startLine); // Check if the link if within the mouse position - if (this._positionIsInRange(position, range)) { + if (positionIsInRange(position, range)) { found = true; callback({ text: link.url?.toString() || '', range, - activate: (event: MouseEvent, text: string) => { - this._activateCallback(event, text); - }, + activate: (event: MouseEvent, text: string) => this._activateCallback(event, text), hover: (event: MouseEvent, text: string) => { + // TODO: This tooltip timer is currently not totally reliable setTimeout(() => { this._tooltipCallback(event, text, convertBufferRangeToViewport(range, this._xterm.buffer.active.viewportY)); }, 200); }, - leave: () => { - this._leaveCallback(); - } + leave: () => this._leaveCallback() }); } }); @@ -69,19 +66,6 @@ export class TerminalWebLinkProvider implements ILinkProvider { callback(undefined); } } - - private _positionIsInRange(position: IBufferCellPosition, range: IBufferRange): boolean { - if (position.y < range.start.y || position.y > range.end.y) { - return false; - } - if (position.y === range.start.y && position.x < range.start.x) { - return false; - } - if (position.y === range.end.y && position.x > range.end.x) { - return false; - } - return true; - } } class TerminalLinkAdapter implements ILinkComputerTarget { diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts index 3909aef1d50fc..848cfbba16cff 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts @@ -405,7 +405,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance { return false; }); } - this._linkManager = this._instantiationService.createInstance(TerminalLinkManager, xterm, this._processManager, this._configHelper); + this._linkManager = this._instantiationService.createInstance(TerminalLinkManager, xterm, this._processManager!, this._configHelper); this._linkManager.onBeforeHandleLink(e => { e.terminal = this; this._onBeforeHandleLink.fire(e); diff --git a/src/vs/workbench/contrib/terminal/test/browser/links/terminalRegexLocalLinkProvider.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalRegexLocalLinkProvider.test.ts new file mode 100644 index 0000000000000..7abbd2c40542f --- /dev/null +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalRegexLocalLinkProvider.test.ts @@ -0,0 +1,58 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { TerminalRegexLocalLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider'; +import { Terminal, ILink, IBufferRange, IBufferCellPosition } from 'xterm'; +import { OperatingSystem } from 'vs/base/common/platform'; + +suite('Workbench - TerminalRegexLocalLinkProvider', () => { + async function assertLink(text: string, expected: { text: string, range: [number, number][] }) { + const xterm = new Terminal(); + // TODO: Test process manager + const provider = new TerminalRegexLocalLinkProvider(xterm, OperatingSystem.Linux, () => { }, () => { }, () => { }, (_, cb) => { cb(true); }); + + // Write the text and wait for the parser to finish + await new Promise(r => xterm.write(text, r)); + + // Calculate positions just outside of link boundaries + const noLinkPositions: IBufferCellPosition[] = [ + { x: expected.range[0][0] - 1, y: expected.range[0][1] }, + { x: expected.range[1][0] + 1, y: expected.range[1][1] } + ]; + + // Ensure outside positions do not detect the link + for (let i = 0; i < noLinkPositions.length; i++) { + const link = await new Promise(r => provider.provideLink(noLinkPositions[i], r)); + assert.equal(link, undefined, `Just outside range boundary should not result in link, link found at (${link?.range.start.x}, ${link?.range.start.y}) to (${link?.range.end.x}, ${link?.range.end.y}) while checking (${noLinkPositions[i].x}, ${noLinkPositions[i].y})`); + } + + // Convert range from [[startx, starty], [endx, endy]] to an IBufferRange + const linkRange: IBufferRange = { + start: { x: expected.range[0][0], y: expected.range[0][1] }, + end: { x: expected.range[1][0], y: expected.range[1][1] }, + }; + + // Calculate positions inside the link boundaries + const linkPositions: IBufferCellPosition[] = [ + linkRange.start, + linkRange.end + ]; + + // Ensure inside positions do detect the link + for (let i = 0; i < linkPositions.length; i++) { + const link = await new Promise(r => provider.provideLink(linkPositions[i], r)); + + assert.ok(link); + assert.deepEqual(link!.text, expected.text); + assert.deepEqual(link!.range, linkRange); + } + } + + // These tests are based on LinkComputer.test.ts + test('Simple unix paths', async () => { + await assertLink('"/foo"', { range: [[2, 1], [5, 1]], text: '/foo' }); + }); +}); diff --git a/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts index 3c51ca1fb51a8..a19c55a60a6a6 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts @@ -8,81 +8,79 @@ import { TerminalWebLinkProvider } from 'vs/workbench/contrib/terminal/browser/l import { Terminal, ILink, IBufferRange, IBufferCellPosition } from 'xterm'; suite('Workbench - TerminalWebLinkProvider', () => { - suite('TerminalWebLinkProvider', () => { - async function assertLink(text: string, expected: { text: string, range: [number, number][] }) { - const xterm = new Terminal(); - const provider = new TerminalWebLinkProvider(xterm, () => { }, () => { }, () => { }); + async function assertLink(text: string, expected: { text: string, range: [number, number][] }) { + const xterm = new Terminal(); + const provider = new TerminalWebLinkProvider(xterm, () => { }, () => { }, () => { }); - // Write the text and wait for the parser to finish - await new Promise(r => xterm.write(text, r)); + // Write the text and wait for the parser to finish + await new Promise(r => xterm.write(text, r)); - // Calculate positions just outside of link boundaries - const noLinkPositions: IBufferCellPosition[] = [ - { x: expected.range[0][0] - 1, y: expected.range[0][1] }, - { x: expected.range[1][0] + 1, y: expected.range[1][1] } - ]; + // Calculate positions just outside of link boundaries + const noLinkPositions: IBufferCellPosition[] = [ + { x: expected.range[0][0] - 1, y: expected.range[0][1] }, + { x: expected.range[1][0] + 1, y: expected.range[1][1] } + ]; - // Ensure outside positions do not detect the link - for (let i = 0; i < noLinkPositions.length; i++) { - const link = await new Promise(r => provider.provideLink(noLinkPositions[i], r)); - assert.equal(link, undefined, `Just outside range boundary should not result in link, link found at: (${link?.range.start.x}, ${link?.range.start.y}) to (${link?.range.end.x}, ${link?.range.end.y})`); - } + // Ensure outside positions do not detect the link + for (let i = 0; i < noLinkPositions.length; i++) { + const link = await new Promise(r => provider.provideLink(noLinkPositions[i], r)); + assert.equal(link, undefined, `Just outside range boundary should not result in link, link found at: (${link?.range.start.x}, ${link?.range.start.y}) to (${link?.range.end.x}, ${link?.range.end.y})`); + } - // Convert range from [[startx, starty], [endx, endy]] to an IBufferRange - const linkRange: IBufferRange = { - start: { x: expected.range[0][0], y: expected.range[0][1] }, - end: { x: expected.range[1][0], y: expected.range[1][1] }, - }; + // Convert range from [[startx, starty], [endx, endy]] to an IBufferRange + const linkRange: IBufferRange = { + start: { x: expected.range[0][0], y: expected.range[0][1] }, + end: { x: expected.range[1][0], y: expected.range[1][1] }, + }; - // Calculate positions inside the link boundaries - const linkPositions: IBufferCellPosition[] = [ - linkRange.start, - linkRange.end - ]; + // Calculate positions inside the link boundaries + const linkPositions: IBufferCellPosition[] = [ + linkRange.start, + linkRange.end + ]; - // Ensure inside positions do detect the link - for (let i = 0; i < linkPositions.length; i++) { - const link = await new Promise(r => provider.provideLink(linkPositions[i], r)); + // Ensure inside positions do detect the link + for (let i = 0; i < linkPositions.length; i++) { + const link = await new Promise(r => provider.provideLink(linkPositions[i], r)); - assert.ok(link); - assert.deepEqual(link!.text, expected.text); - assert.deepEqual(link!.range, linkRange); - } + assert.ok(link); + assert.deepEqual(link!.text, expected.text); + assert.deepEqual(link!.range, linkRange); } + } - // These tests are based on LinkComputer.test.ts - test('LinkComputer cases', async () => { - await assertLink('x = "http://foo.bar";', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); - await assertLink('x = (http://foo.bar);', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); - await assertLink('x = \'http://foo.bar\';', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); - await assertLink('x = http://foo.bar ;', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); - await assertLink('x = ;', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); - await assertLink('x = {http://foo.bar};', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); - await assertLink('(see http://foo.bar)', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); - await assertLink('[see http://foo.bar]', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); - await assertLink('{see http://foo.bar}', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); - await assertLink('', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); - await assertLink('http://foo.bar', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); - await assertLink('// Click here to learn more. https://go.microsoft.com/fwlink/?LinkID=513275&clcid=0x409', { range: [[30, 1], [7, 2]], text: 'https://go.microsoft.com/fwlink/?LinkID=513275&clcid=0x409' }); - await assertLink('// Click here to learn more. https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx', { range: [[30, 1], [28, 2]], text: 'https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx' }); - await assertLink('// https://github.com/projectkudu/kudu/blob/master/Kudu.Core/Scripts/selectNodeVersion.js', { range: [[4, 1], [9, 2]], text: 'https://github.com/projectkudu/kudu/blob/master/Kudu.Core/Scripts/selectNodeVersion.js' }); - await assertLink('', { range: [[49, 1], [14, 2]], text: 'https://go.microsoft.com/fwlink/?LinkId=166007' }); - await assertLink('For instructions, see https://go.microsoft.com/fwlink/?LinkId=166007.', { range: [[23, 1], [68, 1]], text: 'https://go.microsoft.com/fwlink/?LinkId=166007' }); - await assertLink('For instructions, see https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx.', { range: [[23, 1], [21, 2]], text: 'https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx' }); - await assertLink('x = "https://en.wikipedia.org/wiki/Zürich";', { range: [[6, 1], [41, 1]], text: 'https://en.wikipedia.org/wiki/Zürich' }); - await assertLink('請參閱 http://go.microsoft.com/fwlink/?LinkId=761051。', { range: [[8, 1], [53, 1]], text: 'http://go.microsoft.com/fwlink/?LinkId=761051' }); - await assertLink('（請參閱 http://go.microsoft.com/fwlink/?LinkId=761051）', { range: [[10, 1], [55, 1]], text: 'http://go.microsoft.com/fwlink/?LinkId=761051' }); - await assertLink('x = "file:///foo.bar";', { range: [[6, 1], [20, 1]], text: 'file:///foo.bar' }); - await assertLink('x = "file://c:/foo.bar";', { range: [[6, 1], [22, 1]], text: 'file://c:/foo.bar' }); - await assertLink('x = "file://shares/foo.bar";', { range: [[6, 1], [26, 1]], text: 'file://shares/foo.bar' }); - await assertLink('x = "file://shäres/foo.bar";', { range: [[6, 1], [26, 1]], text: 'file://shäres/foo.bar' }); - await assertLink('Some text, then http://www.bing.com.', { range: [[17, 1], [35, 1]], text: 'http://www.bing.com' }); - await assertLink('let url = `http://***/_api/web/lists/GetByTitle(\'Teambuildingaanvragen\')/items`;', { range: [[12, 1], [78, 1]], text: 'http://***/_api/web/lists/GetByTitle(\'Teambuildingaanvragen\')/items' }); - await assertLink('7. At this point, ServiceMain has been called. There is no functionality presently in ServiceMain, but you can consult the [MSDN documentation](https://msdn.microsoft.com/en-us/library/windows/desktop/ms687414(v=vs.85).aspx) to add functionality as desired!', { range: [[66, 2], [64, 3]], text: 'https://msdn.microsoft.com/en-us/library/windows/desktop/ms687414(v=vs.85).aspx' }); - await assertLink('let x = "http://[::1]:5000/connect/token"', { range: [[10, 1], [40, 1]], text: 'http://[::1]:5000/connect/token' }); - await assertLink('2. Navigate to **https://portal.azure.com**', { range: [[18, 1], [41, 1]], text: 'https://portal.azure.com' }); - await assertLink('POST|https://portal.azure.com|2019-12-05|', { range: [[6, 1], [29, 1]], text: 'https://portal.azure.com' }); - await assertLink('aa https://foo.bar/[this is foo site] aa', { range: [[5, 1], [38, 1]], text: 'https://foo.bar/[this is foo site]' }); - }); + // These tests are based on LinkComputer.test.ts + test('LinkComputer cases', async () => { + await assertLink('x = "http://foo.bar";', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); + await assertLink('x = (http://foo.bar);', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); + await assertLink('x = \'http://foo.bar\';', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); + await assertLink('x = http://foo.bar ;', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); + await assertLink('x = ;', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); + await assertLink('x = {http://foo.bar};', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); + await assertLink('(see http://foo.bar)', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); + await assertLink('[see http://foo.bar]', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); + await assertLink('{see http://foo.bar}', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); + await assertLink('', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); + await assertLink('http://foo.bar', { range: [[6, 1], [19, 1]], text: 'http://foo.bar' }); + await assertLink('// Click here to learn more. https://go.microsoft.com/fwlink/?LinkID=513275&clcid=0x409', { range: [[30, 1], [7, 2]], text: 'https://go.microsoft.com/fwlink/?LinkID=513275&clcid=0x409' }); + await assertLink('// Click here to learn more. https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx', { range: [[30, 1], [28, 2]], text: 'https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx' }); + await assertLink('// https://github.com/projectkudu/kudu/blob/master/Kudu.Core/Scripts/selectNodeVersion.js', { range: [[4, 1], [9, 2]], text: 'https://github.com/projectkudu/kudu/blob/master/Kudu.Core/Scripts/selectNodeVersion.js' }); + await assertLink('', { range: [[49, 1], [14, 2]], text: 'https://go.microsoft.com/fwlink/?LinkId=166007' }); + await assertLink('For instructions, see https://go.microsoft.com/fwlink/?LinkId=166007.', { range: [[23, 1], [68, 1]], text: 'https://go.microsoft.com/fwlink/?LinkId=166007' }); + await assertLink('For instructions, see https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx.', { range: [[23, 1], [21, 2]], text: 'https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx' }); + await assertLink('x = "https://en.wikipedia.org/wiki/Zürich";', { range: [[6, 1], [41, 1]], text: 'https://en.wikipedia.org/wiki/Zürich' }); + await assertLink('請參閱 http://go.microsoft.com/fwlink/?LinkId=761051。', { range: [[8, 1], [53, 1]], text: 'http://go.microsoft.com/fwlink/?LinkId=761051' }); + await assertLink('（請參閱 http://go.microsoft.com/fwlink/?LinkId=761051）', { range: [[10, 1], [55, 1]], text: 'http://go.microsoft.com/fwlink/?LinkId=761051' }); + await assertLink('x = "file:///foo.bar";', { range: [[6, 1], [20, 1]], text: 'file:///foo.bar' }); + await assertLink('x = "file://c:/foo.bar";', { range: [[6, 1], [22, 1]], text: 'file://c:/foo.bar' }); + await assertLink('x = "file://shares/foo.bar";', { range: [[6, 1], [26, 1]], text: 'file://shares/foo.bar' }); + await assertLink('x = "file://shäres/foo.bar";', { range: [[6, 1], [26, 1]], text: 'file://shäres/foo.bar' }); + await assertLink('Some text, then http://www.bing.com.', { range: [[17, 1], [35, 1]], text: 'http://www.bing.com' }); + await assertLink('let url = `http://***/_api/web/lists/GetByTitle(\'Teambuildingaanvragen\')/items`;', { range: [[12, 1], [78, 1]], text: 'http://***/_api/web/lists/GetByTitle(\'Teambuildingaanvragen\')/items' }); + await assertLink('7. At this point, ServiceMain has been called. There is no functionality presently in ServiceMain, but you can consult the [MSDN documentation](https://msdn.microsoft.com/en-us/library/windows/desktop/ms687414(v=vs.85).aspx) to add functionality as desired!', { range: [[66, 2], [64, 3]], text: 'https://msdn.microsoft.com/en-us/library/windows/desktop/ms687414(v=vs.85).aspx' }); + await assertLink('let x = "http://[::1]:5000/connect/token"', { range: [[10, 1], [40, 1]], text: 'http://[::1]:5000/connect/token' }); + await assertLink('2. Navigate to **https://portal.azure.com**', { range: [[18, 1], [41, 1]], text: 'https://portal.azure.com' }); + await assertLink('POST|https://portal.azure.com|2019-12-05|', { range: [[6, 1], [29, 1]], text: 'https://portal.azure.com' }); + await assertLink('aa https://foo.bar/[this is foo site] aa', { range: [[5, 1], [38, 1]], text: 'https://foo.bar/[this is foo site]' }); }); }); From 4da7703e9efda76378309a60cf70a00dbc628ea3 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 15:30:09 -0700 Subject: [PATCH 05/14] Port old link format tests --- .../browser/links/terminalWebLinkProvider.ts | 2 +- .../terminalRegexLocalLinkProvider.test.ts | 100 ++++++++++++++++-- .../links/terminalWebLinkProvider.test.ts | 6 +- 3 files changed, 93 insertions(+), 15 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts index cd0cb7a32daae..26faef0b97e68 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Terminal, IViewportRange, ILinkProvider, IBufferCellPosition, ILink, IBufferRange, IBufferLine } from 'xterm'; +import { Terminal, IViewportRange, ILinkProvider, IBufferCellPosition, ILink, IBufferLine } from 'xterm'; import { ILinkComputerTarget, LinkComputer } from 'vs/editor/common/modes/linkComputer'; import { getXtermLineContent, convertLinkRangeToBuffer, convertBufferRangeToViewport, positionIsInRange } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers'; diff --git a/src/vs/workbench/contrib/terminal/test/browser/links/terminalRegexLocalLinkProvider.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalRegexLocalLinkProvider.test.ts index 7abbd2c40542f..d84ac7762e0e0 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/links/terminalRegexLocalLinkProvider.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalRegexLocalLinkProvider.test.ts @@ -7,12 +7,67 @@ import * as assert from 'assert'; import { TerminalRegexLocalLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider'; import { Terminal, ILink, IBufferRange, IBufferCellPosition } from 'xterm'; import { OperatingSystem } from 'vs/base/common/platform'; +import { format } from 'vs/base/common/strings'; + +const unixLinks = [ + '/foo', + '~/foo', + './foo', + '../foo', + '/foo/bar', + 'foo/bar' +]; + +const windowsLinks = [ + 'c:\\foo', + 'c:/foo', + '.\\foo', + './foo', + '..\\foo', + '~\\foo', + '~/foo', + 'c:/foo/bar', + 'c:\\foo\\bar', + 'c:\\foo/bar\\baz', + 'foo/bar', + 'foo/bar', + 'foo\\bar' +]; + +interface LinkFormatInfo { + urlFormat: string; + line?: string; + column?: string; +} + +const supportedLinkFormats: LinkFormatInfo[] = [ + { urlFormat: '{0}' }, + { urlFormat: '{0} on line {1}', line: '5' }, + { urlFormat: '{0} on line {1}, column {2}', line: '5', column: '3' }, + { urlFormat: '{0}:line {1}', line: '5' }, + { urlFormat: '{0}:line {1}, column {2}', line: '5', column: '3' }, + { urlFormat: '{0}({1})', line: '5' }, + { urlFormat: '{0} ({1})', line: '5' }, + { urlFormat: '{0}({1},{2})', line: '5', column: '3' }, + { urlFormat: '{0} ({1},{2})', line: '5', column: '3' }, + { urlFormat: '{0}({1}, {2})', line: '5', column: '3' }, + { urlFormat: '{0} ({1}, {2})', line: '5', column: '3' }, + { urlFormat: '{0}:{1}', line: '5' }, + { urlFormat: '{0}:{1}:{2}', line: '5', column: '3' }, + { urlFormat: '{0}[{1}]', line: '5' }, + { urlFormat: '{0} [{1}]', line: '5' }, + { urlFormat: '{0}[{1},{2}]', line: '5', column: '3' }, + { urlFormat: '{0} [{1},{2}]', line: '5', column: '3' }, + { urlFormat: '{0}[{1}, {2}]', line: '5', column: '3' }, + { urlFormat: '{0} [{1}, {2}]', line: '5', column: '3' }, + { urlFormat: '{0}",{1}', line: '5' } +]; suite('Workbench - TerminalRegexLocalLinkProvider', () => { - async function assertLink(text: string, expected: { text: string, range: [number, number][] }) { + async function assertLink(text: string, os: OperatingSystem, expected: { text: string, range: [number, number][] }) { const xterm = new Terminal(); // TODO: Test process manager - const provider = new TerminalRegexLocalLinkProvider(xterm, OperatingSystem.Linux, () => { }, () => { }, () => { }, (_, cb) => { cb(true); }); + const provider = new TerminalRegexLocalLinkProvider(xterm, os, () => { }, () => { }, () => { }, (_, cb) => { cb(true); }); // Write the text and wait for the parser to finish await new Promise(r => xterm.write(text, r)); @@ -26,7 +81,7 @@ suite('Workbench - TerminalRegexLocalLinkProvider', () => { // Ensure outside positions do not detect the link for (let i = 0; i < noLinkPositions.length; i++) { const link = await new Promise(r => provider.provideLink(noLinkPositions[i], r)); - assert.equal(link, undefined, `Just outside range boundary should not result in link, link found at (${link?.range.start.x}, ${link?.range.start.y}) to (${link?.range.end.x}, ${link?.range.end.y}) while checking (${noLinkPositions[i].x}, ${noLinkPositions[i].y})`); + assert.equal(link, undefined, `Just outside range boundary should not result in link, link found at (${link?.range.start.x}, ${link?.range.start.y}) to (${link?.range.end.x}, ${link?.range.end.y}) while checking (${noLinkPositions[i].x}, ${noLinkPositions[i].y})\nExpected link text=${expected.text}\nActual link text=${link?.text}`); } // Convert range from [[startx, starty], [endx, endy]] to an IBufferRange @@ -44,15 +99,40 @@ suite('Workbench - TerminalRegexLocalLinkProvider', () => { // Ensure inside positions do detect the link for (let i = 0; i < linkPositions.length; i++) { const link = await new Promise(r => provider.provideLink(linkPositions[i], r)); - - assert.ok(link); - assert.deepEqual(link!.text, expected.text); - assert.deepEqual(link!.range, linkRange); + assert.deepEqual(link?.text, expected.text); + assert.deepEqual(link?.range, linkRange); } } - // These tests are based on LinkComputer.test.ts - test('Simple unix paths', async () => { - await assertLink('"/foo"', { range: [[2, 1], [5, 1]], text: '/foo' }); + [{ name: 'Linux', os: OperatingSystem.Linux }, { name: 'macOS', os: OperatingSystem.Macintosh }].forEach(config => { + suite(config.name, () => { + unixLinks.forEach(baseLink => { + test(`Link "${baseLink}"`, async () => { + for (let i = 0; i < supportedLinkFormats.length; i++) { + const linkFormat = supportedLinkFormats[i]; + const formattedLink = format(linkFormat.urlFormat, baseLink, linkFormat.line, linkFormat.column); + await assertLink(formattedLink, config.os, { text: formattedLink, range: [[1, 1], [formattedLink.length, 1]] }); + await assertLink(` ${formattedLink} `, config.os, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); + await assertLink(`(${formattedLink})`, config.os, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); + await assertLink(`[${formattedLink}]`, config.os, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); + } + }); + }); + }); + }); + + suite('Windows', () => { + windowsLinks.forEach(baseLink => { + test(`Link "${baseLink}"`, async () => { + for (let i = 0; i < supportedLinkFormats.length; i++) { + const linkFormat = supportedLinkFormats[i]; + const formattedLink = format(linkFormat.urlFormat, baseLink, linkFormat.line, linkFormat.column); + await assertLink(formattedLink, OperatingSystem.Windows, { text: formattedLink, range: [[1, 1], [formattedLink.length, 1]] }); + await assertLink(` ${formattedLink} `, OperatingSystem.Windows, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); + await assertLink(`(${formattedLink})`, OperatingSystem.Windows, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); + await assertLink(`[${formattedLink}]`, OperatingSystem.Windows, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); + } + }); + }); }); }); diff --git a/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts index a19c55a60a6a6..58db9d4964316 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts @@ -42,10 +42,8 @@ suite('Workbench - TerminalWebLinkProvider', () => { // Ensure inside positions do detect the link for (let i = 0; i < linkPositions.length; i++) { const link = await new Promise(r => provider.provideLink(linkPositions[i], r)); - - assert.ok(link); - assert.deepEqual(link!.text, expected.text); - assert.deepEqual(link!.range, linkRange); + assert.deepEqual(link?.text, expected.text); + assert.deepEqual(link?.range, linkRange); } } From a354eb8b2283d37a6a1baa73ccccb73815740ec1 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 15:35:00 -0700 Subject: [PATCH 06/14] Rename to validated --- .../browser/links/terminalLinkManager.ts | 6 +++-- ... => terminalValidatedLocalLinkProvider.ts} | 22 +------------------ ...erminalValidatedLocalLinkProvider.test.ts} | 6 ++--- 3 files changed, 8 insertions(+), 26 deletions(-) rename src/vs/workbench/contrib/terminal/browser/links/{terminalRegexLocalLinkProvider.ts => terminalValidatedLocalLinkProvider.ts} (88%) rename src/vs/workbench/contrib/terminal/test/browser/links/{terminalRegexLocalLinkProvider.test.ts => terminalValidatedLocalLinkProvider.test.ts} (94%) diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts index 2e16150ca3435..ce2668f172469 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts @@ -22,7 +22,7 @@ import { IMarkdownString, MarkdownString } from 'vs/base/common/htmlContent'; import { Emitter, Event } from 'vs/base/common/event'; import { ILogService } from 'vs/platform/log/common/log'; import { TerminalWebLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider'; -import { TerminalRegexLocalLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider'; +import { TerminalValidatedLocalLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider'; const pathPrefix = '(\\.\\.?|\\~)'; const pathSeparatorClause = '\\/'; @@ -83,6 +83,7 @@ export class TerminalLinkManager extends DisposableStore { private _linkMatchers: number[] = []; private _webLinksAddon: ITerminalAddon | undefined; private _linkProvider: IDisposable | undefined; + private _linkProvider2: IDisposable | undefined; private _hasBeforeHandleLinkListeners = false; protected static _LINK_INTERCEPT_THRESHOLD = LINK_INTERCEPT_THRESHOLD; @@ -174,6 +175,7 @@ export class TerminalLinkManager extends DisposableStore { this.registerLinkProvider(); } else { this._linkProvider?.dispose(); + this._linkProvider2?.dispose(); this._registerLinkMatchers(); } } @@ -292,7 +294,7 @@ export class TerminalLinkManager extends DisposableStore { this._tooltipCallback(event, link, location, this._handleLocalLink.bind(this, link)); }; const wrappedLinkActivateCallback = this._wrapLinkHandler(this._handleLocalLink.bind(this)); - this._linkProvider = this._xterm.registerLinkProvider(new TerminalRegexLocalLinkProvider(this._xterm, this._processManager.os || OS, wrappedLinkActivateCallback, tooltipLinkCallback, this._leaveCallback, this._validateLocalLink.bind(this))); + this._linkProvider2 = this._xterm.registerLinkProvider(new TerminalValidatedLocalLinkProvider(this._xterm, this._processManager.os || OS, wrappedLinkActivateCallback, tooltipLinkCallback, this._leaveCallback, this._validateLocalLink.bind(this))); } protected _wrapLinkHandler(handler: (link: string) => void): XtermLinkMatcherHandler { diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts similarity index 88% rename from src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts rename to src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts index 37305aa6b8983..852e9e5da4d7a 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts @@ -33,14 +33,7 @@ const lineAndColumnClause = [ '(([^:\\s\\(\\)<>\'\"\\[\\]]*)(:(\\d+))?(:(\\d+))?)' // (file path):336, (file path):336:9 ].join('|').replace(/ /g, `[${'\u00A0'} ]`); -// Changing any regex may effect this value, hence changes this as well if required. -// const winLineAndColumnMatchIndex = 12; -// const unixLineAndColumnMatchIndex = 11; - -// Each line and column clause have 6 groups (ie no. of expressions in round brackets) -// const lineAndColumnClauseGroupCount = 6; - -export class TerminalRegexLocalLinkProvider implements ILinkProvider { +export class TerminalValidatedLocalLinkProvider implements ILinkProvider { constructor( private readonly _xterm: Terminal, private readonly _processOperatingSystem: OperatingSystem, @@ -139,17 +132,4 @@ export class TerminalRegexLocalLinkProvider implements ILinkProvider { // Append line and column number regex return new RegExp(`${baseLocalLinkClause}(${lineAndColumnClause})`); } - - // private _positionIsInRange(position: IBufferCellPosition, range: IBufferRange): boolean { - // if (position.y < range.start.y || position.y > range.end.y) { - // return false; - // } - // if (position.y === range.start.y && position.x < range.start.x) { - // return false; - // } - // if (position.y === range.end.y && position.x > range.end.x) { - // return false; - // } - // return true; - // } } diff --git a/src/vs/workbench/contrib/terminal/test/browser/links/terminalRegexLocalLinkProvider.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalValidatedLocalLinkProvider.test.ts similarity index 94% rename from src/vs/workbench/contrib/terminal/test/browser/links/terminalRegexLocalLinkProvider.test.ts rename to src/vs/workbench/contrib/terminal/test/browser/links/terminalValidatedLocalLinkProvider.test.ts index d84ac7762e0e0..68d60c0d6b950 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/links/terminalRegexLocalLinkProvider.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalValidatedLocalLinkProvider.test.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; -import { TerminalRegexLocalLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalRegexLocalLinkProvider'; +import { TerminalValidatedLocalLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider'; import { Terminal, ILink, IBufferRange, IBufferCellPosition } from 'xterm'; import { OperatingSystem } from 'vs/base/common/platform'; import { format } from 'vs/base/common/strings'; @@ -63,11 +63,11 @@ const supportedLinkFormats: LinkFormatInfo[] = [ { urlFormat: '{0}",{1}', line: '5' } ]; -suite('Workbench - TerminalRegexLocalLinkProvider', () => { +suite('Workbench - TerminalValidatedLocalLinkProvider', () => { async function assertLink(text: string, os: OperatingSystem, expected: { text: string, range: [number, number][] }) { const xterm = new Terminal(); // TODO: Test process manager - const provider = new TerminalRegexLocalLinkProvider(xterm, os, () => { }, () => { }, () => { }, (_, cb) => { cb(true); }); + const provider = new TerminalValidatedLocalLinkProvider(xterm, os, () => { }, () => { }, () => { }, (_, cb) => { cb(true); }); // Write the text and wait for the parser to finish await new Promise(r => xterm.write(text, r)); From cdcf6183ccb55823f8dc1ef1f62766891dfbd015 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 16:04:20 -0700 Subject: [PATCH 07/14] Create word link provider --- .../browser/links/terminalLinkHelpers.ts | 2 + .../browser/links/terminalLinkManager.ts | 25 ++- .../terminalValidatedLocalLinkProvider.ts | 4 +- .../browser/links/terminalWebLinkProvider.ts | 4 +- .../browser/links/terminalWordLinkProvider.ts | 180 ++++++++++++++++++ 5 files changed, 206 insertions(+), 9 deletions(-) create mode 100644 src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts index d04bcf16344ef..802737579e4e7 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts @@ -6,6 +6,8 @@ import { IViewportRange, IBufferRange, IBufferLine, IBuffer, IBufferCellPosition } from 'xterm'; import { IRange } from 'vs/editor/common/core/range'; +export const TOOLTIP_HOVER_THRESHOLD = 500; + export function convertLinkRangeToBuffer(lines: IBufferLine[], bufferWidth: number, range: IRange, startLine: number) { const bufferRange: IBufferRange = { start: { diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts index ce2668f172469..0f7be3ef124dd 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts @@ -23,6 +23,9 @@ import { Emitter, Event } from 'vs/base/common/event'; import { ILogService } from 'vs/platform/log/common/log'; import { TerminalWebLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider'; import { TerminalValidatedLocalLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider'; +import { TerminalWordLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { IQuickInputService } from 'vs/platform/quickinput/common/quickInput'; const pathPrefix = '(\\.\\.?|\\~)'; const pathSeparatorClause = '\\/'; @@ -84,6 +87,7 @@ export class TerminalLinkManager extends DisposableStore { private _webLinksAddon: ITerminalAddon | undefined; private _linkProvider: IDisposable | undefined; private _linkProvider2: IDisposable | undefined; + private _linkProvider3: IDisposable | undefined; private _hasBeforeHandleLinkListeners = false; protected static _LINK_INTERCEPT_THRESHOLD = LINK_INTERCEPT_THRESHOLD; @@ -109,7 +113,9 @@ export class TerminalLinkManager extends DisposableStore { @IConfigurationService private readonly _configurationService: IConfigurationService, @ITerminalInstanceService private readonly _terminalInstanceService: ITerminalInstanceService, @IFileService private readonly _fileService: IFileService, - @ILogService private readonly _logService: ILogService + @ILogService private readonly _logService: ILogService, + @IInstantiationService private readonly _instantiationService: IInstantiationService, + @IQuickInputService private readonly _quickInputService: IQuickInputService ) { super(); @@ -176,6 +182,7 @@ export class TerminalLinkManager extends DisposableStore { } else { this._linkProvider?.dispose(); this._linkProvider2?.dispose(); + this._linkProvider3?.dispose(); this._registerLinkMatchers(); } } @@ -283,18 +290,26 @@ export class TerminalLinkManager extends DisposableStore { public registerLinkProvider(): void { // Web links - const tooltipCallback = (event: MouseEvent, link: string, location: IViewportRange) => { + const tooltipWebCallback = (event: MouseEvent, link: string, location: IViewportRange) => { this._tooltipCallback(event, link, location, this._handleHypertextLink.bind(this, link)); }; const wrappedActivateCallback = this._wrapLinkHandler(this._handleHypertextLink.bind(this)); - this._linkProvider = this._xterm.registerLinkProvider(new TerminalWebLinkProvider(this._xterm, wrappedActivateCallback, tooltipCallback, this._leaveCallback)); + this._linkProvider = this._xterm.registerLinkProvider(new TerminalWebLinkProvider(this._xterm, wrappedActivateCallback, tooltipWebCallback, this._leaveCallback)); // Local links - const tooltipLinkCallback = (event: MouseEvent, link: string, location: IViewportRange) => { + const tooltipValidatedLocalCallback = (event: MouseEvent, link: string, location: IViewportRange) => { this._tooltipCallback(event, link, location, this._handleLocalLink.bind(this, link)); }; const wrappedLinkActivateCallback = this._wrapLinkHandler(this._handleLocalLink.bind(this)); - this._linkProvider2 = this._xterm.registerLinkProvider(new TerminalValidatedLocalLinkProvider(this._xterm, this._processManager.os || OS, wrappedLinkActivateCallback, tooltipLinkCallback, this._leaveCallback, this._validateLocalLink.bind(this))); + this._linkProvider2 = this._xterm.registerLinkProvider(new TerminalValidatedLocalLinkProvider(this._xterm, this._processManager.os || OS, wrappedLinkActivateCallback, tooltipValidatedLocalCallback, this._leaveCallback, this._validateLocalLink.bind(this))); + + const tooltipWordCallback = (event: MouseEvent, link: string, location: IViewportRange) => { + this._tooltipCallback(event, link, location, link => this._quickInputService.quickAccess.show(link)); + }; + const wrappedWordActivateCallback = this._wrapLinkHandler(link => { + this._quickInputService.quickAccess.show(link); + }); + this._linkProvider3 = this._xterm.registerLinkProvider(this._instantiationService.createInstance(TerminalWordLinkProvider, this._xterm, wrappedWordActivateCallback, tooltipWordCallback, this._leaveCallback)); } protected _wrapLinkHandler(handler: (link: string) => void): XtermLinkMatcherHandler { diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts index 852e9e5da4d7a..dc0c4c37ab3cf 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Terminal, ILinkProvider, IViewportRange, IBufferCellPosition, ILink, IBufferLine } from 'xterm'; -import { getXtermLineContent, convertLinkRangeToBuffer, convertBufferRangeToViewport, positionIsInRange } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers'; +import { getXtermLineContent, convertLinkRangeToBuffer, convertBufferRangeToViewport, positionIsInRange, TOOLTIP_HOVER_THRESHOLD } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers'; import { OperatingSystem } from 'vs/base/common/platform'; const pathPrefix = '(\\.\\.?|\\~)'; @@ -109,7 +109,7 @@ export class TerminalValidatedLocalLinkProvider implements ILinkProvider { // TODO: This tooltip timer is currently not totally reliable setTimeout(() => { this._tooltipCallback(event, text, convertBufferRangeToViewport(bufferRange, this._xterm.buffer.active.viewportY)); - }, 200); + }, TOOLTIP_HOVER_THRESHOLD); }, leave: () => this._leaveCallback() }); diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts index 26faef0b97e68..35818d931b88d 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts @@ -5,7 +5,7 @@ import { Terminal, IViewportRange, ILinkProvider, IBufferCellPosition, ILink, IBufferLine } from 'xterm'; import { ILinkComputerTarget, LinkComputer } from 'vs/editor/common/modes/linkComputer'; -import { getXtermLineContent, convertLinkRangeToBuffer, convertBufferRangeToViewport, positionIsInRange } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers'; +import { getXtermLineContent, convertLinkRangeToBuffer, convertBufferRangeToViewport, positionIsInRange, TOOLTIP_HOVER_THRESHOLD } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers'; export class TerminalWebLinkProvider implements ILinkProvider { private _linkComputerTarget: ILinkComputerTarget | undefined; @@ -55,7 +55,7 @@ export class TerminalWebLinkProvider implements ILinkProvider { // TODO: This tooltip timer is currently not totally reliable setTimeout(() => { this._tooltipCallback(event, text, convertBufferRangeToViewport(range, this._xterm.buffer.active.viewportY)); - }, 200); + }, TOOLTIP_HOVER_THRESHOLD); }, leave: () => this._leaveCallback() }); diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts new file mode 100644 index 0000000000000..bbd0cfc580968 --- /dev/null +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts @@ -0,0 +1,180 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Terminal, ILinkProvider, IViewportRange, IBufferCellPosition, ILink } from 'xterm'; +import { convertBufferRangeToViewport, TOOLTIP_HOVER_THRESHOLD } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers'; +import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { ITerminalConfiguration, TERMINAL_CONFIG_SECTION } from 'vs/workbench/contrib/terminal/common/terminal'; + +export class TerminalWordLinkProvider implements ILinkProvider { + constructor( + private readonly _xterm: Terminal, + private readonly _activateCallback: (event: MouseEvent, uri: string) => void, + private readonly _tooltipCallback: (event: MouseEvent, uri: string, location: IViewportRange) => boolean | void, + private readonly _leaveCallback: () => void, + @IConfigurationService private readonly _configurationService: IConfigurationService + ) { + } + + public provideLink(position: IBufferCellPosition, callback: (link: ILink | undefined) => void): void { + const start: IBufferCellPosition = { x: position.x, y: position.y }; + const end: IBufferCellPosition = { x: position.x, y: position.y }; + + // let startLine = position.y - 1; + // let endLine = startLine; + + // const lines: IBufferLine[] = [ + // this._xterm.buffer.active.getLine(startLine)! + // ]; + + // while (this._xterm.buffer.active.getLine(startLine)?.isWrapped) { + // lines.unshift(this._xterm.buffer.active.getLine(startLine - 1)!); + // startLine--; + // } + + // while (this._xterm.buffer.active.getLine(endLine + 1)?.isWrapped) { + // lines.push(this._xterm.buffer.active.getLine(endLine + 1)!); + // endLine++; + // } + + // TODO: Support wrapping + + // Expand to the left until a word separator is hit + const line = this._xterm.buffer.active.getLine(position.y - 1)!; + let text = ''; + start.x++; // The hovered cell is considered first + for (let x = position.x; x > 0; x--) { + const char = line.getCell(x - 1)?.getChars(); + if (!char) { + break; + } + const config = this._configurationService.getValue(TERMINAL_CONFIG_SECTION); + if (config.wordSeparators.indexOf(char) >= 0) { + break; + } + start.x--; + text = char + text; + } + + // Expand to the right until a word separator is hit + // end.x++; // The hovered cell is considered first + for (let x = position.x + 1; x < line.length; x++) { + const char = line.getCell(x - 1)?.getChars(); + if (!char) { + break; + } + const config = this._configurationService.getValue(TERMINAL_CONFIG_SECTION); + if (config.wordSeparators.indexOf(char) >= 0) { + break; + } + end.x++; + text += char; + } + + // TODO: Only show word links when modifier is down? + // TODO: Only show tooltip if no mouse movement has happened; copy how editor works + + // No links were found (the hovered cell is whitespace) + if (text.length === 0) { + callback(undefined); + return; + } + + const range = { start, end }; + callback({ + text, + range, + activate: (event: MouseEvent, text: string) => this._activateCallback(event, text), + hover: (event: MouseEvent, text: string) => { + // TODO: This tooltip timer is currently not totally reliable + setTimeout(() => { + this._tooltipCallback(event, text, convertBufferRangeToViewport(range, this._xterm.buffer.active.viewportY)); + }, TOOLTIP_HOVER_THRESHOLD); + }, + leave: () => this._leaveCallback() + }); + + // let startLine = position.y - 1; + // let endLine = startLine; + + // const lines: IBufferLine[] = [ + // this._xterm.buffer.active.getLine(startLine)! + // ]; + + // while (this._xterm.buffer.active.getLine(startLine)?.isWrapped) { + // lines.unshift(this._xterm.buffer.active.getLine(startLine - 1)!); + // startLine--; + // } + + // while (this._xterm.buffer.active.getLine(endLine + 1)?.isWrapped) { + // lines.push(this._xterm.buffer.active.getLine(endLine + 1)!); + // endLine++; + // } + + // const text = getXtermLineContent(this._xterm.buffer.active, startLine, endLine); + + // clone regex to do a global search on text + // const rex = new RegExp(this._localLinkRegex, 'g'); + // let match; + // let stringIndex = -1; + // while ((match = rex.exec(text)) !== null) { + // // const uri = match[typeof matcher.matchIndex !== 'number' ? 0 : matcher.matchIndex]; + // const uri = match[0]; + // if (!uri) { + // // something matched but does not comply with the given matchIndex + // // since this is most likely a bug the regex itself we simply do nothing here + // // this._logService.debug('match found without corresponding matchIndex', match, matcher); + // break; + // } + + // // Get index, match.index is for the outer match which includes negated chars + // // therefore we cannot use match.index directly, instead we search the position + // // of the match group in text again + // // also correct regex and string search offsets for the next loop run + // stringIndex = text.indexOf(uri, stringIndex + 1); + // rex.lastIndex = stringIndex + uri.length; + // if (stringIndex < 0) { + // // invalid stringIndex (should not have happened) + // break; + // } + + // // Convert the link text's string index into a wrapped buffer range + // const bufferRange = convertLinkRangeToBuffer(lines, this._xterm.cols, { + // startColumn: stringIndex + 1, + // startLineNumber: 1, + // endColumn: stringIndex + uri.length + 1, + // endLineNumber: 1 + // }, startLine); + + // if (positionIsInRange(position, bufferRange)) { + // this._validationCallback(uri, isValid => { + // // TODO: Discard if buffers have changes or if another link was added for this line + // if (isValid) { + // callback({ + // text: uri, + // range: bufferRange, + // activate: (event: MouseEvent, text: string) => this._activateCallback(event, text), + // hover: (event: MouseEvent, text: string) => { + // // TODO: This tooltip timer is currently not totally reliable + // setTimeout(() => { + // this._tooltipCallback(event, text, convertBufferRangeToViewport(bufferRange, this._xterm.buffer.active.viewportY)); + // }, 200); + // }, + // leave: () => this._leaveCallback() + // }); + // } else { + // // TODO: Support multiple matches from the regexes + // callback(undefined); + // } + // }); + // } else { + // callback(undefined); + // } + // return; + // } + + // callback(undefined); + } +} From 411d4a136fe4260a08f406284e4a0ae932742d0f Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 16:06:29 -0700 Subject: [PATCH 08/14] Fix compile --- .../browser/links/terminalLinkManager.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkManager.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkManager.test.ts index 1ef7431d9f0bd..ee739f8b562fd 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkManager.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkManager.test.ts @@ -89,7 +89,7 @@ suite('Workbench - TerminalLinkHandler', () => { const terminalLinkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Windows, userHome: '' - } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); + } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!, null!, null!); function testLink(link: string, linkUrl: string, lineNo?: string, columnNo?: string) { assert.equal(terminalLinkHandler.extractLinkUrl(link), linkUrl); assert.equal(terminalLinkHandler.extractLinkUrl(`:${link}:`), linkUrl); @@ -165,7 +165,7 @@ suite('Workbench - TerminalLinkHandler', () => { const terminalLinkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Linux, userHome: '' - } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); + } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!, null!, null!); function testLink(link: string, linkUrl: string, lineNo?: string, columnNo?: string) { assert.equal(terminalLinkHandler.extractLinkUrl(link), linkUrl); assert.equal(terminalLinkHandler.extractLinkUrl(`:${link}:`), linkUrl); @@ -233,7 +233,7 @@ suite('Workbench - TerminalLinkHandler', () => { const linkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Windows, userHome: 'C:\\Users\\Me' - } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); + } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!, null!, null!); linkHandler.processCwd = 'C:\\base'; assert.equal(linkHandler.preprocessPath('./src/file1'), 'C:\\base\\src\\file1'); @@ -246,7 +246,7 @@ suite('Workbench - TerminalLinkHandler', () => { const linkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Windows, userHome: 'C:\\Users\\M e' - } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); + } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!, null!, null!); linkHandler.processCwd = 'C:\\base dir'; assert.equal(linkHandler.preprocessPath('./src/file1'), 'C:\\base dir\\src\\file1'); @@ -260,7 +260,7 @@ suite('Workbench - TerminalLinkHandler', () => { const linkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Linux, userHome: '/home/me' - } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); + } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!, null!, null!); linkHandler.processCwd = '/base'; assert.equal(linkHandler.preprocessPath('./src/file1'), '/base/src/file1'); @@ -273,7 +273,7 @@ suite('Workbench - TerminalLinkHandler', () => { const linkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Linux, userHome: '/home/me' - } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); + } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!, null!, null!); assert.equal(linkHandler.preprocessPath('./src/file1'), null); assert.equal(linkHandler.preprocessPath('src/file2'), null); @@ -287,7 +287,7 @@ suite('Workbench - TerminalLinkHandler', () => { const linkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Linux, userHome: '' - } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!); + } as any, testConfigHelper, null!, null!, null!, new MockTerminalInstanceService(), null!, null!, null!, null!); function assertAreGoodMatches(matches: RegExpMatchArray | null) { if (matches) { @@ -318,7 +318,7 @@ suite('Workbench - TerminalLinkHandler', () => { const linkHandler = new TestTerminalLinkManager(new TestXterm() as any, { os: OperatingSystem.Linux, userHome: '' - } as any, testConfigHelper, null!, null!, new TestConfigurationService(), new MockTerminalInstanceService(), null!, null!); + } as any, testConfigHelper, null!, null!, new TestConfigurationService(), new MockTerminalInstanceService(), null!, null!, null!, null!); linkHandler.onBeforeHandleLink(e => { if (e.link === 'https://www.microsoft.com') { intercepted = true; From 45cbea0e635e631c3cd6fd58a0deb233efdfd6ce Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 16:24:10 -0700 Subject: [PATCH 09/14] Add tests for word link provider --- .../browser/links/terminalWordLinkProvider.ts | 110 +----------------- .../links/terminalWebLinkProvider.test.ts | 2 +- .../links/terminalWordLinkProvider.test.ts | 78 +++++++++++++ 3 files changed, 85 insertions(+), 105 deletions(-) create mode 100644 src/vs/workbench/contrib/terminal/test/browser/links/terminalWordLinkProvider.test.ts diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts index bbd0cfc580968..9f6b8960b27ae 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts @@ -22,23 +22,6 @@ export class TerminalWordLinkProvider implements ILinkProvider { const start: IBufferCellPosition = { x: position.x, y: position.y }; const end: IBufferCellPosition = { x: position.x, y: position.y }; - // let startLine = position.y - 1; - // let endLine = startLine; - - // const lines: IBufferLine[] = [ - // this._xterm.buffer.active.getLine(startLine)! - // ]; - - // while (this._xterm.buffer.active.getLine(startLine)?.isWrapped) { - // lines.unshift(this._xterm.buffer.active.getLine(startLine - 1)!); - // startLine--; - // } - - // while (this._xterm.buffer.active.getLine(endLine + 1)?.isWrapped) { - // lines.push(this._xterm.buffer.active.getLine(endLine + 1)!); - // endLine++; - // } - // TODO: Support wrapping // Expand to the left until a word separator is hit @@ -58,6 +41,12 @@ export class TerminalWordLinkProvider implements ILinkProvider { text = char + text; } + // No links were found (the hovered cell is whitespace) + if (text.length === 0) { + callback(undefined); + return; + } + // Expand to the right until a word separator is hit // end.x++; // The hovered cell is considered first for (let x = position.x + 1; x < line.length; x++) { @@ -76,12 +65,6 @@ export class TerminalWordLinkProvider implements ILinkProvider { // TODO: Only show word links when modifier is down? // TODO: Only show tooltip if no mouse movement has happened; copy how editor works - // No links were found (the hovered cell is whitespace) - if (text.length === 0) { - callback(undefined); - return; - } - const range = { start, end }; callback({ text, @@ -95,86 +78,5 @@ export class TerminalWordLinkProvider implements ILinkProvider { }, leave: () => this._leaveCallback() }); - - // let startLine = position.y - 1; - // let endLine = startLine; - - // const lines: IBufferLine[] = [ - // this._xterm.buffer.active.getLine(startLine)! - // ]; - - // while (this._xterm.buffer.active.getLine(startLine)?.isWrapped) { - // lines.unshift(this._xterm.buffer.active.getLine(startLine - 1)!); - // startLine--; - // } - - // while (this._xterm.buffer.active.getLine(endLine + 1)?.isWrapped) { - // lines.push(this._xterm.buffer.active.getLine(endLine + 1)!); - // endLine++; - // } - - // const text = getXtermLineContent(this._xterm.buffer.active, startLine, endLine); - - // clone regex to do a global search on text - // const rex = new RegExp(this._localLinkRegex, 'g'); - // let match; - // let stringIndex = -1; - // while ((match = rex.exec(text)) !== null) { - // // const uri = match[typeof matcher.matchIndex !== 'number' ? 0 : matcher.matchIndex]; - // const uri = match[0]; - // if (!uri) { - // // something matched but does not comply with the given matchIndex - // // since this is most likely a bug the regex itself we simply do nothing here - // // this._logService.debug('match found without corresponding matchIndex', match, matcher); - // break; - // } - - // // Get index, match.index is for the outer match which includes negated chars - // // therefore we cannot use match.index directly, instead we search the position - // // of the match group in text again - // // also correct regex and string search offsets for the next loop run - // stringIndex = text.indexOf(uri, stringIndex + 1); - // rex.lastIndex = stringIndex + uri.length; - // if (stringIndex < 0) { - // // invalid stringIndex (should not have happened) - // break; - // } - - // // Convert the link text's string index into a wrapped buffer range - // const bufferRange = convertLinkRangeToBuffer(lines, this._xterm.cols, { - // startColumn: stringIndex + 1, - // startLineNumber: 1, - // endColumn: stringIndex + uri.length + 1, - // endLineNumber: 1 - // }, startLine); - - // if (positionIsInRange(position, bufferRange)) { - // this._validationCallback(uri, isValid => { - // // TODO: Discard if buffers have changes or if another link was added for this line - // if (isValid) { - // callback({ - // text: uri, - // range: bufferRange, - // activate: (event: MouseEvent, text: string) => this._activateCallback(event, text), - // hover: (event: MouseEvent, text: string) => { - // // TODO: This tooltip timer is currently not totally reliable - // setTimeout(() => { - // this._tooltipCallback(event, text, convertBufferRangeToViewport(bufferRange, this._xterm.buffer.active.viewportY)); - // }, 200); - // }, - // leave: () => this._leaveCallback() - // }); - // } else { - // // TODO: Support multiple matches from the regexes - // callback(undefined); - // } - // }); - // } else { - // callback(undefined); - // } - // return; - // } - - // callback(undefined); } } diff --git a/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts index 58db9d4964316..15e562586621e 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalWebLinkProvider.test.ts @@ -24,7 +24,7 @@ suite('Workbench - TerminalWebLinkProvider', () => { // Ensure outside positions do not detect the link for (let i = 0; i < noLinkPositions.length; i++) { const link = await new Promise(r => provider.provideLink(noLinkPositions[i], r)); - assert.equal(link, undefined, `Just outside range boundary should not result in link, link found at: (${link?.range.start.x}, ${link?.range.start.y}) to (${link?.range.end.x}, ${link?.range.end.y})`); + assert.equal(link, undefined, `Just outside range boundary should not result in link, link found at (${link?.range.start.x}, ${link?.range.start.y}) to (${link?.range.end.x}, ${link?.range.end.y}) while checking (${noLinkPositions[i].x}, ${noLinkPositions[i].y})\nExpected link text=${expected.text}\nActual link text=${link?.text}`); } // Convert range from [[startx, starty], [endx, endy]] to an IBufferRange diff --git a/src/vs/workbench/contrib/terminal/test/browser/links/terminalWordLinkProvider.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalWordLinkProvider.test.ts new file mode 100644 index 0000000000000..1c137d3c8ae99 --- /dev/null +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalWordLinkProvider.test.ts @@ -0,0 +1,78 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { Terminal, ILink, IBufferRange, IBufferCellPosition } from 'xterm'; +import { TerminalWordLinkProvider } from 'vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider'; +import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; +import { TestConfigurationService } from 'vs/platform/configuration/test/common/testConfigurationService'; +import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; + +suite('Workbench - TerminalWordLinkProvider', () => { + + let instantiationService: TestInstantiationService; + let configurationService: TestConfigurationService; + + setup(() => { + instantiationService = new TestInstantiationService(); + configurationService = new TestConfigurationService(); + instantiationService.stub(IConfigurationService, configurationService); + }); + + async function assertLink(text: string, expected: { text: string, range: [number, number][] }) { + const xterm = new Terminal(); + const provider = instantiationService.createInstance(TerminalWordLinkProvider, xterm, () => { }, () => { }, () => { }); + + // Write the text and wait for the parser to finish + await new Promise(r => xterm.write(text, r)); + + // Calculate positions just outside of link boundaries + const noLinkPositions: IBufferCellPosition[] = [ + { x: expected.range[0][0] - 1, y: expected.range[0][1] }, + { x: expected.range[1][0] + 1, y: expected.range[1][1] } + ]; + + // Ensure outside positions do not detect the link + for (let i = 0; i < noLinkPositions.length; i++) { + const link = await new Promise(r => provider.provideLink(noLinkPositions[i], r)); + assert.equal(link, undefined, `Just outside range boundary should not result in link, link found at (${link?.range.start.x}, ${link?.range.start.y}) to (${link?.range.end.x}, ${link?.range.end.y}) while checking (${noLinkPositions[i].x}, ${noLinkPositions[i].y})\nExpected link text=${expected.text}\nActual link text=${link?.text}`); + } + + // Convert range from [[startx, starty], [endx, endy]] to an IBufferRange + const linkRange: IBufferRange = { + start: { x: expected.range[0][0], y: expected.range[0][1] }, + end: { x: expected.range[1][0], y: expected.range[1][1] }, + }; + + // Calculate positions inside the link boundaries + const linkPositions: IBufferCellPosition[] = [ + linkRange.start, + linkRange.end + ]; + + // Ensure inside positions do detect the link + for (let i = 0; i < linkPositions.length; i++) { + const link = await new Promise(r => provider.provideLink(linkPositions[i], r)); + assert.deepEqual(link?.text, expected.text); + assert.deepEqual(link?.range, linkRange); + } + } + + test('should link words as defined by wordSeparators', async () => { + await configurationService.setUserConfiguration('terminal', { integrated: { wordSeparators: ' ()[]' } }); + await assertLink('foo', { range: [[1, 1], [3, 1]], text: 'foo' }); + await assertLink(' foo ', { range: [[2, 1], [4, 1]], text: 'foo' }); + await assertLink('(foo)', { range: [[2, 1], [4, 1]], text: 'foo' }); + await assertLink('[foo]', { range: [[2, 1], [4, 1]], text: 'foo' }); + await assertLink('{foo}', { range: [[1, 1], [5, 1]], text: '{foo}' }); + + await configurationService.setUserConfiguration('terminal', { integrated: { wordSeparators: ' ' } }); + await assertLink('foo', { range: [[1, 1], [3, 1]], text: 'foo' }); + await assertLink(' foo ', { range: [[2, 1], [4, 1]], text: 'foo' }); + await assertLink('(foo)', { range: [[1, 1], [5, 1]], text: '(foo)' }); + await assertLink('[foo]', { range: [[1, 1], [5, 1]], text: '[foo]' }); + await assertLink('{foo}', { range: [[1, 1], [5, 1]], text: '{foo}' }); + }); +}); From 0bf79935bf015a0261661618e5d9742f6f07d4e5 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 16:28:16 -0700 Subject: [PATCH 10/14] Improve behavior of tooltips --- .../links/terminalValidatedLocalLinkProvider.ts | 11 ++++++++--- .../terminal/browser/links/terminalWebLinkProvider.ts | 11 ++++++++--- .../browser/links/terminalWordLinkProvider.ts | 11 ++++++++--- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts index dc0c4c37ab3cf..34919cd12cd7c 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts @@ -101,17 +101,22 @@ export class TerminalValidatedLocalLinkProvider implements ILinkProvider { this._validationCallback(uri, isValid => { // TODO: Discard if buffers have changes or if another link was added for this line if (isValid) { + let timeout: number | undefined; callback({ text: uri, range: bufferRange, activate: (event: MouseEvent, text: string) => this._activateCallback(event, text), hover: (event: MouseEvent, text: string) => { - // TODO: This tooltip timer is currently not totally reliable - setTimeout(() => { + timeout = window.setTimeout(() => { this._tooltipCallback(event, text, convertBufferRangeToViewport(bufferRange, this._xterm.buffer.active.viewportY)); }, TOOLTIP_HOVER_THRESHOLD); }, - leave: () => this._leaveCallback() + leave: () => { + if (timeout !== undefined) { + window.clearTimeout(timeout); + } + this._leaveCallback(); + } }); } else { // TODO: Support multiple matches from the regexes diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts index 35818d931b88d..8d0e9909832e3 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalWebLinkProvider.ts @@ -47,17 +47,22 @@ export class TerminalWebLinkProvider implements ILinkProvider { if (positionIsInRange(position, range)) { found = true; + let timeout: number | undefined; callback({ text: link.url?.toString() || '', range, activate: (event: MouseEvent, text: string) => this._activateCallback(event, text), hover: (event: MouseEvent, text: string) => { - // TODO: This tooltip timer is currently not totally reliable - setTimeout(() => { + timeout = window.setTimeout(() => { this._tooltipCallback(event, text, convertBufferRangeToViewport(range, this._xterm.buffer.active.viewportY)); }, TOOLTIP_HOVER_THRESHOLD); }, - leave: () => this._leaveCallback() + leave: () => { + if (timeout !== undefined) { + window.clearTimeout(timeout); + } + this._leaveCallback(); + } }); } }); diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts index 9f6b8960b27ae..0d71a56e5330d 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts @@ -66,17 +66,22 @@ export class TerminalWordLinkProvider implements ILinkProvider { // TODO: Only show tooltip if no mouse movement has happened; copy how editor works const range = { start, end }; + let timeout: number | undefined; callback({ text, range, activate: (event: MouseEvent, text: string) => this._activateCallback(event, text), hover: (event: MouseEvent, text: string) => { - // TODO: This tooltip timer is currently not totally reliable - setTimeout(() => { + timeout = window.setTimeout(() => { this._tooltipCallback(event, text, convertBufferRangeToViewport(range, this._xterm.buffer.active.viewportY)); }, TOOLTIP_HOVER_THRESHOLD); }, - leave: () => this._leaveCallback() + leave: () => { + if (timeout !== undefined) { + window.clearTimeout(timeout); + } + this._leaveCallback(); + } }); } } From a7b5a7f1d9ce8f4be57d7d362e547f07c16885f9 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 17:05:00 -0700 Subject: [PATCH 11/14] Support emoji in links --- .../browser/links/terminalLinkHelpers.ts | 7 ++- .../browser/links/terminalLinkHelpers.test.ts | 45 +++++++++++++++---- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts index 802737579e4e7..ebc5e97af7eab 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers.ts @@ -28,10 +28,15 @@ export function convertLinkRangeToBuffer(lines: IBufferLine[], bufferWidth: numb let lineOffset = 0; const line = lines[y]; for (let x = 0; x < Math.min(bufferWidth, lineLength + lineOffset); x++) { - const width = line.getCell(x)?.getWidth(); + const cell = line.getCell(x)!; + const width = cell.getWidth(); if (width === 2) { lineOffset++; } + const char = cell.getChars(); + if (char.length > 1) { + lineOffset -= char.length - 1; + } } startOffset += lineOffset; } diff --git a/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHelpers.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHelpers.test.ts index dcad739385fb8..af67b867c1a7b 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHelpers.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkHelpers.test.ts @@ -31,6 +31,17 @@ suite('Workbench - Terminal Link Helpers', () => { end: { x: 7 + 1, y: 2 } }); }); + test('should convert ranges for combining characters before the link', () => { + const lines = createBufferLineArray([ + { text: 'A🙂 http://', width: 11 }, + { text: 't.com/f/', width: 9 } + ]); + const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 4 + 1, startLineNumber: 1, endColumn: 19 + 1, endLineNumber: 1 }, 0); + assert.deepEqual(bufferRange, { + start: { x: 4, y: 1 }, + end: { x: 7, y: 2 } + }); + }); test('should convert ranges for wide characters inside the link', () => { const lines = createBufferLineArray([ { text: 'AA http://t', width: 11 }, @@ -53,6 +64,17 @@ suite('Workbench - Terminal Link Helpers', () => { end: { x: 7 + 2, y: 2 } }); }); + test('should convert ranges for emoji before before and wide inside the link', () => { + const lines = createBufferLineArray([ + { text: 'A🙂 http://', width: 11 }, + { text: 't.com/文/', width: 9 } + ]); + const bufferRange = convertLinkRangeToBuffer(lines, 11, { startColumn: 4 + 1, startLineNumber: 1, endColumn: 19 + 1, endLineNumber: 1 }, 0); + assert.deepEqual(bufferRange, { + start: { x: 4, y: 1 }, + end: { x: 7 + 1, y: 2 } + }); + }); test('should convert ranges for ascii characters (link starts on wrapped)', () => { const lines = createBufferLineArray([ { text: 'AAAAAAAAAAA', width: 11 }, @@ -156,23 +178,28 @@ class TestBufferLine implements IBufferLine { } getCell(x: number): IBufferCell | undefined { // Create a fake line of cells and use that to resolve the width - let cells: string = ''; - let offset = 0; - for (let i = 0; i <= x - offset; i++) { - const char = this._text.charAt(i); - cells += char; + let cells: string[] = []; + let wideNullCellOffset = 0; // There is no null 0 width char after a wide char + let emojiOffset = 0; // Skip chars as emoji are multiple characters + for (let i = 0; i <= x - wideNullCellOffset + emojiOffset; i++) { + let char = this._text.charAt(i); + if (char === '\ud83d') { + // Make "🙂" + char += '\ude42'; + } + cells.push(char); if (this._text.charAt(i) === TEST_WIDE_CHAR) { // Skip the next character as it's width is 0 - cells += TEST_NULL_CHAR; - offset++; + cells.push(TEST_NULL_CHAR); + wideNullCellOffset++; } } return { getChars: () => { - return x >= cells.length ? '' : cells.charAt(x); + return x >= cells.length ? '' : cells[x]; }, getWidth: () => { - switch (cells.charAt(x)) { + switch (cells[x]) { case TEST_WIDE_CHAR: return 2; case TEST_NULL_CHAR: return 0; default: return 1; From 65f89d0973be825fe82cdd9f72d4bb38c856ddb0 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 18:37:34 -0700 Subject: [PATCH 12/14] Polish PR --- .../browser/links/terminalLinkManager.ts | 30 ++++++++++--------- .../terminalValidatedLocalLinkProvider.ts | 2 -- .../browser/links/terminalWordLinkProvider.ts | 3 -- ...terminalValidatedLocalLinkProvider.test.ts | 1 - 4 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts index 0f7be3ef124dd..8d33aebf5ed87 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalLinkManager.ts @@ -5,7 +5,7 @@ import * as nls from 'vs/nls'; import { URI } from 'vs/base/common/uri'; -import { DisposableStore, IDisposable } from 'vs/base/common/lifecycle'; +import { DisposableStore, IDisposable, dispose } from 'vs/base/common/lifecycle'; import { IOpenerService } from 'vs/platform/opener/common/opener'; import { TerminalWidgetManager, WidgetVerticalAlignment } from 'vs/workbench/contrib/terminal/browser/terminalWidgetManager'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; @@ -85,9 +85,7 @@ export class TerminalLinkManager extends DisposableStore { private readonly _leaveCallback: () => void; private _linkMatchers: number[] = []; private _webLinksAddon: ITerminalAddon | undefined; - private _linkProvider: IDisposable | undefined; - private _linkProvider2: IDisposable | undefined; - private _linkProvider3: IDisposable | undefined; + private _linkProviders: IDisposable[] = []; private _hasBeforeHandleLinkListeners = false; protected static _LINK_INTERCEPT_THRESHOLD = LINK_INTERCEPT_THRESHOLD; @@ -180,9 +178,8 @@ export class TerminalLinkManager extends DisposableStore { this._deregisterLinkMatchers(); this.registerLinkProvider(); } else { - this._linkProvider?.dispose(); - this._linkProvider2?.dispose(); - this._linkProvider3?.dispose(); + dispose(this._linkProviders); + this._linkProviders.length = 0; this._registerLinkMatchers(); } } @@ -294,22 +291,27 @@ export class TerminalLinkManager extends DisposableStore { this._tooltipCallback(event, link, location, this._handleHypertextLink.bind(this, link)); }; const wrappedActivateCallback = this._wrapLinkHandler(this._handleHypertextLink.bind(this)); - this._linkProvider = this._xterm.registerLinkProvider(new TerminalWebLinkProvider(this._xterm, wrappedActivateCallback, tooltipWebCallback, this._leaveCallback)); + this._linkProviders.push(this._xterm.registerLinkProvider( + new TerminalWebLinkProvider(this._xterm, wrappedActivateCallback, tooltipWebCallback, this._leaveCallback) + )); - // Local links + // Validated local links const tooltipValidatedLocalCallback = (event: MouseEvent, link: string, location: IViewportRange) => { this._tooltipCallback(event, link, location, this._handleLocalLink.bind(this, link)); }; const wrappedLinkActivateCallback = this._wrapLinkHandler(this._handleLocalLink.bind(this)); - this._linkProvider2 = this._xterm.registerLinkProvider(new TerminalValidatedLocalLinkProvider(this._xterm, this._processManager.os || OS, wrappedLinkActivateCallback, tooltipValidatedLocalCallback, this._leaveCallback, this._validateLocalLink.bind(this))); + this._linkProviders.push(this._xterm.registerLinkProvider( + new TerminalValidatedLocalLinkProvider(this._xterm, this._processManager.os || OS, wrappedLinkActivateCallback, tooltipValidatedLocalCallback, this._leaveCallback, this._validateLocalLink.bind(this)) + )); + // Word links const tooltipWordCallback = (event: MouseEvent, link: string, location: IViewportRange) => { this._tooltipCallback(event, link, location, link => this._quickInputService.quickAccess.show(link)); }; - const wrappedWordActivateCallback = this._wrapLinkHandler(link => { - this._quickInputService.quickAccess.show(link); - }); - this._linkProvider3 = this._xterm.registerLinkProvider(this._instantiationService.createInstance(TerminalWordLinkProvider, this._xterm, wrappedWordActivateCallback, tooltipWordCallback, this._leaveCallback)); + const wrappedWordActivateCallback = this._wrapLinkHandler(link => this._quickInputService.quickAccess.show(link)); + this._linkProviders.push(this._xterm.registerLinkProvider( + this._instantiationService.createInstance(TerminalWordLinkProvider, this._xterm, wrappedWordActivateCallback, tooltipWordCallback, this._leaveCallback) + )); } protected _wrapLinkHandler(handler: (link: string) => void): XtermLinkMatcherHandler { diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts index 34919cd12cd7c..21f83756038fd 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalValidatedLocalLinkProvider.ts @@ -99,7 +99,6 @@ export class TerminalValidatedLocalLinkProvider implements ILinkProvider { if (positionIsInRange(position, bufferRange)) { this._validationCallback(uri, isValid => { - // TODO: Discard if buffers have changes or if another link was added for this line if (isValid) { let timeout: number | undefined; callback({ @@ -119,7 +118,6 @@ export class TerminalValidatedLocalLinkProvider implements ILinkProvider { } }); } else { - // TODO: Support multiple matches from the regexes callback(undefined); } }); diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts index 0d71a56e5330d..830541732c745 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts @@ -62,9 +62,6 @@ export class TerminalWordLinkProvider implements ILinkProvider { text += char; } - // TODO: Only show word links when modifier is down? - // TODO: Only show tooltip if no mouse movement has happened; copy how editor works - const range = { start, end }; let timeout: number | undefined; callback({ diff --git a/src/vs/workbench/contrib/terminal/test/browser/links/terminalValidatedLocalLinkProvider.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalValidatedLocalLinkProvider.test.ts index 68d60c0d6b950..a2ebaf8ed3772 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/links/terminalValidatedLocalLinkProvider.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalValidatedLocalLinkProvider.test.ts @@ -66,7 +66,6 @@ const supportedLinkFormats: LinkFormatInfo[] = [ suite('Workbench - TerminalValidatedLocalLinkProvider', () => { async function assertLink(text: string, os: OperatingSystem, expected: { text: string, range: [number, number][] }) { const xterm = new Terminal(); - // TODO: Test process manager const provider = new TerminalValidatedLocalLinkProvider(xterm, os, () => { }, () => { }, () => { }, (_, cb) => { cb(true); }); // Write the text and wait for the parser to finish From 5d8275f7c27a8a103f653da9faf25eb2febae406 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 18:45:13 -0700 Subject: [PATCH 13/14] Fix words to link up until the end of the line --- .../contrib/terminal/browser/links/terminalWordLinkProvider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts b/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts index 830541732c745..01c4251d4d5c8 100644 --- a/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts +++ b/src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkProvider.ts @@ -49,7 +49,7 @@ export class TerminalWordLinkProvider implements ILinkProvider { // Expand to the right until a word separator is hit // end.x++; // The hovered cell is considered first - for (let x = position.x + 1; x < line.length; x++) { + for (let x = position.x + 1; x <= line.length; x++) { const char = line.getCell(x - 1)?.getChars(); if (!char) { break; From 4b798196366da0c1d1b1508f4fca9cb819469043 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Sun, 12 Apr 2020 19:03:57 -0700 Subject: [PATCH 14/14] Remove mac tests --- ...terminalValidatedLocalLinkProvider.test.ts | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/test/browser/links/terminalValidatedLocalLinkProvider.test.ts b/src/vs/workbench/contrib/terminal/test/browser/links/terminalValidatedLocalLinkProvider.test.ts index a2ebaf8ed3772..3c63cde4bab61 100644 --- a/src/vs/workbench/contrib/terminal/test/browser/links/terminalValidatedLocalLinkProvider.test.ts +++ b/src/vs/workbench/contrib/terminal/test/browser/links/terminalValidatedLocalLinkProvider.test.ts @@ -103,33 +103,35 @@ suite('Workbench - TerminalValidatedLocalLinkProvider', () => { } } - [{ name: 'Linux', os: OperatingSystem.Linux }, { name: 'macOS', os: OperatingSystem.Macintosh }].forEach(config => { - suite(config.name, () => { - unixLinks.forEach(baseLink => { - test(`Link "${baseLink}"`, async () => { - for (let i = 0; i < supportedLinkFormats.length; i++) { - const linkFormat = supportedLinkFormats[i]; + suite('Linux/macOS', () => { + unixLinks.forEach(baseLink => { + suite(`Link: ${baseLink}`, () => { + for (let i = 0; i < supportedLinkFormats.length; i++) { + const linkFormat = supportedLinkFormats[i]; + test(`Format: ${linkFormat.urlFormat}`, async () => { const formattedLink = format(linkFormat.urlFormat, baseLink, linkFormat.line, linkFormat.column); - await assertLink(formattedLink, config.os, { text: formattedLink, range: [[1, 1], [formattedLink.length, 1]] }); - await assertLink(` ${formattedLink} `, config.os, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); - await assertLink(`(${formattedLink})`, config.os, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); - await assertLink(`[${formattedLink}]`, config.os, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); - } - }); + await assertLink(formattedLink, OperatingSystem.Linux, { text: formattedLink, range: [[1, 1], [formattedLink.length, 1]] }); + await assertLink(` ${formattedLink} `, OperatingSystem.Linux, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); + await assertLink(`(${formattedLink})`, OperatingSystem.Linux, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); + await assertLink(`[${formattedLink}]`, OperatingSystem.Linux, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); + }); + } }); }); }); suite('Windows', () => { windowsLinks.forEach(baseLink => { - test(`Link "${baseLink}"`, async () => { + suite(`Link "${baseLink}"`, () => { for (let i = 0; i < supportedLinkFormats.length; i++) { const linkFormat = supportedLinkFormats[i]; - const formattedLink = format(linkFormat.urlFormat, baseLink, linkFormat.line, linkFormat.column); - await assertLink(formattedLink, OperatingSystem.Windows, { text: formattedLink, range: [[1, 1], [formattedLink.length, 1]] }); - await assertLink(` ${formattedLink} `, OperatingSystem.Windows, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); - await assertLink(`(${formattedLink})`, OperatingSystem.Windows, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); - await assertLink(`[${formattedLink}]`, OperatingSystem.Windows, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); + test(`Format: ${linkFormat.urlFormat}`, async () => { + const formattedLink = format(linkFormat.urlFormat, baseLink, linkFormat.line, linkFormat.column); + await assertLink(formattedLink, OperatingSystem.Windows, { text: formattedLink, range: [[1, 1], [formattedLink.length, 1]] }); + await assertLink(` ${formattedLink} `, OperatingSystem.Windows, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); + await assertLink(`(${formattedLink})`, OperatingSystem.Windows, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); + await assertLink(`[${formattedLink}]`, OperatingSystem.Windows, { text: formattedLink, range: [[2, 1], [formattedLink.length + 1, 1]] }); + }); } }); });