From 195bc9751ef3ca6fd49da82b32a3649219eb2d3d Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Tue, 19 Sep 2023 19:13:01 -0400 Subject: [PATCH] Improve perf/experience with connected remotes - Avoids blocking UI & improves perf of enrichment of autolinks & PRs - Affects inline blame, hovers, statusbar blame, autolinks Improves performance of autolink detection Improves line tracking & state management - Avoids using a class for state - Only resets when needed - Adds more state storage & updates state rather than replacing it Adds a new CacheProvider for better caching --- src/annotations/autolinks.ts | 253 +++++++----- src/annotations/blameAnnotationProvider.ts | 15 +- src/annotations/lineAnnotationController.ts | 259 ++++++------ src/cache.ts | 222 +++++++++++ src/commands/openAssociatedPullRequestOnRemote.ts | 2 +- src/commands/openPullRequestOnRemote.ts | 4 +- src/container.ts | 10 + src/errors.ts | 9 + src/git/formatters/commitFormatter.ts | 44 ++- src/git/gitProviderService.ts | 86 ++-- src/git/models/commit.ts | 48 ++- src/git/models/issue.ts | 22 +- src/git/remotes/richRemoteProvider.ts | 287 +++++++------- src/hovers/hovers.ts | 265 ++++--------- src/hovers/lineHoverController.ts | 31 +- src/statusbar/statusBarController.ts | 353 +++++++++-------- src/system/cancellation.ts | 440 +++++++++++++++++++++ src/system/iterable.ts | 4 +- src/system/promise.ts | 226 +++++------ src/trackers/gitLineTracker.ts | 46 +-- src/trackers/lineTracker.ts | 93 +++-- src/views/nodes/autolinkedItemNode.ts | 67 ++-- src/views/nodes/autolinkedItemsNode.ts | 63 ++- src/views/nodes/commitNode.ts | 31 +- src/views/nodes/fileRevisionAsCommitNode.ts | 31 +- src/views/nodes/rebaseStatusNode.ts | 31 +- src/webviews/commitDetails/commitDetailsWebview.ts | 66 ++-- src/webviews/settings/settingsWebview.ts | 2 +- 28 files changed, 1826 insertions(+), 1184 deletions(-) create mode 100644 src/cache.ts diff --git a/src/annotations/autolinks.ts b/src/annotations/autolinks.ts index 9729447..226bfbf 100644 --- a/src/annotations/autolinks.ts +++ b/src/annotations/autolinks.ts @@ -7,14 +7,14 @@ import type { IssueOrPullRequest } from '../git/models/issue'; import { getIssueOrPullRequestHtmlIcon, getIssueOrPullRequestMarkdownIcon } from '../git/models/issue'; import type { GitRemote } from '../git/models/remote'; import type { RemoteProviderReference } from '../git/models/remoteProvider'; +import type { RichRemoteProvider } from '../git/remotes/richRemoteProvider'; +import type { MaybePausedResult } from '../system/cancellation'; import { configuration } from '../system/configuration'; 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 type { PromiseCancelledErrorWithId } from '../system/promise'; -import { PromiseCancelledError, raceAll } from '../system/promise'; import { encodeHtmlWeak, escapeMarkdown, escapeRegex, getSuperscript } from '../system/string'; const emptyAutolinkMap = Object.freeze(new Map()); @@ -32,6 +32,16 @@ export interface Autolink { description?: string; } +export type EnrichedAutolink = [ + issueOrPullRequest: Promise | undefined, + autolink: Autolink, +]; + +export type MaybeEnrichedAutolink = readonly [ + issueOrPullRequest: MaybePausedResult | undefined, + autolink: Autolink, +]; + export function serializeAutolink(value: Autolink): Autolink { const serialized: Autolink = { provider: value.provider @@ -58,7 +68,7 @@ export interface CacheableAutolinkReference extends AutolinkReference { text: string, outputFormat: 'html' | 'markdown' | 'plaintext', tokenMapping: Map, - issuesOrPullRequests?: Map, + enrichedAutolinks?: Map, footnotes?: Map, ) => string) | null; @@ -74,7 +84,7 @@ export interface DynamicAutolinkReference { text: string, outputFormat: 'html' | 'markdown' | 'plaintext', tokenMapping: Map, - issuesOrPullRequests?: Map, + enrichedAutolinks?: Map, footnotes?: Map, ) => string) | null; @@ -125,112 +135,123 @@ export class Autolinks implements Disposable { } } + getAutolinks(message: string, remote?: GitRemote): Map; + // eslint-disable-next-line @typescript-eslint/unified-signatures + getAutolinks(message: string, remote: GitRemote, options?: { excludeCustom?: boolean }): Map; @debug({ args: { 0: '', 1: false, }, }) - getAutolinks(message: string, remote?: GitRemote): Map { - const provider = remote?.provider; - // If a remote is provided but there is no provider return an empty set - if (remote != null && remote.provider == null) return emptyAutolinkMap; + getAutolinks(message: string, remote?: GitRemote, options?: { excludeCustom?: boolean }): Map { + const refsets: [ + RemoteProviderReference | undefined, + (AutolinkReference | DynamicAutolinkReference)[] | CacheableAutolinkReference[], + ][] = []; + if (remote?.provider?.autolinks?.length) { + refsets.push([remote.provider, remote.provider.autolinks]); + } + if (this._references.length && (remote?.provider == null || !options?.excludeCustom)) { + refsets.push([undefined, this._references]); + } + if (refsets.length === 0) return emptyAutolinkMap; const autolinks = new Map(); let match; let num; - for (const ref of provider?.autolinks ?? this._references) { - if (!isCacheable(ref)) { - if (isDynamic(ref)) { - ref.parse(message, autolinks); + for (const [provider, refs] of refsets) { + for (const ref of refs) { + if (!isCacheable(ref)) { + if (isDynamic(ref)) { + ref.parse(message, autolinks); + } + continue; } - continue; - } - ensureCachedRegex(ref, 'plaintext'); + ensureCachedRegex(ref, 'plaintext'); - do { - match = ref.messageRegex.exec(message); - if (match == null) break; + do { + match = ref.messageRegex.exec(message); + if (match == null) break; - [, , , num] = match; + [, , , num] = match; - autolinks.set(num, { - provider: provider, - id: num, - prefix: ref.prefix, - url: ref.url?.replace(numRegex, num), - title: ref.title?.replace(numRegex, num), + autolinks.set(num, { + provider: provider, + id: num, + prefix: ref.prefix, + url: ref.url?.replace(numRegex, num), + title: ref.title?.replace(numRegex, num), - type: ref.type, - description: ref.description?.replace(numRegex, num), - }); - } while (true); + type: ref.type, + description: ref.description?.replace(numRegex, num), + }); + } while (true); + } } return autolinks; } - async getLinkedIssuesAndPullRequests( - message: string, - remote: GitRemote, - options?: { autolinks?: Map; timeout?: never }, - ): Promise | undefined>; - async getLinkedIssuesAndPullRequests( + async getEnrichedAutolinks( message: string, - remote: GitRemote, - options: { autolinks?: Map; timeout: number }, - ): Promise< - | Map>> - | undefined - >; - @debug({ + remote: GitRemote | undefined, + ): Promise | undefined>; + async getEnrichedAutolinks( + autolinks: Map, + remote: GitRemote | undefined, + ): Promise | undefined>; + @debug({ args: { - 0: '', - 1: false, - 2: options => `autolinks=${options?.autolinks != null}, timeout=${options?.timeout}`, + 0: messageOrAutolinks => + typeof messageOrAutolinks === 'string' ? '' : `autolinks=${messageOrAutolinks.size}`, + 1: remote => remote?.remoteKey, }, }) - async getLinkedIssuesAndPullRequests( - message: string, - remote: GitRemote, - options?: { autolinks?: Map; timeout?: number }, - ) { - if (!remote.hasRichIntegration()) return undefined; - - const { provider } = remote; - const connected = provider.maybeConnected ?? (await provider.isConnected()); - if (!connected) return undefined; - - let autolinks = options?.autolinks; - if (autolinks == null) { - autolinks = this.getAutolinks(message, remote); + async getEnrichedAutolinks( + messageOrAutolinks: string | Map, + remote: GitRemote | undefined, + ): Promise | undefined> { + if (typeof messageOrAutolinks === 'string') { + messageOrAutolinks = this.getAutolinks(messageOrAutolinks, remote); } - - if (autolinks.size === 0) return undefined; - - const issuesOrPullRequests = await raceAll( - autolinks.keys(), - id => provider.getIssueOrPullRequest(id), - options?.timeout, - ); - - // Remove any issues or pull requests that were not found - for (const [id, issueOrPullRequest] of issuesOrPullRequests) { - if (issueOrPullRequest == null) { - issuesOrPullRequests.delete(id); + if (messageOrAutolinks.size === 0) return undefined; + + let provider: RichRemoteProvider | undefined; + if (remote?.hasRichIntegration()) { + ({ provider } = remote); + const connected = remote.provider.maybeConnected ?? (await remote.provider.isConnected()); + if (!connected) { + provider = undefined; } } - return issuesOrPullRequests.size !== 0 ? issuesOrPullRequests : undefined; + return new Map( + map( + messageOrAutolinks, + ([id, link]) => + [ + id, + [ + provider != null && + link.provider?.id === provider.id && + link.provider?.domain === provider.domain + ? provider.getIssueOrPullRequest(id) + : undefined, + link, + ] satisfies EnrichedAutolink, + ] as const, + ), + ); } @debug({ args: { 0: '', 2: remotes => remotes?.length, - 3: issuesOrPullRequests => issuesOrPullRequests?.size, + 3: issuesAndPullRequests => issuesAndPullRequests?.size, 4: footnotes => footnotes?.size, }, }) @@ -238,7 +259,7 @@ export class Autolinks implements Disposable { text: string, outputFormat: 'html' | 'markdown' | 'plaintext', remotes?: GitRemote[], - issuesOrPullRequests?: Map, + enrichedAutolinks?: Map, footnotes?: Map, ): string { const includeFootnotesInText = outputFormat === 'plaintext' && footnotes == null; @@ -251,7 +272,7 @@ export class Autolinks implements Disposable { for (const ref of this._references) { if (this.ensureAutolinkCached(ref)) { if (ref.tokenize != null) { - text = ref.tokenize(text, outputFormat, tokenMapping, issuesOrPullRequests, footnotes); + text = ref.tokenize(text, outputFormat, tokenMapping, enrichedAutolinks, footnotes); } } } @@ -268,7 +289,7 @@ export class Autolinks implements Disposable { for (const ref of r.provider.autolinks) { if (this.ensureAutolinkCached(ref)) { if (ref.tokenize != null) { - text = ref.tokenize(text, outputFormat, tokenMapping, issuesOrPullRequests, footnotes); + text = ref.tokenize(text, outputFormat, tokenMapping, enrichedAutolinks, footnotes); } } } @@ -302,7 +323,7 @@ export class Autolinks implements Disposable { text: string, outputFormat: 'html' | 'markdown' | 'plaintext', tokenMapping: Map, - issuesOrPullRequests?: Map, + enrichedAutolinks?: Map, footnotes?: Map, ) => { let footnoteIndex: number; @@ -319,11 +340,24 @@ export class Autolinks implements Disposable { if (ref.title) { title = ` "${ref.title.replace(numRegex, num)}`; - const issue = issuesOrPullRequests?.get(num); - if (issue != null) { - if (issue instanceof PromiseCancelledError) { - title += `\n${GlyphChars.Dash.repeat(2)}\nDetails timed out`; + const issueResult = enrichedAutolinks?.get(num)?.[0]; + if (issueResult?.value != null) { + if (issueResult.paused) { + if (footnotes != null) { + let name = ref.description?.replace(numRegex, num); + if (name == null) { + name = `Custom Autolink ${ref.prefix}${num}`; + } + footnoteIndex = footnotes.size + 1; + footnotes.set( + footnoteIndex, + `[${getIssueOrPullRequestMarkdownIcon()} ${name} $(loading~spin)](${url}${title}")`, + ); + } + + title += `\n${GlyphChars.Dash.repeat(2)}\nLoading...`; } else { + const issue = issueResult.value; const issueTitle = escapeMarkdown(issue.title.trim()); const issueTitleQuoteEscaped = issueTitle.replace(/"/g, '\\"'); @@ -345,6 +379,16 @@ export class Autolinks implements Disposable { issue.closed ? 'Closed' : 'Opened' }, ${fromNow(issue.closedDate ?? issue.date)}`; } + } else if (footnotes != null) { + let name = ref.description?.replace(numRegex, num); + if (name == null) { + name = `Custom Autolink ${ref.prefix}${num}`; + } + footnoteIndex = footnotes.size + 1; + footnotes.set( + footnoteIndex, + `[${getIssueOrPullRequestMarkdownIcon()} ${name}](${url}${title}")`, + ); } title += '"'; } @@ -366,11 +410,24 @@ export class Autolinks implements Disposable { if (ref.title) { title = `"${encodeHtmlWeak(ref.title.replace(numRegex, num))}`; - const issue = issuesOrPullRequests?.get(num); - if (issue != null) { - if (issue instanceof PromiseCancelledError) { - title += `\n${GlyphChars.Dash.repeat(2)}\nDetails timed out`; + const issueResult = enrichedAutolinks?.get(num)?.[0]; + if (issueResult?.value != null) { + if (issueResult.paused) { + if (footnotes != null) { + let name = ref.description?.replace(numRegex, num); + if (name == null) { + name = `Custom Autolink ${ref.prefix}${num}`; + } + footnoteIndex = footnotes.size + 1; + footnotes.set( + footnoteIndex, + `${getIssueOrPullRequestHtmlIcon()} ${name}`, + ); + } + + title += `\n${GlyphChars.Dash.repeat(2)}\nLoading...`; } else { + const issue = issueResult.value; const issueTitle = encodeHtmlWeak(issue.title.trim()); const issueTitleQuoteEscaped = issueTitle.replace(/"/g, '"'); @@ -392,6 +449,16 @@ export class Autolinks implements Disposable { issue.closed ? 'Closed' : 'Opened' }, ${fromNow(issue.closedDate ?? issue.date)}`; } + } else if (footnotes != null) { + let name = ref.description?.replace(numRegex, num); + if (name == null) { + name = `Custom Autolink ${ref.prefix}${num}`; + } + footnoteIndex = footnotes.size + 1; + footnotes.set( + footnoteIndex, + `${getIssueOrPullRequestHtmlIcon()} ${name}`, + ); } title += '"'; } @@ -407,19 +474,19 @@ export class Autolinks implements Disposable { return text.replace( ref.messageRegex, (_: string, prefix: string, linkText: string, num: string) => { - const issue = issuesOrPullRequests?.get(num); - if (issue == null) return linkText; + const issueResult = enrichedAutolinks?.get(num)?.[0]; + if (issueResult?.value == null) return linkText; if (footnotes != null) { footnoteIndex = footnotes.size + 1; footnotes.set( footnoteIndex, `${linkText}: ${ - issue instanceof PromiseCancelledError - ? 'Details timed out' - : `${issue.title} ${GlyphChars.Dot} ${ - issue.closed ? 'Closed' : 'Opened' - }, ${fromNow(issue.closedDate ?? issue.date)}` + issueResult.paused + ? 'Loading...' + : `${issueResult.value.title} ${GlyphChars.Dot} ${ + issueResult.value.closed ? 'Closed' : 'Opened' + }, ${fromNow(issueResult.value.closedDate ?? issueResult.value.date)}` }`, ); } diff --git a/src/annotations/blameAnnotationProvider.ts b/src/annotations/blameAnnotationProvider.ts index 88ebaae..3facb38 100644 --- a/src/annotations/blameAnnotationProvider.ts +++ b/src/annotations/blameAnnotationProvider.ts @@ -172,7 +172,13 @@ export abstract class BlameAnnotationProviderBase extends AnnotationProviderBase await Promise.all([ providers.details ? this.getDetailsHoverMessage(commit, document) : undefined, providers.changes - ? changesMessage(commit, await GitUri.fromUri(document.uri), position.line, document) + ? changesMessage( + this.container, + commit, + await GitUri.fromUri(document.uri), + position.line, + document, + ) : undefined, ]) ).filter((m?: T): m is T => Boolean(m)); @@ -190,13 +196,12 @@ export abstract class BlameAnnotationProviderBase extends AnnotationProviderBase editorLine = commitLine.originalLine - 1; const cfg = configuration.get('hovers'); - return detailsMessage(commit, await GitUri.fromUri(document.uri), editorLine, { + return detailsMessage(this.container, commit, await GitUri.fromUri(document.uri), editorLine, { autolinks: cfg.autolinks.enabled, dateFormat: configuration.get('defaultDateFormat'), format: cfg.detailsMarkdownFormat, - pullRequests: { - enabled: cfg.pullRequests.enabled, - }, + pullRequests: cfg.pullRequests.enabled, + timeout: 250, }); } } diff --git a/src/annotations/lineAnnotationController.ts b/src/annotations/lineAnnotationController.ts index b050eb9..5a976e1 100644 --- a/src/annotations/lineAnnotationController.ts +++ b/src/annotations/lineAnnotationController.ts @@ -1,29 +1,21 @@ -import type { - CancellationToken, - ConfigurationChangeEvent, - DecorationOptions, - TextEditor, - TextEditorDecorationType, -} from 'vscode'; +import type { ConfigurationChangeEvent, DecorationOptions, TextEditor, TextEditorDecorationType } from 'vscode'; import { CancellationTokenSource, DecorationRangeBehavior, Disposable, Range, window } from 'vscode'; import { GlyphChars, Schemes } from '../constants'; import type { Container } from '../container'; import { CommitFormatter } from '../git/formatters/commitFormatter'; -import type { GitCommit } from '../git/models/commit'; import type { PullRequest } from '../git/models/pullRequest'; import { detailsMessage } from '../hovers/hovers'; +import type { MaybePausedResult } from '../system/cancellation'; +import { pauseOnCancelOrTimeoutMap } from '../system/cancellation'; import { configuration } from '../system/configuration'; import { debug, log } from '../system/decorators/log'; import { once } from '../system/event'; import { debounce } from '../system/function'; -import { count, every, filter } from '../system/iterable'; import { Logger } from '../system/logger'; -import type { LogScope } from '../system/logger.scope'; import { getLogScope, setLogScopeExit } from '../system/logger.scope'; -import type { PromiseCancelledErrorWithId } from '../system/promise'; -import { PromiseCancelledError, raceAll } from '../system/promise'; +import { getSettledValue } from '../system/promise'; import { isTextEditor } from '../system/utils'; -import type { LinesChangeEvent } from '../trackers/gitLineTracker'; +import type { GitLineState, LinesChangeEvent } from '../trackers/gitLineTracker'; import { getInlineDecoration } from './annotations'; const annotationDecoration: TextEditorDecorationType = window.createTextEditorDecorationType({ @@ -158,37 +150,30 @@ export class LineAnnotationController implements Disposable { editor.setDecorations(annotationDecoration, []); } - private async getPullRequests( + private getPullRequestsForLines( repoPath: string, - lines: [number, GitCommit][], - { timeout }: { timeout?: number } = {}, - ) { - if (lines.length === 0) return undefined; + lines: Map, + ): Map> { + const prs = new Map>(); + if (lines.size === 0) return prs; - const remote = await this.container.git.getBestRemoteWithRichProvider(repoPath); - if (remote?.provider == null) return undefined; + const remotePromise = this.container.git.getBestRemoteWithRichProvider(repoPath); - const refs = new Set(); + for (const [, state] of lines) { + if (state.commit.isUncommitted) continue; - for (const [, commit] of lines) { - refs.add(commit.ref); + let pr = prs.get(state.commit.ref); + if (pr == null) { + pr = remotePromise.then(remote => state.commit.getAssociatedPullRequest(remote)); + prs.set(state.commit.ref, pr); + } } - if (refs.size === 0) return undefined; - - const { provider } = remote; - const prs = await raceAll( - refs.values(), - ref => this.container.git.getPullRequestForCommit(ref, provider), - timeout, - ); - if (prs.size === 0 || every(prs.values(), pr => pr == null)) return undefined; - return prs; } @debug({ args: false }) - private async refresh(editor: TextEditor | undefined, options?: { prs?: Map }) { + private async refresh(editor: TextEditor | undefined) { if (editor == null && this._editor == null) return; const scope = getLogScope(); @@ -253,22 +238,36 @@ export class LineAnnotationController implements Disposable { .join()}`; } - const commitLines = new Map(); + const commitPromises = new Map>(); + const lines = new Map(); for (const selection of selections) { const state = this.container.lineTracker.getState(selection.active); if (state?.commit == null) { Logger.debug(scope, `Line ${selection.active} returned no commit`); continue; } - commitLines.set(selection.active, state.commit); + + if (state.commit.message == null && !commitPromises.has(state.commit.ref)) { + commitPromises.set(state.commit.ref, state.commit.ensureFullDetails()); + } + lines.set(selection.active, state); } const repoPath = trackedDocument.uri.repoPath; - // TODO: Make this configurable? - const timeout = 100; - const [getBranchAndTagTips, prs] = await Promise.all([ - CommitFormatter.has(cfg.format, 'tips') ? this.container.git.getBranchesAndTagsTipsFn(repoPath) : undefined, + let hoverOptions: RequireSome[4], 'autolinks' | 'pullRequests'> | undefined; + // Live Share (vsls schemes) don't support `languages.registerHoverProvider` so we'll need to add them to the decoration directly + if (editor.document.uri.scheme === Schemes.Vsls || editor.document.uri.scheme === Schemes.VslsScc) { + const hoverCfg = configuration.get('hovers'); + hoverOptions = { + autolinks: hoverCfg.autolinks.enabled, + dateFormat: configuration.get('defaultDateFormat'), + format: hoverCfg.detailsMarkdownFormat, + pullRequests: hoverCfg.pullRequests.enabled, + }; + } + + const getPullRequests = repoPath != null && cfg.pullRequests.enabled && CommitFormatter.has( @@ -278,73 +277,109 @@ export class LineAnnotationController implements Disposable { 'pullRequestAgoOrDate', 'pullRequestDate', 'pullRequestState', - ) - ? options?.prs ?? - this.getPullRequests(repoPath, [...filter(commitLines, ([, commit]) => !commit.isUncommitted)], { - timeout: timeout, - }) - : undefined, - ]); + ); - if (prs != null) { - this._cancellation?.cancel(); - this._cancellation = new CancellationTokenSource(); - void this.waitForAnyPendingPullRequests(editor, prs, this._cancellation.token, timeout, scope); - } + this._cancellation?.cancel(); + this._cancellation = new CancellationTokenSource(); + const cancellation = this._cancellation.token; - const decorations = []; + const getBranchAndTagTipsPromise = CommitFormatter.has(cfg.format, 'tips') + ? this.container.git.getBranchesAndTagsTipsFn(repoPath) + : undefined; - let hoverOptions: RequireSome[3], 'autolinks' | 'pullRequests'> | undefined; - // Live Share (vsls schemes) don't support `languages.registerHoverProvider` so we'll need to add them to the decoration directly - if (editor.document.uri.scheme === Schemes.Vsls || editor.document.uri.scheme === Schemes.VslsScc) { - const hoverCfg = configuration.get('hovers'); - hoverOptions = { - autolinks: hoverCfg.autolinks.enabled, - dateFormat: configuration.get('defaultDateFormat'), - format: hoverCfg.detailsMarkdownFormat, - pullRequests: { - enabled: hoverCfg.pullRequests.enabled, - }, - }; - } + async function updateDecorations( + container: Container, + editor: TextEditor, + getBranchAndTagTips: Awaited | undefined, + prs: Map> | undefined, + timeout?: number, + ) { + const decorations = []; + + for (const [l, state] of lines) { + const commit = state.commit; + if (commit == null || (commit.isUncommitted && cfg.uncommittedChangesFormat === '')) continue; - for (const [l, commit] of commitLines) { - if (commit.isUncommitted && cfg.uncommittedChangesFormat === '') continue; - - const pr = prs?.get(commit.ref); - const decoration = getInlineDecoration( - commit, - // await GitUri.fromUri(editor.document.uri), - // l, - commit.isUncommitted ? cfg.uncommittedChangesFormat ?? cfg.format : cfg.format, - { - dateFormat: cfg.dateFormat === null ? configuration.get('defaultDateFormat') : cfg.dateFormat, - getBranchAndTagTips: getBranchAndTagTips, - pullRequestOrRemote: pr, - pullRequestPendingMessage: `PR ${GlyphChars.Ellipsis}`, - }, - cfg.scrollable, - ) as DecorationOptions; - decoration.range = editor.document.validateRange(new Range(l, maxSmallIntegerV8, l, maxSmallIntegerV8)); - - if (hoverOptions != null) { - decoration.hoverMessage = await detailsMessage( + const pr = prs?.get(commit.ref); + + const decoration = getInlineDecoration( commit, - trackedDocument.uri, - l, - pr != null - ? { - ...hoverOptions, - pullRequests: { ...hoverOptions.pullRequests, pr: pr }, - } - : hoverOptions, - ); + // await GitUri.fromUri(editor.document.uri), + // l, + commit.isUncommitted ? cfg.uncommittedChangesFormat ?? cfg.format : cfg.format, + { + dateFormat: cfg.dateFormat === null ? configuration.get('defaultDateFormat') : cfg.dateFormat, + getBranchAndTagTips: getBranchAndTagTips, + pullRequest: pr?.value, + pullRequestPendingMessage: `PR ${GlyphChars.Ellipsis}`, + }, + cfg.scrollable, + ) as DecorationOptions; + decoration.range = editor.document.validateRange(new Range(l, maxSmallIntegerV8, l, maxSmallIntegerV8)); + + if (hoverOptions != null) { + decoration.hoverMessage = await detailsMessage(container, commit, trackedDocument.uri, l, { + ...hoverOptions, + pullRequest: pr?.value, + timeout: timeout, + }); + } + + decorations.push(decoration); } - decorations.push(decoration); + editor.setDecorations(annotationDecoration, decorations); } - editor.setDecorations(annotationDecoration, decorations); + // TODO: Make this configurable? + const timeout = 100; + const prsResult = getPullRequests + ? await pauseOnCancelOrTimeoutMap( + this.getPullRequestsForLines(repoPath, lines), + true, + cancellation, + timeout, + async result => { + if ( + result.reason !== 'timedout' || + cancellation.isCancellationRequested || + editor !== this._editor + ) { + return; + } + + // If the PRs are taking too long, refresh the decorations once they complete + + Logger.debug( + scope, + `${GlyphChars.Dot} pull request queries took too long (over ${timeout} ms)`, + ); + + const [getBranchAndTagTipsResult, prsResult] = await Promise.allSettled([ + getBranchAndTagTipsPromise, + result.value, + ]); + + if (cancellation.isCancellationRequested || editor !== this._editor) return; + + const prs = getSettledValue(prsResult); + const getBranchAndTagTips = getSettledValue(getBranchAndTagTipsResult); + + Logger.debug(scope, `${GlyphChars.Dot} pull request queries completed; updating...`); + + void updateDecorations(this.container, editor, getBranchAndTagTips, prs); + }, + ) + : undefined; + + const [getBranchAndTagTipsResult] = await Promise.allSettled([ + getBranchAndTagTipsPromise, + ...commitPromises.values(), + ]); + + if (cancellation.isCancellationRequested) return; + + await updateDecorations(this.container, editor, getSettledValue(getBranchAndTagTipsResult), prsResult, 100); } private setLineTracker(enabled: boolean) { @@ -361,32 +396,4 @@ export class LineAnnotationController implements Disposable { this.container.lineTracker.unsubscribe(this); } - - private async waitForAnyPendingPullRequests( - editor: TextEditor, - prs: Map< - string, - PullRequest | PromiseCancelledErrorWithId> | undefined - >, - cancellationToken: CancellationToken, - timeout: number, - scope: LogScope | undefined, - ) { - // If there are any PRs that timed out, refresh the annotation(s) once they complete - const prCount = count(prs.values(), pr => pr instanceof PromiseCancelledError); - if (cancellationToken.isCancellationRequested || prCount === 0) return; - - Logger.debug(scope, `${GlyphChars.Dot} ${prCount} pull request queries took too long (over ${timeout} ms)`); - - const resolved = new Map(); - for (const [key, value] of prs) { - resolved.set(key, value instanceof PromiseCancelledError ? await value.promise : value); - } - - if (cancellationToken.isCancellationRequested || editor !== this._editor) return; - - Logger.debug(scope, `${GlyphChars.Dot} ${prCount} pull request queries completed; refreshing...`); - - void this.refresh(editor, { prs: resolved }); - } } diff --git a/src/cache.ts b/src/cache.ts new file mode 100644 index 0000000..e048c82 --- /dev/null +++ b/src/cache.ts @@ -0,0 +1,222 @@ +// import type { EnrichedAutolink } from './annotations/autolinks'; +import type { Disposable } from './api/gitlens'; +import type { Container } from './container'; +import type { DefaultBranch } from './git/models/defaultBranch'; +import type { IssueOrPullRequest } from './git/models/issue'; +import type { PullRequest } from './git/models/pullRequest'; +import { PullRequestState } from './git/models/pullRequest'; +import type { GitRemote } from './git/models/remote'; +import type { RepositoryMetadata } from './git/models/repositoryMetadata'; +import type { RemoteProvider } from './git/remotes/remoteProvider'; +import type { RichRemoteProvider } from './git/remotes/richRemoteProvider'; +import { isPromise } from './system/promise'; + +type Caches = { + defaultBranch: { key: `repo:${string}`; value: DefaultBranch }; + // enrichedAutolinksBySha: { key: `sha:${string}:${string}`; value: Map }; + issuesOrPrsById: { key: `id:${string}:${string}`; value: IssueOrPullRequest }; + prByBranch: { key: `branch:${string}:${string}`; value: PullRequest }; + prsBySha: { key: `sha:${string}:${string}`; value: PullRequest }; + repoMetadata: { key: `repo:${string}`; value: RepositoryMetadata }; +}; + +type Cache = keyof Caches; +type CacheKey = Caches[T]['key']; +type CacheValue = Caches[T]['value']; +type CacheResult = Promise | T | undefined; + +type Cacheable = () => { value: CacheResult; expiresAt?: number }; +type Cached = + | { + value: T | undefined; + expiresAt?: number; + etag?: string; + } + | { + value: Promise; + expiresAt?: never; // Don't set an expiration on promises as they will resolve to a value with the desired expiration + etag?: string; + }; + +export class CacheProvider implements Disposable { + private readonly _cache = new Map<`${Cache}:${CacheKey}`, Cached>>>(); + + // eslint-disable-next-line @typescript-eslint/no-useless-constructor + constructor(_container: Container) {} + + dispose() { + this._cache.clear(); + } + + delete(cache: T, key: CacheKey) { + this._cache.delete(`${cache}:${key}`); + } + + get( + cache: T, + key: CacheKey, + etag: string | undefined, + cacheable: Cacheable>, + ): CacheResult> { + const item = this._cache.get(`${cache}:${key}`); + + if ( + item == null || + (item.expiresAt != null && item.expiresAt > 0 && item.expiresAt < Date.now()) || + (item.etag != null && item.etag !== etag) + ) { + const { value, expiresAt } = cacheable(); + return this.set(cache, key, value, etag, expiresAt)?.value as CacheResult>; + } + + return item.value as CacheResult>; + } + + getIssueOrPullRequest( + id: string, + remoteOrProvider: RichRemoteProvider | GitRemote, + cacheable: Cacheable, + ): CacheResult { + const { key, etag } = getRemoteKeyAndEtag(remoteOrProvider); + return this.get('issuesOrPrsById', `id:${id}:${key}`, etag, cacheable); + } + + // getEnrichedAutolinks( + // sha: string, + // remoteOrProvider: RichRemoteProvider | GitRemote, + // cacheable: Cacheable>, + // ): CacheResult> { + // const { key, etag } = getRemoteKeyAndEtag(remoteOrProvider); + // return this.get('enrichedAutolinksBySha', `sha:${sha}:${key}`, etag, cacheable); + // } + + getPullRequestForBranch( + branch: string, + remoteOrProvider: RichRemoteProvider | GitRemote, + cacheable: Cacheable, + ): CacheResult { + const cache = 'prByBranch'; + const { key, etag } = getRemoteKeyAndEtag(remoteOrProvider); + // Wrap the cacheable so we can also add the result to the issuesOrPrsById cache + return this.get(cache, `branch:${branch}:${key}`, etag, this.wrapPullRequestCacheable(cacheable, key, etag)); + } + + getPullRequestForSha( + sha: string, + remoteOrProvider: RichRemoteProvider | GitRemote, + cacheable: Cacheable, + ): CacheResult { + const cache = 'prsBySha'; + const { key, etag } = getRemoteKeyAndEtag(remoteOrProvider); + // Wrap the cacheable so we can also add the result to the issuesOrPrsById cache + return this.get(cache, `sha:${sha}:${key}`, etag, this.wrapPullRequestCacheable(cacheable, key, etag)); + } + + getRepositoryDefaultBranch( + remoteOrProvider: RichRemoteProvider | GitRemote, + cacheable: Cacheable, + ): CacheResult { + const { key, etag } = getRemoteKeyAndEtag(remoteOrProvider); + return this.get('defaultBranch', `repo:${key}`, etag, cacheable); + } + + getRepositoryMetadata( + remoteOrProvider: RichRemoteProvider | GitRemote, + cacheable: Cacheable, + ): CacheResult { + const { key, etag } = getRemoteKeyAndEtag(remoteOrProvider); + return this.get('repoMetadata', `repo:${key}`, etag, cacheable); + } + + private set( + cache: T, + key: CacheKey, + value: CacheResult>, + etag: string | undefined, + expiresAt?: number, + ): Cached>> { + let item: Cached>>; + if (isPromise(value)) { + void value.then( + v => { + this.set(cache, key, v, etag, expiresAt); + }, + () => { + this.delete(cache, key); + }, + ); + + item = { value: value, etag: etag }; + } else { + item = { value: value, etag: etag, expiresAt: expiresAt ?? getExpiresAt(cache, value) }; + } + + this._cache.set(`${cache}:${key}`, item); + return item; + } + + private wrapPullRequestCacheable( + cacheable: Cacheable, + key: string, + etag: string | undefined, + ): Cacheable { + return () => { + const item = cacheable(); + if (isPromise(item.value)) { + void item.value.then(v => { + if (v != null) { + this.set('issuesOrPrsById', `id:${v.id}:${key}`, v, etag); + } + }); + } + + return item; + }; + } +} + +function getExpiresAt(cache: T, value: CacheValue | undefined): number { + const now = Date.now(); + const defaultExpiresAt = now + 60 * 60 * 1000; // 1 hour + + switch (cache) { + case 'defaultBranch': + case 'repoMetadata': + return 0; // Never expires + case 'issuesOrPrsById': { + if (value == null) return 0; // Never expires + + // Open issues expire after 1 hour, but closed issues expire after 12 hours unless recently updated and then expire in 1 hour + + const issueOrPr = value as CacheValue<'issuesOrPrsById'>; + if (!issueOrPr.closed) return defaultExpiresAt; + + const updatedAgo = now - (issueOrPr.closedDate ?? issueOrPr.date).getTime(); + return now + (updatedAgo > 14 * 24 * 60 * 60 * 1000 ? 12 : 1) * 60 * 60 * 1000; + } + case 'prByBranch': + case 'prsBySha': { + if (value == null) return cache === 'prByBranch' ? defaultExpiresAt : 0 /* Never expires */; + + // Open prs expire after 1 hour, but closed/merge prs expire after 12 hours unless recently updated and then expire in 1 hour + + const pr = value as CacheValue<'prsBySha'>; + if (pr.state === PullRequestState.Open) return defaultExpiresAt; + + const updatedAgo = now - (pr.closedDate ?? pr.mergedDate ?? pr.date).getTime(); + return now + (updatedAgo > 14 * 24 * 60 * 60 * 1000 ? 12 : 1) * 60 * 60 * 1000; + } + // case 'enrichedAutolinksBySha': + default: + return value == null ? 0 /* Never expires */ : defaultExpiresAt; + } +} + +function getRemoteKeyAndEtag(remoteOrProvider: RemoteProvider | GitRemote) { + return { + key: remoteOrProvider.remoteKey, + etag: remoteOrProvider.hasRichIntegration() + ? `${remoteOrProvider.remoteKey}:${remoteOrProvider.maybeConnected ?? false}` + : remoteOrProvider.remoteKey, + }; +} diff --git a/src/commands/openAssociatedPullRequestOnRemote.ts b/src/commands/openAssociatedPullRequestOnRemote.ts index a586bf7..c839fef 100644 --- a/src/commands/openAssociatedPullRequestOnRemote.ts +++ b/src/commands/openAssociatedPullRequestOnRemote.ts @@ -56,7 +56,7 @@ export class OpenAssociatedPullRequestOnRemoteCommand extends ActiveEditorComman const commit = await repo.getCommit('HEAD'); if (commit == null) return; - pr = await this.container.git.getPullRequestForCommit(commit.ref, remote.provider); + pr = await commit.getAssociatedPullRequest(remote); if (pr == null) return; } diff --git a/src/commands/openPullRequestOnRemote.ts b/src/commands/openPullRequestOnRemote.ts index b687f79..293d32f 100644 --- a/src/commands/openPullRequestOnRemote.ts +++ b/src/commands/openPullRequestOnRemote.ts @@ -36,9 +36,9 @@ export class OpenPullRequestOnRemoteCommand extends Command { if (args?.repoPath == null || args?.ref == null) return; const remote = await this.container.git.getBestRemoteWithRichProvider(args.repoPath); - if (remote?.provider == null) return; + if (!remote?.hasRichIntegration()) return; - const pr = await this.container.git.getPullRequestForCommit(args.ref, remote.provider); + const pr = await remote.provider.getPullRequestForCommit(args.ref); if (pr == null) return; args = { ...args }; diff --git a/src/container.ts b/src/container.ts index 0eebddd..b086150 100644 --- a/src/container.ts +++ b/src/container.ts @@ -7,6 +7,7 @@ import { FileAnnotationController } from './annotations/fileAnnotationController import { LineAnnotationController } from './annotations/lineAnnotationController'; import { ActionRunners } from './api/actionRunners'; import { setDefaultGravatarsStyle } from './avatars'; +import { CacheProvider } from './cache'; import { GitCodeLensController } from './codelens/codeLensController'; import type { ToggleFileAnnotationCommandArgs } from './commands'; import type { FileAnnotationType, ModeConfig } from './config'; @@ -355,6 +356,15 @@ export class Container { return this._branchesView; } + private _cache: CacheProvider | undefined; + get cache() { + if (this._cache == null) { + this._disposables.push((this._cache = new CacheProvider(this))); + } + + return this._cache; + } + private readonly _codeLensController: GitCodeLensController; get codeLens() { return this._codeLensController; diff --git a/src/errors.ts b/src/errors.ts index a6ca1df..8d1d83e 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,4 +1,5 @@ import type { Uri } from 'vscode'; +import { CancellationError as _CancellationError } from 'vscode'; import type { Response } from '@env/fetch'; import type { RequiredSubscriptionPlans, Subscription } from './subscription'; import { isSubscriptionPaidPlan } from './subscription'; @@ -85,6 +86,14 @@ export class AuthenticationError extends Error { } } +export class CancellationError extends _CancellationError { + constructor() { + super(); + + Error.captureStackTrace?.(this, CancellationError); + } +} + export class ExtensionNotFoundError extends Error { constructor( public readonly extensionId: string, diff --git a/src/git/formatters/commitFormatter.ts b/src/git/formatters/commitFormatter.ts index 1188108..0848a12 100644 --- a/src/git/formatters/commitFormatter.ts +++ b/src/git/formatters/commitFormatter.ts @@ -1,4 +1,5 @@ import type { Uri } from 'vscode'; +import type { MaybeEnrichedAutolink } from '../../annotations/autolinks'; import type { Action, ActionContext, @@ -24,7 +25,7 @@ import { arePlusFeaturesEnabled } from '../../plus/subscription/utils'; import type { ShowInCommitGraphCommandArgs } from '../../plus/webviews/graph/protocol'; import { configuration } from '../../system/configuration'; import { join, map } from '../../system/iterable'; -import { PromiseCancelledError } from '../../system/promise'; +import { isPromise } from '../../system/promise'; import type { TokenOptions } from '../../system/string'; import { encodeHtmlWeak, escapeMarkdown, getSuperscript } from '../../system/string'; import type { ContactPresence } from '../../vsls/vsls'; @@ -32,7 +33,6 @@ import type { PreviousLineComparisonUrisResult } from '../gitProvider'; import type { GitCommit } from '../models/commit'; import { isCommit } from '../models/commit'; import { uncommitted, uncommittedStaged } from '../models/constants'; -import type { IssueOrPullRequest } from '../models/issue'; import { PullRequest } from '../models/pullRequest'; import { getReferenceFromRevision, isUncommittedStaged, shortenRevision } from '../models/reference'; import { GitRemote } from '../models/remote'; @@ -41,7 +41,6 @@ import type { FormatOptions, RequiredTokenOptions } from './formatter'; import { Formatter } from './formatter'; export interface CommitFormatOptions extends FormatOptions { - autolinkedIssuesOrPullRequests?: Map; avatarSize?: number; dateStyle?: DateStyle; editor?: { line: number; uri: Uri }; @@ -59,10 +58,11 @@ export interface CommitFormatOptions extends FormatOptions { tips?: string; }; }; + enrichedAutolinks?: Map; messageAutolinks?: boolean; messageIndent?: number; messageTruncateAtNewLine?: boolean; - pullRequestOrRemote?: PullRequest | PromiseCancelledError | GitRemote; + pullRequest?: PullRequest | Promise; pullRequestPendingMessage?: string; presence?: ContactPresence; previousLineComparisonUris?: PreviousLineComparisonUrisResult; @@ -156,14 +156,14 @@ export class CommitFormatter extends Formatter { } private get _pullRequestDate() { - const { pullRequestOrRemote: pr } = this._options; + const { pullRequest: pr } = this._options; if (pr == null || !PullRequest.is(pr)) return ''; return pr.formatDate(this._options.dateFormat) ?? ''; } private get _pullRequestDateAgo() { - const { pullRequestOrRemote: pr } = this._options; + const { pullRequest: pr } = this._options; if (pr == null || !PullRequest.is(pr)) return ''; return pr.formatDateFromNow() ?? ''; @@ -443,15 +443,16 @@ export class CommitFormatter extends Formatter { )} "Open in Commit Graph")`; } - if (this._options.remotes != null && this._options.remotes.length !== 0) { - const providers = GitRemote.getHighlanderProviders(this._options.remotes); + const { pullRequest: pr, remotes } = this._options; + + if (remotes?.length) { + const providers = GitRemote.getHighlanderProviders(remotes); commands += `  [$(globe)](${OpenCommitOnRemoteCommand.getMarkdownCommandArgs( this._item.sha, )} "Open Commit on ${providers?.length ? providers[0].name : 'Remote'}")`; } - const { pullRequestOrRemote: pr } = this._options; if (pr != null) { if (PullRequest.is(pr)) { commands += `${separator}[$(git-pull-request) PR #${ @@ -465,13 +466,20 @@ export class CommitFormatter extends Formatter { }\n${GlyphChars.Dash.repeat(2)}\n${escapeMarkdown(pr.title).replace(/"/g, '\\"')}\n${ pr.state }, ${pr.formatDateFromNow()}")`; - } else if (pr instanceof PromiseCancelledError) { + } else if (isPromise(pr)) { commands += `${separator}[$(git-pull-request) PR $(loading~spin)](command:${Commands.RefreshHover} "Searching for a Pull Request (if any) that introduced this commit...")`; - } else if (pr.provider != null && configuration.get('integrations.enabled')) { - commands += `${separator}[$(plug) Connect to ${pr.provider.name}${ + } + } else if (remotes != null) { + const [remote] = remotes; + if ( + remote?.hasRichIntegration() && + !remote.provider.maybeConnected && + configuration.get('integrations.enabled') + ) { + commands += `${separator}[$(plug) Connect to ${remote?.provider.name}${ GlyphChars.Ellipsis - }](${ConnectRemoteProviderCommand.getMarkdownCommandArgs(pr)} "Connect to ${ - pr.provider.name + }](${ConnectRemoteProviderCommand.getMarkdownCommandArgs(remote)} "Connect to ${ + remote.provider.name } to enable the display of the Pull Request (if any) that introduced this commit")`; } } @@ -648,7 +656,7 @@ export class CommitFormatter extends Formatter { message, outputFormat, this._options.remotes, - this._options.autolinkedIssuesOrPullRequests, + this._options.enrichedAutolinks, this._options.footnotes, ); } @@ -670,7 +678,7 @@ export class CommitFormatter extends Formatter { } get pullRequest(): string { - const { pullRequestOrRemote: pr } = this._options; + const { pullRequest: pr } = this._options; // TODO: Implement html rendering if (pr == null || this._options.outputFormat === 'html') { return this._padOrTruncate('', this._options.tokenOptions.pullRequest); @@ -713,7 +721,7 @@ export class CommitFormatter extends Formatter { } else { text = `PR #${pr.id}`; } - } else if (pr instanceof PromiseCancelledError) { + } else if (isPromise(pr)) { text = this._options.outputFormat === 'markdown' ? `[PR $(loading~spin)](command:${Commands.RefreshHover} "Searching for a Pull Request (if any) that introduced this commit...")` @@ -738,7 +746,7 @@ export class CommitFormatter extends Formatter { } get pullRequestState(): string { - const { pullRequestOrRemote: pr } = this._options; + const { pullRequest: pr } = this._options; return this._padOrTruncate( pr == null || !PullRequest.is(pr) ? '' : pr.state ?? '', this._options.tokenOptions.pullRequestState, diff --git a/src/git/gitProviderService.ts b/src/git/gitProviderService.ts index 3260593..ace45f4 100644 --- a/src/git/gitProviderService.ts +++ b/src/git/gitProviderService.ts @@ -15,7 +15,7 @@ import { resetAvatarCache } from '../avatars'; import type { CoreGitConfiguration } from '../constants'; import { GlyphChars, Schemes } from '../constants'; import type { Container } from '../container'; -import { AccessDeniedError, ProviderNotFoundError } from '../errors'; +import { AccessDeniedError, CancellationError, ProviderNotFoundError } from '../errors'; import type { FeatureAccess, Features, PlusFeatures, RepoFeatureAccess } from '../features'; import type { SubscriptionChangeEvent } from '../plus/subscription/subscriptionService'; import type { RepoComparisonKey } from '../repositories'; @@ -1978,56 +1978,6 @@ export class GitProviderService implements Disposable { } } - async getPullRequestForCommit( - ref: string, - remote: GitRemote, - options?: { timeout?: number }, - ): Promise; - async getPullRequestForCommit( - ref: string, - provider: RichRemoteProvider, - options?: { timeout?: number }, - ): Promise; - @gate((ref, remoteOrProvider, options) => { - const provider = GitRemote.is(remoteOrProvider) ? remoteOrProvider.provider : remoteOrProvider; - return `${ref}${ - provider != null ? `|${provider.id}:${provider.domain}/${provider.path}` : '' - }|${options?.timeout}`; - }) - @debug({ args: { 1: remoteOrProvider => remoteOrProvider.name } }) - async getPullRequestForCommit( - ref: string, - remoteOrProvider: GitRemote | RichRemoteProvider, - options?: { timeout?: number }, - ): Promise { - if (isUncommitted(ref)) return undefined; - - let provider; - if (GitRemote.is(remoteOrProvider)) { - ({ provider } = remoteOrProvider); - if (!provider?.hasRichIntegration()) return undefined; - } else { - provider = remoteOrProvider; - } - - let promiseOrPR = provider.getPullRequestForCommit(ref); - if (promiseOrPR == null || !isPromise(promiseOrPR)) { - return promiseOrPR; - } - - if (options?.timeout != null && options.timeout > 0) { - promiseOrPR = cancellable(promiseOrPR, options.timeout); - } - - try { - return await promiseOrPR; - } catch (ex) { - if (ex instanceof PromiseCancelledError) throw ex; - - return undefined; - } - } - @debug({ args: { 0: remoteOrProvider => remoteOrProvider.name } }) async getMyPullRequests( remoteOrProvider: GitRemote | RichRemoteProvider, @@ -2118,14 +2068,16 @@ export class GitProviderService implements Disposable { @log() async getBestRemoteWithProvider( repoPath: string | Uri, + cancellation?: CancellationToken, ): Promise | undefined> { - const remotes = await this.getBestRemotesWithProviders(repoPath); + const remotes = await this.getBestRemotesWithProviders(repoPath, cancellation); return remotes[0]; } @log() async getBestRemotesWithProviders( repoPath: string | Uri, + cancellation?: CancellationToken, ): Promise[]> { if (repoPath == null) return []; if (typeof repoPath === 'string') { @@ -2136,10 +2088,12 @@ export class GitProviderService implements Disposable { let remotes = this._bestRemotesCache.get(cacheKey); if (remotes == null) { async function getBest(this: GitProviderService) { - const remotes = await this.getRemotesWithProviders(repoPath, { sort: true }); + const remotes = await this.getRemotesWithProviders(repoPath, { sort: true }, cancellation); if (remotes.length === 0) return []; if (remotes.length === 1) return [...remotes]; + if (cancellation?.isCancellationRequested) throw new CancellationError(); + const defaultRemote = remotes.find(r => r.default)?.name; const currentBranchRemote = (await this.getBranch(remotes[0].repoPath))?.getRemoteName(); @@ -2174,7 +2128,12 @@ export class GitProviderService implements Disposable { (p.maybeConnected || (p.maybeConnected === undefined && p.shouldConnect && (await p.isConnected()))) ) { - const repo = await p.getRepositoryMetadata(); + if (cancellation?.isCancellationRequested) throw new CancellationError(); + + const repo = await p.getRepositoryMetadata(cancellation); + + if (cancellation?.isCancellationRequested) throw new CancellationError(); + if (repo != null) { weight += repo.isFork ? -3 : 3; // Once we've found the "original" (not a fork) don't bother looking for more @@ -2202,13 +2161,17 @@ export class GitProviderService implements Disposable { async getBestRemoteWithRichProvider( repoPath: string | Uri, options?: { includeDisconnected?: boolean }, + cancellation?: CancellationToken, ): Promise | undefined> { - const remotes = await this.getBestRemotesWithProviders(repoPath); + const remotes = await this.getBestRemotesWithProviders(repoPath, cancellation); const includeDisconnected = options?.includeDisconnected ?? false; for (const r of remotes) { - if (r.hasRichIntegration() && (includeDisconnected || r.provider.maybeConnected === true)) { - return r; + if (r.hasRichIntegration()) { + if (includeDisconnected || r.provider.maybeConnected === true) return r; + if (r.provider.maybeConnected === undefined && r.default) { + if (await r.provider.isConnected()) return r; + } } } @@ -2216,7 +2179,11 @@ export class GitProviderService implements Disposable { } @log() - async getRemotes(repoPath: string | Uri, options?: { sort?: boolean }): Promise { + async getRemotes( + repoPath: string | Uri, + options?: { sort?: boolean }, + _cancellation?: CancellationToken, + ): Promise { if (repoPath == null) return []; const { provider, path } = this.getProvider(repoPath); @@ -2227,8 +2194,9 @@ export class GitProviderService implements Disposable { async getRemotesWithProviders( repoPath: string | Uri, options?: { sort?: boolean }, + cancellation?: CancellationToken, ): Promise[]> { - const remotes = await this.getRemotes(repoPath, options); + const remotes = await this.getRemotes(repoPath, options, cancellation); return remotes.filter( (r: GitRemote): r is GitRemote => r.provider != null, ); diff --git a/src/git/models/commit.ts b/src/git/models/commit.ts index 7047f84..34a63d2 100644 --- a/src/git/models/commit.ts +++ b/src/git/models/commit.ts @@ -1,4 +1,5 @@ import { Uri } from 'vscode'; +import type { EnrichedAutolink } from '../../annotations/autolinks'; import { getAvatarUri, getCachedAvatarUri } from '../../avatars'; import type { GravatarDefaultStyle } from '../../config'; import { DateSource, DateStyle } from '../../config'; @@ -8,11 +9,10 @@ import { formatDate, fromNow } from '../../system/date'; import { gate } from '../../system/decorators/gate'; import { memoize } from '../../system/decorators/memoize'; import { getLoggableName } from '../../system/logger'; -import { cancellable } from '../../system/promise'; import { pad, pluralize } from '../../system/string'; import type { PreviousLineComparisonUrisResult } from '../gitProvider'; import { GitUri } from '../gitUri'; -import type { RichRemoteProvider } from '../remotes/richRemoteProvider'; +import type { RemoteProvider } from '../remotes/remoteProvider'; import { uncommitted, uncommittedStaged } from './constants'; import type { GitFile } from './file'; import { GitFileChange, GitFileWorkingTreeStatus } from './file'; @@ -54,8 +54,8 @@ export class GitCommit implements GitRevisionReference { stashName?: string | undefined, stashOnRef?: string | undefined, ) { - this.ref = this.sha; - this.shortSha = this.sha.substring(0, this.container.CommitShaFormatting.length); + this.ref = sha; + this.shortSha = sha.substring(0, this.container.CommitShaFormatting.length); this.tips = tips; if (stashName) { @@ -75,6 +75,9 @@ export class GitCommit implements GitRevisionReference { } else { this._summary = summary; } + } else if (isUncommitted(sha, true)) { + this._summary = summary; + this._message = 'Uncommitted Changes'; } else { this._summary = `${summary} ${GlyphChars.Ellipsis}`; } @@ -201,8 +204,6 @@ export class GitCommit implements GitRevisionReference { // If the commit is "uncommitted", then have the files list be all uncommitted files if (this.isUncommitted) { - this._message = 'Uncommitted Changes'; - const repository = this.container.git.getRepository(this.repoPath); this._etagFileSystem = repository?.etagFileSystem; @@ -417,23 +418,30 @@ export class GitCommit implements GitRevisionReference { return status; } - private _pullRequest: Promise | undefined; - async getAssociatedPullRequest(options?: { - remote?: GitRemote; - timeout?: number; - }): Promise { - if (this._pullRequest == null) { - async function getCore(this: GitCommit): Promise { - const remote = - options?.remote ?? (await this.container.git.getBestRemoteWithRichProvider(this.repoPath)); - if (remote?.provider == null) return undefined; + async getAssociatedPullRequest(remote?: GitRemote): Promise { + remote ??= await this.container.git.getBestRemoteWithRichProvider(this.repoPath); + return remote?.hasRichIntegration() ? remote.provider.getPullRequestForCommit(this.ref) : undefined; + } - return this.container.git.getPullRequestForCommit(this.ref, remote, options); - } - this._pullRequest = getCore.call(this); + async getEnrichedAutolinks(remote?: GitRemote): Promise | undefined> { + if (this.isUncommitted) return undefined; + + remote ??= await this.container.git.getBestRemoteWithRichProvider(this.repoPath); + if (!remote?.hasRichIntegration()) return undefined; + + // TODO@eamodio should we cache these? Seems like we would use more memory than it's worth + // async function getCore(this: GitCommit): Promise | undefined> { + if (this.message == null) { + await this.ensureFullDetails(); } - return cancellable(this._pullRequest, options?.timeout); + return this.container.autolinks.getEnrichedAutolinks(this.message ?? this.summary, remote); + // } + + // const enriched = this.container.cache.getEnrichedAutolinks(this.sha, remote, () => ({ + // value: getCore.call(this), + // })); + // return enriched; } getAvatarUri(options?: { defaultStyle?: GravatarDefaultStyle; size?: number }): Uri | Promise { diff --git a/src/git/models/issue.ts b/src/git/models/issue.ts index cfc0d01..729449f 100644 --- a/src/git/models/issue.ts +++ b/src/git/models/issue.ts @@ -68,7 +68,13 @@ export function serializeIssueOrPullRequest(value: IssueOrPullRequest): IssueOrP return serialized; } -export function getIssueOrPullRequestHtmlIcon(issue: IssueOrPullRequest): string { +export function getIssueOrPullRequestHtmlIcon(issue?: IssueOrPullRequest): string { + if (issue == null) { + return ``; + } + if (issue.type === IssueOrPullRequestType.PullRequest) { if (issue.closed) { return ``; } -export function getIssueOrPullRequestMarkdownIcon(issue: IssueOrPullRequest): string { +export function getIssueOrPullRequestMarkdownIcon(issue?: IssueOrPullRequest): string { + if (issue == null) { + return `$(link)`; + } + if (issue.type === IssueOrPullRequestType.PullRequest) { if (issue.closed) { return `$(issues)`; } -export function getIssueOrPullRequestThemeIcon(issue: IssueOrPullRequest): ThemeIcon { +export function getIssueOrPullRequestThemeIcon(issue?: IssueOrPullRequest): ThemeIcon { + if (issue == null) { + return new ThemeIcon('link', new ThemeColor('gitlens.closedAutolinkedIssueIconColor' satisfies Colors)); + } + if (issue.type === IssueOrPullRequestType.PullRequest) { if (issue.closed) { return new ThemeIcon( diff --git a/src/git/remotes/richRemoteProvider.ts b/src/git/remotes/richRemoteProvider.ts index 5f034d2..457781c 100644 --- a/src/git/remotes/richRemoteProvider.ts +++ b/src/git/remotes/richRemoteProvider.ts @@ -1,4 +1,10 @@ -import type { AuthenticationSession, AuthenticationSessionsChangeEvent, Event, MessageItem } from 'vscode'; +import type { + AuthenticationSession, + AuthenticationSessionsChangeEvent, + CancellationToken, + Event, + MessageItem, +} from 'vscode'; import { authentication, Disposable, EventEmitter, window } from 'vscode'; import { wrapForForcedInsecureSSL } from '@env/fetch'; import { isWeb } from '@env/platform'; @@ -12,7 +18,6 @@ import { gate } from '../../system/decorators/gate'; import { debug, log } from '../../system/decorators/log'; import { Logger } from '../../system/logger'; import { getLogScope } from '../../system/logger.scope'; -import { isPromise } from '../../system/promise'; import type { Account } from '../models/author'; import type { DefaultBranch } from '../models/defaultBranch'; import type { IssueOrPullRequest, SearchedIssue } from '../models/issue'; @@ -167,8 +172,6 @@ export abstract class RichRemoteProvider extends RemoteProvider implements Dispo } this.resetRequestExceptionCount(); - this._repoMetadata = undefined; - this._prsByCommit.clear(); this._session = null; if (connected) { @@ -283,7 +286,6 @@ export abstract class RichRemoteProvider extends RemoteProvider implements Dispo }, ): Promise; - @gate() @debug() async getDefaultBranch(): Promise { const scope = getLogScope(); @@ -291,98 +293,75 @@ export abstract class RichRemoteProvider extends RemoteProvider implements Dispo const connected = this.maybeConnected ?? (await this.isConnected()); if (!connected) return undefined; - try { - const defaultBranch = await this.getProviderDefaultBranch(this._session!); - this.resetRequestExceptionCount(); - return defaultBranch; - } catch (ex) { - Logger.error(ex, scope); - - if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) { - this.trackRequestException(); - } - return undefined; - } + const defaultBranch = this.container.cache.getRepositoryDefaultBranch(this, () => ({ + value: (async () => { + try { + const result = await this.getProviderDefaultBranch(this._session!); + this.resetRequestExceptionCount(); + return result; + } catch (ex) { + Logger.error(ex, scope); + + if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) { + this.trackRequestException(); + } + return undefined; + } + })(), + })); + return defaultBranch; } protected abstract getProviderDefaultBranch({ accessToken, }: AuthenticationSession): Promise; - private _repoMetadata: RepositoryMetadata | undefined; + private _ignoreSSLErrors = new Map(); + getIgnoreSSLErrors(): boolean | 'force' { + if (isWeb) return false; - @gate() - @debug() - async getRepositoryMetadata(): Promise { - if (this._repoMetadata != null) return this._repoMetadata; + let ignoreSSLErrors = this._ignoreSSLErrors.get(this.id); + if (ignoreSSLErrors === undefined) { + const cfg = configuration + .get('remotes') + ?.find(remote => remote.type.toLowerCase() === this.id && remote.domain === this.domain); + ignoreSSLErrors = cfg?.ignoreSSLErrors ?? false; + this._ignoreSSLErrors.set(this.id, ignoreSSLErrors); + } + return ignoreSSLErrors; + } + + @debug() + async getRepositoryMetadata(_cancellation?: CancellationToken): Promise { const scope = getLogScope(); const connected = this.maybeConnected ?? (await this.isConnected()); if (!connected) return undefined; - try { - const metadata = await this.getProviderRepositoryMetadata(this._session!); - this._repoMetadata = metadata; - this.resetRequestExceptionCount(); - return metadata; - } catch (ex) { - Logger.error(ex, scope); - - if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) { - this.trackRequestException(); - } - return undefined; - } + const metadata = this.container.cache.getRepositoryMetadata(this, () => ({ + value: (async () => { + try { + const result = await this.getProviderRepositoryMetadata(this._session!); + this.resetRequestExceptionCount(); + return result; + } catch (ex) { + Logger.error(ex, scope); + + if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) { + this.trackRequestException(); + } + return undefined; + } + })(), + })); + return metadata; } protected abstract getProviderRepositoryMetadata({ accessToken, }: AuthenticationSession): Promise; - @gate() - @debug() - async searchMyPullRequests(): Promise { - const scope = getLogScope(); - - try { - const pullRequests = await this.searchProviderMyPullRequests(this._session!); - this.resetRequestExceptionCount(); - return pullRequests; - } catch (ex) { - Logger.error(ex, scope); - - if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) { - this.trackRequestException(); - } - return undefined; - } - } - protected abstract searchProviderMyPullRequests( - session: AuthenticationSession, - ): Promise; - - @gate() - @debug() - async searchMyIssues(): Promise { - const scope = getLogScope(); - - try { - const issues = await this.searchProviderMyIssues(this._session!); - this.resetRequestExceptionCount(); - return issues; - } catch (ex) { - Logger.error(ex, scope); - - if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) { - this.trackRequestException(); - } - return undefined; - } - } - protected abstract searchProviderMyIssues(session: AuthenticationSession): Promise; - - @gate() @debug() async getIssueOrPullRequest(id: string): Promise { const scope = getLogScope(); @@ -390,34 +369,23 @@ export abstract class RichRemoteProvider extends RemoteProvider implements Dispo const connected = this.maybeConnected ?? (await this.isConnected()); if (!connected) return undefined; - try { - const issueOrPullRequest = await this.getProviderIssueOrPullRequest(this._session!, id); - this.resetRequestExceptionCount(); - return issueOrPullRequest; - } catch (ex) { - Logger.error(ex, scope); - - if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) { - this.trackRequestException(); - } - return undefined; - } - } - - private _ignoreSSLErrors = new Map(); - getIgnoreSSLErrors(): boolean | 'force' { - if (isWeb) return false; - - let ignoreSSLErrors = this._ignoreSSLErrors.get(this.id); - if (ignoreSSLErrors === undefined) { - const cfg = configuration - .get('remotes') - ?.find(remote => remote.type.toLowerCase() === this.id && remote.domain === this.domain); - ignoreSSLErrors = cfg?.ignoreSSLErrors ?? false; - this._ignoreSSLErrors.set(this.id, ignoreSSLErrors); - } - - return ignoreSSLErrors; + const issueOrPR = this.container.cache.getIssueOrPullRequest(id, this, () => ({ + value: (async () => { + try { + const result = await this.getProviderIssueOrPullRequest(this._session!, id); + this.resetRequestExceptionCount(); + return result; + } catch (ex) { + Logger.error(ex, scope); + + if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) { + this.trackRequestException(); + } + return undefined; + } + })(), + })); + return issueOrPR; } protected abstract getProviderIssueOrPullRequest( @@ -425,7 +393,6 @@ export abstract class RichRemoteProvider extends RemoteProvider implements Dispo id: string, ): Promise; - @gate() @debug() async getPullRequestForBranch( branch: string, @@ -439,19 +406,25 @@ export abstract class RichRemoteProvider extends RemoteProvider implements Dispo const connected = this.maybeConnected ?? (await this.isConnected()); if (!connected) return undefined; - try { - const pr = await this.getProviderPullRequestForBranch(this._session!, branch, options); - this.resetRequestExceptionCount(); - return pr; - } catch (ex) { - Logger.error(ex, scope); - - if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) { - this.trackRequestException(); - } - return undefined; - } + const pr = this.container.cache.getPullRequestForBranch(branch, this, () => ({ + value: (async () => { + try { + const result = await this.getProviderPullRequestForBranch(this._session!, branch, options); + this.resetRequestExceptionCount(); + return result; + } catch (ex) { + Logger.error(ex, scope); + + if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) { + this.trackRequestException(); + } + return undefined; + } + })(), + })); + return pr; } + protected abstract getProviderPullRequestForBranch( session: AuthenticationSession, branch: string, @@ -461,48 +434,78 @@ export abstract class RichRemoteProvider extends RemoteProvider implements Dispo }, ): Promise; - private _prsByCommit = new Map | PullRequest | null>(); - @debug() - getPullRequestForCommit(ref: string): Promise | PullRequest | undefined { - let pr = this._prsByCommit.get(ref); - if (pr === undefined) { - pr = this.getPullRequestForCommitCore(ref); - this._prsByCommit.set(ref, pr); - } - if (pr == null || !isPromise(pr)) return pr ?? undefined; + async getPullRequestForCommit(ref: string): Promise { + const scope = getLogScope(); - return pr.then(pr => pr ?? undefined); + const connected = this.maybeConnected ?? (await this.isConnected()); + if (!connected) return undefined; + + const pr = this.container.cache.getPullRequestForSha(ref, this, () => ({ + value: (async () => { + try { + const result = await this.getProviderPullRequestForCommit(this._session!, ref); + this.resetRequestExceptionCount(); + return result; + } catch (ex) { + Logger.error(ex, scope); + + if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) { + this.trackRequestException(); + } + return undefined; + } + })(), + })); + return pr; } + protected abstract getProviderPullRequestForCommit( + session: AuthenticationSession, + ref: string, + ): Promise; + + @gate() @debug() - private async getPullRequestForCommitCore(ref: string) { + async searchMyIssues(): Promise { const scope = getLogScope(); - const connected = this.maybeConnected ?? (await this.isConnected()); - if (!connected) return null; - try { - const pr = (await this.getProviderPullRequestForCommit(this._session!, ref)) ?? null; - this._prsByCommit.set(ref, pr); + const issues = await this.searchProviderMyIssues(this._session!); this.resetRequestExceptionCount(); - return pr; + return issues; } catch (ex) { Logger.error(ex, scope); - this._prsByCommit.delete(ref); - if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) { this.trackRequestException(); } - return null; + return undefined; } } + protected abstract searchProviderMyIssues(session: AuthenticationSession): Promise; - protected abstract getProviderPullRequestForCommit( + @gate() + @debug() + async searchMyPullRequests(): Promise { + const scope = getLogScope(); + + try { + const pullRequests = await this.searchProviderMyPullRequests(this._session!); + this.resetRequestExceptionCount(); + return pullRequests; + } catch (ex) { + Logger.error(ex, scope); + + if (ex instanceof AuthenticationError || ex instanceof ProviderRequestClientError) { + this.trackRequestException(); + } + return undefined; + } + } + protected abstract searchProviderMyPullRequests( session: AuthenticationSession, - ref: string, - ): Promise; + ): Promise; @gate() private async ensureSession( diff --git a/src/hovers/hovers.ts b/src/hovers/hovers.ts index e668cdf..cecc1c4 100644 --- a/src/hovers/hovers.ts +++ b/src/hovers/hovers.ts @@ -1,9 +1,9 @@ import type { CancellationToken, TextDocument } from 'vscode'; import { MarkdownString } from 'vscode'; -import { hrtime } from '@env/hrtime'; +import type { EnrichedAutolink } from '../annotations/autolinks'; import { DiffWithCommand, ShowQuickCommitCommand } from '../commands'; import { GlyphChars } from '../constants'; -import { Container } from '../container'; +import type { Container } from '../container'; import { CommitFormatter } from '../git/formatters/commitFormatter'; import { GitUri } from '../git/gitUri'; import type { GitCommit } from '../git/models/commit'; @@ -11,16 +11,14 @@ import { uncommittedStaged } from '../git/models/constants'; import type { GitDiffHunk, GitDiffHunkLine } from '../git/models/diff'; import type { PullRequest } from '../git/models/pullRequest'; import { isUncommittedStaged, shortenRevision } from '../git/models/reference'; -import { GitRemote } from '../git/models/remote'; +import type { GitRemote } from '../git/models/remote'; +import type { RemoteProvider } from '../git/remotes/remoteProvider'; +import { pauseOnCancelOrTimeout, pauseOnCancelOrTimeoutMapTuplePromise } from '../system/cancellation'; import { configuration } from '../system/configuration'; -import { count } from '../system/iterable'; -import { Logger } from '../system/logger'; -import { LogLevel } from '../system/logger.constants'; -import { getNewLogScope } from '../system/logger.scope'; -import { getSettledValue, PromiseCancelledError } from '../system/promise'; -import { getDurationMilliseconds } from '../system/string'; +import { getSettledValue } from '../system/promise'; export async function changesMessage( + container: Container, commit: GitCommit, uri: GitUri, editorLine: number, // 0-based, Git is 1-based @@ -59,11 +57,11 @@ export async function changesMessage( editorLine = commitLine.line - 1; // TODO: Doesn't work with dirty files -- pass in editor? or contents? - let hunkLine = await Container.instance.git.getDiffForLine(uri, editorLine, ref, documentRef); + let hunkLine = await container.git.getDiffForLine(uri, editorLine, ref, documentRef); // If we didn't find a diff & ref is undefined (meaning uncommitted), check for a staged diff if (hunkLine == null && ref == null && documentRef !== uncommittedStaged) { - hunkLine = await Container.instance.git.getDiffForLine(uri, editorLine, undefined, uncommittedStaged); + hunkLine = await container.git.getDiffForLine(uri, editorLine, undefined, uncommittedStaged); } return hunkLine != null ? getDiffFromHunkLine(hunkLine) : undefined; @@ -188,80 +186,103 @@ export async function localChangesMessage( } export async function detailsMessage( + container: Container, commit: GitCommit, uri: GitUri, editorLine: number, // 0-based, Git is 1-based options: Readonly<{ autolinks?: boolean; - cancellationToken?: CancellationToken; + cancellation?: CancellationToken; dateFormat: string | null; + enrichedAutolinks?: Promise | undefined> | undefined; format: string; - pullRequests?: Readonly<{ - enabled: boolean; - pr?: PullRequest | PromiseCancelledError>; - }>; getBranchAndTagTips?: ( sha: string, options?: { compact?: boolean | undefined; icons?: boolean | undefined }, ) => string | undefined; + pullRequest?: Promise | PullRequest | undefined; + pullRequests?: boolean; + remotes?: GitRemote[]; + timeout?: number; }>, -): Promise { - let message = commit.message ?? commit.summary; - if (commit.message == null && !commit.isUncommitted) { - await commit.ensureFullDetails(); - message = commit.message ?? commit.summary; - - if (options?.cancellationToken?.isCancellationRequested) return new MarkdownString(); +): Promise { + const remotesResult = await pauseOnCancelOrTimeout( + options?.remotes ?? container.git.getBestRemotesWithProviders(commit.repoPath), + options?.cancellation, + options?.timeout, + ); + + let remotes: GitRemote[] | undefined; + let remote: GitRemote | undefined; + if (remotesResult.paused) { + if (remotesResult.reason === 'cancelled') return undefined; + // If we timed out, just continue without the remotes + } else { + remotes = remotesResult.value; + [remote] = remotes; } - const [ - remotesResult, - previousLineComparisonUrisResult, - autolinkedIssuesOrPullRequestsResult, - prResult, - presenceResult, - ] = await Promise.allSettled([ - Container.instance.git.getRemotesWithProviders(commit.repoPath, { sort: true }), - commit.isUncommitted ? commit.getPreviousComparisonUrisForLine(editorLine, uri.sha) : undefined, - getAutoLinkedIssuesOrPullRequests(message, commit.repoPath), - options?.pullRequests?.pr ?? - getPullRequestForCommitOrBestRemote(commit.ref, commit.repoPath, { - pullRequests: - options?.pullRequests?.enabled !== false && - CommitFormatter.has( - options.format, - 'pullRequest', - 'pullRequestAgo', - 'pullRequestAgoOrDate', - 'pullRequestDate', - 'pullRequestState', - ), - }), - Container.instance.vsls.maybeGetPresence(commit.author.email), - ]); - - if (options?.cancellationToken?.isCancellationRequested) return new MarkdownString(); - - const remotes = getSettledValue(remotesResult); - const previousLineComparisonUris = getSettledValue(previousLineComparisonUrisResult); - const autolinkedIssuesOrPullRequests = getSettledValue(autolinkedIssuesOrPullRequestsResult); + + const cfg = configuration.get('hovers'); + const autolinks = + remote?.provider != null && + (options?.autolinks || (options?.autolinks !== false && cfg.autolinks.enabled && cfg.autolinks.enhanced)) && + CommitFormatter.has(cfg.detailsMarkdownFormat, 'message'); + const prs = + remote?.hasRichIntegration() && + remote.provider.maybeConnected !== false && + (options?.pullRequests || (options?.pullRequests !== false && cfg.pullRequests.enabled)) && + CommitFormatter.has( + options.format, + 'pullRequest', + 'pullRequestAgo', + 'pullRequestAgoOrDate', + 'pullRequestDate', + 'pullRequestState', + ); + + const [enrichedAutolinksResult, prResult, presenceResult, previousLineComparisonUrisResult] = + await Promise.allSettled([ + autolinks + ? pauseOnCancelOrTimeoutMapTuplePromise( + options?.enrichedAutolinks ?? commit.getEnrichedAutolinks(remote), + options?.cancellation, + options?.timeout, + ) + : undefined, + prs + ? pauseOnCancelOrTimeout( + options?.pullRequest ?? commit.getAssociatedPullRequest(remote), + options?.cancellation, + options?.timeout, + ) + : undefined, + container.vsls.maybeGetPresence(commit.author.email), + commit.isUncommitted ? commit.getPreviousComparisonUrisForLine(editorLine, uri.sha) : undefined, + commit.message == null ? commit.ensureFullDetails() : undefined, + ]); + + if (options?.cancellation?.isCancellationRequested) return undefined; + + const enrichedResult = getSettledValue(enrichedAutolinksResult); const pr = getSettledValue(prResult); const presence = getSettledValue(presenceResult); + const previousLineComparisonUris = getSettledValue(previousLineComparisonUrisResult); // Remove possible duplicate pull request - if (pr != null && !(pr instanceof PromiseCancelledError || pr instanceof GitRemote)) { - autolinkedIssuesOrPullRequests?.delete(pr.id); + if (pr?.value != null && !pr.paused && enrichedResult?.value != null && !enrichedResult.paused) { + enrichedResult.value?.delete(pr.value.id); } const details = await CommitFormatter.fromTemplateAsync(options.format, commit, { - autolinkedIssuesOrPullRequests: autolinkedIssuesOrPullRequests, + enrichedAutolinks: enrichedResult?.value != null && !enrichedResult.paused ? enrichedResult.value : undefined, dateFormat: options.dateFormat === null ? 'MMMM Do, YYYY h:mma' : options.dateFormat, editor: { line: editorLine, uri: uri, }, getBranchAndTagTips: options?.getBranchAndTagTips, - messageAutolinks: options?.autolinks, - pullRequestOrRemote: pr, + messageAutolinks: options?.autolinks || (options?.autolinks !== false && cfg.autolinks.enabled), + pullRequest: pr?.value, presence: presence, previousLineComparisonUris: previousLineComparisonUris, outputFormat: 'markdown', @@ -287,129 +308,3 @@ function getDiffFromHunkLine(hunkLine: GitDiffHunkLine, diffStyle?: 'line' | 'hu hunkLine.current == null ? '' : `\n+ ${hunkLine.current.line.trim()}` }\n\`\`\``; } - -async function getAutoLinkedIssuesOrPullRequests(message: string, repoPath: string) { - const scope = getNewLogScope('Hovers.getAutoLinkedIssuesOrPullRequests'); - Logger.debug(scope, `${GlyphChars.Dash} message=`); - - const start = hrtime(); - - const cfg = configuration.get('hovers'); - if ( - !cfg.autolinks.enabled || - !cfg.autolinks.enhanced || - !CommitFormatter.has(cfg.detailsMarkdownFormat, 'message') - ) { - Logger.debug(scope, `completed [${getDurationMilliseconds(start)}ms]`); - - return undefined; - } - - const remote = await Container.instance.git.getBestRemoteWithRichProvider(repoPath); - if (remote?.provider == null) { - Logger.debug(scope, `completed [${getDurationMilliseconds(start)}ms]`); - - return undefined; - } - - // TODO: Make this configurable? - const timeout = 250; - - try { - const autolinks = await Container.instance.autolinks.getLinkedIssuesAndPullRequests(message, remote, { - timeout: timeout, - }); - - if (autolinks != null && Logger.enabled(LogLevel.Debug)) { - // If there are any issues/PRs that timed out, log it - const prCount = count(autolinks.values(), pr => pr instanceof PromiseCancelledError); - if (prCount !== 0) { - Logger.debug( - scope, - `timed out ${ - GlyphChars.Dash - } ${prCount} issue/pull request queries took too long (over ${timeout} ms) [${getDurationMilliseconds( - start, - )}ms]`, - ); - - // const pending = [ - // ...Iterables.map(autolinks.values(), issueOrPullRequest => - // issueOrPullRequest instanceof CancelledPromiseError - // ? issueOrPullRequest.promise - // : undefined, - // ), - // ]; - // void Promise.all(pending).then(() => { - // Logger.debug( - // scope, - // `${GlyphChars.Dot} ${count} issue/pull request queries completed; refreshing...`, - // ); - // void executeCoreCommand('editor.action.showHover'); - // }); - - return autolinks; - } - } - - Logger.debug(scope, `completed [${getDurationMilliseconds(start)}ms]`); - - return autolinks; - } catch (ex) { - Logger.error(ex, scope, `failed [${getDurationMilliseconds(start)}ms]`); - - return undefined; - } -} - -async function getPullRequestForCommitOrBestRemote( - ref: string, - repoPath: string, - options?: { - pullRequests?: boolean; - }, -) { - const scope = getNewLogScope('Hovers.getPullRequestForCommit'); - Logger.debug(scope, `${GlyphChars.Dash} ref=${ref}`); - - const start = hrtime(); - - if (!options?.pullRequests) { - Logger.debug(scope, `completed [${getDurationMilliseconds(start)}ms]`); - - return undefined; - } - - const remote = await Container.instance.git.getBestRemoteWithRichProvider(repoPath, { includeDisconnected: true }); - if (remote?.provider == null) { - Logger.debug(scope, `completed [${getDurationMilliseconds(start)}ms]`); - - return undefined; - } - - const { provider } = remote; - const connected = provider.maybeConnected ?? (await provider.isConnected()); - if (!connected) { - Logger.debug(scope, `completed [${getDurationMilliseconds(start)}ms]`); - - return remote; - } - - try { - const pr = await Container.instance.git.getPullRequestForCommit(ref, provider, { timeout: 250 }); - - Logger.debug(scope, `completed [${getDurationMilliseconds(start)}ms]`); - - return pr; - } catch (ex) { - if (ex instanceof PromiseCancelledError) { - Logger.debug(scope, `timed out [${getDurationMilliseconds(start)}ms]`); - - return ex; - } - - Logger.error(ex, scope, `failed [${getDurationMilliseconds(start)}ms]`); - - return undefined; - } -} diff --git a/src/hovers/lineHoverController.ts b/src/hovers/lineHoverController.ts index b231f08..946bf5c 100644 --- a/src/hovers/lineHoverController.ts +++ b/src/hovers/lineHoverController.ts @@ -86,7 +86,7 @@ export class LineHoverController implements Disposable { async provideDetailsHover( document: TextDocument, position: Position, - _token: CancellationToken, + token: CancellationToken, ): Promise { if (!this.container.lineTracker.includes(position.line)) return undefined; @@ -122,16 +122,17 @@ export class LineHoverController implements Disposable { editorLine = commitLine.originalLine - 1; const trackedDocument = await this.container.tracker.get(document); - if (trackedDocument == null) return undefined; - - const message = await detailsMessage(commit, trackedDocument.uri, editorLine, { - autolinks: cfg.autolinks.enabled, - dateFormat: configuration.get('defaultDateFormat'), - format: cfg.detailsMarkdownFormat, - pullRequests: { - enabled: cfg.pullRequests.enabled, - }, - }); + if (trackedDocument == null || token.isCancellationRequested) return undefined; + + const message = + (await detailsMessage(this.container, commit, trackedDocument.uri, editorLine, { + autolinks: cfg.autolinks.enabled, + cancellation: token, + dateFormat: configuration.get('defaultDateFormat'), + format: cfg.detailsMarkdownFormat, + pullRequests: cfg.pullRequests.enabled, + timeout: 250, + })) ?? 'Cancelled'; return new Hover(message, range); } @@ -178,7 +179,13 @@ export class LineHoverController implements Disposable { const trackedDocument = await this.container.tracker.get(document); if (trackedDocument == null) return undefined; - const message = await changesMessage(commit, trackedDocument.uri, position.line, trackedDocument.document); + const message = await changesMessage( + this.container, + commit, + trackedDocument.uri, + position.line, + trackedDocument.document, + ); if (message == null) return undefined; return new Hover(message, range); diff --git a/src/statusbar/statusBarController.ts b/src/statusbar/statusBarController.ts index 7108b6b..c2f5beb 100644 --- a/src/statusbar/statusBarController.ts +++ b/src/statusbar/statusBarController.ts @@ -1,30 +1,28 @@ -import type { CancellationToken, ConfigurationChangeEvent, StatusBarItem, TextEditor, Uri } from 'vscode'; +import type { ConfigurationChangeEvent, StatusBarItem, TextEditor, Uri } from 'vscode'; import { CancellationTokenSource, Disposable, MarkdownString, StatusBarAlignment, window } from 'vscode'; import type { ToggleFileChangesAnnotationCommandArgs } from '../commands/toggleFileAnnotations'; import { FileAnnotationType, StatusBarCommand } from '../config'; import { Commands, GlyphChars } from '../constants'; import type { Container } from '../container'; import { CommitFormatter } from '../git/formatters/commitFormatter'; -import type { GitCommit } from '../git/models/commit'; import type { PullRequest } from '../git/models/pullRequest'; import { detailsMessage } from '../hovers/hovers'; +import type { MaybePausedResult } from '../system/cancellation'; +import { pauseOnCancelOrTimeout } from '../system/cancellation'; import { asCommand } from '../system/command'; import { configuration } from '../system/configuration'; import { debug } from '../system/decorators/log'; import { once } from '../system/event'; import { Logger } from '../system/logger'; -import type { LogScope } from '../system/logger.scope'; import { getLogScope } from '../system/logger.scope'; -import { PromiseCancelledError } from '../system/promise'; +import { getSettledValue } from '../system/promise'; import { isTextEditor } from '../system/utils'; -import type { LinesChangeEvent } from '../trackers/gitLineTracker'; +import type { GitLineState, LinesChangeEvent } from '../trackers/gitLineTracker'; export class StatusBarController implements Disposable { - private _pullRequestCancellation: CancellationTokenSource | undefined; - private _tooltipCancellation: CancellationTokenSource | undefined; - private _tooltipDelayTimer: ReturnType | undefined; - + private _cancellation: CancellationTokenSource | undefined; private readonly _disposable: Disposable; + private _selectedSha: string | undefined; private _statusBarBlame: StatusBarItem | undefined; private _statusBarMode: StatusBarItem | undefined; @@ -146,7 +144,7 @@ export class StatusBarController implements Disposable { if (!e.pending && e.selections != null) { const state = this.container.lineTracker.getState(e.selections[0].active); if (state?.commit != null) { - void this.updateBlame(e.editor!, state.commit); + void this.updateBlame(e.editor!, state); return; } @@ -156,121 +154,101 @@ export class StatusBarController implements Disposable { if (clear) { this.clearBlame(); - } else if (this._statusBarBlame != null) { - this._statusBarBlame.text = this._statusBarBlame.text.replace('$(git-commit)', '$(watch)'); + } else if (this._statusBarBlame?.text.startsWith('$(git-commit)')) { + this._statusBarBlame.text = `$(watch)${this._statusBarBlame.text.substring(13)}`; } } clearBlame() { - this._pullRequestCancellation?.cancel(); - this._tooltipCancellation?.cancel(); + this._selectedSha = undefined; + this._cancellation?.cancel(); this._statusBarBlame?.hide(); } - @debug({ args: false }) - private async updateBlame(editor: TextEditor, commit: GitCommit, options?: { pr?: PullRequest | null }) { + @debug({ + args: { + 0: false, + 1: s => s.commit?.sha, + }, + }) + private async updateBlame(editor: TextEditor, state: GitLineState) { const cfg = configuration.get('statusBar'); - if (!cfg.enabled || this._statusBarBlame == null || !isTextEditor(editor)) return; + if (!cfg.enabled || this._statusBarBlame == null || !isTextEditor(editor)) { + this._cancellation?.cancel(); + this._selectedSha = undefined; - const scope = getLogScope(); + return; + } - const showPullRequests = - cfg.pullRequests.enabled && - (CommitFormatter.has( - cfg.format, - 'pullRequest', - 'pullRequestAgo', - 'pullRequestAgoOrDate', - 'pullRequestDate', - 'pullRequestState', - ) || - CommitFormatter.has( - cfg.tooltipFormat, - 'pullRequest', - 'pullRequestAgo', - 'pullRequestAgoOrDate', - 'pullRequestDate', - 'pullRequestState', - )); + const { commit } = state; + if (commit == null) { + this._cancellation?.cancel(); - // TODO: Make this configurable? - const timeout = 100; - const [getBranchAndTagTips, pr] = await Promise.all([ - CommitFormatter.has(cfg.format, 'tips') || CommitFormatter.has(cfg.tooltipFormat, 'tips') - ? this.container.git.getBranchesAndTagsTipsFn(commit.repoPath) - : undefined, - showPullRequests && options?.pr === undefined - ? this.getPullRequest(commit, { timeout: timeout }) - : options?.pr ?? undefined, - ]); - - if (pr != null) { - this._pullRequestCancellation?.cancel(); - this._pullRequestCancellation = new CancellationTokenSource(); - void this.waitForPendingPullRequest( - editor, - commit, - pr, - this._pullRequestCancellation.token, - timeout, - scope, - ); + return; } - this._statusBarBlame.text = `$(git-commit) ${CommitFormatter.fromTemplate(cfg.format, commit, { - dateFormat: cfg.dateFormat === null ? configuration.get('defaultDateFormat') : cfg.dateFormat, - getBranchAndTagTips: getBranchAndTagTips, - messageTruncateAtNewLine: true, - pullRequestOrRemote: pr, - pullRequestPendingMessage: 'PR $(watch)', - })}`; + // We can avoid refreshing if the commit is the same, except when the commit is uncommitted, since we need to incorporate the line number in the hover + if (this._selectedSha === commit.sha && !commit.isUncommitted) { + if (this._statusBarBlame?.text.startsWith('$(watch)')) { + this._statusBarBlame.text = `$(git-commit)${this._statusBarBlame.text.substring(8)}`; + } - let tooltip: string; + return; + } + + const scope = getLogScope(); + this._selectedSha = commit.sha; + + this._cancellation?.cancel(); + this._cancellation = new CancellationTokenSource(); + const cancellation = this._cancellation.token; + + let actionTooltip: string; switch (cfg.command) { case StatusBarCommand.CopyRemoteCommitUrl: - tooltip = 'Click to Copy Remote Commit URL'; + actionTooltip = 'Click to Copy Remote Commit URL'; break; case StatusBarCommand.CopyRemoteFileUrl: this._statusBarBlame.command = Commands.CopyRemoteFileUrl; - tooltip = 'Click to Copy Remote File Revision URL'; + actionTooltip = 'Click to Copy Remote File Revision URL'; break; case StatusBarCommand.DiffWithPrevious: this._statusBarBlame.command = Commands.DiffLineWithPrevious; - tooltip = 'Click to Open Line Changes with Previous Revision'; + actionTooltip = 'Click to Open Line Changes with Previous Revision'; break; case StatusBarCommand.DiffWithWorking: this._statusBarBlame.command = Commands.DiffLineWithWorking; - tooltip = 'Click to Open Line Changes with Working File'; + actionTooltip = 'Click to Open Line Changes with Working File'; break; case StatusBarCommand.OpenCommitOnRemote: - tooltip = 'Click to Open Commit on Remote'; + actionTooltip = 'Click to Open Commit on Remote'; break; case StatusBarCommand.OpenFileOnRemote: - tooltip = 'Click to Open Revision on Remote'; + actionTooltip = 'Click to Open Revision on Remote'; break; case StatusBarCommand.RevealCommitInView: - tooltip = 'Click to Reveal Commit in the Side Bar'; + actionTooltip = 'Click to Reveal Commit in the Side Bar'; break; case StatusBarCommand.ShowCommitsInView: - tooltip = 'Click to Search for Commit'; + actionTooltip = 'Click to Search for Commit'; break; case StatusBarCommand.ShowQuickCommitDetails: - tooltip = 'Click to Show Commit'; + actionTooltip = 'Click to Show Commit'; break; case StatusBarCommand.ShowQuickCommitFileDetails: - tooltip = 'Click to Show Commit (file)'; + actionTooltip = 'Click to Show Commit (file)'; break; case StatusBarCommand.ShowQuickCurrentBranchHistory: - tooltip = 'Click to Show Branch History'; + actionTooltip = 'Click to Show Branch History'; break; case StatusBarCommand.ShowQuickFileHistory: - tooltip = 'Click to Show File History'; + actionTooltip = 'Click to Show File History'; break; case StatusBarCommand.ToggleCodeLens: - tooltip = 'Click to Toggle Git CodeLens'; + actionTooltip = 'Click to Toggle Git CodeLens'; break; case StatusBarCommand.ToggleFileBlame: - tooltip = 'Click to Toggle File Blame'; + actionTooltip = 'Click to Toggle File Blame'; break; case StatusBarCommand.ToggleFileChanges: { if (commit.file != null) { @@ -286,7 +264,7 @@ export class StatusBarController implements Disposable { ], }); } - tooltip = 'Click to Toggle File Changes'; + actionTooltip = 'Click to Toggle File Changes'; break; } case StatusBarCommand.ToggleFileChangesOnly: { @@ -303,114 +281,147 @@ export class StatusBarController implements Disposable { ], }); } - tooltip = 'Click to Toggle File Changes'; + actionTooltip = 'Click to Toggle File Changes'; break; } case StatusBarCommand.ToggleFileHeatmap: - tooltip = 'Click to Toggle File Heatmap'; + actionTooltip = 'Click to Toggle File Heatmap'; break; } - this._statusBarBlame.tooltip = tooltip; + this._statusBarBlame.tooltip = new MarkdownString(`Loading... \n\n---\n\n${actionTooltip}`); this._statusBarBlame.accessibilityInformation = { - label: `${this._statusBarBlame.text}\n${tooltip}`, + label: `${this._statusBarBlame.text}\n${actionTooltip}`, }; - if (this._tooltipDelayTimer != null) { - clearTimeout(this._tooltipDelayTimer); - } - this._tooltipCancellation?.cancel(); - - this._tooltipDelayTimer = setTimeout(() => { - this._tooltipDelayTimer = undefined; - this._tooltipCancellation = new CancellationTokenSource(); - - void this.updateCommitTooltip( - this._statusBarBlame!, - commit, - tooltip, - getBranchAndTagTips, - { - enabled: showPullRequests || pr != null, - pr: pr, - }, - this._tooltipCancellation.token, - ); - }, 500); + const remotes = await this.container.git.getBestRemotesWithProviders(commit.repoPath); + const [remote] = remotes; - this._statusBarBlame.show(); - } + const defaultDateFormat = configuration.get('defaultDateFormat'); + const getBranchAndTagTipsPromise = + CommitFormatter.has(cfg.format, 'tips') || CommitFormatter.has(cfg.tooltipFormat, 'tips') + ? this.container.git.getBranchesAndTagsTipsFn(commit.repoPath) + : undefined; + + const showPullRequests = + !commit.isUncommitted && + remote?.hasRichIntegration() && + cfg.pullRequests.enabled && + (CommitFormatter.has( + cfg.format, + 'pullRequest', + 'pullRequestAgo', + 'pullRequestAgoOrDate', + 'pullRequestDate', + 'pullRequestState', + ) || + CommitFormatter.has( + cfg.tooltipFormat, + 'pullRequest', + 'pullRequestAgo', + 'pullRequestAgoOrDate', + 'pullRequestDate', + 'pullRequestState', + )); - private async getPullRequest( - commit: GitCommit, - { timeout }: { timeout?: number } = {}, - ): Promise> | undefined> { - const remote = await this.container.git.getBestRemoteWithRichProvider(commit.repoPath); - if (remote?.provider == null) return undefined; - - const { provider } = remote; - try { - return await this.container.git.getPullRequestForCommit(commit.ref, provider, { timeout: timeout }); - } catch (ex) { - return ex instanceof PromiseCancelledError ? ex : undefined; + function setBlameText( + statusBarItem: StatusBarItem, + getBranchAndTagTips: Awaited | undefined, + pr: Promise | PullRequest | undefined, + ) { + statusBarItem.text = `$(git-commit) ${CommitFormatter.fromTemplate(cfg.format, commit, { + dateFormat: cfg.dateFormat === null ? defaultDateFormat : cfg.dateFormat, + getBranchAndTagTips: getBranchAndTagTips, + messageTruncateAtNewLine: true, + pullRequest: pr, + pullRequestPendingMessage: 'PR $(watch)', + remotes: remotes, + })}`; + statusBarItem.accessibilityInformation = { + label: `${statusBarItem.text}\n${actionTooltip}`, + }; } - } - private async updateCommitTooltip( - statusBarItem: StatusBarItem, - commit: GitCommit, - actionTooltip: string, - getBranchAndTagTips: - | (( - sha: string, - options?: { compact?: boolean | undefined; icons?: boolean | undefined } | undefined, - ) => string | undefined) - | undefined, - pullRequests: { - enabled: boolean; - pr: PullRequest | PromiseCancelledError> | undefined; - }, - cancellationToken: CancellationToken, - ) { - if (cancellationToken.isCancellationRequested) return; - - const tooltip = await detailsMessage(commit, commit.getGitUri(), commit.lines[0].line, { - autolinks: true, - cancellationToken: cancellationToken, - dateFormat: configuration.get('defaultDateFormat'), - format: configuration.get('statusBar.tooltipFormat'), - getBranchAndTagTips: getBranchAndTagTips, - pullRequests: pullRequests, - }); - - if (cancellationToken.isCancellationRequested) return; - - tooltip.appendMarkdown(`\n\n---\n\n${actionTooltip}`); - statusBarItem.tooltip = tooltip; - statusBarItem.accessibilityInformation = { - label: `${statusBarItem.text}\n${actionTooltip}`, - }; - } + async function getBlameTooltip( + container: Container, + getBranchAndTagTips: Awaited | undefined, + pr: Promise | PullRequest | undefined, + timeout?: number, + ) { + return detailsMessage(container, commit, commit.getGitUri(), commit.lines[0].line, { + autolinks: true, + cancellation: cancellation, + dateFormat: defaultDateFormat, + format: cfg.tooltipFormat, + getBranchAndTagTips: getBranchAndTagTips, + pullRequest: pr, + pullRequests: showPullRequests && pr != null, + remotes: remotes, + timeout: timeout, + }); + } + + let prResult: MaybePausedResult | undefined; + if (showPullRequests) { + // TODO: Make this configurable? + const timeout = 100; + + prResult = await pauseOnCancelOrTimeout( + commit.getAssociatedPullRequest(remote), + cancellation, + timeout, + async result => { + if (result.reason !== 'timedout' || this._statusBarBlame == null) return; + + // If the PR is taking too long, refresh the status bar once it completes - private async waitForPendingPullRequest( - editor: TextEditor, - commit: GitCommit, - pr: PullRequest | PromiseCancelledError> | undefined, - cancellationToken: CancellationToken, - timeout: number, - scope: LogScope | undefined, - ) { - if (cancellationToken.isCancellationRequested || !(pr instanceof PromiseCancelledError)) return; + Logger.debug(scope, `${GlyphChars.Dot} pull request query took too long (over ${timeout} ms)`); - // If the PR timed out, refresh the status bar once it completes - Logger.debug(scope, `${GlyphChars.Dot} pull request query took too long (over ${timeout} ms)`); + const [getBranchAndTagTipsResult, prResult] = await Promise.allSettled([ + getBranchAndTagTipsPromise, + result.value, + ]); - pr = await pr.promise; + if (cancellation.isCancellationRequested || this._statusBarBlame == null) return; - if (cancellationToken.isCancellationRequested) return; + const pr = getSettledValue(prResult); + const getBranchAndTagTips = getSettledValue(getBranchAndTagTipsResult); - Logger.debug(scope, `${GlyphChars.Dot} pull request query completed; refreshing...`); + Logger.debug(scope, `${GlyphChars.Dot} pull request query completed; updating...`); - void this.updateBlame(editor, commit, { pr: pr ?? null }); + setBlameText(this._statusBarBlame, getBranchAndTagTips, pr); + + const tooltip = await getBlameTooltip(this.container, getBranchAndTagTips, pr); + if (tooltip != null) { + this._statusBarBlame.tooltip = tooltip.appendMarkdown(`\n\n---\n\n${actionTooltip}`); + } + }, + ); + } + + const getBranchAndTagTips = getBranchAndTagTipsPromise != null ? await getBranchAndTagTipsPromise : undefined; + + if (cancellation.isCancellationRequested) return; + + setBlameText(this._statusBarBlame, getBranchAndTagTips, prResult?.value); + this._statusBarBlame.show(); + + const tooltipResult = await pauseOnCancelOrTimeout( + getBlameTooltip(this.container, getBranchAndTagTips, prResult?.value, 20), + cancellation, + 100, + async result => { + if (result.reason !== 'timedout' || this._statusBarBlame == null) return; + + const tooltip = await result.value; + if (tooltip != null) { + this._statusBarBlame.tooltip = tooltip.appendMarkdown(`\n\n---\n\n${actionTooltip}`); + } + }, + ); + + if (!cancellation.isCancellationRequested && !tooltipResult.paused && tooltipResult.value != null) { + this._statusBarBlame.tooltip = tooltipResult.value.appendMarkdown(`\n\n---\n\n${actionTooltip}`); + } } } diff --git a/src/system/cancellation.ts b/src/system/cancellation.ts index dfe3402..b4f15aa 100644 --- a/src/system/cancellation.ts +++ b/src/system/cancellation.ts @@ -1,5 +1,7 @@ import type { CancellationToken, Disposable } from 'vscode'; import { CancellationTokenSource } from 'vscode'; +import { map } from './iterable'; +import { isPromise } from './promise'; export class TimedCancellationSource implements CancellationTokenSource, Disposable { private readonly cancellation = new CancellationTokenSource(); @@ -23,3 +25,441 @@ export class TimedCancellationSource implements CancellationTokenSource, Disposa return this.cancellation.token; } } + +type PausedResult = { + value: Promise; + paused: true; + reason: 'cancelled' | 'timedout'; +}; + +export type CompletedResult = { + value: T; + paused: false; +}; + +export type MaybePausedResult = PausedResult | CompletedResult; + +export function pauseOnCancelOrTimeout( + promise: T | Promise, + cancellation?: undefined, + timeout?: undefined, +): Promise>; +export function pauseOnCancelOrTimeout( + promise: T | Promise, + cancellation?: CancellationToken, + timeout?: number | CancellationToken, + continuation?: (result: PausedResult) => void | Promise, +): Promise>; +export function pauseOnCancelOrTimeout( + promise: T | Promise, + cancellation?: CancellationToken, + timeout?: number | CancellationToken, + continuation?: (result: PausedResult) => void | Promise, +): Promise> { + if (!isPromise(promise)) { + return Promise.resolve({ value: promise, paused: false } satisfies MaybePausedResult); + } + + if (cancellation == null && timeout == null) { + return promise.then(value => ({ value: value, paused: false }) satisfies CompletedResult); + } + + let disposeCancellation: Disposable | undefined; + let disposeTimeout: Disposable | undefined; + + const result = Promise.race([ + promise.then(value => { + disposeCancellation?.dispose(); + disposeTimeout?.dispose(); + + if (cancellation?.isCancellationRequested) { + return { + value: Promise.resolve(value), + paused: true, + reason: 'cancelled', + } satisfies MaybePausedResult; + } + + return { value: value, paused: false } satisfies MaybePausedResult; + }), + new Promise>(resolve => { + const resolver = (reason: 'cancelled' | 'timedout') => { + disposeCancellation?.dispose(); + disposeTimeout?.dispose(); + + resolve({ + value: promise, + paused: true, + reason: reason, + } satisfies MaybePausedResult); + }; + + disposeCancellation = cancellation?.onCancellationRequested(() => resolver('cancelled')); + if (timeout != null) { + if (typeof timeout === 'number') { + const timer = setTimeout(() => resolver('timedout'), timeout); + disposeTimeout = { dispose: () => clearTimeout(timer) }; + } else { + disposeTimeout = timeout.onCancellationRequested(() => resolver('timedout')); + } + } + }), + ]); + + return continuation == null + ? result + : result.then(r => { + if (r.paused) { + setTimeout(() => continuation(r), 0); + } + return r; + }); +} + +export async function pauseOnCancelOrTimeoutMap( + source: Map>, + ignoreErrors: true, + cancellation?: CancellationToken, + timeout?: number | CancellationToken, + continuation?: (result: PausedResult>>) => void | Promise, +): Promise>>; +export async function pauseOnCancelOrTimeoutMap( + source: Map>, + ignoreErrors?: boolean, + cancellation?: CancellationToken, + timeout?: number | CancellationToken, + continuation?: (result: PausedResult>>) => void | Promise, +): Promise>>; +export async function pauseOnCancelOrTimeoutMap( + source: Map>, + ignoreErrors?: boolean, + cancellation?: CancellationToken, + timeout?: number | CancellationToken, + continuation?: (result: PausedResult>>) => void | Promise, +): Promise>> { + if (source.size === 0) return source as unknown as Map>; + + // Change the timeout to a cancellation token if it is a number to avoid creating lots of timers + let timeoutCancellation: CancellationTokenSource | undefined; + if (timeout != null && typeof timeout === 'number') { + timeoutCancellation = new TimedCancellationSource(timeout); + timeout = timeoutCancellation.token; + } + + const results = await Promise.all( + map(source, ([id, promise]) => + pauseOnCancelOrTimeout( + promise.catch(ex => (ignoreErrors || !(ex instanceof Error) ? undefined : ex)), + cancellation, + timeout, + ).then(result => [id, result] as const), + ), + ); + + timeoutCancellation?.dispose(); + + if (continuation != null) { + if (results.some(([, r]) => r.paused)) { + async function getContinuationValue() { + const completed = new Map>(); + + for (const [id, result] of results) { + completed.set(id, { value: result.paused ? await result.value : result.value, paused: false }); + } + + return completed; + } + + const cancelled = results.some(([, r]) => r.paused && r.reason === 'cancelled'); + + void continuation({ + value: getContinuationValue(), + paused: true, + reason: cancelled ? 'cancelled' : 'timedout', + }); + } + } + + return new Map>(results); +} + +export async function pauseOnCancelOrTimeoutMapPromise( + source: Promise> | undefined>, + ignoreErrors: true, + cancellation?: CancellationToken, + timeout?: number | CancellationToken, + continuation?: (result: PausedResult>>) => void | Promise, +): Promise> | undefined>>; +export async function pauseOnCancelOrTimeoutMapPromise( + source: Promise> | undefined>, + ignoreErrors?: boolean, + cancellation?: CancellationToken, + timeout?: number | CancellationToken, + continuation?: (result: PausedResult>>) => void | Promise, +): Promise> | undefined>>; +export async function pauseOnCancelOrTimeoutMapPromise( + source: Promise> | undefined>, + ignoreErrors?: boolean, + cancellation?: CancellationToken, + timeout?: number | CancellationToken, + continuation?: (result: PausedResult>>) => void | Promise, +): Promise> | undefined>> { + // Change the timeout to a cancellation token if it is a number to avoid creating lots of timers + let timeoutCancellation: CancellationTokenSource | undefined; + if (timeout != null && typeof timeout === 'number') { + timeoutCancellation = new TimedCancellationSource(timeout); + timeout = timeoutCancellation.token; + } + + const mapPromise = source.then(m => + m == null ? m : pauseOnCancelOrTimeoutMap(m, ignoreErrors, cancellation, timeout, continuation), + ); + + void mapPromise.then(() => timeoutCancellation?.dispose()); + + const result = await pauseOnCancelOrTimeout(source, cancellation, timeout); + return result.paused + ? { value: mapPromise, paused: result.paused, reason: result.reason } + : { value: await mapPromise, paused: false }; +} + +export async function pauseOnCancelOrTimeoutMapTuple( + source: Map | undefined, ...U]>, + cancellation?: undefined, + timeout?: undefined, +): Promise | undefined, ...U]>>; +export async function pauseOnCancelOrTimeoutMapTuple( + source: Map | undefined, ...U]>, + cancellation?: CancellationToken, + timeout?: number | CancellationToken, + continuation?: ( + result: PausedResult | undefined, ...U]>>, + ) => void | Promise, +): Promise | undefined, ...U]>>; +export async function pauseOnCancelOrTimeoutMapTuple( + source: Map | undefined, ...U]>, + cancellation?: CancellationToken, + timeout?: number | CancellationToken, + continuation?: ( + result: PausedResult | undefined, ...U]>>, + ) => void | Promise, +): Promise | undefined, ...U]>> { + if (source.size === 0) { + return source as unknown as Map | undefined, ...U]>; + } + + // Change the timeout to a cancellation token if it is a number to avoid creating lots of timers + let timeoutCancellation: CancellationTokenSource | undefined; + if (timeout != null && typeof timeout === 'number') { + timeoutCancellation = new TimedCancellationSource(timeout); + timeout = timeoutCancellation.token; + } + + const results = await Promise.all( + map(source, ([id, [promise, ...rest]]) => + promise == null + ? ([id, [undefined, ...rest]] as const) + : pauseOnCancelOrTimeout( + promise.catch(() => undefined), + cancellation, + timeout, + ).then(result => [id, [result as MaybePausedResult | undefined, ...rest]] as const), + ), + ); + + timeoutCancellation?.dispose(); + + if (continuation != null) { + if (results.some(([, [r]]) => r?.paused ?? false)) { + async function getContinuationValue() { + const completed = new Map | undefined, ...U]>(); + + for (const [id, [r, ...rest]] of results) { + completed.set(id, [ + { value: r?.paused ? await r.value : r?.value, paused: false }, + ...rest, + ] as const); + } + + return completed; + } + + const cancelled = results.some(([, [r]]) => r?.paused && r.reason === 'cancelled'); + + void continuation({ + value: getContinuationValue(), + paused: true, + reason: cancelled ? 'cancelled' : 'timedout', + }); + } + } + + return new Map | undefined, ...U]>(results); +} + +export async function pauseOnCancelOrTimeoutMapTuplePromise( + source: Promise | undefined, ...U]> | undefined>, + cancellation?: undefined, + timeout?: undefined, +): Promise | undefined, ...U]> | undefined>>; +export async function pauseOnCancelOrTimeoutMapTuplePromise( + source: Promise | undefined, ...U]> | undefined>, + cancellation?: CancellationToken, + timeout?: number | CancellationToken, + continuation?: ( + result: PausedResult | undefined, ...U]>>, + ) => void | Promise, +): Promise | undefined, ...U]> | undefined>>; +export async function pauseOnCancelOrTimeoutMapTuplePromise( + source: Promise | undefined, ...U]> | undefined>, + cancellation?: CancellationToken, + timeout?: number | CancellationToken, + continuation?: ( + result: PausedResult | undefined, ...U]>>, + ) => void | Promise, +): Promise | undefined, ...U]> | undefined>> { + // Change the timeout to a cancellation token if it is a number to avoid creating lots of timers + let timeoutCancellation: CancellationTokenSource | undefined; + if (timeout != null && typeof timeout === 'number') { + timeoutCancellation = new TimedCancellationSource(timeout); + timeout = timeoutCancellation.token; + } + + const mapPromise = source.then(m => + m == null ? m : pauseOnCancelOrTimeoutMapTuple(m, cancellation, timeout, continuation), + ); + + void mapPromise.then(() => timeoutCancellation?.dispose()); + + const result = await pauseOnCancelOrTimeout(source, cancellation, timeout); + return result.paused + ? { value: mapPromise, paused: result.paused, reason: result.reason } + : { value: await mapPromise, paused: false }; +} + +// type PromiseKeys = { +// [K in keyof T]: T[K] extends Promise | undefined ? K : never; +// }[keyof T]; +// type WithCompletedResult> = Omit & { +// [K in U]: CompletedResult | undefined> | undefined; +// }; +// type WithMaybePausedResult> = Omit & { +// [K in U]: MaybePausedResult | undefined> | undefined; +// }; + +// export async function pauseOnCancelOrTimeoutMapOnProp>( +// source: Map, +// prop: U, +// cancellation?: undefined, +// timeout?: undefined, +// ): Promise>>; +// export async function pauseOnCancelOrTimeoutMapOnProp>( +// source: Map, +// prop: U, +// cancellation?: CancellationToken, +// timeout?: number | CancellationToken, +// continuation?: (result: PausedResult>>) => void | Promise, +// ): Promise>>; +// export async function pauseOnCancelOrTimeoutMapOnProp>( +// source: Map, +// prop: U, +// cancellation?: CancellationToken, +// timeout?: number | CancellationToken, +// continuation?: (result: PausedResult>>) => void | Promise, +// ): Promise>> { +// if (source.size === 0) { +// return source as unknown as Map>; +// } + +// // Change the timeout to a cancellation token if it is a number to avoid creating lots of timers +// let timeoutCancellation: CancellationTokenSource | undefined; +// if (timeout != null && typeof timeout === 'number') { +// timeoutCancellation = new TimedCancellationSource(timeout); +// timeout = timeoutCancellation.token; +// } + +// const results = await Promise.all( +// map(source, ([id, item]) => +// item[prop] == null +// ? ([id, item as WithMaybePausedResult] as const) +// : pauseOnCancelOrTimeout( +// (item[prop] as Promise).catch(() => undefined), +// cancellation, +// timeout, +// ).then(result => { +// (item as any)[prop] = result; +// return [id, item as WithMaybePausedResult] as const; +// }), +// ), +// ); + +// timeoutCancellation?.dispose(); + +// if (continuation != null) { +// if (results.some(([, r]) => (r as any)[prop]?.paused ?? false)) { +// async function getContinuationValue() { +// const completed = new Map>(); + +// for (const [id, result] of results) { +// const r = result[prop]; // as MaybePausedResult> | undefined; +// (result as /*WithCompletedResult*/ any)[prop] = r?.paused ? await r.value : r?.value; +// completed.set(id, result as WithCompletedResult); +// } + +// return completed; +// } + +// const cancelled = results.some(([, result]) => { +// const r = result[prop]; +// return r?.paused && r.reason === 'cancelled'; +// }); + +// void continuation({ +// value: getContinuationValue(), +// paused: true, +// reason: cancelled ? 'cancelled' : 'timedout', +// }); +// } +// } + +// return new Map>(results); +// } + +// export async function pauseOnCancelOrTimeoutMapOnPropPromise>( +// source: Promise | undefined>, +// prop: U, +// cancellation?: undefined, +// timeout?: undefined, +// ): Promise> | undefined>>; +// export async function pauseOnCancelOrTimeoutMapOnPropPromise>( +// source: Promise | undefined>, +// prop: U, +// cancellation?: CancellationToken, +// timeout?: number | CancellationToken, +// continuation?: (result: PausedResult>>) => void | Promise, +// ): Promise> | undefined>>; +// export async function pauseOnCancelOrTimeoutMapOnPropPromise>( +// source: Promise | undefined>, +// prop: U, +// cancellation?: CancellationToken, +// timeout?: number | CancellationToken, +// continuation?: (result: PausedResult>>) => void | Promise, +// ): Promise> | undefined>> { +// // Change the timeout to a cancellation token if it is a number to avoid creating lots of timers +// let timeoutCancellation: CancellationTokenSource | undefined; +// if (timeout != null && typeof timeout === 'number') { +// timeoutCancellation = new TimedCancellationSource(timeout); +// timeout = timeoutCancellation.token; +// } + +// const mapPromise = source.then(m => +// m == null ? m : pauseOnCancelOrTimeoutMapOnProp(m, prop, cancellation, timeout, continuation), +// ); + +// void mapPromise.then(() => timeoutCancellation?.dispose()); + +// const result = await pauseOnCancelOrTimeout(source, cancellation, timeout); +// return result.paused +// ? { value: mapPromise, paused: result.paused, reason: result.reason } +// : { value: await mapPromise, paused: false }; +// } diff --git a/src/system/iterable.ts b/src/system/iterable.ts index 5b97719..c9bb071 100644 --- a/src/system/iterable.ts +++ b/src/system/iterable.ts @@ -200,8 +200,10 @@ export function* take(source: Iterable | IterableIterator, count: numbe } } -export function* union(...sources: (Iterable | IterableIterator)[]): Iterable { +export function* union(...sources: (Iterable | IterableIterator | undefined)[]): Iterable { for (const source of sources) { + if (source == null) continue; + for (const item of source) { yield item; } diff --git a/src/system/promise.ts b/src/system/promise.ts index 13f89ee..09d1ac7 100644 --- a/src/system/promise.ts +++ b/src/system/promise.ts @@ -1,5 +1,4 @@ import type { CancellationToken, Disposable } from 'vscode'; -import { map } from './iterable'; export type PromiseOrValue = Promise | T; @@ -34,19 +33,22 @@ export function any(...promises: Promise[]): Promise { export async function* asSettled(promises: Promise[]): AsyncIterable> { const map = new Map( - promises.map((promise, i) => [ - i, - promise.then( - v => - ({ index: i, value: v, status: 'fulfilled' }) as unknown as PromiseFulfilledResult & { - index: number; - }, - e => - ({ index: i, reason: e, status: 'rejected' }) as unknown as PromiseRejectedResult & { - index: number; - }, - ), - ]), + promises.map( + (promise, i) => + [ + i, + promise.then( + v => + ({ index: i, value: v, status: 'fulfilled' }) as unknown as PromiseFulfilledResult & { + index: number; + }, + e => + ({ index: i, reason: e, status: 'rejected' }) as unknown as PromiseRejectedResult & { + index: number; + }, + ), + ] as const, + ), ); while (map.size) { @@ -65,16 +67,6 @@ export class PromiseCancelledError = Promise> extend } } -export class PromiseCancelledErrorWithId = Promise> extends PromiseCancelledError { - constructor( - public readonly id: TKey, - promise: T, - message: string, - ) { - super(promise, message); - } -} - export function cancellable( promise: Promise, timeoutOrToken?: number | CancellationToken, @@ -159,121 +151,97 @@ export function defer(): Deferred { return deferred; } -export function getSettledValue(promise: PromiseSettledResult): T | undefined; -export function getSettledValue(promise: PromiseSettledResult, defaultValue: NonNullable): NonNullable; +export function getSettledValue(promise: PromiseSettledResult | undefined): T | undefined; +export function getSettledValue( + promise: PromiseSettledResult | undefined, + defaultValue: NonNullable, +): NonNullable; export function getSettledValue( - promise: PromiseSettledResult, + promise: PromiseSettledResult | undefined, defaultValue: T | undefined = undefined, ): T | typeof defaultValue { - return promise.status === 'fulfilled' ? promise.value : defaultValue; + return promise?.status === 'fulfilled' ? promise.value : defaultValue; } export function isPromise(obj: PromiseLike | T): obj is Promise { - return obj instanceof Promise || typeof (obj as PromiseLike)?.then === 'function'; -} - -export function progress(promise: Promise, intervalMs: number, onProgress: () => boolean): Promise { - return new Promise((resolve, reject) => { - let timer: ReturnType | undefined; - timer = setInterval(() => { - if (onProgress()) { - if (timer != null) { - clearInterval(timer); - timer = undefined; - } - } - }, intervalMs); - - promise.then( - () => { - if (timer != null) { - clearInterval(timer); - timer = undefined; - } - - resolve(promise); - }, - ex => { - if (timer != null) { - clearInterval(timer); - timer = undefined; - } - - reject(ex); - }, - ); - }); -} - -export function raceAll( - promises: Promise[], - timeout?: number, -): Promise<(TPromise | PromiseCancelledError>)[]>; -export function raceAll( - promises: Map>, - timeout?: number, -): Promise>>>; -export function raceAll( - ids: Iterable, - fn: (id: T) => Promise, - timeout?: number, -): Promise>>>; -export async function raceAll( - promisesOrIds: Promise[] | Map> | Iterable, - timeoutOrFn?: number | ((id: T) => Promise), - timeout?: number, -) { - let promises; - if (timeoutOrFn != null && typeof timeoutOrFn !== 'number') { - promises = new Map(map]>(promisesOrIds as Iterable, id => [id, timeoutOrFn(id)])); - } else { - timeout = timeoutOrFn; - promises = promisesOrIds as Promise[] | Map>; - } - - if (promises instanceof Map) { - return new Map( - await Promise.all( - map<[T, Promise], Promise<[T, TPromise | PromiseCancelledErrorWithId>]>>( - promises.entries(), - timeout == null - ? ([id, promise]) => promise.then(p => [id, p]) - : ([id, promise]) => - Promise.race([ - promise, - - new Promise>>(resolve => - setTimeout( - () => resolve(new PromiseCancelledErrorWithId(id, promise, 'TIMED OUT')), - timeout, - ), - ), - ]).then(p => [id, p]), - ), - ), - ); - } - - return Promise.all( - timeout == null - ? promises - : promises.map(p => - Promise.race([ - p, - new Promise>>(resolve => - setTimeout(() => resolve(new PromiseCancelledError(p, 'TIMED OUT')), timeout), - ), - ]), - ), - ); + return obj != null && (obj instanceof Promise || typeof (obj as PromiseLike)?.then === 'function'); } -export async function wait(ms: number): Promise { - await new Promise(resolve => setTimeout(resolve, ms)); +// export function progress(promise: Promise, intervalMs: number, onProgress: () => boolean): Promise { +// return new Promise((resolve, reject) => { +// let timer: ReturnType | undefined; +// timer = setInterval(() => { +// if (onProgress()) { +// if (timer != null) { +// clearInterval(timer); +// timer = undefined; +// } +// } +// }, intervalMs); + +// promise.then( +// () => { +// if (timer != null) { +// clearInterval(timer); +// timer = undefined; +// } + +// resolve(promise); +// }, +// ex => { +// if (timer != null) { +// clearInterval(timer); +// timer = undefined; +// } + +// reject(ex); +// }, +// ); +// }); +// } + +// export async function resolveMap( +// source: Map>, +// ignoreErrors?: false, +// ): Promise>; +// export async function resolveMap( +// source: Promise> | undefined>, +// ignoreErrors?: false, +// ): Promise | undefined>; +// export async function resolveMap( +// source: Map>, +// ignoreErrors: true, +// ): Promise | undefined>; +// export async function resolveMap( +// source: Promise> | undefined>, +// ignoreErrors: true, +// ): Promise>; +// export async function resolveMap( +// source: Map> | Promise> | undefined>, +// ignoreErrors?: boolean, +// ): Promise | undefined> { +// if (isPromise(source)) { +// const map = await source; +// if (map == null) return undefined; + +// source = map; +// } + +// const promises = map(source, ([id, promise]) => +// promise.then( +// p => [id, p as T | Error | undefined] as const, +// ex => [id, (ignoreErrors || !(ex instanceof Error) ? undefined : ex) as T | Error | undefined] as const, +// ), +// ); +// return new Map(await Promise.all(promises)); +// } + +export function wait(ms: number): Promise { + return new Promise(resolve => setTimeout(resolve, ms)); } -export async function waitUntilNextTick(): Promise { - await new Promise(resolve => queueMicrotask(resolve)); +export function waitUntilNextTick(): Promise { + return new Promise(resolve => queueMicrotask(resolve)); } export class AggregateError extends Error { diff --git a/src/trackers/gitLineTracker.ts b/src/trackers/gitLineTracker.ts index 83cfaaa..ab28fd0 100644 --- a/src/trackers/gitLineTracker.ts +++ b/src/trackers/gitLineTracker.ts @@ -18,12 +18,8 @@ import { LineTracker } from './lineTracker'; export * from './lineTracker'; -export class GitLineState { - constructor(public readonly commit: GitCommit | undefined) { - if (commit != null && commit.file == null) { - debugger; - } - } +export interface GitLineState { + commit: GitCommit; } export class GitLineTracker extends LineTracker { @@ -32,8 +28,6 @@ export class GitLineTracker extends LineTracker { } protected override async fireLinesChanged(e: LinesChangeEvent) { - this.reset(); - let updated = false; if (!this.suspended && !e.pending && e.selections != null && e.editor != null) { updated = await this.updateState(e.selections, e.editor); @@ -68,19 +62,16 @@ export class GitLineTracker extends LineTracker { @debug({ args: { - 0: e => - `editor=${e.editor.document.uri.toString(true)}, doc=${e.document.uri.toString(true)}, blameable=${ - e.blameable - }`, + 0: e => `editor/doc=${e.editor.document.uri.toString(true)}, blameable=${e.blameable}`, }, }) private onBlameStateChanged(_e: DocumentBlameStateChangeEvent) { - this.trigger('editor'); + this.notifyLinesChanged('editor'); } @debug({ args: { - 0: e => `editor=${e.editor.document.uri.toString(true)}, doc=${e.document.uri.toString(true)}`, + 0: e => `editor/doc=${e.editor.document.uri.toString(true)}`, }, }) private onContentChanged(e: DocumentContentChangeEvent) { @@ -94,13 +85,13 @@ export class GitLineTracker extends LineTracker { ), ) ) { - this.trigger('editor'); + this.notifyLinesChanged('editor'); } } @debug({ args: { - 0: e => `editor=${e.editor.document.uri.toString(true)}, doc=${e.document.uri.toString(true)}`, + 0: e => `editor/doc=${e.editor.document.uri.toString(true)}`, }, }) private onDirtyIdleTriggered(e: DocumentDirtyIdleTriggerEvent) { @@ -112,10 +103,7 @@ export class GitLineTracker extends LineTracker { @debug({ args: { - 0: e => - `editor=${e.editor.document.uri.toString(true)}, doc=${e.document.uri.toString(true)}, dirty=${ - e.dirty - }`, + 0: e => `editor/doc=${e.editor.document.uri.toString(true)}, dirty=${e.dirty}`, }, }) private onDirtyStateChanged(e: DocumentDirtyStateChangeEvent) { @@ -158,7 +146,11 @@ export class GitLineTracker extends LineTracker { return false; } - this.setState(blameLine.line.line - 1, new GitLineState(blameLine.commit)); + if (blameLine.commit != null && blameLine.commit.file == null) { + debugger; + } + + this.setState(blameLine.line.line - 1, { commit: blameLine.commit }); } else { const blame = await this.container.git.getBlame(trackedDocument.uri, editor.document); if (blame == null) { @@ -169,7 +161,17 @@ export class GitLineTracker extends LineTracker { for (const selection of selections) { const commitLine = blame.lines[selection.active]; - this.setState(selection.active, new GitLineState(blame.commits.get(commitLine.sha))); + const commit = blame.commits.get(commitLine.sha); + if (commit != null && commit.file == null) { + debugger; + } + + if (commit == null) { + debugger; + this.resetState(selection.active); + } else { + this.setState(selection.active, { commit: commit }); + } } } diff --git a/src/trackers/lineTracker.ts b/src/trackers/lineTracker.ts index 5c36c14..1490e54 100644 --- a/src/trackers/lineTracker.ts +++ b/src/trackers/lineTracker.ts @@ -28,7 +28,6 @@ export class LineTracker implements Disposable { protected _disposable: Disposable | undefined; private _editor: TextEditor | undefined; - private readonly _state = new Map(); dispose() { @@ -41,31 +40,38 @@ export class LineTracker implements Disposable { if (editor === this._editor) return; if (editor != null && !isTextEditor(editor)) return; - this.reset(); this._editor = editor; - this._selections = LineTracker.toLineSelections(editor?.selections); + this._selections = toLineSelections(editor?.selections); - this.trigger('editor'); + this.notifyLinesChanged('editor'); } private onTextEditorSelectionChanged(e: TextEditorSelectionChangeEvent) { // If this isn't for our cached editor and its not a real editor -- kick out if (this._editor !== e.textEditor && !isTextEditor(e.textEditor)) return; - const selections = LineTracker.toLineSelections(e.selections); + const selections = toLineSelections(e.selections); if (this._editor === e.textEditor && this.includes(selections)) return; - this.reset(); this._editor = e.textEditor; this._selections = selections; - this.trigger(this._editor === e.textEditor ? 'selection' : 'editor'); + this.notifyLinesChanged(this._editor === e.textEditor ? 'selection' : 'editor'); } getState(line: number): T | undefined { return this._state.get(line); } + resetState(line?: number) { + if (line != null) { + this._state.delete(line); + return; + } + + this._state.clear(); + } + setState(line: number, state: T | undefined) { this._state.set(line, state); } @@ -79,7 +85,7 @@ export class LineTracker implements Disposable { includes(line: number, options?: { activeOnly: boolean }): boolean; includes(lineOrSelections: number | LineSelection[], options?: { activeOnly: boolean }): boolean { if (typeof lineOrSelections !== 'number') { - return LineTracker.includes(lineOrSelections, this._selections); + return isIncluded(lineOrSelections, this._selections); } if (this._selections == null || this._selections.length === 0) return false; @@ -101,11 +107,7 @@ export class LineTracker implements Disposable { } refresh() { - this.trigger('editor'); - } - - reset() { - this._state.clear(); + this.notifyLinesChanged('editor'); } private _subscriptions = new Map(); @@ -161,10 +163,7 @@ export class LineTracker implements Disposable { if (this._subscriptions.size !== 0) return; - if (this._linesChangedDebounced != null) { - this._linesChangedDebounced.cancel(); - } - + this._fireLinesChangedDebounced?.cancel(); this._disposable?.dispose(); this._disposable = undefined; } @@ -182,7 +181,7 @@ export class LineTracker implements Disposable { this._suspended = false; this.onResume?.(); - this.trigger('editor'); + this.notifyLinesChanged('editor'); } protected onSuspend?(): void; @@ -193,27 +192,25 @@ export class LineTracker implements Disposable { this._suspended = true; this.onSuspend?.(); - this.trigger('editor'); + this.notifyLinesChanged('editor'); } protected fireLinesChanged(e: LinesChangeEvent) { this._onDidChangeActiveLines.fire(e); } - protected trigger(reason: 'editor' | 'selection') { - this.onLinesChanged({ editor: this._editor, selections: this.selections, reason: reason }); - } - - private _linesChangedDebounced: Deferrable<(e: LinesChangeEvent) => void> | undefined; + private _fireLinesChangedDebounced: Deferrable<(e: LinesChangeEvent) => void> | undefined; + protected notifyLinesChanged(reason: 'editor' | 'selection') { + if (reason === 'editor') { + this.resetState(); + } - private onLinesChanged(e: LinesChangeEvent) { + const e: LinesChangeEvent = { editor: this._editor, selections: this.selections, reason: reason }; if (e.selections == null) { queueMicrotask(() => { if (e.editor !== window.activeTextEditor) return; - if (this._linesChangedDebounced != null) { - this._linesChangedDebounced.cancel(); - } + this._fireLinesChangedDebounced?.cancel(); this.fireLinesChanged(e); }); @@ -221,11 +218,12 @@ export class LineTracker implements Disposable { return; } - if (this._linesChangedDebounced == null) { - this._linesChangedDebounced = debounce((e: LinesChangeEvent) => { + if (this._fireLinesChangedDebounced == null) { + this._fireLinesChangedDebounced = debounce((e: LinesChangeEvent) => { if (e.editor !== window.activeTextEditor) return; + // Make sure we are still on the same lines - if (!LineTracker.includes(e.selections, LineTracker.toLineSelections(e.editor?.selections))) { + if (!isIncluded(e.selections, toLineSelections(e.editor?.selections))) { return; } @@ -234,28 +232,27 @@ export class LineTracker implements Disposable { } // If we have no pending moves, then fire an immediate pending event, and defer the real event - if (!this._linesChangedDebounced.pending?.()) { + if (!this._fireLinesChangedDebounced.pending?.()) { this.fireLinesChanged({ ...e, pending: true }); } - this._linesChangedDebounced(e); + this._fireLinesChangedDebounced(e); } +} - static includes(selections: LineSelection[] | undefined, inSelections: LineSelection[] | undefined): boolean { - if (selections == null && inSelections == null) return true; - if (selections == null || inSelections == null || selections.length !== inSelections.length) return false; - - let match; +function isIncluded(selections: LineSelection[] | undefined, within: LineSelection[] | undefined): boolean { + if (selections == null && within == null) return true; + if (selections == null || within == null || selections.length !== within.length) return false; - return selections.every((s, i) => { - match = inSelections[i]; - return s.active === match.active && s.anchor === match.anchor; - }); - } + let match; + return selections.every((s, i) => { + match = within[i]; + return s.active === match.active && s.anchor === match.anchor; + }); +} - static toLineSelections(selections: readonly Selection[]): LineSelection[]; - static toLineSelections(selections: readonly Selection[] | undefined): LineSelection[] | undefined; - static toLineSelections(selections: readonly Selection[] | undefined) { - return selections?.map(s => ({ active: s.active.line, anchor: s.anchor.line })); - } +function toLineSelections(selections: readonly Selection[]): LineSelection[]; +function toLineSelections(selections: readonly Selection[] | undefined): LineSelection[] | undefined; +function toLineSelections(selections: readonly Selection[] | undefined) { + return selections?.map(s => ({ active: s.active.line, anchor: s.anchor.line })); } diff --git a/src/views/nodes/autolinkedItemNode.ts b/src/views/nodes/autolinkedItemNode.ts index 614a71f..95256e9 100644 --- a/src/views/nodes/autolinkedItemNode.ts +++ b/src/views/nodes/autolinkedItemNode.ts @@ -9,6 +9,7 @@ import { IssueOrPullRequestType, } from '../../git/models/issue'; import { fromNow } from '../../system/date'; +import { isPromise } from '../../system/promise'; import type { ViewsWithCommits } from '../viewBase'; import { ContextValues, getViewNodeId, ViewNode } from './viewNode'; @@ -17,7 +18,8 @@ export class AutolinkedItemNode extends ViewNode { view: ViewsWithCommits, protected override readonly parent: ViewNode, public readonly repoPath: string, - public readonly item: Autolink | IssueOrPullRequest, + public readonly item: Autolink, + private enrichedItem: Promise | IssueOrPullRequest | undefined, ) { super(GitUri.fromRepoPath(repoPath), view, parent); @@ -37,53 +39,68 @@ export class AutolinkedItemNode extends ViewNode { } getTreeItem(): TreeItem { - if (!isIssueOrPullRequest(this.item)) { - const { provider } = this.item; + const enriched = this.enrichedItem; + const pending = isPromise(enriched); + if (pending) { + void enriched.then(item => { + this.enrichedItem = item; + this.view.triggerNodeChange(this); + }); + } + + if (pending || enriched == null) { + const autolink = this.item; + const { provider } = autolink; - const item = new TreeItem(`${this.item.prefix}${this.item.id}`, TreeItemCollapsibleState.None); + const item = new TreeItem( + autolink.description ?? `Autolink ${autolink.prefix}${autolink.id}`, + TreeItemCollapsibleState.None, + ); item.description = provider?.name ?? 'Custom'; item.iconPath = new ThemeIcon( - this.item.type == null + pending + ? 'loading~spin' + : autolink.type == null ? 'link' - : this.item.type === AutolinkType.PullRequest + : autolink.type === AutolinkType.PullRequest ? 'git-pull-request' : 'issues', ); item.contextValue = ContextValues.AutolinkedItem; item.tooltip = new MarkdownString( `${ - this.item.description - ? `Autolinked ${this.item.description}` + autolink.description + ? `Autolinked ${autolink.description}` : `${ - this.item.type == null + autolink.type == null ? 'Autolinked' - : this.item.type === AutolinkType.PullRequest + : autolink.type === AutolinkType.PullRequest ? 'Autolinked Pull Request' : 'Autolinked Issue' - } ${this.item.prefix}${this.item.id}` - } \\\n[${this.item.url}](${this.item.url}${this.item.title != null ? ` "${this.item.title}"` : ''})`, + } ${autolink.prefix}${autolink.id}` + } \\\n[${autolink.url}](${autolink.url}${autolink.title != null ? ` "${autolink.title}"` : ''})`, ); return item; } - const relativeTime = fromNow(this.item.closedDate ?? this.item.date); + const relativeTime = fromNow(enriched.closedDate ?? enriched.date); - const item = new TreeItem(`${this.item.id}: ${this.item.title}`, TreeItemCollapsibleState.None); + const item = new TreeItem(`${enriched.id}: ${enriched.title}`, TreeItemCollapsibleState.None); item.description = relativeTime; - item.iconPath = getIssueOrPullRequestThemeIcon(this.item); + item.iconPath = getIssueOrPullRequestThemeIcon(enriched); item.contextValue = - this.item.type === IssueOrPullRequestType.PullRequest + enriched.type === IssueOrPullRequestType.PullRequest ? ContextValues.PullRequest : ContextValues.AutolinkedIssue; const linkTitle = ` "Open ${ - this.item.type === IssueOrPullRequestType.PullRequest ? 'Pull Request' : 'Issue' - } \\#${this.item.id} on ${this.item.provider.name}"`; + enriched.type === IssueOrPullRequestType.PullRequest ? 'Pull Request' : 'Issue' + } \\#${enriched.id} on ${enriched.provider.name}"`; const tooltip = new MarkdownString( - `${getIssueOrPullRequestMarkdownIcon(this.item)} [**${this.item.title.trim()}**](${ - this.item.url - }${linkTitle}) \\\n[#${this.item.id}](${this.item.url}${linkTitle}) was ${ - this.item.closed ? 'closed' : 'opened' + `${getIssueOrPullRequestMarkdownIcon(enriched)} [**${enriched.title.trim()}**](${ + enriched.url + }${linkTitle}) \\\n[#${enriched.id}](${enriched.url}${linkTitle}) was ${ + enriched.closed ? 'closed' : 'opened' } ${relativeTime}`, true, ); @@ -96,6 +113,6 @@ export class AutolinkedItemNode extends ViewNode { } } -function isIssueOrPullRequest(item: Autolink | IssueOrPullRequest): item is IssueOrPullRequest { - return 'closed' in item && typeof item.closed === 'boolean'; -} +// function isIssueOrPullRequest(item: Autolink | IssueOrPullRequest): item is IssueOrPullRequest { +// return 'closed' in item && typeof item.closed === 'boolean'; +// } diff --git a/src/views/nodes/autolinkedItemsNode.ts b/src/views/nodes/autolinkedItemsNode.ts index c53309d..37f2ade 100644 --- a/src/views/nodes/autolinkedItemsNode.ts +++ b/src/views/nodes/autolinkedItemsNode.ts @@ -1,12 +1,11 @@ import { TreeItem, TreeItemCollapsibleState } from 'vscode'; -import type { Autolink } from '../../annotations/autolinks'; import { GitUri } from '../../git/gitUri'; -import type { IssueOrPullRequest } from '../../git/models/issue'; import type { GitLog } from '../../git/models/log'; import { PullRequest } from '../../git/models/pullRequest'; +import { pauseOnCancelOrTimeoutMapTuple } from '../../system/cancellation'; import { gate } from '../../system/decorators/gate'; import { debug } from '../../system/decorators/log'; -import { union } from '../../system/iterable'; +import { getSettledValue } from '../../system/promise'; import type { ViewsWithCommits } from '../viewBase'; import { AutolinkedItemNode } from './autolinkedItemNode'; import { LoadMoreNode, MessageNode } from './common'; @@ -44,36 +43,20 @@ export class AutolinkedItemsNode extends ViewNode { let children: ViewNode[] | undefined; if (commits.length) { + const remote = await this.view.container.git.getBestRemoteWithProvider(this.repoPath); const combineMessages = commits.map(c => c.message).join('\n'); - let items: Map; - - const customAutolinks = this.view.container.autolinks.getAutolinks(combineMessages); + const [enrichedAutolinksResult /*, ...prsResults*/] = await Promise.allSettled([ + this.view.container.autolinks + .getEnrichedAutolinks(combineMessages, remote) + .then(enriched => + enriched != null ? pauseOnCancelOrTimeoutMapTuple(enriched, undefined, 250) : undefined, + ), + // Only get PRs from the first 100 commits to attempt to avoid hitting the api limits + // ...commits.slice(0, 100).map(c => this.remote.provider.getPullRequestForCommit(c.sha)), + ]); - const remote = await this.view.container.git.getBestRemoteWithProvider(this.repoPath); - if (remote != null) { - const providerAutolinks = this.view.container.autolinks.getAutolinks(combineMessages, remote); - - items = providerAutolinks; - - const [autolinkedMapResult /*, ...prsResults*/] = await Promise.allSettled([ - this.view.container.autolinks.getLinkedIssuesAndPullRequests(combineMessages, remote, { - autolinks: providerAutolinks, - }), - // Only get PRs from the first 100 commits to attempt to avoid hitting the api limits - // ...commits.slice(0, 100).map(c => this.remote.provider.getPullRequestForCommit(c.sha)), - ]); - - if (autolinkedMapResult.status === 'fulfilled' && autolinkedMapResult.value != null) { - for (const [id, issue] of autolinkedMapResult.value) { - items.set(id, issue); - } - } - - items = new Map(union(items, customAutolinks)); - } else { - items = customAutolinks; - } + const enrichedAutolinks = getSettledValue(enrichedAutolinksResult); // for (const result of prsResults) { // if (result.status !== 'fulfilled' || result.value == null) continue; @@ -81,14 +64,22 @@ export class AutolinkedItemsNode extends ViewNode { // items.set(result.value.id, result.value); // } - children = [...items.values()].map(item => - PullRequest.is(item) - ? new PullRequestNode(this.view, this, item, this.log.repoPath) - : new AutolinkedItemNode(this.view, this, this.repoPath, item), - ); + if (enrichedAutolinks?.size) { + children = [...enrichedAutolinks.values()].map(([issueOrPullRequest, autolink]) => + issueOrPullRequest != null && PullRequest.is(issueOrPullRequest?.value) + ? new PullRequestNode(this.view, this, issueOrPullRequest.value, this.log.repoPath) + : new AutolinkedItemNode( + this.view, + this, + this.repoPath, + autolink, + issueOrPullRequest?.value, + ), + ); + } } - if (children == null || children.length === 0) { + if (!children?.length) { children = [new MessageNode(this.view, this, 'No autolinked issues or pull requests could be found.')]; } diff --git a/src/views/nodes/commitNode.ts b/src/views/nodes/commitNode.ts index b34130f..3739478 100644 --- a/src/views/nodes/commitNode.ts +++ b/src/views/nodes/commitNode.ts @@ -12,6 +12,7 @@ import type { GitRevisionReference } from '../../git/models/reference'; import type { GitRemote } from '../../git/models/remote'; import type { RichRemoteProvider } from '../../git/remotes/richRemoteProvider'; import { makeHierarchical } from '../../system/array'; +import { pauseOnCancelOrTimeoutMapTuplePromise } from '../../system/cancellation'; import { configuration } from '../../system/configuration'; import { getContext } from '../../system/context'; import { gate } from '../../system/decorators/gate'; @@ -222,7 +223,7 @@ export class CommitNode extends ViewRefNode> { if (this._cancellationTokenSource != null) { this._cancellationTokenSource.cancel(); - this._cancellationTokenSource.dispose(); this._cancellationTokenSource = undefined; } @@ -503,38 +503,38 @@ export class CommitDetailsWebviewProvider implements WebviewProvider issueOrPullRequest?.value)] + : undefined, pullRequest: pr, }); @@ -544,8 +544,8 @@ export class CommitDetailsWebviewProvider implements WebviewProvider(i: T | undefined): i is T => i != null) + // autolinkedIssuesAndPullRequests != null + // ? [...autolinkedIssuesAndPullRequests.values()].filter((i: T | undefined): i is T => i != null) // : undefined, // }; } @@ -795,16 +795,8 @@ export class CommitDetailsWebviewProvider implements WebviewProvider, + enrichedAutolinks?: Map, ) { let message = CommitFormatter.fromTemplate(`\${message}`, commit); const index = message.indexOf('\n'); @@ -854,7 +846,7 @@ export class CommitDetailsWebviewProvider implements WebviewProvider { try { preview = CommitFormatter.fromTemplate(params.format, commit, { dateFormat: configuration.get('defaultDateFormat'), - pullRequestOrRemote: pr, + pullRequest: pr, messageTruncateAtNewLine: true, }); } catch {