Skip to content

Commit

Permalink
Merge pull request #169889 from microsoft/tyriar/169867
Browse files Browse the repository at this point in the history 
Optimize TerminalWordLinkDetector._parseWords
  • Loading branch information
@Tyriar
Tyriar authored Dec 27, 2022
2 parents f285d46 + 3e05e21 commit 048752c
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 9 deletions.
27 changes: 20 additions & 7 deletions src/vs/workbench/contrib/terminal/browser/links/terminalWordLinkDetector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { URI } from 'vs/base/common/uri';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { matchesScheme } from 'vs/platform/opener/common/opener';
import { IProductService } from 'vs/platform/product/common/productService';
import { TerminalSettingId } from 'vs/platform/terminal/common/terminal';
import { ITerminalSimpleLink, ITerminalLinkDetector, TerminalBuiltinLinkType } from 'vs/workbench/contrib/terminal/browser/links/links';
import { convertLinkRangeToBuffer, getXtermLineContent } from 'vs/workbench/contrib/terminal/browser/links/terminalLinkHelpers';
import { ITerminalConfiguration, TERMINAL_CONFIG_SECTION } from 'vs/workbench/contrib/terminal/common/terminal';
Expand All @@ -32,16 +33,23 @@ export class TerminalWordLinkDetector implements ITerminalLinkDetector {
// quite small.
readonly maxLinkLength = 100;

private _separatorCodes!: Uint32Array;

constructor(
readonly xterm: Terminal,
@IConfigurationService private readonly _configurationService: IConfigurationService,
@IProductService private readonly _productService: IProductService,
) {
this._refreshSeparatorCodes();
this._configurationService.onDidChangeConfiguration(e => {
if (e.affectsConfiguration(TerminalSettingId.WordSeparators)) {
this._refreshSeparatorCodes();
}
});
}

detect(lines: IBufferLine[], startLine: number, endLine: number): ITerminalSimpleLink[] {
const links: ITerminalSimpleLink[] = [];
const wordSeparators = this._configurationService.getValue<ITerminalConfiguration>(TERMINAL_CONFIG_SECTION).wordSeparators;

// Get the text representation of the wrapped line
const text = getXtermLineContent(this.xterm.buffer.active, startLine, endLine, this.xterm.cols);
Expand All @@ -50,7 +58,7 @@ export class TerminalWordLinkDetector implements ITerminalLinkDetector {
}

// Parse out all words from the wrapped line
const words: Word[] = this._parseWords(text, wordSeparators);
const words: Word[] = this._parseWords(text);

// Map the words to ITerminalLink objects
for (const word of words) {
Expand Down Expand Up @@ -98,15 +106,12 @@ export class TerminalWordLinkDetector implements ITerminalLinkDetector {
return links;
}

private _parseWords(text: string, separators: string): Word[] {
private _parseWords(text: string): Word[] {
const words: Word[] = [];

const wordSeparators: string[] = separators.split('');
const characters = text.split('');

let startIndex = 0;
for (let i = 0; i < text.length; i++) {
if (wordSeparators.includes(characters[i])) {
if (this._separatorCodes.includes(text.charCodeAt(i))) {
words.push({ startIndex, endIndex: i, text: text.substring(startIndex, i) });
startIndex = i + 1;
}
Expand All @@ -117,4 +122,12 @@ export class TerminalWordLinkDetector implements ITerminalLinkDetector {

return words;
}

private _refreshSeparatorCodes(): void {
const separators = this._configurationService.getValue<ITerminalConfiguration>(TERMINAL_CONFIG_SECTION).wordSeparators;
this._separatorCodes = new Uint32Array(separators.length);
for (let i = 0; i < separators.length; i++) {
this._separatorCodes[i] = separators.charCodeAt(i);
}
}
}
3 changes: 2 additions & 1 deletion src/vs/workbench/contrib/terminal/test/browser/links/terminalLinkManager.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ const defaultTerminalConfig: Partial<ITerminalConfiguration> = {
scrollback: 1000,
fastScrollSensitivity: 2,
mouseWheelScrollSensitivity: 1,
unicodeVersion: '11'
unicodeVersion: '11',
wordSeparators: ' ()[]{}\',"`─‘’'
};

class TestLinkManager extends TerminalLinkManager {
Expand Down
13 changes: 12 additions & 1 deletion src/vs/workbench/contrib/terminal/test/browser/links/terminalWordLinkDetector.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ suite('Workbench - TerminalWordLinkDetector', () => {
let detector: TerminalWordLinkDetector;
let xterm: Terminal;

setup(() => {
setup(async () => {
const instantiationService = new TestInstantiationService();
configurationService = new TestConfigurationService();
await configurationService.setUserConfiguration('terminal', { integrated: { wordSeparators: '' } });

instantiationService.stub(IConfigurationService, configurationService);
instantiationService.set(IProductService, TestProductService);
Expand All @@ -39,6 +40,7 @@ suite('Workbench - TerminalWordLinkDetector', () => {
suite('should link words as defined by wordSeparators', () => {
test('" ()[]"', async () => {
await configurationService.setUserConfiguration('terminal', { integrated: { wordSeparators: ' ()[]' } });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as any);
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' }]);
Expand All @@ -47,6 +49,7 @@ suite('Workbench - TerminalWordLinkDetector', () => {
});
test('" "', async () => {
await configurationService.setUserConfiguration('terminal', { integrated: { wordSeparators: ' ' } });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as any);
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)' }]);
Expand All @@ -55,6 +58,7 @@ suite('Workbench - TerminalWordLinkDetector', () => {
});
test('" []"', async () => {
await configurationService.setUserConfiguration('terminal', { integrated: { wordSeparators: ' []' } });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as any);
await assertLink('aabbccdd.txt ', [{ range: [[1, 1], [12, 1]], text: 'aabbccdd.txt' }]);
await assertLink(' aabbccdd.txt ', [{ range: [[2, 1], [13, 1]], text: 'aabbccdd.txt' }]);
await assertLink(' [aabbccdd.txt] ', [{ range: [[3, 1], [14, 1]], text: 'aabbccdd.txt' }]);
Expand All @@ -65,13 +69,15 @@ suite('Workbench - TerminalWordLinkDetector', () => {
// with a wide character, which the terminalLinkHelper currently doesn't account for
test.skip('should support wide characters', async () => {
await configurationService.setUserConfiguration('terminal', { integrated: { wordSeparators: ' []' } });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as any);
await assertLink('我是学生.txt ', [{ range: [[1, 1], [12, 1]], text: '我是学生.txt' }]);
await assertLink(' 我是学生.txt ', [{ range: [[2, 1], [13, 1]], text: '我是学生.txt' }]);
await assertLink(' [我是学生.txt] ', [{ range: [[3, 1], [14, 1]], text: '我是学生.txt' }]);
});

test('should support multiple link results', async () => {
await configurationService.setUserConfiguration('terminal', { integrated: { wordSeparators: ' ' } });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as any);
await assertLink('foo bar', [
{ range: [[1, 1], [3, 1]], text: 'foo' },
{ range: [[5, 1], [7, 1]], text: 'bar' }
Expand All @@ -80,6 +86,7 @@ suite('Workbench - TerminalWordLinkDetector', () => {

test('should remove trailing colon in the link results', async () => {
await configurationService.setUserConfiguration('terminal', { integrated: { wordSeparators: ' ' } });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as any);
await assertLink('foo:5:6: bar:0:32:', [
{ range: [[1, 1], [7, 1]], text: 'foo:5:6' },
{ range: [[10, 1], [17, 1]], text: 'bar:0:32' }
Expand All @@ -88,12 +95,14 @@ suite('Workbench - TerminalWordLinkDetector', () => {

test('should support wrapping', async () => {
await configurationService.setUserConfiguration('terminal', { integrated: { wordSeparators: ' ' } });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as any);
await assertLink('fsdjfsdkfjslkdfjskdfjsldkfjsdlkfjslkdjfskldjflskdfjskldjflskdfjsdklfjsdklfjsldkfjsdlkfjsdlkfjsdlkfjsldkfjslkdfjsdlkfjsldkfjsdlkfjskdfjsldkfjsdlkfjslkdfjsdlkfjsldkfjsldkfjsldkfjslkdfjsdlkfjslkdfjsdklfsd', [
{ range: [[1, 1], [41, 3]], text: 'fsdjfsdkfjslkdfjskdfjsldkfjsdlkfjslkdjfskldjflskdfjskldjflskdfjsdklfjsdklfjsldkfjsdlkfjsdlkfjsdlkfjsldkfjslkdfjsdlkfjsldkfjsdlkfjskdfjsldkfjsdlkfjslkdfjsdlkfjsldkfjsldkfjsldkfjslkdfjsdlkfjslkdfjsdklfsd' },
]);
});
test('should support wrapping with multiple links', async () => {
await configurationService.setUserConfiguration('terminal', { integrated: { wordSeparators: ' ' } });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as any);
await assertLink('fsdjfsdkfjslkdfjskdfjsldkfj sdlkfjslkdjfskldjflskdfjskldjflskdfj sdklfjsdklfjsldkfjsdlkfjsdlkfjsdlkfjsldkfjslkdfjsdlkfjsldkfjsdlkfjskdfjsldkfjsdlkfjslkdfjsdlkfjsldkfjsldkfjsldkfjslkdfjsdlkfjslkdfjsdklfsd', [
{ range: [[1, 1], [27, 1]], text: 'fsdjfsdkfjslkdfjskdfjsldkfj' },
{ range: [[29, 1], [64, 1]], text: 'sdlkfjslkdjfskldjflskdfjskldjflskdfj' },
Expand All @@ -102,10 +111,12 @@ suite('Workbench - TerminalWordLinkDetector', () => {
});
test('does not return any links for empty text', async () => {
await configurationService.setUserConfiguration('terminal', { integrated: { wordSeparators: ' ' } });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as any);
await assertLink('', []);
});
test('should support file scheme links', async () => {
await configurationService.setUserConfiguration('terminal', { integrated: { wordSeparators: ' ' } });
configurationService.onDidChangeConfigurationEmitter.fire({ affectsConfiguration: () => true } as any);
await assertLink('file:///C:/users/test/file.txt ', [{ range: [[1, 1], [30, 1]], text: 'file:///C:/users/test/file.txt' }]);
await assertLink('file:///C:/users/test/file.txt:1:10 ', [{ range: [[1, 1], [35, 1]], text: 'file:///C:/users/test/file.txt:1:10' }]);
});
Expand Down

0 comments on commit 048752c

Please sign in to comment.