Skip to content

Commit d6d172b

Browse files
authored
refactor(terminal): extract common link related code to terminal-link.ts (#4549)
1. Move regex patterns and constants to terminal-link.ts 2. Create ILinkInfo interface for link information 3. Extract extractLineInfoFromMatch function to common module 4. Update imports and usages in related files 5. Remove duplicate code in link providers
1 parent 473f45d commit d6d172b

File tree

5 files changed

+339
-101
lines changed

5 files changed

+339
-101
lines changed

packages/terminal-next/__tests__/browser/links/validated-local-link-provider.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,4 +286,79 @@ describe('Workbench - TerminalValidatedLocalLinkProvider', () => {
286286
},
287287
]);
288288
});
289+
290+
describe('Special formats', () => {
291+
const isWindows = false;
292+
test('Python error output format', async () => {
293+
await assertLink(' File "/path/to/file.py", line 3, in test_func', isWindows, [
294+
{
295+
text: '/path/to/file.py", line 3',
296+
range: [
297+
[9, 1],
298+
[33, 1],
299+
],
300+
},
301+
]);
302+
await assertLink(' File "/absolute/path/script.py", line 123, column 45', isWindows, [
303+
{
304+
text: '/absolute/path/script.py", line 123, column 45',
305+
range: [
306+
[11, 1],
307+
[56, 1],
308+
],
309+
},
310+
]);
311+
});
312+
313+
test('File paths with line and column numbers', async () => {
314+
await assertLink('/path/to/file.py:2, column 2', isWindows, [
315+
{
316+
text: '/path/to/file.py:2',
317+
range: [
318+
[1, 1],
319+
[18, 1],
320+
],
321+
},
322+
]);
323+
324+
await assertLink('/path/to/file.py:2, col 2', isWindows, [
325+
{
326+
text: '/path/to/file.py:2',
327+
range: [
328+
[1, 1],
329+
[18, 1],
330+
],
331+
},
332+
]);
333+
334+
await assertLink('/path/to/file.py:2:3', isWindows, [
335+
{
336+
text: '/path/to/file.py:2:3',
337+
range: [
338+
[1, 1],
339+
[20, 1],
340+
],
341+
},
342+
]);
343+
});
344+
345+
test('Multiple formats in one line', async () => {
346+
await assertLink('Error in /path/to/file.py:2, column 2 and /another/file.js:5:6', isWindows, [
347+
{
348+
text: '/path/to/file.py:2',
349+
range: [
350+
[10, 1],
351+
[27, 1],
352+
],
353+
},
354+
{
355+
text: '/another/file.js:5:6',
356+
range: [
357+
[43, 1],
358+
[62, 1],
359+
],
360+
},
361+
]);
362+
});
363+
});
289364
});

packages/terminal-next/src/browser/links/link-manager.ts

Lines changed: 26 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,21 @@ import {
2525
ITerminalHoverManagerService,
2626
ITerminalService,
2727
} from '../../common';
28+
import {
29+
ILinkInfo,
30+
extractLineInfoFromMatch,
31+
getLineAndColumnClause,
32+
unixLocalLinkClause,
33+
winDrivePrefix,
34+
winLocalLinkClause,
35+
} from '../../common/terminal-link';
2836
import { XTermCore } from '../../common/xterm-private';
2937
import { TerminalClient } from '../terminal.client';
3038

3139
import { TerminalExternalLinkProviderAdapter } from './external-link-provider-adapter';
3240
import { TerminalLink } from './link';
3341
import { TerminalProtocolLinkProvider } from './protocol-link-provider';
34-
import {
35-
TerminalValidatedLocalLinkProvider,
36-
lineAndColumnClause,
37-
lineAndColumnClauseGroupCount,
38-
unixLineAndColumnMatchIndex,
39-
unixLocalLinkClause,
40-
winDrivePrefix,
41-
winLineAndColumnMatchIndex,
42-
winLocalLinkClause,
43-
} from './validated-local-link-provider';
42+
import { TerminalValidatedLocalLinkProvider } from './validated-local-link-provider';
4443
import { TerminalWordLinkProvider } from './word-link-provider';
4544

