Skip to content

Commit ff3369f

Browse files
TyriarTyriar
authored
Merge pull request microsoft#185888 from microsoft/tyriar/178287
Support terminal links with end row/col
2 parents 052ac9c + f0e8f86 commit ff3369f

File tree

4 files changed

+96
-41
lines changed

4 files changed

+96
-41
lines changed

‎src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkOpeners.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,10 @@ export class TerminalLocalFileLinkOpener implements ITerminalLinkOpener {
3737
}
3838
const linkSuffix = link.parsedLink ? link.parsedLink.suffix : getLinkSuffix(link.text);
3939
const selection: ITextEditorSelection | undefined = linkSuffix?.row === undefined ? undefined : {
40-
startLineNumber: linkSuffix?.row ?? 1,
41-
startColumn: linkSuffix?.col ?? 1
40+
startLineNumber: linkSuffix.row ?? 1,
41+
startColumn: linkSuffix.col ?? 1,
42+
endLineNumber: linkSuffix.rowEnd,
43+
endColumn: linkSuffix.colEnd
4244
};
4345
await this._editorService.openEditor({
4446
resource: link.uri,

‎src/vs/workbench/contrib/terminalContrib/links/browser/terminalLinkParsing.ts‎

Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ export interface IParsedLink {
2121
export interface ILinkSuffix {
2222
row: number | undefined;
2323
col: number | undefined;
24+
rowEnd: number | undefined;
25+
colEnd: number | undefined;
2426
suffix: ILinkPartialRange;
2527
}
2628

@@ -42,12 +44,20 @@ const linkSuffixRegex = new Lazy<RegExp>(() => generateLinkSuffixRegex(false));
4244
function generateLinkSuffixRegex(eolOnly: boolean) {
4345
let ri = 0;
4446
let ci = 0;
45-
function l(): string {
47+
let rei = 0;
48+
let cei = 0;
49+
function r(): string {
4650
return `(?<row${ri++}>\\d+)`;
4751
}
4852
function c(): string {
4953
return `(?<col${ci++}>\\d+)`;
5054
}
55+
function re(): string {
56+
return `(?<rowEnd${rei++}>\\d+)`;
57+
}
58+
function ce(): string {
59+
return `(?<colEnd${cei++}>\\d+)`;
60+
}
5161

5262
const eolSuffix = eolOnly ? '$' : '';
5363

@@ -62,12 +72,12 @@ function generateLinkSuffixRegex(eolOnly: boolean) {
6272
// foo:339
6373
// foo:339:12
6474
// foo 339
65-
// foo 339:12 [#140780]
75+
// foo 339:12 [#140780]
6676
// "foo",339
6777
// "foo",339:12
68-
`(?::| |['"],)${l()}(:${c()})?` + eolSuffix,
69-
// The quotes below are optional [#171652]
70-
// "foo", line 339 [#40468]
78+
`(?::| |['"],)${r()}(:${c()})?` + eolSuffix,
79+
// The quotes below are optional [#171652]
80+
// "foo", line 339 [#40468]
7181
// "foo", line 339, col 12
7282
// "foo", line 339, column 12
7383
// "foo":line 339
@@ -80,18 +90,19 @@ function generateLinkSuffixRegex(eolOnly: boolean) {
8090
// "foo" on line 339, col 12
8191
// "foo" on line 339, column 12
8292
// "foo" line 339 column 12
83-
// "foo", line 339, character 12 [#171880]
84-
// "foo", line 339, characters 12-13 [#171880]
85-
// "foo", lines 339-340 [#171880]
86-
`['"]?(?:,? |: ?| on )lines? ${l()}(?:-\\d+)?(?:,? (?:col(?:umn)?|characters?) ${c()}(?:-\\d+)?)?` + eolSuffix,
93+
// "foo", line 339, character 12 [#171880]
94+
// "foo", line 339, characters 12-14 [#171880]
95+
// "foo", lines 339-341 [#171880]
96+
// "foo", lines 339-341, characters 12-14 [#178287]
97+
`['"]?(?:,? |: ?| on )lines? ${r()}(?:-${re()})?(?:,? (?:col(?:umn)?|characters?) ${c()}(?:-${ce()})?)?` + eolSuffix,
8798
// foo(339)
8899
// foo(339,12)
89100
// foo(339, 12)
90101
// foo (339)
91102
// ...
92103
// foo: (339)
93104
// ...
94-
`:? ?[\\[\\(]${l()}(?:, ?${c()})?[\\]\\)]` + eolSuffix,
105+
`:? ?[\\[\\(]${r()}(?:, ?${c()})?[\\]\\)]` + eolSuffix,
95106
];
96107

97108
const suffixClause = lineAndColumnRegexClauses
@@ -129,25 +140,6 @@ export function removeLinkQueryString(link: string): string {
129140
return link.substring(0, index);
130141
}
131142

132-
/**
133-
* Returns the optional link suffix which contains line and column information.
134-
* @param link The link to parse.
135-
*/
136-
export function getLinkSuffix(link: string): ILinkSuffix | null {
137-
const matches = linkSuffixRegexEol.value.exec(link);
138-
const groups = matches?.groups;
139-
if (!groups || matches.length < 1) {
140-
return null;
141-
}
142-
const rowString = groups.row0 || groups.row1 || groups.row2;
143-
const colString = groups.col0 || groups.col1 || groups.col2;
144-
return {
145-
row: rowString !== undefined ? parseInt(rowString) : undefined,
146-
col: colString !== undefined ? parseInt(colString) : undefined,
147-
suffix: { index: matches.index, text: matches[0] }
148-
};
149-
}
150-
151143
export function detectLinkSuffixes(line: string): ILinkSuffix[] {
152144
// Find all suffixes on the line. Since the regex global flag is used, lastIndex will be updated
153145
// in place such that there are no overlapping matches.
@@ -164,20 +156,35 @@ export function detectLinkSuffixes(line: string): ILinkSuffix[] {
164156
return results;
165157
}
166158

159+
/**
160+
* Returns the optional link suffix which contains line and column information.
161+
* @param link The link to parse.
162+
*/
163+
export function getLinkSuffix(link: string): ILinkSuffix | null {
164+
return toLinkSuffix(linkSuffixRegexEol.value.exec(link));
165+
}
166+
167167
export function toLinkSuffix(match: RegExpExecArray | null): ILinkSuffix | null {
168168
const groups = match?.groups;
169169
if (!groups || match.length < 1) {
170170
return null;
171171
}
172-
const rowString = groups.row0 || groups.row1 || groups.row2;
173-
const colString = groups.col0 || groups.col1 || groups.col2;
174172
return {
175-
row: rowString !== undefined ? parseInt(rowString) : undefined,
176-
col: colString !== undefined ? parseInt(colString) : undefined,
173+
row: parseIntOptional(groups.row0 || groups.row1 || groups.row2),
174+
col: parseIntOptional(groups.col0 || groups.col1 || groups.col2),
175+
rowEnd: parseIntOptional(groups.rowEnd0 || groups.rowEnd1 || groups.rowEnd2),
176+
colEnd: parseIntOptional(groups.colEnd0 || groups.colEnd1 || groups.colEnd2),
177177
suffix: { index: match.index, text: match[0] }
178178
};
179179
}
180180

181+
function parseIntOptional(value: string | undefined): number | undefined {
182+
if (value === undefined) {
183+
return value;
184+
}
185+
return parseInt(value);
186+
}
187+
181188
// This defines valid path characters for a link with a suffix, the first `[]` of the regex includes
182189
// characters the path is not allowed to _start_ with, the second `[]` includes characters not
183190
// allowed at all in the path. If the characters show up in both regexes the link will stop at that

‎src/vs/workbench/contrib/terminalContrib/links/test/browser/terminalLinkOpeners.test.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,9 @@ suite('Workbench - TerminalLinkOpeners', () => {
332332
source: 'editor',
333333
selection: {
334334
startColumn: 5,
335-
startLineNumber: 10
335+
startLineNumber: 10,
336+
endColumn: undefined,
337+
endLineNumber: undefined
336338
},
337339
});
338340
});
@@ -421,7 +423,9 @@ suite('Workbench - TerminalLinkOpeners', () => {
421423
source: 'editor',
422424
selection: {
423425
startColumn: 5,
424-
startLineNumber: 10
426+
startLineNumber: 10,
427+
endColumn: undefined,
428+
endLineNumber: undefined
425429
},
426430
});
427431
await opener.open({
@@ -434,7 +438,9 @@ suite('Workbench - TerminalLinkOpeners', () => {
434438
source: 'editor',
435439
selection: {
436440
startColumn: 5,
437-
startLineNumber: 10
441+
startLineNumber: 10,
442+
endColumn: undefined,
443+
endLineNumber: undefined
438444
},
439445
});
440446
});

‎src/vs/workbench/contrib/terminalContrib/links/test/browser/terminalLinkParsing.test.ts‎

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,11 @@ interface ITestLink {
1111
link: string;
1212
prefix: string | undefined;
1313
suffix: string | undefined;
14+
// TODO: These has vars would be nicer as a flags enum
1415
hasRow: boolean;
1516
hasCol: boolean;
17+
hasRowEnd?: boolean;
18+
hasColEnd?: boolean;
1619
}
1720

1821
const operatingSystems: ReadonlyArray<OperatingSystem> = [
@@ -33,6 +36,8 @@ const osLabel: { [key: number | OperatingSystem]: string } = {
3336

3437
const testRow = 339;
3538
const testCol = 12;
39+
const testRowEnd = 341;
40+
const testColEnd = 14;
3641
const testLinks: ITestLink[] = [
3742
// Simple
3843
{ link: 'foo', prefix: undefined, suffix: undefined, hasRow: false, hasCol: false },
@@ -117,8 +122,9 @@ const testLinks: ITestLink[] = [
117122

118123
// OCaml-style
119124
{ link: '"foo", line 339, character 12', prefix: '"', suffix: '", line 339, character 12', hasRow: true, hasCol: true },
120-
{ link: '"foo", line 339, characters 12-13', prefix: '"', suffix: '", line 339, characters 12-13', hasRow: true, hasCol: true },
121-
{ link: '"foo", lines 339-340', prefix: '"', suffix: '", lines 339-340', hasRow: true, hasCol: false },
125+
{ link: '"foo", line 339, characters 12-14', prefix: '"', suffix: '", line 339, characters 12-14', hasRow: true, hasCol: true, hasColEnd: true },
126+
{ link: '"foo", lines 339-341', prefix: '"', suffix: '", lines 339-341', hasRow: true, hasCol: false, hasRowEnd: true },
127+
{ link: '"foo", lines 339-341, characters 12-14', prefix: '"', suffix: '", lines 339-341, characters 12-14', hasRow: true, hasCol: true, hasRowEnd: true, hasColEnd: true },
122128

123129
// Non-breaking space
124130
{ link: 'foo\u00A0339:12', prefix: undefined, suffix: '\u00A0339:12', hasRow: true, hasCol: true },
@@ -148,6 +154,8 @@ suite('TerminalLinkParsing', () => {
148154
testLink.suffix === undefined ? null : {
149155
row: testLink.hasRow ? testRow : undefined,
150156
col: testLink.hasCol ? testCol : undefined,
157+
rowEnd: testLink.hasRowEnd ? testRowEnd : undefined,
158+
colEnd: testLink.hasColEnd ? testColEnd : undefined,
151159
suffix: {
152160
index: testLink.link.length - testLink.suffix.length,
153161
text: testLink.suffix
@@ -165,6 +173,8 @@ suite('TerminalLinkParsing', () => {
165173
testLink.suffix === undefined ? [] : [{
166174
row: testLink.hasRow ? testRow : undefined,
167175
col: testLink.hasCol ? testCol : undefined,
176+
rowEnd: testLink.hasRowEnd ? testRowEnd : undefined,
177+
colEnd: testLink.hasColEnd ? testColEnd : undefined,
168178
suffix: {
169179
index: testLink.link.length - testLink.suffix.length,
170180
text: testLink.suffix
@@ -181,6 +191,8 @@ suite('TerminalLinkParsing', () => {
181191
{
182192
col: 2,
183193
row: 1,
194+
rowEnd: undefined,
195+
colEnd: undefined,
184196
suffix: {
185197
index: 3,
186198
text: '(1, 2)'
@@ -189,6 +201,8 @@ suite('TerminalLinkParsing', () => {
189201
{
190202
col: 4,
191203
row: 3,
204+
rowEnd: undefined,
205+
colEnd: undefined,
192206
suffix: {
193207
index: 13,
194208
text: '[3, 4]'
@@ -197,6 +211,8 @@ suite('TerminalLinkParsing', () => {
197211
{
198212
col: undefined,
199213
row: 5,
214+
rowEnd: undefined,
215+
colEnd: undefined,
200216
suffix: {
201217
index: 23,
202218
text: ' on line 5'
@@ -233,6 +249,8 @@ suite('TerminalLinkParsing', () => {
233249
suffix: {
234250
col: 2,
235251
row: 1,
252+
rowEnd: undefined,
253+
colEnd: undefined,
236254
suffix: {
237255
index: 3,
238256
text: '(1, 2)'
@@ -248,6 +266,8 @@ suite('TerminalLinkParsing', () => {
248266
suffix: {
249267
col: 4,
250268
row: 3,
269+
rowEnd: undefined,
270+
colEnd: undefined,
251271
suffix: {
252272
index: 13,
253273
text: '[3, 4]'
@@ -266,6 +286,8 @@ suite('TerminalLinkParsing', () => {
266286
suffix: {
267287
col: undefined,
268288
row: 5,
289+
rowEnd: undefined,
290+
colEnd: undefined,
269291
suffix: {
270292
index: 24,
271293
text: '" on line 5'
@@ -292,6 +314,8 @@ suite('TerminalLinkParsing', () => {
292314
suffix: {
293315
row: 5,
294316
col: 6,
317+
rowEnd: undefined,
318+
colEnd: undefined,
295319
suffix: {
296320
index: 4,
297321
text: '", line 5, col 6'
@@ -318,6 +342,8 @@ suite('TerminalLinkParsing', () => {
318342
suffix: {
319343
row: 5,
320344
col: 6,
345+
rowEnd: undefined,
346+
colEnd: undefined,
321347
suffix: {
322348
index: 10,
323349
text: '", line 5, col 6'
@@ -353,6 +379,8 @@ suite('TerminalLinkParsing', () => {
353379
suffix: {
354380
row: 5,
355381
col: 6,
382+
rowEnd: undefined,
383+
colEnd: undefined,
356384
suffix: {
357385
index: 41,
358386
text: '", line 5, col 6'
@@ -392,6 +420,8 @@ suite('TerminalLinkParsing', () => {
392420
suffix: {
393421
col: undefined,
394422
row: 400,
423+
rowEnd: undefined,
424+
colEnd: undefined,
395425
suffix: {
396426
index: 27,
397427
text: ':400'
@@ -446,6 +476,8 @@ suite('TerminalLinkParsing', () => {
446476
suffix: {
447477
col: undefined,
448478
row: 400,
479+
rowEnd: undefined,
480+
colEnd: undefined,
449481
suffix: {
450482
index: 1 + osTestPath[os].length,
451483
text: ':400'
@@ -466,6 +498,8 @@ suite('TerminalLinkParsing', () => {
466498
suffix: {
467499
col: undefined,
468500
row: 400,
501+
rowEnd: undefined,
502+
colEnd: undefined,
469503
suffix: {
470504
index: 1 + osTestPath[os].length,
471505
text: ':400'
@@ -573,7 +607,7 @@ suite('TerminalLinkParsing', () => {
573607
const link2 = testLinksWithSuffix[i + 1];
574608
const link3 = testLinksWithSuffix[i + 2];
575609
const line = ` ${link1.link} ${link2.link} ${link3.link} `;
576-
test('`' + line + '`', () => {
610+
test('`' + line.replaceAll('\u00A0', '<nbsp>') + '`', () => {
577611
strictEqual(detectLinks(line, OperatingSystem.Linux).length, 3);
578612
ok(link1.suffix);
579613
ok(link2.suffix);
@@ -590,6 +624,8 @@ suite('TerminalLinkParsing', () => {
590624
suffix: {
591625
row: link1.hasRow ? testRow : undefined,
592626
col: link1.hasCol ? testCol : undefined,
627+
rowEnd: link1.hasRowEnd ? testRowEnd : undefined,
628+
colEnd: link1.hasColEnd ? testColEnd : undefined,
593629
suffix: {
594630
index: 1 + (link1.link.length - link1.suffix.length),
595631
text: link1.suffix
@@ -608,6 +644,8 @@ suite('TerminalLinkParsing', () => {
608644
suffix: {
609645
row: link2.hasRow ? testRow : undefined,
610646
col: link2.hasCol ? testCol : undefined,
647+
rowEnd: link2.hasRowEnd ? testRowEnd : undefined,
648+
colEnd: link2.hasColEnd ? testColEnd : undefined,
611649
suffix: {
612650
index: (detectedLink1.prefix?.index ?? detectedLink1.path.index) + link1.link.length + 1 + (link2.link.length - link2.suffix.length),
613651
text: link2.suffix
@@ -626,6 +664,8 @@ suite('TerminalLinkParsing', () => {
626664
suffix: {
627665
row: link3.hasRow ? testRow : undefined,
628666
col: link3.hasCol ? testCol : undefined,
667+
rowEnd: link3.hasRowEnd ? testRowEnd : undefined,
668+
colEnd: link3.hasColEnd ? testColEnd : undefined,
629669
suffix: {
630670
index: (detectedLink2.prefix?.index ?? detectedLink2.path.index) + link2.link.length + 1 + (link3.link.length - link3.suffix.length),
631671
text: link3.suffix

0 commit comments

Comments
 (0)