GitHub - avatar resolution improvements (#238239)

* Add support for size

* Add support for extracting user id from GitHub no-reply email addresses

* Fix git blame email parsing

* Extrat link into function

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<Map<string, string | undefined>>;
+	provideAvatar(repository: Repository, query: AvatarQuery): ProviderResult<Map<string, string | undefined>>;
 	provideHoverCommands(repository: Repository): ProviderResult<Command[]>;
 	provideMessageLinks(repository: Repository, message: string): ProviderResult<string>;
 }

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) {

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;

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<Map<string, string | undefined> | undefined> {
 	for (const provider of registry.getSourceControlHistoryItemDetailsProviders()) {
 		const result = await provider.provideAvatar(new ApiRepository(repository), query);

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<Map<string, string | undefined> | undefined> {
-		this._logger.trace(`[GitHubSourceControlHistoryItemDetailsProvider][provideAvatar] Avatar resolution for ${query.length} commit(s) in ${repository.rootUri.fsPath}.`);
+	async provideAvatar(repository: Repository, query: AvatarQuery): Promise<Map<string, string | undefined> | 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<AvatarQuery>(query, compareAvatarQuery);
+			const authorQuery = groupBy<AvatarQueryCommit>(query.commits, compareAvatarQuery);
 
 			const results = new Map<string, string | undefined>();
 			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}`;
 	}

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) {

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<Map<string, string | undefined> | undefined>;
+	provideAvatar(repository: Repository, query: AvatarQuery): Promise<Map<string, string | undefined> | undefined>;
 	provideHoverCommands(repository: Repository): Promise<Command[] | undefined>;
 	provideMessageLinks(repository: Repository, message: string): Promise<string | undefined>;
 }