Skip to content

Commit 625e1ab

Browse files
committed
Improves branch autolinks
1 parent 6daf177 commit 625e1ab

File tree

5 files changed

+148
-62
lines changed

5 files changed

+148
-62
lines changed

ThirdPartyNotices.txt

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ This project incorporates components from the projects listed below.
3232
27. react version 16.8.4 (https://github.com/facebook/react)
3333
28. signal-utils version 0.20.0 (https://github.com/proposal-signals/signal-utils)
3434
29. slug version 10.0.0 (https://github.com/Trott/slug)
35-
30. sortablejs version 1.15.0 (https://github.com/SortableJS/Sortable)
35+
30. slugify version 1.6.6 (https://github.com/simov/slugify)
36+
31. sortablejs version 1.15.0 (https://github.com/SortableJS/Sortable)
3637

3738
%% @gk-nzaytsev/fast-string-truncated-width NOTICES AND INFORMATION BEGIN HERE
3839
=========================================
@@ -2219,6 +2220,33 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
22192220
=========================================
22202221
END OF slug NOTICES AND INFORMATION
22212222

2223+
%% slugify NOTICES AND INFORMATION BEGIN HERE
2224+
=========================================
2225+
The MIT License (MIT)
2226+
2227+
Copyright (c) Simeon Velichkov <simeonvelichkov@gmail.com>
2228+
2229+
Permission is hereby granted, free of charge, to any person obtaining a copy
2230+
of this software and associated documentation files (the "Software"), to deal
2231+
in the Software without restriction, including without limitation the rights
2232+
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
2233+
copies of the Software, and to permit persons to whom the Software is
2234+
furnished to do so, subject to the following conditions:
2235+
2236+
The above copyright notice and this permission notice shall be included in all
2237+
copies or substantial portions of the Software.
2238+
2239+
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
2240+
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
2241+
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
2242+
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
2243+
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
2244+
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
2245+
SOFTWARE.
2246+
2247+
=========================================
2248+
END OF slugify NOTICES AND INFORMATION
2249+
22222250
%% sortablejs NOTICES AND INFORMATION BEGIN HERE
22232251
=========================================
22242252
MIT License

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20030,6 +20030,7 @@
2003020030
"react-dom": "16.8.4",
2003120031
"signal-utils": "0.20.0",
2003220032
"slug": "10.0.0",
20033+
"slugify": "^1.6.6",
2003320034
"sortablejs": "1.15.0"
2003420035
},
2003520036
"devDependencies": {

pnpm-lock.yaml

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/autolinks/__tests__/autolinks.test.ts

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,17 @@ import * as assert from 'assert';
22
import { suite, test } from 'mocha';
33
import { map } from '../../system/iterable';
44
import type { Autolink, RefSet } from '../autolinks.utils';
5-
import { getAutolinks, getBranchAutolinks } from '../autolinks.utils';
5+
import { calculatePriority, getAutolinks, getBranchAutolinks } from '../autolinks.utils';
66

7-
const mockRefSets = (prefixes: string[] = ['']): RefSet[] =>
7+
const mockRefSets = (prefixes: string[] = [''], title = 'test'): RefSet[] =>
88
prefixes.map(prefix => [
99
{ domain: 'test', icon: '1', id: '1', name: 'test' },
1010
[
1111
{
1212
alphanumeric: false,
1313
ignoreCase: false,
1414
prefix: prefix,
15-
title: 'test',
15+
title: title,
1616
url: '<num>',
1717
description: 'test',
1818
},
@@ -48,6 +48,24 @@ suite('Autolinks Test Suite', () => {
4848
assertAutolinks(getAutolinks('test message 123 sd', mockRefSets()), ['123']);
4949
});
5050

51+
test('Test autolink priority comparation', () => {
52+
assert.equal(
53+
calculatePriority('1', 0, '1', 1) > calculatePriority('1', 0, '1', 0) || 'bigger chunk fall',
54+
true,
55+
);
56+
assert.equal(
57+
calculatePriority('1', 0, '1', 2) > calculatePriority('1', 1, '1', 0) || 'less edge distance fall',
58+
true,
59+
);
60+
assert.equal(
61+
calculatePriority('1', 0, '1', 2) > calculatePriority('1', 0, '1.1', 2) || 'single number fall',
62+
true,
63+
);
64+
assert.equal(
65+
calculatePriority('2', 0, '2', 2) > calculatePriority('1', 0, '1', 2) || 'bigger number fall',
66+
true,
67+
);
68+
});
5169
/**
5270
* 16.1.1^ - improved branch name autolinks matching
5371
*/
@@ -57,7 +75,14 @@ suite('Autolinks Test Suite', () => {
5775
assertAutolinks(getBranchAutolinks('folder/release/16/issue-1', mockRefSets([''])), ['1']);
5876
assertAutolinks(getBranchAutolinks('folder/release/16.1/issue-1', mockRefSets([''])), ['1']);
5977
assertAutolinks(getBranchAutolinks('folder/release/16.1.1/1', mockRefSets([''])), ['1']);
60-
// skip one in case of single chunk
78+
assertAutolinks(getBranchAutolinks('release-2024', mockRefSets([''])), []);
79+
assertAutolinks(getBranchAutolinks('v-2024', mockRefSets([''])), []);
80+
assertAutolinks(getBranchAutolinks('v2024', mockRefSets([''])), []);
81+
// cannot be definitely handled
82+
assertAutolinks(getBranchAutolinks('some-issue-in-release-2024', mockRefSets([''])), ['2024']);
83+
assertAutolinks(getBranchAutolinks('folder/release-notes-16-1', mockRefSets([''])), ['16']);
84+
assertAutolinks(getBranchAutolinks('folder/16-1-release-notes', mockRefSets([''])), ['16']);
85+
// skip next in case of single chunk
6186
assertAutolinks(getBranchAutolinks('folder/release-16/1', mockRefSets([''])), ['1']);
6287
assertAutolinks(getBranchAutolinks('folder/release-16.1/1', mockRefSets([''])), ['1']);
6388
assertAutolinks(getBranchAutolinks('folder/release-16.1.2/1', mockRefSets([''])), ['1']);
@@ -66,24 +91,33 @@ suite('Autolinks Test Suite', () => {
6691
* Added chunk matching logic for non-prefixed numbers:
6792
* - XX - is more likely issue number
6893
* - XX.XX - is less likely issue number, but still possible
69-
* - XX.XX.XX - is more likely not issue number
94+
* - XX.XX.XX - is more likely not issue number: seems like a date or version number
7095
*/
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']);
96+
assertAutolinks(getBranchAutolinks('issue-2024', mockRefSets([''])), ['2024']);
97+
assertAutolinks(getBranchAutolinks('issue-2024.1', mockRefSets([''])), ['2024']);
98+
assertAutolinks(getBranchAutolinks('issue-2024.1.1', mockRefSets([''])), []);
8199

82100
// the chunk that is more close to the end is more likely actual issue number
83101
assertAutolinks(getBranchAutolinks('1-epic-folder/10-issue/100-subissue', mockRefSets([''])), [
84102
'100',
85103
'10',
86104
'1',
87105
]);
106+
107+
// ignore numbers from title
108+
assertAutolinks(getBranchAutolinks('folder/100-content-content-16', mockRefSets([''], '100-content-content')), [
109+
'16',
110+
]);
111+
assertAutolinks(getBranchAutolinks('folder/100-content-content-16', mockRefSets([''], 'content-content-16')), [
112+
'100',
113+
]);
114+
115+
// consider edge distance and issue key length to sort
116+
assertAutolinks(getBranchAutolinks('2-some-issue-in-release-2024', mockRefSets([''])), ['2024', '2']);
117+
assertAutolinks(getBranchAutolinks('2024-some-issue-in-release-2', mockRefSets([''])), ['2024', '2']);
118+
assertAutolinks(getBranchAutolinks('some-2-issue-in-release-2024', mockRefSets([''])), ['2024', '2']);
119+
assertAutolinks(getBranchAutolinks('4048-issue-in-release-2024.1', mockRefSets([''])), ['4048', '2024']);
120+
// less numbers - more likely issue key
121+
assertAutolinks(getBranchAutolinks('1-issue-in-release-2024.1', mockRefSets([''])), ['1', '2024']);
88122
});
89123
});

src/autolinks/autolinks.utils.ts

Lines changed: 66 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
import slugify from 'slugify';
12
import { IssueIntegrationId } from '../constants.integrations';
23
import type { IssueOrPullRequest } from '../git/models/issue';
34
import type { ProviderReference } from '../git/models/remoteProvider';
45
import type { ResourceDescriptor } from '../plus/integrations/integration';
5-
import { flatMap } from '../system/iterable';
6+
import { flatMap, forEach } from '../system/iterable';
67
import { escapeMarkdown } from '../system/markdown';
78
import type { MaybePausedResult } from '../system/promise';
89
import { encodeHtmlWeak, escapeRegex } from '../system/string';
@@ -117,7 +118,7 @@ export function isDynamic(ref: AutolinkReference | DynamicAutolinkReference): re
117118
return !('prefix' in ref) && !('url' in ref);
118119
}
119120

120-
function isCacheable(ref: AutolinkReference | DynamicAutolinkReference): ref is CacheableAutolinkReference {
121+
export function isCacheable(ref: AutolinkReference | DynamicAutolinkReference): ref is CacheableAutolinkReference {
121122
return 'prefix' in ref && ref.prefix != null && 'url' in ref && ref.url != null;
122123
}
123124

@@ -131,17 +132,18 @@ export type RefSet = [
131132
* @returns non-0 result that means a probability of the autolink `b` is more relevant of the autolink `a`
132133
*/
133134
function compareAutolinks(a: Autolink, b: Autolink): number {
135+
// believe that link with prefix is definitely more relevant that just a number
134136
if (b.prefix.length - a.prefix.length) {
135137
return b.prefix.length - a.prefix.length;
136138
}
139+
// if custom priority provided, let's consider it first
137140
if (a.priority || b.priority) {
138141
if ((b.priority ?? '') > (a.priority ?? '')) {
139142
return 1;
140143
}
141144
if ((b.priority ?? '') < (a.priority ?? '')) {
142145
return -1;
143146
}
144-
return 0;
145147
}
146148
// consider that if the number is in the start, it's the most relevant link
147149
if (b.index === 0) return 1;
@@ -184,6 +186,7 @@ function ensureCachedRegex(ref: CacheableAutolinkReference, outputFormat: 'html'
184186
ref.ignoreCase ? 'gi' : 'g',
185187
);
186188
if (!ref.prefix && !ref.alphanumeric) {
189+
// use a different regex for non-prefixed refs
187190
ref.branchNameRegex =
188191
/(?<numberChunkBeginning>^|\/|-|_)(?<numberChunk>(?<issueKeyNumber>\d+)(((-|\.|_)\d+){0,1}))(?<numberChunkEnding>$|\/|-|_)/gi;
189192
} else {
@@ -245,27 +248,26 @@ export function getAutolinks(message: string, refsets: Readonly<RefSet[]>) {
245248
return autolinks;
246249
}
247250

248-
function calculatePriority(
249-
input: string,
251+
/** returns lexicographic priority value ready to sort */
252+
export function calculatePriority(
250253
issueKey: string,
254+
edgeDistance: number,
251255
numberGroup: string,
252-
index: number,
253256
chunkIndex: number = 0,
254257
): string {
255-
const edgeDistance = Math.min(index, input.length - index + numberGroup.length - 1);
256258
const isSingleNumber = issueKey === numberGroup;
257259
return `
258-
${String.fromCharCode('a'.charCodeAt(0) + chunkIndex)}:
259-
${String.fromCharCode('a'.charCodeAt(0) - edgeDistance)}:
260-
${String.fromCharCode('a'.charCodeAt(0) + Number(isSingleNumber))}
260+
${String.fromCharCode('a'.charCodeAt(0) + chunkIndex)}:\
261+
${String.fromCharCode('a'.charCodeAt(0) - edgeDistance)}:\
262+
${String.fromCharCode('a'.charCodeAt(0) + Number(isSingleNumber))}:\
263+
${String.fromCharCode('a'.charCodeAt(0) + Number(issueKey))}
261264
`;
262265
}
263266

264267
export function getBranchAutolinks(branchName: string, refsets: Readonly<RefSet[]>) {
265268
const autolinks = new Map<string, Autolink>();
266269

267270
let match;
268-
let num;
269271
for (const [provider, refs] of refsets) {
270272
for (const ref of refs) {
271273
if (
@@ -278,16 +280,19 @@ export function getBranchAutolinks(branchName: string, refsets: Readonly<RefSet[
278280

279281
ensureCachedRegex(ref, 'plaintext');
280282
let chunks = [branchName];
283+
// use more complex logic for refs with no prefix
281284
const nonPrefixedRef = !ref.prefix && !ref.alphanumeric;
282285
if (nonPrefixedRef) {
283286
chunks = branchName.split('/');
284287
}
285288
let chunkIndex = 0;
286-
const chunkMap = new Map<string, number>();
289+
const chunkIndexMap = new Map<string, number>();
287290
let skip = false;
288-
const matches = flatMap(chunks, chunk => {
289-
const releaseMatch = /^release(s?)((?<releaseNum>-[\d.-]+)?)$/gm.exec(chunk);
291+
// know chunk indexes, skip release-like chunks or chunk pairs like release-1 or release/1
292+
let matches: IterableIterator<RegExpExecArray> | undefined = flatMap(chunks, chunk => {
293+
const releaseMatch = /^(v|ver?|versions?|releases?)((?<releaseNum>[\d.-]+)?)$/gm.exec(chunk);
290294
if (releaseMatch) {
295+
// number in the next chunk should be ignored
291296
if (!releaseMatch.groups?.releaseNum) {
292297
skip = true;
293298
}
@@ -298,48 +303,63 @@ export function getBranchAutolinks(branchName: string, refsets: Readonly<RefSet[
298303
return [];
299304
}
300305
const match = chunk.matchAll(ref.branchNameRegex);
301-
chunkMap.set(chunk, chunkIndex++);
306+
chunkIndexMap.set(chunk, chunkIndex++);
302307
return match;
303308
});
309+
/** additional matches list to skip numbers that are mentioned inside the ref title */
310+
const refTitlesMatches: IterableIterator<RegExpExecArray>[] = [];
311+
/** indicates that we should remove any matched link from the map */
312+
let unwanted = false;
304313
do {
305-
match = matches.next();
306-
if (!match.value?.groups) break;
314+
match = matches?.next();
315+
if (match?.done && refTitlesMatches.length) {
316+
// check ref titles on unwanted matches
317+
matches = refTitlesMatches.shift();
318+
unwanted = true;
319+
continue;
320+
}
321+
if (!match?.value?.groups) break;
307322

308-
num = match?.value?.groups.issueKeyNumber;
323+
const { issueKeyNumber: issueKey, numberChunk = issueKey } = match.value.groups;
324+
const input = match.value.input;
309325
let index = match.value.index;
310-
const linkUrl = ref.url?.replace(numRegex, num);
326+
const entryEdgeDistance = Math.min(index, input.length - index - numberChunk.length - 1);
327+
328+
const linkUrl = ref.url?.replace(numRegex, issueKey);
311329
// strange case (I would say synthetic), but if we parse the link twice, use the most relevant of them
312330
const existingIndex = autolinks.get(linkUrl)?.index;
313331
if (existingIndex != null) {
314332
index = Math.min(index, existingIndex);
315333
}
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-
);
323-
autolinks.set(linkUrl, {
324-
...ref,
325-
provider: provider,
326-
id: num,
327-
index: index,
328-
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,
338-
title: ref.title?.replace(numRegex, num),
339-
description: ref.description?.replace(numRegex, num),
340-
descriptor: ref.descriptor,
341-
});
342-
} while (!match.done);
334+
335+
// fill refTitlesMatches for non-prefixed refs
336+
if (!unwanted && nonPrefixedRef && ref.title) {
337+
refTitlesMatches.push(slugify(ref.title).matchAll(ref.branchNameRegex));
338+
}
339+
340+
if (!unwanted) {
341+
autolinks.set(linkUrl, {
342+
...ref,
343+
provider: provider,
344+
id: issueKey,
345+
index: index,
346+
url: linkUrl,
347+
priority: nonPrefixedRef
348+
? calculatePriority(
349+
issueKey,
350+
entryEdgeDistance,
351+
match.value.groups.numberChunk,
352+
chunkIndexMap.get(match.value.input),
353+
)
354+
: undefined,
355+
title: ref.title?.replace(numRegex, issueKey),
356+
description: ref.description?.replace(numRegex, issueKey),
357+
descriptor: ref.descriptor,
358+
});
359+
} else {
360+
autolinks.delete(linkUrl);
361+
}
362+
} while (true);
343363
}
344364
}
345365

0 commit comments

Comments
 (0)