4645
const { posix, win32 } = path;
@@ -249,23 +248,22 @@ export class TerminalLinkManager extends Disposable {
249248
protected get _localLinkRegex(): RegExp {
250249
const baseLocalLinkClause = this._client.os === OperatingSystem.Windows ? winLocalLinkClause : unixLocalLinkClause;
251250
// Append line and column number regex
252-
return new RegExp(`${baseLocalLinkClause}(${lineAndColumnClause})`);
251+
return new RegExp(`${baseLocalLinkClause}(${getLineAndColumnClause()})`);
253252
}
254253

255254
private async _handleLocalLink(link: string): Promise<void> {
256-
// TODO: This gets resolved again but doesn't need to as it's already validated
257-
const resolvedLink = await this._resolvePath(link);
255+
const lineColumnInfo: LineColumnInfo = this.extractLineColumnInfo(link);
256+
const resolvedLink = await this._resolvePath(lineColumnInfo.filePath);
258257
if (!resolvedLink) {
259258
return;
260259
}
261-
const lineColumnInfo: LineColumnInfo = this.extractLineColumnInfo(link);
262260
const range: ITextEditorSelection = {
263261
startLineNumber: lineColumnInfo.lineNumber,
264262
endLineNumber: lineColumnInfo.lineNumber,
265263
startColumn: lineColumnInfo.columnNumber,
266264
endColumn: lineColumnInfo.columnNumber,
267265
};
268-
await this._editorService.open(resolvedLink.uri, { range });
266+
await this._editorService.open(resolvedLink.uri, { range, focus: true });
269267
}
270268

271269
private _handleHypertextLink(url: string): void {
@@ -399,39 +397,31 @@ export class TerminalLinkManager extends Disposable {
399397
}
400398
}
401399

400+
private _extractLineInfoFromMatch(match: RegExpExecArray): ILinkInfo {
401+
return extractLineInfoFromMatch(match);
402+
}
403+
402404
/**
403405
* Returns line and column number of URl if that is present.
404406
*
405407
* @param link Url link which may contain line and column number.
406408
*/
407409
public extractLineColumnInfo(link: string): LineColumnInfo {
408-
const matches: string[] | null = this._localLinkRegex.exec(link);
410+
const matches: RegExpExecArray | null = this._localLinkRegex.exec(link);
409411
const lineColumnInfo: LineColumnInfo = {
410412
lineNumber: 1,
411413
columnNumber: 1,
414+
filePath: link,
412415
};
413-
414416
if (!matches) {
415417
return lineColumnInfo;
416418
}
417-
418-
const lineAndColumnMatchIndex =
419-
this._client.os === OperatingSystem.Windows ? winLineAndColumnMatchIndex : unixLineAndColumnMatchIndex;
420-
for (let i = 0; i < lineAndColumnClause.length; i++) {
421-
const lineMatchIndex = lineAndColumnMatchIndex + lineAndColumnClauseGroupCount * i;
422-
const rowNumber = matches[lineMatchIndex];
423-
if (rowNumber) {
424-
lineColumnInfo['lineNumber'] = parseInt(rowNumber, 10);
425-
// Check if column number exists
426-
const columnNumber = matches[lineMatchIndex + 2];
427-
if (columnNumber) {
428-
lineColumnInfo['columnNumber'] = parseInt(columnNumber, 10);
429-
}
430-
break;
431-
}
432-
}
433-
434-
return lineColumnInfo;
419+
const lineInfo = this._extractLineInfoFromMatch(matches);
420+
return {
421+
filePath: lineInfo.filePath ?? link,
422+
lineNumber: lineInfo.line ?? 1,
423+
columnNumber: lineInfo.column ?? 1,
424+
};
435425
}
436426

437427
/**
@@ -449,6 +439,7 @@ export class TerminalLinkManager extends Disposable {
449439
}
450440

451441
export interface LineColumnInfo {
442+
filePath: string;
452443
lineNumber: number;
453444
columnNumber: number;
454445
}

packages/terminal-next/src/browser/links/link.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,17 @@ export class TerminalLink extends Disposable implements ILink {
4343
public readonly range: IBufferRange,
4444
public readonly text: string,
4545
private readonly _viewportY: number,
46-
private readonly _activateCallback: (event: MouseEvent | undefined, uri: string) => void,
46+
private readonly _activateCallback: (
47+
event: MouseEvent | undefined,
48+
uri: string,
49+
lineInfo?: {
50+
filePath?: string;
51+
line?: number;
52+
column?: number;
53+
endLine?: number;
54+
endColumn?: number;
55+
},
56+
) => void,
4757
private readonly _tooltipCallback: (
4858
link: TerminalLink,
4959
viewportRange: IViewportRange,
@@ -52,6 +62,13 @@ export class TerminalLink extends Disposable implements ILink {
5262
) => IDisposable | undefined,
5363
private readonly _isHighConfidenceLink: boolean,
5464
readonly label: string | undefined,
65+
private readonly _lineInfo?: {
66+
filePath?: string;
67+
line?: number;
68+
column?: number;
69+
endLine?: number;
70+
endColumn?: number;
71+
},
5572
) {
5673
super();
5774
this.decorations = {
@@ -71,7 +88,7 @@ export class TerminalLink extends Disposable implements ILink {
7188
}
7289

7390
activate(event: MouseEvent | undefined, text: string): void {
74-
this._activateCallback(event, text);
91+
this._activateCallback(event, text, this._lineInfo);
7592
}
7693

7794
hover(event: MouseEvent, text: string): void {

0 commit comments

Comments
 (0)