From 7936163842edab029231443eec35176b31c5831a Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Mon, 2 Dec 2019 03:36:08 -0500 Subject: [PATCH] Adds issue titles to autolinks (if connected) --- src/annotations/autolinks.ts | 137 +++++++++++++++++++++++++++------- src/config.ts | 1 - src/git/formatters/commitFormatter.ts | 9 ++- src/git/models/issue.ts | 9 +++ src/git/models/models.ts | 1 + src/git/remotes/github.ts | 8 +- src/git/remotes/provider.ts | 19 ++++- src/github/github.ts | 58 +++++++++++++- src/hovers/hovers.ts | 4 +- 9 files changed, 211 insertions(+), 35 deletions(-) create mode 100644 src/git/models/issue.ts diff --git a/src/annotations/autolinks.ts b/src/annotations/autolinks.ts index 8a137ca..7503236 100644 --- a/src/annotations/autolinks.ts +++ b/src/annotations/autolinks.ts @@ -2,23 +2,35 @@ import { ConfigurationChangeEvent, Disposable } from 'vscode'; import { AutolinkReference, configuration } from '../configuration'; import { Container } from '../container'; -import { Strings } from '../system'; +import { Dates, Strings } from '../system'; import { Logger } from '../logger'; -import { GitRemote } from '../git/git'; +import { GitRemote, Issue } from '../git/git'; +import { RemoteProviderWithApi } from '../git/remotes/provider'; +import { GlyphChars } from '../constants'; const numRegex = //g; +export interface CacheableAutolinkReference extends AutolinkReference { + linkify?: ((text: string) => string) | null; + markdownRegex?: RegExp; + textRegex?: RegExp; +} + export interface DynamicAutolinkReference { linkify: (text: string) => string; } -function requiresGenerator(ref: AutolinkReference | DynamicAutolinkReference): ref is AutolinkReference { - return ref.linkify === undefined; +function isDynamic(ref: AutolinkReference | DynamicAutolinkReference): ref is DynamicAutolinkReference { + return (ref as AutolinkReference).prefix === undefined && (ref as AutolinkReference).url === undefined; +} + +function isCacheable(ref: AutolinkReference | DynamicAutolinkReference): ref is CacheableAutolinkReference { + return (ref as AutolinkReference).prefix !== undefined && (ref as AutolinkReference).url !== undefined; } export class Autolinks implements Disposable { protected _disposable: Disposable | undefined; - private _references: AutolinkReference[] = []; + private _references: CacheableAutolinkReference[] = []; constructor() { this._disposable = Disposable.from(configuration.onDidChange(this.onConfigurationChanged, this)); @@ -36,14 +48,51 @@ export class Autolinks implements Disposable { } } - linkify(text: string, remotes?: GitRemote[]) { - for (const ref of this._references) { - if (requiresGenerator(ref)) { - ref.linkify = this._getAutolinkGenerator(ref); + async getIssueLinks(text: string, remotes: GitRemote[]) { + if (remotes.length === 0) return undefined; + + const issues = new Map(); + + for (const r of remotes) { + if (!(r.provider instanceof RemoteProviderWithApi)) continue; + if (!(await r.provider.isConnected())) continue; + + for (const ref of r.provider.autolinks) { + if (!isCacheable(ref)) continue; + + if (ref.textRegex === undefined) { + ref.textRegex = new RegExp(`(?<=^|\\s)(${ref.prefix}([0-9]+))\\b`, 'g'); + } + + let match; + let num; + let id; + let issue; + do { + match = ref.textRegex.exec(text); + if (match == null) break; + + [, , num] = match; + + id = Number(num); + issue = await r.provider.getIssue(id); + if (issue != null) { + issues.set(id, issue); + } + } while (true); } + } - if (ref.linkify != null) { - text = ref.linkify(text); + if (issues.size === 0) return undefined; + return issues; + } + + linkify(text: string, remotes?: GitRemote[], issues?: Map) { + for (const ref of this._references) { + if (this.ensureAutolinkCached(ref, issues)) { + if (ref.linkify != null) { + text = ref.linkify(text); + } } } @@ -52,12 +101,10 @@ export class Autolinks implements Disposable { if (r.provider === undefined) continue; for (const ref of r.provider.autolinks) { - if (requiresGenerator(ref)) { - ref.linkify = this._getAutolinkGenerator(ref); - } - - if (ref.linkify != null) { - text = ref.linkify(text); + if (this.ensureAutolinkCached(ref, issues)) { + if (ref.linkify != null) { + text = ref.linkify(text); + } } } } @@ -66,19 +113,53 @@ export class Autolinks implements Disposable { return text; } - private _getAutolinkGenerator({ prefix, url, title }: AutolinkReference) { + private ensureAutolinkCached( + ref: CacheableAutolinkReference | DynamicAutolinkReference, + issues?: Map + ): ref is CacheableAutolinkReference | DynamicAutolinkReference { + if (isDynamic(ref)) return true; + try { - const regex = new RegExp( - `(?<=^|\\s)(${Strings.escapeMarkdown(prefix).replace(/\\/g, '\\\\')}([0-9]+))\\b`, - 'g' - ); - const markdown = `[$1](${url.replace(numRegex, '$2')}${ - title ? ` "${title.replace(numRegex, '$2')}"` : '' - })`; - return (text: string) => text.replace(regex, markdown); + if (ref.markdownRegex === undefined) { + ref.markdownRegex = new RegExp( + `(?<=^|\\s)(${Strings.escapeMarkdown(ref.prefix).replace(/\\/g, '\\\\')}([0-9]+))\\b`, + 'g' + ); + } + + if (issues == null || issues.size === 0) { + const markdown = `[$1](${ref.url.replace(numRegex, '$2')}${ + ref.title ? ` "${ref.title.replace(numRegex, '$2')}"` : '' + })`; + ref.linkify = (text: string) => text.replace(ref.markdownRegex!, markdown); + + return true; + } + + ref.linkify = (text: string) => + text.replace(ref.markdownRegex!, (substring, linkText, number) => { + const issue = issues?.get(Number(number)); + + return `[${linkText}](${ref.url.replace(numRegex, number)}${ + ref.title + ? ` "${ref.title.replace(numRegex, number)}${ + issue + ? `\n${GlyphChars.Dash.repeat(2)}\n${issue.title.replace(/([")])/g, '\\$1')}\n${ + issue.closed ? 'Closed' : 'Opened' + }, ${Dates.getFormatter(issue.closedDate ?? issue.date).fromNow()}` + : '' + }"` + : '' + })`; + }); } catch (ex) { - Logger.error(ex, `Failed to create autolink generator: prefix=${prefix}, url=${url}, title=${title}`); - return null; + Logger.error( + ex, + `Failed to create autolink generator: prefix=${ref.prefix}, url=${ref.url}, title=${ref.title}` + ); + ref.linkify = null; } + + return true; } } diff --git a/src/config.ts b/src/config.ts index a463770..728328e 100644 --- a/src/config.ts +++ b/src/config.ts @@ -126,7 +126,6 @@ export interface AutolinkReference { url: string; title?: string; ignoreCase?: boolean; - linkify?: ((text: string) => string) | null; } export enum BranchSorting { diff --git a/src/git/formatters/commitFormatter.ts b/src/git/formatters/commitFormatter.ts index 1fe445e..94e60be 100644 --- a/src/git/formatters/commitFormatter.ts +++ b/src/git/formatters/commitFormatter.ts @@ -11,7 +11,7 @@ import { import { DateStyle, FileAnnotationType } from '../../configuration'; import { GlyphChars } from '../../constants'; import { Container } from '../../container'; -import { CommitPullRequest, GitCommit, GitLogCommit, GitRemote, GitService, GitUri } from '../gitService'; +import { CommitPullRequest, GitCommit, GitLogCommit, GitRemote, GitService, GitUri, Issue } from '../gitService'; import { Strings } from '../../system'; import { FormatOptions, Formatter } from './formatter'; import { ContactPresence } from '../../vsls/vsls'; @@ -25,6 +25,7 @@ const hasTokenRegexMap = new Map(); export interface CommitFormatOptions extends FormatOptions { annotationType?: FileAnnotationType; + autolinkedIssues?: Map; dateStyle?: DateStyle; getBranchAndTagTips?: (sha: string) => string | undefined; line?: number; @@ -355,7 +356,11 @@ export class CommitFormatter extends Formatter { return message; } - message = Container.autolinks.linkify(Strings.escapeMarkdown(message, { quoted: true }), this._options.remotes); + message = Container.autolinks.linkify( + Strings.escapeMarkdown(message, { quoted: true }), + this._options.remotes, + this._options.autolinkedIssues + ); return `\n> ${message}`; } diff --git a/src/git/models/issue.ts b/src/git/models/issue.ts new file mode 100644 index 0000000..79fa260 --- /dev/null +++ b/src/git/models/issue.ts @@ -0,0 +1,9 @@ +'use strict'; + +export interface Issue { + id: number; + date: Date; + title: string; + closed: boolean; + closedDate?: Date; +} diff --git a/src/git/models/models.ts b/src/git/models/models.ts index 60a4204..14b31a4 100644 --- a/src/git/models/models.ts +++ b/src/git/models/models.ts @@ -37,6 +37,7 @@ export * from './commit'; export * from './contributor'; export * from './diff'; export * from './file'; +export * from './issue'; export * from './log'; export * from './logCommit'; export * from './pullRequest'; diff --git a/src/git/remotes/github.ts b/src/git/remotes/github.ts index 1c22c9c..b19dcda 100644 --- a/src/git/remotes/github.ts +++ b/src/git/remotes/github.ts @@ -3,6 +3,7 @@ import { Disposable, env, QuickInputButton, Range, Uri, window } from 'vscode'; import { DynamicAutolinkReference } from '../../annotations/autolinks'; import { AutolinkReference } from '../../config'; import { Container } from '../../container'; +import { Issue } from '../models/issue'; import { PullRequest } from '../models/pullRequest'; import { RemoteProviderWithApi } from './provider'; @@ -94,7 +95,7 @@ export class GitHubRemote extends RemoteProviderWithApi<{ token: string }> { input.placeholder = 'Generate a personal access token from github.com (required)'; input.show(); - }); + }); } finally { input.dispose(); disposable?.dispose(); @@ -134,6 +135,11 @@ export class GitHubRemote extends RemoteProviderWithApi<{ token: string }> { return `${this.baseUrl}?path=${fileName}${line}`; } + protected async onGetIssue({ token }: { token: string }, id: number): Promise { + const [owner, repo] = this.splitPath(); + return (await Container.github).getIssue(token, owner, repo, id, { baseUrl: this.apiBaseUrl }); + } + protected async onGetPullRequestForCommit( { token }: { token: string }, ref: string diff --git a/src/git/remotes/provider.ts b/src/git/remotes/provider.ts index 33e24c3..937b4b3 100644 --- a/src/git/remotes/provider.ts +++ b/src/git/remotes/provider.ts @@ -6,6 +6,7 @@ import { Container } from '../../container'; import { CredentialChangeEvent, CredentialManager } from '../../credentials'; import { Logger } from '../../logger'; import { Messages } from '../../messages'; +import { Issue } from '../models/issue'; import { GitLogCommit } from '../models/logCommit'; import { PullRequest } from '../models/pullRequest'; import { debug, Promises } from '../../system'; @@ -210,7 +211,7 @@ export abstract class RemoteProviderWithApi extends this._onDidChange.fire(); return; - } + } if (e.reason === 'clear' && (e.key === undefined || e.key === this.credentialsKey)) { this._credentials = undefined; @@ -233,6 +234,21 @@ export abstract class RemoteProviderWithApi extends return (await this.credentials()) != null; } + @debug() + async getIssue(id: number): Promise { + const cc = Logger.getCorrelationContext(); + + if (!(await this.isConnected())) return undefined; + + try { + return await this.onGetIssue(this._credentials!, id); + } catch (ex) { + Logger.error(ex, cc); + + return undefined; + } + } + private _prsByCommit = new Map | PullRequest | null>(); @debug() @@ -247,6 +263,7 @@ export abstract class RemoteProviderWithApi extends return pr.then(pr => pr ?? undefined); } + protected abstract onGetIssue(credentials: T, id: number): Promise; protected abstract onGetPullRequestForCommit(credentials: T, ref: string): Promise; protected _credentials: T | null | undefined; diff --git a/src/github/github.ts b/src/github/github.ts index 60a351d..fb2fc05 100644 --- a/src/github/github.ts +++ b/src/github/github.ts @@ -2,7 +2,7 @@ import { graphql } from '@octokit/graphql'; import { Logger } from '../logger'; import { debug } from '../system'; -import { PullRequest, PullRequestState } from '../git/gitService'; +import { Issue, PullRequest, PullRequestState } from '../git/gitService'; export class GitHubApi { @debug() @@ -74,6 +74,62 @@ export class GitHubApi { throw ex; } } + + @debug() + async getIssue( + token: string, + owner: string, + repo: string, + number: number, + options?: { + baseUrl?: string; + } + ): Promise { + const cc = Logger.getCorrelationContext(); + + try { + const query = `query pr($owner: String!, $repo: String!, $number: Int!) { + repository(name: $repo, owner: $owner) { + issue(number: $number) { + createdAt + closed + closedAt + title + } + } +}`; + + const variables = { owner: owner, repo: repo, number: number }; + Logger.debug(cc, `variables: ${JSON.stringify(variables)}`); + + const rsp = await graphql(query, { + ...variables, + headers: { authorization: `token ${token}` }, + ...options + }); + const issue = rsp?.repository?.issue as GitHubIssue | undefined; + if (issue == null) return undefined; + + return { + id: issue.number, + date: new Date(issue.createdAt), + title: issue.title, + closed: issue.closed, + closedDate: issue.closedAt == null ? undefined : new Date(issue.closedAt) + }; + } catch (ex) { + Logger.error(ex, cc); + throw ex; + } + } +} + +interface GitHubIssue { + number: number; + createdAt: string; + closed: boolean; + closedAt: string | null; + title: string; } interface GitHubPullRequest { diff --git a/src/hovers/hovers.ts b/src/hovers/hovers.ts index a05fb41..f704d6c 100644 --- a/src/hovers/hovers.ts +++ b/src/hovers/hovers.ts @@ -154,14 +154,16 @@ export namespace Hovers { const remotes = await Container.git.getRemotes(commit.repoPath, { sort: true }); - const [previousLineDiffUris, pr, presence] = await Promise.all([ + const [previousLineDiffUris, autolinkedIssues, pr, presence] = await Promise.all([ commit.isUncommitted ? commit.getPreviousLineDiffUris(uri, editorLine, uri.sha) : undefined, + Container.autolinks.getIssueLinks(commit.message, remotes), getPullRequestForCommit(commit.ref, remotes), Container.vsls.maybeGetPresence(commit.email).catch(reason => undefined) ]); const details = CommitFormatter.fromTemplate(Container.config.hovers.detailsMarkdownFormat, commit, { annotationType: annotationType, + autolinkedIssues: autolinkedIssues, dateFormat: dateFormat, line: editorLine, markdown: true,