Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding line and column support for terminal #24832

Merged
merged 10 commits into from Apr 24, 2017
Merged

Adding line and column support for terminal #24832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
425ed1b
WIP: Adding line and column support for terminal
Apr 15, 2017
441099c
Adding space along with non-breaking space in regex, fixes for unix p…
Apr 16, 2017
8d5be51
Adding tests for getting information of line and column along with li…
Apr 16, 2017
566e2d7
moving object level constants to class level, seprate method to forma…
Apr 18, 2017
325d9a7
Adding code to generate links tests cases.
Apr 18, 2017
94affbf
Consolidate link regex variables
Tyriar Apr 24, 2017
e848c21
Tidy comments
Tyriar Apr 24, 2017
3247d10
Only evaluate lineAndColumnClause once
Tyriar Apr 24, 2017
4f6cb99
Make extractLineColumnInfo more concise
Tyriar Apr 24, 2017
3984e25
Merge remote-tracking branch 'ups/master' into 21472-adding-line-colu…
Tyriar Apr 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
113 changes: 103 additions & 10 deletions src/vs/workbench/parts/terminal/electron-browser/terminalLinkHandler.ts
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@ import Uri from 'vs/base/common/uri';
import { dispose, IDisposable } from 'vs/base/common/lifecycle';
import { IWorkbenchEditorService } from 'vs/workbench/services/editor/common/editorService';
import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
import { IOpenerService } from 'vs/platform/opener/common/opener';
import { TerminalWidgetManager } from 'vs/workbench/parts/terminal/browser/terminalWidgetManager';
import { TPromise } from 'vs/base/common/winjs.base';

Expand All @@ -20,13 +21,31 @@ const pathSeparatorClause = '\\/';
const excludedPathCharactersClause = '[^\\0\\s!$`&*()\\[\\]+\'":;]'; // '":; are allowed in paths but they are often separators so ignore them
const escapedExcludedPathCharactersClause = '(\\\\s|\\\\!|\\\\$|\\\\`|\\\\&|\\\\*|(|)|\\+)';
/** A regex that matches paths in the form /foo, ~/foo, ./foo, ../foo, foo/bar */
const UNIX_LIKE_LOCAL_LINK_REGEX = new RegExp('((' + pathPrefix + '|(' + excludedPathCharactersClause + '|' + escapedExcludedPathCharactersClause + ')+)?(' + pathSeparatorClause + '(' + excludedPathCharactersClause + '|' + escapedExcludedPathCharactersClause + ')+)+)');
const unixLocalLinkClause = '((' + pathPrefix + '|(' + excludedPathCharactersClause + '|' + escapedExcludedPathCharactersClause + ')+)?(' + pathSeparatorClause + '(' + excludedPathCharactersClause + '|' + escapedExcludedPathCharactersClause + ')+)+)';

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 WINDOWS_LOCAL_LINK_REGEX = new RegExp('((' + winPathPrefix + '|(' + winExcludedPathCharactersClause + ')+)?(' + winPathSeparatorClause + '(' + winExcludedPathCharactersClause + ')+)+)');
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*) 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+)(,(\\d+))?)\\))', // (file path)(45), (file path) (45), (file path)(45,18), (file path) (45,18)
'(([^:\\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 = 15;

// Each line and column clause have 6 groups (ie no. of expressions in round brackets)
const lineAndColumnClauseGroupCount = 6;

