Skip to content

Commit bf3b6b4

Browse files
nzaytsevnzaytsev
committed
Improves branch autolinks experiments
1 parent 9bbeecb commit bf3b6b4

File tree

2 files changed

+134
-26
lines changed

2 files changed

+134
-26
lines changed

‎src/autolinks/__tests__/autolinks.test.ts‎

Lines changed: 52 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ const mockRefSets = (prefixes: string[] = ['']): RefSet[] =>
1313
ignoreCase: false,
1414
prefix: prefix,
1515
title: 'test',
16-
url: 'test/<num>',
16+
url: '<num>',
1717
description: 'test',
1818
},
1919
],
@@ -25,27 +25,65 @@ function assertAutolinks(actual: Map<string, Autolink>, expected: Array<string>)
2525

2626
suite('Autolinks Test Suite', () => {
2727
test('Branch name autolinks', () => {
28-
assertAutolinks(getBranchAutolinks('123', mockRefSets()), ['test/123']);
29-
assertAutolinks(getBranchAutolinks('feature/123', mockRefSets()), ['test/123']);
30-
assertAutolinks(getBranchAutolinks('feature/PRE-123', mockRefSets()), ['test/123']);
31-
assertAutolinks(getBranchAutolinks('123.2', mockRefSets()), ['test/123', 'test/2']);
28+
assertAutolinks(getBranchAutolinks('123', mockRefSets()), ['123']);
29+
assertAutolinks(getBranchAutolinks('feature/123', mockRefSets()), ['123']);
30+
assertAutolinks(getBranchAutolinks('feature/PRE-123', mockRefSets()), ['123']);
31+
assertAutolinks(getBranchAutolinks('123.2', mockRefSets()), ['123']);
3232
assertAutolinks(getBranchAutolinks('123', mockRefSets(['PRE-'])), []);
3333
assertAutolinks(getBranchAutolinks('feature/123', mockRefSets(['PRE-'])), []);
34-
assertAutolinks(getBranchAutolinks('feature/2-fa/123', mockRefSets([''])), ['test/123', 'test/2']);
35-
assertAutolinks(getBranchAutolinks('feature/2-fa/123', mockRefSets([''])), ['test/123', 'test/2']);
36-
// incorrectly solved cat worths to compare the blocks length so that the less block size (without possible link) is more likely a link
37-
assertAutolinks(getBranchAutolinks('feature/2-fa/3', mockRefSets([''])), ['test/2', 'test/3']);
38-
assertAutolinks(getBranchAutolinks('feature/PRE-123', mockRefSets(['PRE-'])), ['test/123']);
39-
assertAutolinks(getBranchAutolinks('feature/PRE-123.2', mockRefSets(['PRE-'])), ['test/123']);
40-
assertAutolinks(getBranchAutolinks('feature/3-123-PRE-123', mockRefSets(['PRE-'])), ['test/123']);
34+
assertAutolinks(getBranchAutolinks('feature/2-fa/123', mockRefSets([''])), ['123', '2']);
35+
assertAutolinks(getBranchAutolinks('feature/2-fa/123', mockRefSets([''])), ['123', '2']);
36+
assertAutolinks(getBranchAutolinks('feature/2-fa/3', mockRefSets([''])), ['3', '2']);
37+
assertAutolinks(getBranchAutolinks('feature/PRE-123', mockRefSets(['PRE-'])), ['123']);
38+
assertAutolinks(getBranchAutolinks('feature/PRE-123.2', mockRefSets(['PRE-'])), ['123']);
39+
assertAutolinks(getBranchAutolinks('feature/3-123-PRE-123', mockRefSets(['PRE-'])), ['123']);
4140
assertAutolinks(
4241
getBranchAutolinks('feature/3-123-PRE-123', mockRefSets(['', 'PRE-'])),
4342

44-
['test/123', 'test/3'],
43+
['123', '3'],
4544
);
4645
});
4746

4847
test('Commit message autolinks', () => {
49-
assertAutolinks(getAutolinks('test message 123 sd', mockRefSets()), ['test/123']);
48+
assertAutolinks(getAutolinks('test message 123 sd', mockRefSets()), ['123']);
49+
});
50+
51+
/**
52+
* 16.1.1^ - improved branch name autolinks matching
53+
*/
54+
test('Improved branch name autolinks matching', () => {
55+
// skip branch names chunks matching '^release(?=(-(?<number-chunk>))$` or other release-like values
56+
// skip pair in case of double chunk
57+
assertAutolinks(getBranchAutolinks('folder/release/16/issue-1', mockRefSets([''])), ['1']);
58+
assertAutolinks(getBranchAutolinks('folder/release/16.1/issue-1', mockRefSets([''])), ['1']);
59+
assertAutolinks(getBranchAutolinks('folder/release/16.1.1/1', mockRefSets([''])), ['1']);
60+
// skip one in case of single chunk
61+
assertAutolinks(getBranchAutolinks('folder/release-16/1', mockRefSets([''])), ['1']);
62+
assertAutolinks(getBranchAutolinks('folder/release-16.1/1', mockRefSets([''])), ['1']);
63+
assertAutolinks(getBranchAutolinks('folder/release-16.1.2/1', mockRefSets([''])), ['1']);
64+
65+
/**
66+
* Added chunk matching logic for non-prefixed numbers:
67+
* - XX - is more likely issue number
68+
* - XX.XX - is less likely issue number, but still possible
69+
* - XX.XX.XX - is more likely not issue number
70+
*/
71+
assertAutolinks(getBranchAutolinks('some-issue-in-release-2024', mockRefSets([''])), ['2024']);
72+
assertAutolinks(getBranchAutolinks('some-issue-in-release-2024.1', mockRefSets([''])), ['2024']);
73+
assertAutolinks(getBranchAutolinks('some-issue-in-release-2024.1.1', mockRefSets([''])), []);
74+
75+
assertAutolinks(getBranchAutolinks('folder/release-notes-16-1', mockRefSets([''])), ['16']);
76+
assertAutolinks(getBranchAutolinks('folder/16-1-release-notes', mockRefSets([''])), ['16']);
77+
78+
// considered the distance from the edges of the chunk as a priority sign
79+
assertAutolinks(getBranchAutolinks('folder/16-content-1-content', mockRefSets([''])), ['16', '1']);
80+
assertAutolinks(getBranchAutolinks('folder/content-1-content-16', mockRefSets([''])), ['16', '1']);
81+
82+
// the chunk that is more close to the end is more likely actual issue number
83+
assertAutolinks(getBranchAutolinks('1-epic-folder/10-issue/100-subissue', mockRefSets([''])), [
84+
'100',
85+
'10',
86+
'1',
87+
]);
5088
});
5189
});

