-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add branchName autolinks #3644
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add branchName autolinks #3644
Changes from 3 commits
871a584
a76a6a2
f0c137b
e457785
635bf2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import * as assert from 'assert'; | ||
| import { suite, test } from 'mocha'; | ||
| import { map } from '../../system/iterable'; | ||
| import type { Autolink, 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/<num>', | ||
| description: 'test', | ||
| }, | ||
| ], | ||
| ]); | ||
|
|
||
| function assertAutolinks(actual: Map<string, Autolink>, expected: Array<string>): void { | ||
| assert.deepEqual([...map(actual.values(), x => x.url)], expected); | ||
| } | ||
|
|
||
| suite('Autolinks Test Suite', () => { | ||
| test('Branch name autolinks', () => { | ||
| 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', () => { | ||
| assertAutolinks(Autolinks._getAutolinks('test message 123 sd', mockRefSets()), ['test/123']); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<string, Autolink>()); | ||
|
|
||
| const numRegex = /<num>/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,13 +38,15 @@ export interface AutolinkReference { | |
| readonly title: string | undefined; | ||
|
|
||
| readonly type?: AutolinkType; | ||
| readonly referenceType?: AutolinkReferenceType; | ||
| readonly description?: string; | ||
| readonly descriptor?: ResourceDescriptor; | ||
| } | ||
|
|
||
| export interface Autolink extends AutolinkReference { | ||
| provider?: ProviderReference; | ||
| id: string; | ||
| index: number; | ||
|
|
||
| tokenize?: | ||
| | (( | ||
|
|
@@ -78,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, | ||
|
|
@@ -105,6 +109,7 @@ export interface CacheableAutolinkReference extends AutolinkReference { | |
| messageHtmlRegex?: RegExp; | ||
| messageMarkdownRegex?: RegExp; | ||
| messageRegex?: RegExp; | ||
| branchNameRegex?: RegExp; | ||
| } | ||
|
|
||
| export interface DynamicAutolinkReference { | ||
|
|
@@ -131,6 +136,11 @@ 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[], | ||
| ]; | ||
|
|
||
| export class Autolinks implements Disposable { | ||
| protected _disposable: Disposable | undefined; | ||
| private _references: CacheableAutolinkReference[] = []; | ||
|
|
@@ -162,30 +172,11 @@ export class Autolinks implements Disposable { | |
| } | ||
| } | ||
|
|
||
| async getAutolinks(message: string, remote?: GitRemote): Promise<Map<string, Autolink>>; | ||
| async getAutolinks( | ||
| message: string, | ||
| remote: GitRemote, | ||
| // eslint-disable-next-line @typescript-eslint/unified-signatures | ||
| options?: { excludeCustom?: boolean }, | ||
| ): Promise<Map<string, Autolink>>; | ||
| @debug<Autolinks['getAutolinks']>({ | ||
| args: { | ||
| 0: '<message>', | ||
| 1: false, | ||
| }, | ||
| }) | ||
| async getAutolinks( | ||
| message: string, | ||
| remote?: GitRemote, | ||
| options?: { excludeCustom?: boolean }, | ||
| ): Promise<Map<string, Autolink>> { | ||
| 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 +186,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 +205,124 @@ 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: Autolink, b: Autolink) { | ||
| // 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.prefix.length - a.prefix.length || b.id.length - a.id.length || -(b.index - a.index); | ||
| } | ||
|
|
||
| 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); | ||
| } | ||
| 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<Map<string, Autolink>> { | ||
| const refsets = await this.getRefsets(remote, options); | ||
| if (refsets.length === 0) return emptyAutolinkMap; | ||
|
|
||
| return Autolinks._getBranchAutolinks(branchName, refsets); | ||
| } | ||
|
|
||
| static _getBranchAutolinks(branchName: string, refsets: Readonly<RefSet[]>) { | ||
| const autolinks = new Map<string, Autolink>(); | ||
|
|
||
| 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')) { | ||
axosoft-ramint marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 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, { | ||
| ...ref, | ||
| provider: provider, | ||
| id: num, | ||
| index: index, | ||
| url: linkUrl, | ||
| title: ref.title?.replace(numRegex, num), | ||
| description: ref.description?.replace(numRegex, num), | ||
| descriptor: ref.descriptor, | ||
| }); | ||
| } while (!match.done); | ||
| } | ||
| } | ||
|
|
||
| return new Map([...autolinks.entries()].sort((a, b) => this.compareAutolinks(a[1], b[1]))); | ||
| } | ||
|
|
||
| async getAutolinks(message: string, remote?: GitRemote): Promise<Map<string, Autolink>>; | ||
| async getAutolinks( | ||
| message: string, | ||
| remote: GitRemote, | ||
| // eslint-disable-next-line @typescript-eslint/unified-signatures | ||
| options?: { excludeCustom?: boolean }, | ||
| ): Promise<Map<string, Autolink>>; | ||
| @debug<Autolinks['getAutolinks']>({ | ||
| args: { | ||
| 0: '<message>', | ||
| 1: false, | ||
| }, | ||
| }) | ||
| async getAutolinks( | ||
| message: string, | ||
| remote?: GitRemote, | ||
| options?: { excludeCustom?: boolean }, | ||
| ): Promise<Map<string, Autolink>> { | ||
| const refsets = await this.getRefsets(remote, options); | ||
| if (refsets.length === 0) return emptyAutolinkMap; | ||
|
|
||
| return Autolinks._getAutolinks(message, refsets); | ||
| } | ||
|
|
||
| static _getAutolinks(message: string, refsets: Readonly<RefSet[]>) { | ||
| const autolinks = new Map<string, Autolink>(); | ||
| let match; | ||
| let num; | ||
| for (const [provider, refs] of refsets) { | ||
|
|
@@ -236,13 +338,14 @@ export class Autolinks implements Disposable { | |
|
|
||
| do { | ||
| match = ref.messageRegex.exec(message); | ||
| if (match == null) break; | ||
| if (!match) break; | ||
|
|
||
| [, , , num] = match; | ||
|
|
||
| autolinks.set(num, { | ||
| provider: provider, | ||
| id: num, | ||
| index: match.index, | ||
| prefix: ref.prefix, | ||
| url: ref.url?.replace(numRegex, num), | ||
| alphanumeric: ref.alphanumeric, | ||
|
|
@@ -625,7 +728,7 @@ function ensureCachedRegex( | |
| function ensureCachedRegex( | ||
| ref: CacheableAutolinkReference, | ||
| outputFormat: 'plaintext', | ||
| ): asserts ref is RequireSome<CacheableAutolinkReference, 'messageRegex'>; | ||
| ): asserts ref is RequireSome<CacheableAutolinkReference, 'messageRegex' | 'branchNameRegex'>; | ||
| 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 +749,12 @@ function ensureCachedRegex(ref: CacheableAutolinkReference, outputFormat: 'html' | |
| `(^|\\s|\\(|\\[|\\{)(${escapeRegex(ref.prefix)}(${ref.alphanumeric ? '\\w' : '\\d'}+))\\b`, | ||
| ref.ignoreCase ? 'gi' : 'g', | ||
| ); | ||
| ref.branchNameRegex = new RegExp( | ||
| `(^|\\-|_|\\.|\\/)(?<prefix>${ref.prefix})(?<issueKeyNumber>${ | ||
| ref.alphanumeric ? '\\w' : '\\d' | ||
| }+)(?=$|\\-|_|\\.|\\/)`, | ||
| 'gi', | ||
| ); | ||
|
Comment on lines
+759
to
+764
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just for my own learning/benefit, can you explain how you formulated this regex?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's an easy regex playground service called https://regex101.com/, I always test the regexes there before I put them to the code. |
||
| } | ||
|
|
||
| return true; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from './autolinks'; |
Uh oh!
There was an error while loading. Please reload this page.