/** Higher than local link, lower than hypertext */
const CUSTOM_LINK_PRIORITY = -1;
/** Lowest */
Expand All @@ -39,12 +58,19 @@ export class TerminalLinkHandler {
private _tooltipDisposables: IDisposable[] = [];
private _widgetManager: TerminalWidgetManager;

private _localLinkPattern: RegExp;

constructor(
private _xterm: any,
private _platform: platform.Platform,
@IOpenerService private _openerService: IOpenerService,
@IWorkbenchEditorService private _editorService: IWorkbenchEditorService,
@IWorkspaceContextService private _contextService: IWorkspaceContextService
) {
const baseLocalLinkClause = _platform === platform.Platform.Windows ? winLocalLinkClause : unixLocalLinkClause;
// Append line and column number regex
this._localLinkPattern = new RegExp(`${baseLocalLinkClause}(${lineAndColumnClause})`);

this._xterm.setHypertextLinkHandler(this._wrapLinkHandler(() => true));
this._xterm.setHypertextValidationCallback((uri: string, element: HTMLElement, callback: (isValid: boolean) => void) => {
this._validateWebLink(uri, element, callback);
Expand Down Expand Up @@ -76,8 +102,8 @@ export class TerminalLinkHandler {
this._handleLocalLink(url);
return;
});

return this._xterm.registerLinkMatcher(this._localLinkRegex, wrappedHandler, {
matchIndex: 1,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Match index 1 only matches first group of regex, but we need other groups as well which have line and column number information.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But matchIndex is still generally supported right? Custom link matchers need to be able to specify it still.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tyriar , Yes for custom link matchers MatchIndex is supported, it will follow given matchIndex.

validationCallback: (link: string, element: HTMLElement, callback: (isValid: boolean) => void) => this._validateLocalLink(link, element, callback),
priority: LOCAL_LINK_PRIORITY
});
Expand All @@ -99,19 +125,26 @@ export class TerminalLinkHandler {
}

protected get _localLinkRegex(): RegExp {
if (this._platform === platform.Platform.Windows) {
return WINDOWS_LOCAL_LINK_REGEX;
}
return UNIX_LIKE_LOCAL_LINK_REGEX;
return this._localLinkPattern;
}

private _handleLocalLink(link: string): TPromise<void> {
return this._resolvePath(link).then(resolvedLink => {
if (!resolvedLink) {
return void 0;
}
const resource = Uri.file(path.normalize(path.resolve(resolvedLink)));
return this._editorService.openEditor({ resource }).then(() => void 0);

let normalizedPath = path.normalize(path.resolve(resolvedLink));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be pulled into a formatLocalLinkPath or similar named function that returns a URI in form <path>#line,col?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that will be better will do that

const normalizedUrl = this.extractLinkUrl(normalizedPath);

normalizedPath = this._formatLocalLinkPath(normalizedPath);

let resource = Uri.file(normalizedUrl);
resource = resource.with({
fragment: Uri.parse(normalizedPath).fragment
});

return this._openerService.open(resource);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this._editorService.openEditor ignore the line/col?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I first tried with openEditor, but it didn't worked. Hence used open service.

});
}

Expand Down Expand Up @@ -188,12 +221,72 @@ export class TerminalLinkHandler {
return TPromise.as(void 0);
}

const linkUrl = this.extractLinkUrl(link);
// Open an editor if the path exists
return pfs.fileExists(link).then(isFile => {
return pfs.fileExists(linkUrl).then(isFile => {
if (!isFile) {
return null;
}
return link;
});
}

/**
* Appends line number and column number to link if they exists.
* @param link link to format, will become link#line_num,col_num.
*/
private _formatLocalLinkPath(link: string): string {
const lineColumnInfo: LineColumnInfo = this.extractLineColumnInfo(link);
if (lineColumnInfo.lineNumber) {
link += `#${lineColumnInfo.lineNumber}`;

if (lineColumnInfo.columnNumber) {
link += `,${lineColumnInfo.columnNumber}`;
}
}

return link;
}

/**
* Returns line and column number of URl if that is present.
*
* @param link Url link which may contain line and column number.
*/
public extractLineColumnInfo(link: string): LineColumnInfo {
const matches: string[] = this._localLinkRegex.exec(link);
const lineColumnInfo: LineColumnInfo = {};
const lineAndColumnMatchIndex = this._platform === platform.Platform.Windows ? winLineAndColumnMatchIndex : unixLineAndColumnMatchIndex;

for (let i = 0; i < lineAndColumnClause.length; i++) {
const lineMatchIndex = lineAndColumnMatchIndex + (lineAndColumnClauseGroupCount * i);
const rowNumber = matches[lineMatchIndex];
if (rowNumber) {
lineColumnInfo['lineNumber'] = rowNumber;
// Check if column number exists
const columnNumber = matches[lineMatchIndex + 2];
if (columnNumber) {
lineColumnInfo['columnNumber'] = columnNumber;
}
break;
}
}

return lineColumnInfo;
}

/**
* Returns url from link as link may contain line and column information.
*
* @param link url link which may contain line and column number.
*/
public extractLinkUrl(link: string): string {
const matches: string[] = this._localLinkRegex.exec(link);
return matches[1];
}
}

export interface LineColumnInfo {
lineNumber?: string;
columnNumber?: string;
};
156 changes: 122 additions & 34 deletions src/vs/workbench/parts/terminal/test/electron-browser/terminalLinkHandler.test.ts
View file
Open in desktop
Expand Up @@ -7,9 +7,10 @@

import * as assert from 'assert';
import { Platform } from 'vs/base/common/platform';
import { TerminalLinkHandler } from 'vs/workbench/parts/terminal/electron-browser/terminalLinkHandler';
import { TerminalLinkHandler, LineColumnInfo } from 'vs/workbench/parts/terminal/electron-browser/terminalLinkHandler';
import { IWorkspace, WorkspaceContextService } from 'vs/platform/workspace/common/workspace';
import URI from 'vs/base/common/uri';
import * as strings from 'vs/base/common/strings';
import * as path from 'path';
import * as sinon from 'sinon';

Expand Down Expand Up @@ -37,6 +38,12 @@ class TestURI extends URI {
}
}

interface LinkFormatInfo {
urlFormat: string;
line?: string;
column?: string;
}

class TestWorkspace implements IWorkspace {
resource: URI;
constructor(basePath: string) {
Expand All @@ -47,47 +54,128 @@ class TestWorkspace implements IWorkspace {
suite('Workbench - TerminalLinkHandler', () => {
suite('localLinkRegex', () => {
test('Windows', () => {
const regex = new TestTerminalLinkHandler(new TestXterm(), Platform.Windows, null, null).localLinkRegex;
function testLink(link: string) {
assert.equal(` ${link} `.match(regex)[1], link);
assert.equal(`:${link}:`.match(regex)[1], link);
assert.equal(`;${link};`.match(regex)[1], link);
assert.equal(`(${link})`.match(regex)[1], link);
const terminalLinkHandler = new TestTerminalLinkHandler(new TestXterm(), Platform.Windows, 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);
assert.equal(terminalLinkHandler.extractLinkUrl(`;${link};`), linkUrl);
assert.equal(terminalLinkHandler.extractLinkUrl(`(${link})`), linkUrl);

if (lineNo) {
const lineColumnInfo: LineColumnInfo = terminalLinkHandler.extractLineColumnInfo(link);
assert.equal(lineColumnInfo.lineNumber, lineNo);

if (columnNo) {
assert.equal(lineColumnInfo.columnNumber, columnNo);
}
}
}

function generateAndTestLinks() {
const linkUrls = [
'c:\\foo',
'c:/foo',
'.\\foo',
'./foo',
'..\\foo',
'~\\foo',
'~/foo',
'c:/a/long/path',
'c:\\a\\long\\path',
'c:\\mixed/slash\\path',
'a/relative/path'
];

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}', line: '5' },
{ urlFormat: '{0}:{1}:{2}', line: '5', column: '3' }
];

linkUrls.forEach(linkUrl => {
supportedLinkFormats.forEach(linkFormatInfo => {
testLink(
strings.format(linkFormatInfo.urlFormat, linkUrl, linkFormatInfo.line, linkFormatInfo.column),
linkUrl,
linkFormatInfo.line,
linkFormatInfo.column
);
});
});
}
testLink('c:\\foo');
testLink('c:/foo');
testLink('.\\foo');
testLink('./foo');
testLink('..\\foo');
testLink('../foo');
testLink('~\\foo');
testLink('~/foo');
testLink('c:/a/long/path');
testLink('c:\\a\\long\\path');
testLink('c:\\mixed/slash\\path');
testLink('a/relative/path');

generateAndTestLinks();
});

test('Linux', () => {
const regex = new TestTerminalLinkHandler(new TestXterm(), Platform.Linux, null, null).localLinkRegex;
function testLink(link: string) {
assert.equal(` ${link} `.match(regex)[1], link);
assert.equal(`:${link}:`.match(regex)[1], link);
assert.equal(`;${link};`.match(regex)[1], link);
assert.equal(`(${link})`.match(regex)[1], link);
const terminalLinkHandler = new TestTerminalLinkHandler(new TestXterm(), Platform.Linux, 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);
assert.equal(terminalLinkHandler.extractLinkUrl(`;${link};`), linkUrl);
assert.equal(terminalLinkHandler.extractLinkUrl(`(${link})`), linkUrl);

if (lineNo) {
const lineColumnInfo: LineColumnInfo = terminalLinkHandler.extractLineColumnInfo(link);
assert.equal(lineColumnInfo.lineNumber, lineNo);

if (columnNo) {
assert.equal(lineColumnInfo.columnNumber, columnNo);
}
}
}

function generateAndTestLinks() {
const linkUrls = [
'/foo',
'~/foo',
'./foo',
'../foo',
'/a/long/path',
'a/relative/path'
];

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}', line: '5' },
{ urlFormat: '{0}:{1}:{2}', line: '5', column: '3' }
];

linkUrls.forEach(linkUrl => {
supportedLinkFormats.forEach(linkFormatInfo => {
testLink(
strings.format(linkFormatInfo.urlFormat, linkUrl, linkFormatInfo.line, linkFormatInfo.column),
linkUrl,
linkFormatInfo.line,
linkFormatInfo.column
);
});
});
}
testLink('/foo');
testLink('~/foo');
testLink('./foo');
testLink('../foo');
testLink('/a/long/path');
testLink('a/relative/path');

generateAndTestLinks();
});
});

suite('preprocessPath', () => {
test('Windows', () => {
const linkHandler = new TestTerminalLinkHandler(new TestXterm(), Platform.Windows, null,
const linkHandler = new TestTerminalLinkHandler(new TestXterm(), Platform.Windows, null, null,
new WorkspaceContextService(new TestWorkspace('C:\\base')));

let stub = sinon.stub(path, 'join', function (arg1, arg2) {
Expand All @@ -101,7 +189,7 @@ suite('Workbench - TerminalLinkHandler', () => {
});

test('Linux', () => {
const linkHandler = new TestTerminalLinkHandler(new TestXterm(), Platform.Linux, null,
const linkHandler = new TestTerminalLinkHandler(new TestXterm(), Platform.Linux, null, null,
new WorkspaceContextService(new TestWorkspace('/base')));

let stub = sinon.stub(path, 'join', function (arg1, arg2) {
Expand All @@ -115,7 +203,7 @@ suite('Workbench - TerminalLinkHandler', () => {
});

test('No Workspace', () => {
const linkHandler = new TestTerminalLinkHandler(new TestXterm(), Platform.Linux, null, new WorkspaceContextService(null));
const linkHandler = new TestTerminalLinkHandler(new TestXterm(), Platform.Linux, null, null, new WorkspaceContextService(null));

assert.equal(linkHandler.preprocessPath('./src/file1'), null);
assert.equal(linkHandler.preprocessPath('src/file2'), null);
Expand Down