diff --git a/extensions/git/src/api/git.d.ts b/extensions/git/src/api/git.d.ts index 81e778dfa3a..51cfd496fda 100644 --- a/extensions/git/src/api/git.d.ts +++ b/extensions/git/src/api/git.d.ts @@ -326,14 +326,19 @@ export interface BranchProtectionProvider { provideBranchProtection(): BranchProtection[]; } -export interface AvatarQuery { - readonly commit: string; +export interface AvatarQueryCommit { + readonly hash: string; readonly authorName?: string; readonly authorEmail?: string; } +export interface AvatarQuery { + readonly commits: AvatarQueryCommit[]; + readonly size: number; +} + export interface SourceControlHistoryItemDetailsProvider { - provideAvatar(repository: Repository, query: AvatarQuery[]): ProviderResult>; + provideAvatar(repository: Repository, query: AvatarQuery): ProviderResult>; provideHoverCommands(repository: Repository): ProviderResult; provideMessageLinks(repository: Repository, message: string): ProviderResult; } diff --git a/extensions/git/src/blame.ts b/extensions/git/src/blame.ts index b407dbfc4e7..da145d8323f 100644 --- a/extensions/git/src/blame.ts +++ b/extensions/git/src/blame.ts @@ -13,7 +13,9 @@ import { fromGitUri, isGitUri } from './uri'; import { emojify, ensureEmojis } from './emoji'; import { getWorkingTreeAndIndexDiffInformation, getWorkingTreeDiffInformation } from './staging'; import { provideSourceControlHistoryItemAvatar, provideSourceControlHistoryItemHoverCommands, provideSourceControlHistoryItemMessageLinks } from './historyItemDetailsProvider'; -import { AvatarQuery } from './api/git'; +import { AvatarQuery, AvatarQueryCommit } from './api/git'; + +const AVATAR_SIZE = 20; function lineRangesContainLine(changes: readonly TextEditorChange[], lineNumber: number): boolean { return changes.some(c => c.modified.startLineNumber <= lineNumber && lineNumber < c.modified.endLineNumberExclusive); @@ -219,13 +221,16 @@ export class GitBlameController { // Avatar const avatarQuery = { - commit: blameInformation.hash, - authorName: blameInformation.authorName, - authorEmail: blameInformation.authorEmail + commits: [{ + hash: blameInformation.hash, + authorName: blameInformation.authorName, + authorEmail: blameInformation.authorEmail + } satisfies AvatarQueryCommit], + size: AVATAR_SIZE } satisfies AvatarQuery; - const avatarResult = await provideSourceControlHistoryItemAvatar(this._model, repository, [avatarQuery]); - commitAvatar = avatarResult?.get(avatarQuery.commit); + const avatarResult = await provideSourceControlHistoryItemAvatar(this._model, repository, avatarQuery); + commitAvatar = avatarResult?.get(blameInformation.hash); } catch { } } @@ -250,7 +255,7 @@ export class GitBlameController { const authorName = commitInformation?.authorName ?? blameInformation.authorName; const authorEmail = commitInformation?.authorEmail ?? blameInformation.authorEmail; const authorDate = commitInformation?.authorDate ?? blameInformation.authorDate; - const avatar = commitAvatar ? `![${authorName}](${commitAvatar})` : '$(account)'; + const avatar = commitAvatar ? `![${authorName}](${commitAvatar}|width=${AVATAR_SIZE},height=${AVATAR_SIZE})` : '$(account)'; if (authorName) { if (authorEmail) { diff --git a/extensions/git/src/git.ts b/extensions/git/src/git.ts index 7e2fd062f88..4072e4ae706 100644 --- a/extensions/git/src/git.ts +++ b/extensions/git/src/git.ts @@ -1098,7 +1098,7 @@ function parseGitBlame(data: string): BlameInformation[] { authorName = line.substring('author '.length); } if (commitHash && line.startsWith('author-mail ')) { - authorEmail = line.substring('author-mail '.length); + authorEmail = line.substring('author-mail <'.length, line.length - 1); } if (commitHash && line.startsWith('author-time ')) { authorTime = Number(line.substring('author-time '.length)) * 1000; diff --git a/extensions/git/src/historyItemDetailsProvider.ts b/extensions/git/src/historyItemDetailsProvider.ts index c9928b824b9..be0e2b337f8 100644 --- a/extensions/git/src/historyItemDetailsProvider.ts +++ b/extensions/git/src/historyItemDetailsProvider.ts @@ -16,7 +16,7 @@ export interface ISourceControlHistoryItemDetailsProviderRegistry { export async function provideSourceControlHistoryItemAvatar( registry: ISourceControlHistoryItemDetailsProviderRegistry, repository: Repository, - query: AvatarQuery[] + query: AvatarQuery ): Promise | undefined> { for (const provider of registry.getSourceControlHistoryItemDetailsProviders()) { const result = await provider.provideAvatar(new ApiRepository(repository), query); diff --git a/extensions/github/src/historyItemDetailsProvider.ts b/extensions/github/src/historyItemDetailsProvider.ts index 6c4f5aa8adf..fbf6ec93bae 100644 --- a/extensions/github/src/historyItemDetailsProvider.ts +++ b/extensions/github/src/historyItemDetailsProvider.ts @@ -5,11 +5,10 @@ import { authentication, Command, l10n, LogOutputChannel } from 'vscode'; import { Commit, Repository as GitHubRepository, Maybe } from '@octokit/graphql-schema'; -import { API, AvatarQuery, Repository, SourceControlHistoryItemDetailsProvider } from './typings/git'; +import { API, AvatarQuery, AvatarQueryCommit, Repository, SourceControlHistoryItemDetailsProvider } from './typings/git'; import { DisposableStore, getRepositoryDefaultRemote, getRepositoryDefaultRemoteUrl, getRepositoryFromUrl, groupBy, sequentialize } from './util'; import { AuthenticationError, getOctokitGraphql } from './auth'; - -const AVATAR_SIZE = 20; +import { getAvatarLink } from './links'; const ISSUE_EXPRESSION = /(([A-Za-z0-9_.\-]+)\/([A-Za-z0-9_.\-]+))?(#|GH-)([1-9][0-9]*)($|\b)/g; @@ -22,7 +21,7 @@ const ASSIGNABLE_USERS_QUERY = ` login name email - avatarUrl(size: ${AVATAR_SIZE}) + avatarUrl } } } @@ -37,7 +36,7 @@ const COMMIT_AUTHOR_QUERY = ` author { name email - avatarUrl(size: ${AVATAR_SIZE}) + avatarUrl user { id login @@ -62,7 +61,12 @@ interface GitHubUser { readonly avatarUrl: string; } -function compareAvatarQuery(a: AvatarQuery, b: AvatarQuery): number { +function getUserIdFromNoReplyEmail(email: string | undefined): string | undefined { + const match = email?.match(/^([0-9]+)\+[^@]+@users\.noreply\.github\.com$/); + return match?.[1]; +} + +function compareAvatarQuery(a: AvatarQueryCommit, b: AvatarQueryCommit): number { // Email const emailComparison = (a.authorEmail ?? '').localeCompare(b.authorEmail ?? ''); if (emailComparison !== 0) { @@ -88,8 +92,8 @@ export class GitHubSourceControlHistoryItemDetailsProvider implements SourceCont })); } - async provideAvatar(repository: Repository, query: AvatarQuery[]): Promise | undefined> { - this._logger.trace(`[GitHubSourceControlHistoryItemDetailsProvider][provideAvatar] Avatar resolution for ${query.length} commit(s) in ${repository.rootUri.fsPath}.`); + async provideAvatar(repository: Repository, query: AvatarQuery): Promise | undefined> { + this._logger.trace(`[GitHubSourceControlHistoryItemDetailsProvider][provideAvatar] Avatar resolution for ${query.commits.length} commit(s) in ${repository.rootUri.fsPath}.`); if (!this._enabled) { this._logger.trace(`[GitHubSourceControlHistoryItemDetailsProvider][provideAvatar] Avatar resolution is disabled.`); @@ -113,7 +117,7 @@ export class GitHubSourceControlHistoryItemDetailsProvider implements SourceCont } // Group the query by author - const authorQuery = groupBy(query, compareAvatarQuery); + const authorQuery = groupBy(query.commits, compareAvatarQuery); const results = new Map(); await Promise.all(authorQuery.map(async q => { @@ -128,29 +132,33 @@ export class GitHubSourceControlHistoryItemDetailsProvider implements SourceCont // Cache hit if (avatarUrl) { // Add avatar for each commit - for (const { commit } of q) { - results.set(commit, this._getAvatarUrl(avatarUrl, AVATAR_SIZE)); - } + q.forEach(({ hash }) => results.set(hash, avatarUrl)); return; } // Check if any of the commit are being tracked in the list // of known commits that have incomplte author information - if (q.some(({ commit }) => repositoryStore.commits.has(commit))) { - for (const { commit } of q) { - results.set(commit, undefined); - } + if (q.some(({ hash }) => repositoryStore.commits.has(hash))) { + q.forEach(({ hash }) => results.set(hash, undefined)); + return; + } + + // Try to extract the user identifier from GitHub no-reply emails + const userIdFromEmail = getUserIdFromNoReplyEmail(q[0].authorEmail); + if (userIdFromEmail) { + const avatarUrl = getAvatarLink(userIdFromEmail, query.size); + q.forEach(({ hash }) => results.set(hash, avatarUrl)); return; } // Get the commit details - const commitAuthor = await this._getCommitAuthor(descriptor, q[0].commit); + const commitAuthor = await this._getCommitAuthor(descriptor, q[0].hash); if (!commitAuthor) { // The commit has incomplete author information, so // we should not try to query the authors details again - for (const { commit } of q) { - repositoryStore.commits.add(commit); - results.set(commit, undefined); + for (const { hash } of q) { + repositoryStore.commits.add(hash); + results.set(hash, undefined); } return; } @@ -159,9 +167,7 @@ export class GitHubSourceControlHistoryItemDetailsProvider implements SourceCont repositoryStore.users.push(commitAuthor); // Add avatar for each commit - for (const { commit } of q) { - results.set(commit, this._getAvatarUrl(commitAuthor.avatarUrl, AVATAR_SIZE)); - } + q.forEach(({ hash }) => results.set(hash, `${commitAuthor.avatarUrl}&s=${query.size}`)); })); return results; @@ -296,10 +302,6 @@ export class GitHubSourceControlHistoryItemDetailsProvider implements SourceCont } } - private _getAvatarUrl(url: string, size: number): string { - return `${url}|height=${size},width=${size}`; - } - private _getRepositoryKey(descriptor: { owner: string; repo: string }): string { return `${descriptor.owner}/${descriptor.repo}`; } diff --git a/extensions/github/src/links.ts b/extensions/github/src/links.ts index fe97d172249..fdcac0c5cfd 100644 --- a/extensions/github/src/links.ts +++ b/extensions/github/src/links.ts @@ -176,6 +176,10 @@ export async function getLink(gitAPI: GitAPI, useSelection: boolean, shouldEnsur return `${uriWithoutFileSegments}${fileSegments}`; } +export function getAvatarLink(userId: string, size: number): string { + return `https://avatars.githubusercontent.com/u/${userId}?s=${size}`; +} + export function getBranchLink(url: string, branch: string, hostPrefix: string = 'https://github.com') { const repo = getRepositoryFromUrl(url); if (!repo) { diff --git a/extensions/github/src/typings/git.d.ts b/extensions/github/src/typings/git.d.ts index 7ad60e95400..e600b767c7c 100644 --- a/extensions/github/src/typings/git.d.ts +++ b/extensions/github/src/typings/git.d.ts @@ -289,14 +289,19 @@ export interface BranchProtectionProvider { provideBranchProtection(): BranchProtection[]; } -export interface AvatarQuery { - readonly commit: string; +export interface AvatarQueryCommit { + readonly hash: string; readonly authorName?: string; readonly authorEmail?: string; } +export interface AvatarQuery { + readonly commits: AvatarQueryCommit[]; + readonly size: number; +} + export interface SourceControlHistoryItemDetailsProvider { - provideAvatar(repository: Repository, query: AvatarQuery[]): Promise | undefined>; + provideAvatar(repository: Repository, query: AvatarQuery): Promise | undefined>; provideHoverCommands(repository: Repository): Promise; provideMessageLinks(repository: Repository, message: string): Promise; }