‎src/autolinks/autolinks.utils.ts‎

Lines changed: 82 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { IssueIntegrationId } from '../constants.integrations';
22
import type { IssueOrPullRequest } from '../git/models/issue';
33
import type { ProviderReference } from '../git/models/remoteProvider';
44
import type { ResourceDescriptor } from '../plus/integrations/integration';
5+
import { flatMap } from '../system/iterable';
56
import { escapeMarkdown } from '../system/markdown';
67
import type { MaybePausedResult } from '../system/promise';
78
import { encodeHtmlWeak, escapeRegex } from '../system/string';
@@ -30,6 +31,7 @@ export interface Autolink extends AutolinkReference {
3031
provider?: ProviderReference;
3132
id: string;
3233
index?: number;
34+
priority?: string;
3335

3436
tokenize?:
3537
| ((
@@ -129,16 +131,24 @@ export type RefSet = [
129131
* @returns non-0 result that means a probability of the autolink `b` is more relevant of the autolink `a`
130132
*/
131133
function compareAutolinks(a: Autolink, b: Autolink): number {
134+
if (b.prefix.length - a.prefix.length) {
135+
return b.prefix.length - a.prefix.length;
136+
}
137+
if (a.priority || b.priority) {
138+
if ((b.priority ?? '') > (a.priority ?? '')) {
139+
return 1;
140+
}
141+
if ((b.priority ?? '') < (a.priority ?? '')) {
142+
return -1;
143+
}
144+
return 0;
145+
}
132146
// consider that if the number is in the start, it's the most relevant link
133147
if (b.index === 0) return 1;
134148
if (a.index === 0) return -1;
135149

136150
// maybe it worths to use some weight function instead.
137-
return (
138-
b.prefix.length - a.prefix.length ||
139-
b.id.length - a.id.length ||
140-
(b.index != null && a.index != null ? -(b.index - a.index) : 0)
141-
);
151+
return b.id.length - a.id.length || (b.index != null && a.index != null ? -(b.index - a.index) : 0);
142152
}
143153

144154
function ensureCachedRegex(
@@ -173,12 +183,17 @@ function ensureCachedRegex(ref: CacheableAutolinkReference, outputFormat: 'html'
173183
`(^|\\s|\\(|\\[|\\{)(${escapeRegex(ref.prefix)}(${ref.alphanumeric ? '\\w' : '\\d'}+))\\b`,
174184
ref.ignoreCase ? 'gi' : 'g',
175185
);
176-
ref.branchNameRegex = new RegExp(
177-
`(^|\\-|_|\\.|\\/)(?<prefix>${ref.prefix})(?<issueKeyNumber>${
178-
ref.alphanumeric ? '\\w' : '\\d'
179-
}+)(?=$|\\-|_|\\.|\\/)`,
180-
'gi',
181-
);
186+
if (!ref.prefix && !ref.alphanumeric) {
187+
ref.branchNameRegex =
188+
/(?<numberChunkBeginning>^|\/|-|_)(?<numberChunk>(?<issueKeyNumber>\d+)(((-|\.|_)\d+){0,1}))(?<numberChunkEnding>$|\/|-|_)/gi;
189+
} else {
190+
ref.branchNameRegex = new RegExp(
191+
`(^|\\-|_|\\.|\\/)(?<prefix>${ref.prefix})(?<issueKeyNumber>${
192+
ref.alphanumeric ? '\\w' : '\\d'
193+
}+)(?=$|\\-|_|\\.|\\/)`,
194+
'gi',
195+
);
196+
}
182197
}
183198

184199
return true;
@@ -230,6 +245,22 @@ export function getAutolinks(message: string, refsets: Readonly<RefSet[]>) {
230245
return autolinks;
231246
}
232247

248+
function calculatePriority(
249+
input: string,
250+
issueKey: string,
251+
numberGroup: string,
252+
index: number,
253+
chunkIndex: number = 0,
254+
): string {
255+
const edgeDistance = Math.min(index, input.length - index + numberGroup.length - 1);
256+
const isSingleNumber = issueKey === numberGroup;
257+
return `
258+
${String.fromCharCode('a'.charCodeAt(0) + chunkIndex)}:
259+
${String.fromCharCode('a'.charCodeAt(0) - edgeDistance)}:
260+
${String.fromCharCode('a'.charCodeAt(0) + Number(isSingleNumber))}
261+
`;
262+
}
263+
233264
export function getBranchAutolinks(branchName: string, refsets: Readonly<RefSet[]>) {
234265
const autolinks = new Map<string, Autolink>();
235266

@@ -246,7 +277,30 @@ export function getBranchAutolinks(branchName: string, refsets: Readonly<RefSet[
246277
}
247278

248279
ensureCachedRegex(ref, 'plaintext');
249-
const matches = branchName.matchAll(ref.branchNameRegex);
280+
let chunks = [branchName];
281+
const nonPrefixedRef = !ref.prefix && !ref.alphanumeric;
282+
if (nonPrefixedRef) {
283+
chunks = branchName.split('/');
284+
}
285+
let chunkIndex = 0;
286+
const chunkMap = new Map<string, number>();
287+
let skip = false;
288+
const matches = flatMap(chunks, chunk => {
289+
const releaseMatch = /^release(s?)((?<releaseNum>-[\d.-]+)?)$/gm.exec(chunk);
290+
if (releaseMatch) {
291+
if (!releaseMatch.groups?.releaseNum) {
292+
skip = true;
293+
}
294+
return [];
295+
}
296+
if (skip) {
297+
skip = false;
298+
return [];
299+
}
300+
const match = chunk.matchAll(ref.branchNameRegex);
301+
chunkMap.set(chunk, chunkIndex++);
302+
return match;
303+
});
250304
do {
251305
match = matches.next();
252306
if (!match.value?.groups) break;
@@ -259,12 +313,28 @@ export function getBranchAutolinks(branchName: string, refsets: Readonly<RefSet[
259313
if (existingIndex != null) {
260314
index = Math.min(index, existingIndex);
261315
}
316+
console.log(
317+
JSON.stringify(match.value),
318+
match.value.groups.numberChunk,
319+
match.value.groups.numberChunkBeginning,
320+
match.value.input,
321+
match.value.groups.issueKeyNumber,
322+
);
262323
autolinks.set(linkUrl, {
263324
...ref,
264325
provider: provider,
265326
id: num,
266327
index: index,
267328
url: linkUrl,
329+
priority: nonPrefixedRef
330+
? calculatePriority(
331+
match.value.input,
332+
num,
333+
match.value.groups.numberChunk,
334+
index,
335+
chunkMap.get(match.value.input),
336+
)
337+
: undefined,
268338
title: ref.title?.replace(numRegex, num),
269339
description: ref.description?.replace(numRegex, num),
270340
descriptor: ref.descriptor,

0 commit comments

Comments
 (0)