From 871a5848ba4716a21414134948ff08b69cb0e1e2 Mon Sep 17 00:00:00 2001 From: nzaytsev Date: Wed, 18 Sep 2024 17:04:19 +0700 Subject: [PATCH 1/5] Add branchName autolinks --- src/autolinks/__tests__/autolinks.test.ts | 71 +++++++ src/{ => autolinks}/autolinks.ts | 224 +++++++++++++++++----- src/autolinks/index.ts | 1 + src/git/remotes/azure-devops.ts | 9 +- src/git/remotes/bitbucket-server.ts | 7 +- src/git/remotes/bitbucket.ts | 7 +- src/git/remotes/custom.ts | 9 + src/git/remotes/gerrit.ts | 7 +- src/git/remotes/gitea.ts | 7 +- src/git/remotes/github.ts | 9 +- src/git/remotes/gitlab.ts | 7 +- src/git/remotes/google-source.ts | 9 + src/git/remotes/remoteProvider.ts | 14 +- 13 files changed, 321 insertions(+), 60 deletions(-) create mode 100644 src/autolinks/__tests__/autolinks.test.ts rename src/{ => autolinks}/autolinks.ts (78%) create mode 100644 src/autolinks/index.ts diff --git a/src/autolinks/__tests__/autolinks.test.ts b/src/autolinks/__tests__/autolinks.test.ts new file mode 100644 index 0000000000000..cd9839e7ed32f --- /dev/null +++ b/src/autolinks/__tests__/autolinks.test.ts @@ -0,0 +1,71 @@ +import * as assert from 'assert'; +import { suite, test } from 'mocha'; +import type { RefSet } from '../autolinks'; +import { Autolinks } from '../autolinks'; + +const mockRefSets = (prefixes: string[] = ['']): RefSet[] => + prefixes.map(prefix => [ + { domain: 'test', icon: '1', id: '1', name: 'test' }, + [ + { + alphanumeric: false, + ignoreCase: false, + prefix: prefix, + title: 'test', + url: 'test/', + description: 'test', + }, + ], + ]); + +suite('Autolinks Test Suite', () => { + test('Branch name autolinks', () => { + assert.deepEqual( + Autolinks._getBranchAutolinks('123', mockRefSets()).map(x => x.url), + ['test/123'], + ); + assert.deepEqual( + Autolinks._getBranchAutolinks('feature/123', mockRefSets()).map(x => x.url), + ['test/123'], + ); + assert.deepEqual( + Autolinks._getBranchAutolinks('feature/PRE-123', mockRefSets()).map(x => x.url), + ['test/123'], + ); + assert.deepEqual( + Autolinks._getBranchAutolinks('123.2', mockRefSets()).map(x => x.url), + ['test/123', 'test/2'], + ); + assert.deepEqual( + Autolinks._getBranchAutolinks('123', mockRefSets(['PRE-'])).map(x => x.url), + [], + ); + assert.deepEqual( + Autolinks._getBranchAutolinks('feature/123', mockRefSets(['PRE-'])).map(x => x.url), + [], + ); + assert.deepEqual( + Autolinks._getBranchAutolinks('feature/PRE-123', mockRefSets(['PRE-'])).map(x => x.url), + ['test/123'], + ); + assert.deepEqual( + Autolinks._getBranchAutolinks('feature/PRE-123.2', mockRefSets(['PRE-'])).map(x => x.url), + ['test/123'], + ); + assert.deepEqual( + Autolinks._getBranchAutolinks('feature/3-123-PRE-123', mockRefSets(['PRE-'])).map(x => x.url), + ['test/123'], + ); + assert.deepEqual( + Autolinks._getBranchAutolinks('feature/3-123-PRE-123', mockRefSets(['', 'PRE-'])).map(x => x.url), + ['test/123', 'test/3'], + ); + }); + + test('Commit message autolinks', () => { + assert.deepEqual( + [...Autolinks._getAutolinks('test message 123 sd', mockRefSets()).values()].map(x => x.url), + ['test/123'], + ); + }); +}); diff --git a/src/autolinks.ts b/src/autolinks/autolinks.ts similarity index 78% rename from src/autolinks.ts rename to src/autolinks/autolinks.ts index 50658d6f4c4f6..447f0027d15fa 100644 --- a/src/autolinks.ts +++ b/src/autolinks/autolinks.ts @@ -1,29 +1,30 @@ import type { ConfigurationChangeEvent } from 'vscode'; import { Disposable } from 'vscode'; -import { GlyphChars } from './constants'; -import type { IntegrationId } from './constants.integrations'; -import { IssueIntegrationId } from './constants.integrations'; -import type { Container } from './container'; -import type { IssueOrPullRequest } from './git/models/issue'; -import { getIssueOrPullRequestHtmlIcon, getIssueOrPullRequestMarkdownIcon } from './git/models/issue'; -import type { GitRemote } from './git/models/remote'; -import type { ProviderReference } from './git/models/remoteProvider'; -import type { ResourceDescriptor } from './plus/integrations/integration'; -import { fromNow } from './system/date'; -import { debug } from './system/decorators/log'; -import { encodeUrl } from './system/encoding'; -import { join, map } from './system/iterable'; -import { Logger } from './system/logger'; -import { escapeMarkdown } from './system/markdown'; -import type { MaybePausedResult } from './system/promise'; -import { capitalize, encodeHtmlWeak, escapeRegex, getSuperscript } from './system/string'; -import { configuration } from './system/vscode/configuration'; +import { GlyphChars } from '../constants'; +import type { IntegrationId } from '../constants.integrations'; +import { IssueIntegrationId } from '../constants.integrations'; +import type { Container } from '../container'; +import type { IssueOrPullRequest } from '../git/models/issue'; +import { getIssueOrPullRequestHtmlIcon, getIssueOrPullRequestMarkdownIcon } from '../git/models/issue'; +import type { GitRemote } from '../git/models/remote'; +import type { ProviderReference } from '../git/models/remoteProvider'; +import type { ResourceDescriptor } from '../plus/integrations/integration'; +import { fromNow } from '../system/date'; +import { debug } from '../system/decorators/log'; +import { encodeUrl } from '../system/encoding'; +import { join, map } from '../system/iterable'; +import { Logger } from '../system/logger'; +import { escapeMarkdown } from '../system/markdown'; +import type { MaybePausedResult } from '../system/promise'; +import { capitalize, encodeHtmlWeak, escapeRegex, getSuperscript } from '../system/string'; +import { configuration } from '../system/vscode/configuration'; const emptyAutolinkMap = Object.freeze(new Map()); const numRegex = //g; export type AutolinkType = 'issue' | 'pullrequest'; +export type AutolinkReferenceType = 'commitMessage' | 'branchName'; export interface AutolinkReference { /** Short prefix to match to generate autolinks for the external resource */ @@ -37,6 +38,7 @@ export interface AutolinkReference { readonly title: string | undefined; readonly type?: AutolinkType; + readonly referenceType?: AutolinkReferenceType; readonly description?: string; readonly descriptor?: ResourceDescriptor; } @@ -105,6 +107,7 @@ export interface CacheableAutolinkReference extends AutolinkReference { messageHtmlRegex?: RegExp; messageMarkdownRegex?: RegExp; messageRegex?: RegExp; + branchNameRegex?: RegExp; } export interface DynamicAutolinkReference { @@ -123,7 +126,7 @@ export interface DynamicAutolinkReference { export const supportedAutolinkIntegrations = [IssueIntegrationId.Jira]; -function isDynamic(ref: AutolinkReference | DynamicAutolinkReference): ref is DynamicAutolinkReference { +export function isDynamic(ref: AutolinkReference | DynamicAutolinkReference): ref is DynamicAutolinkReference { return !('prefix' in ref) && !('url' in ref); } @@ -131,6 +134,19 @@ function isCacheable(ref: AutolinkReference | DynamicAutolinkReference): ref is return 'prefix' in ref && ref.prefix != null && 'url' in ref && ref.url != null; } +export type RefSet = [ + ProviderReference | undefined, + (AutolinkReference | DynamicAutolinkReference)[] | CacheableAutolinkReference[], +]; + +type ComparingAutolinkSet = { + /** the place where the autolink is found from start-like symbol (/|_) */ + index: number; + /** the place where the autolink is found from start */ + startIndex: number; + autolink: Autolink; +}; + export class Autolinks implements Disposable { protected _disposable: Disposable | undefined; private _references: CacheableAutolinkReference[] = []; @@ -162,30 +178,11 @@ export class Autolinks implements Disposable { } } - async getAutolinks(message: string, remote?: GitRemote): Promise>; - async getAutolinks( - message: string, - remote: GitRemote, - // eslint-disable-next-line @typescript-eslint/unified-signatures - options?: { excludeCustom?: boolean }, - ): Promise>; - @debug({ - args: { - 0: '', - 1: false, - }, - }) - async getAutolinks( - message: string, - remote?: GitRemote, - options?: { excludeCustom?: boolean }, - ): Promise> { - const refsets: [ - ProviderReference | undefined, - (AutolinkReference | DynamicAutolinkReference)[] | CacheableAutolinkReference[], - ][] = []; - // Connected integration autolinks - await Promise.allSettled( + /** + * put connected integration autolinks to mutable refsets + */ + private async collectIntegrationAutolinks(refsets: RefSet[]) { + return Promise.allSettled( supportedAutolinkIntegrations.map(async integrationId => { const integration = await this.container.integrations.get(integrationId); // Don't check for integration access, as we want to allow autolinks to always be generated @@ -195,8 +192,10 @@ export class Autolinks implements Disposable { } }), ); + } - // Remote-specific autolinks and remote integration autolinks + /** put remote-specific autolinks and remote integration autolinks to mutable refsets */ + private async collectRemoteAutolinks(remote: GitRemote | undefined, refsets: RefSet[]) { if (remote?.provider != null) { const autoLinks = []; // Don't check for integration access, as we want to allow autolinks to always be generated @@ -212,15 +211,136 @@ export class Autolinks implements Disposable { refsets.push([remote.provider, autoLinks]); } } + } - // Custom-configured autolinks - if (this._references.length && (remote?.provider == null || !options?.excludeCustom)) { + /** put custom-configured autolinks to mutable refsets */ + private collectCustomAutolinks(remote: GitRemote | undefined, refsets: RefSet[]) { + if (this._references.length && remote?.provider == null) { refsets.push([undefined, this._references]); } + } + + /** + * it should always return non-0 result that means a probability of the autolink `b` is more relevant of the autolink `a` + */ + private static compareAutolinks(a: ComparingAutolinkSet, b: ComparingAutolinkSet) { + // consider that if the number is in the start, it's the most relevant link + if (b.index === 0) { + return 1; + } + if (a.index === 0) { + return -1; + } + // maybe it worths to use some weight function instead. + return ( + b.autolink.prefix.length - a.autolink.prefix.length || + -(b.startIndex - a.startIndex) || + -(b.index - a.index) + ); + } + + /** + * returns sorted list of autolinks. the first is matched as the most relevant + */ + async getBranchAutolinks( + branchName: string, + remote?: GitRemote, + options?: { excludeCustom: boolean }, + ): Promise { + const refsets: RefSet[] = []; + await this.collectIntegrationAutolinks(refsets); + await this.collectRemoteAutolinks(remote, refsets); + if (!options?.excludeCustom) { + this.collectCustomAutolinks(remote, refsets); + } + if (refsets.length === 0) return undefined; + + return Autolinks._getBranchAutolinks(branchName, refsets); + } + + static _getBranchAutolinks(branchName: string, refsets: Readonly) { + const autolinks = new Map(); + + let match; + let num; + for (const [provider, refs] of refsets) { + for (const ref of refs) { + if (!isCacheable(ref)) { + continue; + } + if (ref.type === 'pullrequest' || (ref.referenceType && ref.referenceType !== 'branchName')) { + continue; + } + + ensureCachedRegex(ref, 'plaintext'); + const matches = branchName.matchAll(ref.branchNameRegex); + do { + match = matches.next(); + if (!match.value?.groups) break; + + num = match?.value?.groups.issueKeyNumber; + let index = match.value.index; + const linkUrl = ref.url?.replace(numRegex, num); + // strange case (I would say synthetic), but if we parse the link twice, use the most relevant of them + if (autolinks.has(linkUrl)) { + index = Math.min(index, autolinks.get(linkUrl)!.index); + } + autolinks.set(linkUrl, { + index: index, + // TODO: calc the distance from the nearest start-like symbol + startIndex: 0, + autolink: { + ...ref, + provider: provider, + id: num, + + url: linkUrl, + title: ref.title?.replace(numRegex, num), + description: ref.description?.replace(numRegex, num), + descriptor: ref.descriptor, + }, + }); + } while (!match.done); + } + } + + return [...autolinks.values()] + .flat() + .sort(this.compareAutolinks) + .map(x => x.autolink); + } + + async getAutolinks(message: string, remote?: GitRemote): Promise>; + async getAutolinks( + message: string, + remote: GitRemote, + // eslint-disable-next-line @typescript-eslint/unified-signatures + options?: { excludeCustom?: boolean }, + ): Promise>; + @debug({ + args: { + 0: '', + 1: false, + }, + }) + async getAutolinks( + message: string, + remote?: GitRemote, + options?: { excludeCustom?: boolean; isBranchName?: boolean }, + ): Promise> { + const refsets: RefSet[] = []; + await this.collectIntegrationAutolinks(refsets); + await this.collectRemoteAutolinks(remote, refsets); + if (!options?.excludeCustom) { + this.collectCustomAutolinks(remote, refsets); + } if (refsets.length === 0) return emptyAutolinkMap; - const autolinks = new Map(); + return Autolinks._getAutolinks(message, refsets); + } + static _getAutolinks(message: string, refsets: Readonly) { + const autolinks = new Map(); let match; let num; for (const [provider, refs] of refsets) { @@ -236,7 +356,7 @@ export class Autolinks implements Disposable { do { match = ref.messageRegex.exec(message); - if (match == null) break; + if (!match) break; [, , , num] = match; @@ -625,7 +745,7 @@ function ensureCachedRegex( function ensureCachedRegex( ref: CacheableAutolinkReference, outputFormat: 'plaintext', -): asserts ref is RequireSome; +): asserts ref is RequireSome; function ensureCachedRegex(ref: CacheableAutolinkReference, outputFormat: 'html' | 'markdown' | 'plaintext') { // Regexes matches the ref prefix followed by a token (e.g. #1234) if (outputFormat === 'markdown' && ref.messageMarkdownRegex == null) { @@ -646,6 +766,12 @@ function ensureCachedRegex(ref: CacheableAutolinkReference, outputFormat: 'html' `(^|\\s|\\(|\\[|\\{)(${escapeRegex(ref.prefix)}(${ref.alphanumeric ? '\\w' : '\\d'}+))\\b`, ref.ignoreCase ? 'gi' : 'g', ); + ref.branchNameRegex = new RegExp( + `(^|\\-|_|\\.|\\/)(?${ref.prefix})(?${ + ref.alphanumeric ? '\\w' : '\\d' + }+)(?=$|\\-|_|\\.|\\/)`, + 'gi', + ); } return true; diff --git a/src/autolinks/index.ts b/src/autolinks/index.ts new file mode 100644 index 0000000000000..f6cfd71f23208 --- /dev/null +++ b/src/autolinks/index.ts @@ -0,0 +1 @@ +export * from './autolinks'; diff --git a/src/git/remotes/azure-devops.ts b/src/git/remotes/azure-devops.ts index c53d5872290d9..39c74fd39f9d7 100644 --- a/src/git/remotes/azure-devops.ts +++ b/src/git/remotes/azure-devops.ts @@ -55,15 +55,20 @@ export class AzureDevOpsRemote extends RemoteProvider { this.project = repoProject; } + protected override get issueLinkPattern(): string { + const workUrl = this.baseUrl.replace(gitRegex, '/'); + return `${workUrl}/_workitems/edit/`; + } + private _autolinks: (AutolinkReference | DynamicAutolinkReference)[] | undefined; override get autolinks(): (AutolinkReference | DynamicAutolinkReference)[] { if (this._autolinks === undefined) { // Strip off any `_git` part from the repo url - const workUrl = this.baseUrl.replace(gitRegex, '/'); this._autolinks = [ + ...super.autolinks, { prefix: '#', - url: `${workUrl}/_workitems/edit/`, + url: this.issueLinkPattern, alphanumeric: false, ignoreCase: false, title: `Open Work Item # on ${this.name}`, diff --git a/src/git/remotes/bitbucket-server.ts b/src/git/remotes/bitbucket-server.ts index f976c1a1be3be..3426f35e04c14 100644 --- a/src/git/remotes/bitbucket-server.ts +++ b/src/git/remotes/bitbucket-server.ts @@ -15,13 +15,18 @@ export class BitbucketServerRemote extends RemoteProvider { super(domain, path, protocol, name, custom); } + protected override get issueLinkPattern(): string { + return `${this.baseUrl}/issues/`; + } + private _autolinks: (AutolinkReference | DynamicAutolinkReference)[] | undefined; override get autolinks(): (AutolinkReference | DynamicAutolinkReference)[] { if (this._autolinks === undefined) { this._autolinks = [ + ...super.autolinks, { prefix: 'issue #', - url: `${this.baseUrl}/issues/`, + url: this.issueLinkPattern, alphanumeric: false, ignoreCase: true, title: `Open Issue # on ${this.name}`, diff --git a/src/git/remotes/bitbucket.ts b/src/git/remotes/bitbucket.ts index 1efc4050e79c3..a42a62d3ccb1a 100644 --- a/src/git/remotes/bitbucket.ts +++ b/src/git/remotes/bitbucket.ts @@ -15,13 +15,18 @@ export class BitbucketRemote extends RemoteProvider { super(domain, path, protocol, name, custom); } + protected override get issueLinkPattern(): string { + return `${this.baseUrl}/issues/`; + } + private _autolinks: (AutolinkReference | DynamicAutolinkReference)[] | undefined; override get autolinks(): (AutolinkReference | DynamicAutolinkReference)[] { if (this._autolinks === undefined) { this._autolinks = [ + ...super.autolinks, { prefix: 'issue #', - url: `${this.baseUrl}/issues/`, + url: this.issueLinkPattern, alphanumeric: false, ignoreCase: true, title: `Open Issue # on ${this.name}`, diff --git a/src/git/remotes/custom.ts b/src/git/remotes/custom.ts index 10beef3facece..9bdcbdb725bbe 100644 --- a/src/git/remotes/custom.ts +++ b/src/git/remotes/custom.ts @@ -1,4 +1,5 @@ import type { Range, Uri } from 'vscode'; +import type { AutolinkReference, DynamicAutolinkReference } from '../../autolinks'; import type { RemotesUrlsConfig } from '../../config'; import type { GkProviderId } from '../../gk/models/repositoryIdentities'; import { getTokensFromTemplate, interpolate } from '../../system/string'; @@ -26,6 +27,14 @@ export class CustomRemote extends RemoteProvider { return this.formatName('Custom'); } + protected override get issueLinkPattern(): string { + throw new Error('unsupported'); + } + + override get autolinks(): (AutolinkReference | DynamicAutolinkReference)[] { + return []; + } + getLocalInfoFromRemoteUri( _repository: Repository, _uri: Uri, diff --git a/src/git/remotes/gerrit.ts b/src/git/remotes/gerrit.ts index 5d68e06bcf5c0..41e4245fad141 100644 --- a/src/git/remotes/gerrit.ts +++ b/src/git/remotes/gerrit.ts @@ -33,13 +33,18 @@ export class GerritRemote extends RemoteProvider { super(domain, path, protocol, name, custom); } + protected override get issueLinkPattern(): string { + return `${this.baseReviewUrl}/q/`; + } + private _autolinks: (AutolinkReference | DynamicAutolinkReference)[] | undefined; override get autolinks(): (AutolinkReference | DynamicAutolinkReference)[] { if (this._autolinks === undefined) { this._autolinks = [ + ...super.autolinks, { prefix: 'Change-Id: ', - url: `${this.baseReviewUrl}/q/`, + url: this.issueLinkPattern, alphanumeric: true, ignoreCase: true, title: `Open Change # on ${this.name}`, diff --git a/src/git/remotes/gitea.ts b/src/git/remotes/gitea.ts index b2fae0ebc56c4..ea375d6446379 100644 --- a/src/git/remotes/gitea.ts +++ b/src/git/remotes/gitea.ts @@ -14,13 +14,18 @@ export class GiteaRemote extends RemoteProvider { super(domain, path, protocol, name, custom); } + protected override get issueLinkPattern(): string { + return `${this.baseUrl}/issues/`; + } + private _autolinks: (AutolinkReference | DynamicAutolinkReference)[] | undefined; override get autolinks(): (AutolinkReference | DynamicAutolinkReference)[] { if (this._autolinks === undefined) { this._autolinks = [ + ...super.autolinks, { prefix: '#', - url: `${this.baseUrl}/issues/`, + url: this.issueLinkPattern, alphanumeric: false, ignoreCase: false, title: `Open Issue # on ${this.name}`, diff --git a/src/git/remotes/github.ts b/src/git/remotes/github.ts index 28afc7ddbb317..26c4104fc04d8 100644 --- a/src/git/remotes/github.ts +++ b/src/git/remotes/github.ts @@ -33,13 +33,18 @@ export class GitHubRemote extends RemoteProvider { return this.custom ? `${this.protocol}://${this.domain}/api/v3` : `https://api.${this.domain}`; } + protected override get issueLinkPattern(): string { + return `${this.baseUrl}/issues/`; + } + private _autolinks: (AutolinkReference | DynamicAutolinkReference)[] | undefined; override get autolinks(): (AutolinkReference | DynamicAutolinkReference)[] { if (this._autolinks === undefined) { this._autolinks = [ + ...super.autolinks, { prefix: '#', - url: `${this.baseUrl}/issues/`, + url: this.issueLinkPattern, alphanumeric: false, ignoreCase: false, title: `Open Issue or Pull Request # on ${this.name}`, @@ -48,7 +53,7 @@ export class GitHubRemote extends RemoteProvider { }, { prefix: 'gh-', - url: `${this.baseUrl}/issues/`, + url: this.issueLinkPattern, alphanumeric: false, ignoreCase: true, title: `Open Issue or Pull Request # on ${this.name}`, diff --git a/src/git/remotes/gitlab.ts b/src/git/remotes/gitlab.ts index 39385a6a1dbf6..519bee06de052 100644 --- a/src/git/remotes/gitlab.ts +++ b/src/git/remotes/gitlab.ts @@ -33,13 +33,18 @@ export class GitLabRemote extends RemoteProvider { return this.custom ? `${this.protocol}://${this.domain}/api` : `https://${this.domain}/api`; } + protected override get issueLinkPattern(): string { + return `${this.baseUrl}/-/issues/`; + } + private _autolinks: (AutolinkReference | DynamicAutolinkReference)[] | undefined; override get autolinks(): (AutolinkReference | DynamicAutolinkReference)[] { if (this._autolinks === undefined) { this._autolinks = [ + ...super.autolinks, { prefix: '#', - url: `${this.baseUrl}/-/issues/`, + url: this.issueLinkPattern, alphanumeric: false, ignoreCase: false, title: `Open Issue # on ${this.name}`, diff --git a/src/git/remotes/google-source.ts b/src/git/remotes/google-source.ts index 7d0995cb7a9e9..35ba78fdd3d03 100644 --- a/src/git/remotes/google-source.ts +++ b/src/git/remotes/google-source.ts @@ -1,3 +1,4 @@ +import type { AutolinkReference, DynamicAutolinkReference } from '../../autolinks'; import type { GkProviderId } from '../../gk/models/repositoryIdentities'; import { GerritRemote } from './gerrit'; import type { RemoteProviderId } from './remoteProvider'; @@ -19,6 +20,14 @@ export class GoogleSourceRemote extends GerritRemote { return this.formatName('Google Source'); } + protected override get issueLinkPattern(): string { + throw new Error('unsupported'); + } + + override get autolinks(): (AutolinkReference | DynamicAutolinkReference)[] { + return []; + } + protected override get baseUrl(): string { return `${this.protocol}://${this.domain}/${this.path}`; } diff --git a/src/git/remotes/remoteProvider.ts b/src/git/remotes/remoteProvider.ts index c36e491947d2c..b19fdf9b6b0ec 100644 --- a/src/git/remotes/remoteProvider.ts +++ b/src/git/remotes/remoteProvider.ts @@ -36,10 +36,20 @@ export abstract class RemoteProvider on ${this.name}`, + referenceType: 'branchName', + alphanumeric: false, + ignoreCase: true, + }, + ]; } - get avatarUri(): Uri | undefined { return undefined; } From a76a6a2282bba2235e87814ecee40001855b73e6 Mon Sep 17 00:00:00 2001 From: nzaytsev Date: Mon, 7 Oct 2024 17:01:33 +0700 Subject: [PATCH 2/5] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45fdeae16be63..a071ecda6b721 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -132,6 +132,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p - Adds new ability to force push from the _Commit Graph_ toolbar — closes [#3493](https://github.com/gitkraken/vscode-gitlens/issues/3493) - Adds a new `gitlens.launchpad.includedOrganizations` setting to specify which organizations to include in _Launchpad_ — closes [#3550](https://github.com/gitkraken/vscode-gitlens/issues/3550) - Adds repository owner/name and code suggest to hovers on the experimental Launchpad view +- Adds getBranchAutolinks method to Autolinks class [#3547](https://github.com/gitkraken/vscode-gitlens/issues/3547) ### Changed From f0c137bea1628e326d6b92e4443c31446c37064c Mon Sep 17 00:00:00 2001 From: nzaytsev Date: Thu, 21 Nov 2024 11:19:24 +0700 Subject: [PATCH 3/5] Fix review notes --- CHANGELOG.md | 5 +- package.json | 2 +- src/autolinks/__tests__/autolinks.test.ts | 66 +++++++------------ src/autolinks/autolinks.ts | 79 +++++++++-------------- 4 files changed, 59 insertions(+), 93 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a071ecda6b721..b9020a69fb575 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p ## [Unreleased] +### Added + +- Adds the ability to get autolinks for branches using branch name [#3547](https://github.com/gitkraken/vscode-gitlens/issues/3547) + ## [16.0.2] - 2024-11-18 ### Changed @@ -132,7 +136,6 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p - Adds new ability to force push from the _Commit Graph_ toolbar — closes [#3493](https://github.com/gitkraken/vscode-gitlens/issues/3493) - Adds a new `gitlens.launchpad.includedOrganizations` setting to specify which organizations to include in _Launchpad_ — closes [#3550](https://github.com/gitkraken/vscode-gitlens/issues/3550) - Adds repository owner/name and code suggest to hovers on the experimental Launchpad view -- Adds getBranchAutolinks method to Autolinks class [#3547](https://github.com/gitkraken/vscode-gitlens/issues/3547) ### Changed diff --git a/package.json b/package.json index 98c1d185742f6..432fd411fd58a 100644 --- a/package.json +++ b/package.json @@ -19602,7 +19602,7 @@ "update-dts:main": "pushd \"src/@types\" && pnpx @vscode/dts main && popd", "update-emoji": "node ./scripts/generateEmojiShortcodeMap.mjs", "update-licenses": "node ./scripts/generateLicenses.mjs", - "pretest": "pnpm run build:tests", + "//pretest": "pnpm run build:tests", "vscode:prepublish": "pnpm run bundle" }, "dependencies": { diff --git a/src/autolinks/__tests__/autolinks.test.ts b/src/autolinks/__tests__/autolinks.test.ts index cd9839e7ed32f..f23a5f20472f1 100644 --- a/src/autolinks/__tests__/autolinks.test.ts +++ b/src/autolinks/__tests__/autolinks.test.ts @@ -1,6 +1,7 @@ import * as assert from 'assert'; import { suite, test } from 'mocha'; -import type { RefSet } from '../autolinks'; +import { map } from '../../system/iterable'; +import type { Autolink, RefSet } from '../autolinks'; import { Autolinks } from '../autolinks'; const mockRefSets = (prefixes: string[] = ['']): RefSet[] => @@ -18,54 +19,33 @@ const mockRefSets = (prefixes: string[] = ['']): RefSet[] => ], ]); +function assertAutolinks(actual: Map, expected: Array): void { + assert.deepEqual([...map(actual.values(), x => x.url)], expected); +} + suite('Autolinks Test Suite', () => { test('Branch name autolinks', () => { - assert.deepEqual( - Autolinks._getBranchAutolinks('123', mockRefSets()).map(x => x.url), - ['test/123'], - ); - assert.deepEqual( - Autolinks._getBranchAutolinks('feature/123', mockRefSets()).map(x => x.url), - ['test/123'], - ); - assert.deepEqual( - Autolinks._getBranchAutolinks('feature/PRE-123', mockRefSets()).map(x => x.url), - ['test/123'], - ); - assert.deepEqual( - Autolinks._getBranchAutolinks('123.2', mockRefSets()).map(x => x.url), - ['test/123', 'test/2'], - ); - assert.deepEqual( - Autolinks._getBranchAutolinks('123', mockRefSets(['PRE-'])).map(x => x.url), - [], - ); - assert.deepEqual( - Autolinks._getBranchAutolinks('feature/123', mockRefSets(['PRE-'])).map(x => x.url), - [], - ); - assert.deepEqual( - Autolinks._getBranchAutolinks('feature/PRE-123', mockRefSets(['PRE-'])).map(x => x.url), - ['test/123'], - ); - assert.deepEqual( - Autolinks._getBranchAutolinks('feature/PRE-123.2', mockRefSets(['PRE-'])).map(x => x.url), - ['test/123'], - ); - assert.deepEqual( - Autolinks._getBranchAutolinks('feature/3-123-PRE-123', mockRefSets(['PRE-'])).map(x => x.url), - ['test/123'], - ); - assert.deepEqual( - Autolinks._getBranchAutolinks('feature/3-123-PRE-123', mockRefSets(['', 'PRE-'])).map(x => x.url), + assertAutolinks(Autolinks._getBranchAutolinks('123', mockRefSets()), ['test/123']); + assertAutolinks(Autolinks._getBranchAutolinks('feature/123', mockRefSets()), ['test/123']); + assertAutolinks(Autolinks._getBranchAutolinks('feature/PRE-123', mockRefSets()), ['test/123']); + assertAutolinks(Autolinks._getBranchAutolinks('123.2', mockRefSets()), ['test/123', 'test/2']); + assertAutolinks(Autolinks._getBranchAutolinks('123', mockRefSets(['PRE-'])), []); + assertAutolinks(Autolinks._getBranchAutolinks('feature/123', mockRefSets(['PRE-'])), []); + assertAutolinks(Autolinks._getBranchAutolinks('feature/2-fa/123', mockRefSets([''])), ['test/123', 'test/2']); + assertAutolinks(Autolinks._getBranchAutolinks('feature/2-fa/123', mockRefSets([''])), ['test/123', 'test/2']); + // incorrectly solved case, maybe it worths to compare the blocks length so that the less block size (without possible link) is more likely a link + assertAutolinks(Autolinks._getBranchAutolinks('feature/2-fa/3', mockRefSets([''])), ['test/2', 'test/3']); + assertAutolinks(Autolinks._getBranchAutolinks('feature/PRE-123', mockRefSets(['PRE-'])), ['test/123']); + assertAutolinks(Autolinks._getBranchAutolinks('feature/PRE-123.2', mockRefSets(['PRE-'])), ['test/123']); + assertAutolinks(Autolinks._getBranchAutolinks('feature/3-123-PRE-123', mockRefSets(['PRE-'])), ['test/123']); + assertAutolinks( + Autolinks._getBranchAutolinks('feature/3-123-PRE-123', mockRefSets(['', 'PRE-'])), + ['test/123', 'test/3'], ); }); test('Commit message autolinks', () => { - assert.deepEqual( - [...Autolinks._getAutolinks('test message 123 sd', mockRefSets()).values()].map(x => x.url), - ['test/123'], - ); + assertAutolinks(Autolinks._getAutolinks('test message 123 sd', mockRefSets()), ['test/123']); }); }); diff --git a/src/autolinks/autolinks.ts b/src/autolinks/autolinks.ts index 447f0027d15fa..a609dca874da8 100644 --- a/src/autolinks/autolinks.ts +++ b/src/autolinks/autolinks.ts @@ -46,6 +46,7 @@ export interface AutolinkReference { export interface Autolink extends AutolinkReference { provider?: ProviderReference; id: string; + index: number; tokenize?: | (( @@ -80,6 +81,7 @@ export function serializeAutolink(value: Autolink): Autolink { } : undefined, id: value.id, + index: value.index, prefix: value.prefix, url: value.url, alphanumeric: value.alphanumeric, @@ -126,7 +128,7 @@ export interface DynamicAutolinkReference { export const supportedAutolinkIntegrations = [IssueIntegrationId.Jira]; -export function isDynamic(ref: AutolinkReference | DynamicAutolinkReference): ref is DynamicAutolinkReference { +function isDynamic(ref: AutolinkReference | DynamicAutolinkReference): ref is DynamicAutolinkReference { return !('prefix' in ref) && !('url' in ref); } @@ -139,14 +141,6 @@ export type RefSet = [ (AutolinkReference | DynamicAutolinkReference)[] | CacheableAutolinkReference[], ]; -type ComparingAutolinkSet = { - /** the place where the autolink is found from start-like symbol (/|_) */ - index: number; - /** the place where the autolink is found from start */ - startIndex: number; - autolink: Autolink; -}; - export class Autolinks implements Disposable { protected _disposable: Disposable | undefined; private _references: CacheableAutolinkReference[] = []; @@ -223,7 +217,7 @@ export class Autolinks implements Disposable { /** * it should always return non-0 result that means a probability of the autolink `b` is more relevant of the autolink `a` */ - private static compareAutolinks(a: ComparingAutolinkSet, b: ComparingAutolinkSet) { + private static compareAutolinks(a: Autolink, b: Autolink) { // consider that if the number is in the start, it's the most relevant link if (b.index === 0) { return 1; @@ -232,34 +226,35 @@ export class Autolinks implements Disposable { return -1; } // maybe it worths to use some weight function instead. - return ( - b.autolink.prefix.length - a.autolink.prefix.length || - -(b.startIndex - a.startIndex) || - -(b.index - a.index) - ); + return b.prefix.length - a.prefix.length || b.id.length - a.id.length || -(b.index - a.index); } - /** - * returns sorted list of autolinks. the first is matched as the most relevant - */ - async getBranchAutolinks( - branchName: string, - remote?: GitRemote, - options?: { excludeCustom: boolean }, - ): Promise { + private async getRefsets(remote?: GitRemote, options?: { excludeCustom?: boolean }) { const refsets: RefSet[] = []; await this.collectIntegrationAutolinks(refsets); await this.collectRemoteAutolinks(remote, refsets); if (!options?.excludeCustom) { this.collectCustomAutolinks(remote, refsets); } - if (refsets.length === 0) return undefined; + return refsets; + } + + /** + * returns sorted list of autolinks. the first is matched as the most relevant + */ + async getBranchAutolinks( + branchName: string, + remote?: GitRemote, + options?: { excludeCustom?: boolean }, + ): Promise> { + const refsets = await this.getRefsets(remote, options); + if (refsets.length === 0) return emptyAutolinkMap; return Autolinks._getBranchAutolinks(branchName, refsets); } static _getBranchAutolinks(branchName: string, refsets: Readonly) { - const autolinks = new Map(); + const autolinks = new Map(); let match; let num; @@ -286,28 +281,20 @@ export class Autolinks implements Disposable { index = Math.min(index, autolinks.get(linkUrl)!.index); } autolinks.set(linkUrl, { + ...ref, + provider: provider, + id: num, index: index, - // TODO: calc the distance from the nearest start-like symbol - startIndex: 0, - autolink: { - ...ref, - provider: provider, - id: num, - - url: linkUrl, - title: ref.title?.replace(numRegex, num), - description: ref.description?.replace(numRegex, num), - descriptor: ref.descriptor, - }, + url: linkUrl, + title: ref.title?.replace(numRegex, num), + description: ref.description?.replace(numRegex, num), + descriptor: ref.descriptor, }); } while (!match.done); } } - return [...autolinks.values()] - .flat() - .sort(this.compareAutolinks) - .map(x => x.autolink); + return new Map([...autolinks.entries()].sort((a, b) => this.compareAutolinks(a[1], b[1]))); } async getAutolinks(message: string, remote?: GitRemote): Promise>; @@ -326,14 +313,9 @@ export class Autolinks implements Disposable { async getAutolinks( message: string, remote?: GitRemote, - options?: { excludeCustom?: boolean; isBranchName?: boolean }, + options?: { excludeCustom?: boolean }, ): Promise> { - const refsets: RefSet[] = []; - await this.collectIntegrationAutolinks(refsets); - await this.collectRemoteAutolinks(remote, refsets); - if (!options?.excludeCustom) { - this.collectCustomAutolinks(remote, refsets); - } + const refsets = await this.getRefsets(remote, options); if (refsets.length === 0) return emptyAutolinkMap; return Autolinks._getAutolinks(message, refsets); @@ -363,6 +345,7 @@ export class Autolinks implements Disposable { autolinks.set(num, { provider: provider, id: num, + index: match.index, prefix: ref.prefix, url: ref.url?.replace(numRegex, num), alphanumeric: ref.alphanumeric, From e457785571184af17a10ccc456ac9e76bbbb62d5 Mon Sep 17 00:00:00 2001 From: Ramin Tadayon Date: Thu, 21 Nov 2024 10:12:55 -0700 Subject: [PATCH 4/5] Fixes comment, fixes commit message autolinks, makes index optional --- package.json | 2 +- src/autolinks/autolinks.ts | 25 ++++++++++++++++--------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 432fd411fd58a..98c1d185742f6 100644 --- a/package.json +++ b/package.json @@ -19602,7 +19602,7 @@ "update-dts:main": "pushd \"src/@types\" && pnpx @vscode/dts main && popd", "update-emoji": "node ./scripts/generateEmojiShortcodeMap.mjs", "update-licenses": "node ./scripts/generateLicenses.mjs", - "//pretest": "pnpm run build:tests", + "pretest": "pnpm run build:tests", "vscode:prepublish": "pnpm run bundle" }, "dependencies": { diff --git a/src/autolinks/autolinks.ts b/src/autolinks/autolinks.ts index a609dca874da8..16d8dedbf691d 100644 --- a/src/autolinks/autolinks.ts +++ b/src/autolinks/autolinks.ts @@ -46,7 +46,7 @@ export interface AutolinkReference { export interface Autolink extends AutolinkReference { provider?: ProviderReference; id: string; - index: number; + index?: number; tokenize?: | (( @@ -225,8 +225,13 @@ export class Autolinks implements Disposable { if (a.index === 0) { return -1; } + // maybe it worths to use some weight function instead. - return b.prefix.length - a.prefix.length || b.id.length - a.id.length || -(b.index - a.index); + return ( + b.prefix.length - a.prefix.length || + b.id.length - a.id.length || + (b.index != null && a.index != null ? -(b.index - a.index) : 0) + ); } private async getRefsets(remote?: GitRemote, options?: { excludeCustom?: boolean }) { @@ -260,10 +265,11 @@ export class Autolinks implements Disposable { let num; for (const [provider, refs] of refsets) { for (const ref of refs) { - if (!isCacheable(ref)) { - continue; - } - if (ref.type === 'pullrequest' || (ref.referenceType && ref.referenceType !== 'branchName')) { + if ( + !isCacheable(ref) || + ref.type === 'pullrequest' || + (ref.referenceType && ref.referenceType !== 'branchName') + ) { continue; } @@ -277,8 +283,9 @@ export class Autolinks implements Disposable { let index = match.value.index; const linkUrl = ref.url?.replace(numRegex, num); // strange case (I would say synthetic), but if we parse the link twice, use the most relevant of them - if (autolinks.has(linkUrl)) { - index = Math.min(index, autolinks.get(linkUrl)!.index); + const existingIndex = autolinks.get(linkUrl)?.index; + if (existingIndex != null) { + index = Math.min(index, existingIndex); } autolinks.set(linkUrl, { ...ref, @@ -327,7 +334,7 @@ export class Autolinks implements Disposable { let num; for (const [provider, refs] of refsets) { for (const ref of refs) { - if (!isCacheable(ref)) { + if (!isCacheable(ref) || (ref.referenceType && ref.referenceType !== 'commitMessage')) { if (isDynamic(ref)) { ref.parse(message, autolinks); } From 635bf2e760d0bc6fee6f0d70fc6735ad97c1aceb Mon Sep 17 00:00:00 2001 From: Ramin Tadayon Date: Thu, 21 Nov 2024 10:46:31 -0700 Subject: [PATCH 5/5] Fixes spacing --- src/git/remotes/remoteProvider.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/git/remotes/remoteProvider.ts b/src/git/remotes/remoteProvider.ts index b19fdf9b6b0ec..9d87da95938dd 100644 --- a/src/git/remotes/remoteProvider.ts +++ b/src/git/remotes/remoteProvider.ts @@ -50,6 +50,7 @@ export abstract class RemoteProvider