Open commits from non-checked out PRs in VS Code instead of opening the browser (#7347)

* Open commits from non-checked out PRs in VS Code instead of opening the browser
Fixes #7346

* Wait for GH repos

Author: alexr00

package.json

@@ -52,6 +52,7 @@
 		"onFileSystem:newIssue",
 		"onFileSystem:pr",
 		"onFileSystem:githubpr",
+		"onFileSystem:githubcommit",
 		"onFileSystem:review",
 		"onWebviewPanel:pr.codingAgentSessionLogView"
 	],

src/common/uri.ts

@@ -92,6 +92,29 @@ export interface GitUriOptions {
 	base: boolean;
 }
 
+export interface GitHubCommitUriParams {
+	commit: string;
+	owner: string;
+	repo: string;
+}
+
+export function fromGitHubCommitUri(uri: vscode.Uri): GitHubCommitUriParams | undefined {
+	if (uri.scheme !== Schemes.GitHubCommit || uri.query === '') {
+		return undefined;
+	}
+	try {
+		return JSON.parse(uri.query) as GitHubCommitUriParams;
+	} catch (e) { }
+}
+
+export function toGitHubCommitUri(fileName: string, params: GitHubCommitUriParams): vscode.Uri {
+	return vscode.Uri.from({
+		scheme: Schemes.GitHubCommit,
+		path: `/${fileName}`,
+		query: JSON.stringify(params)
+	});
+}
+
 const ImageMimetypes = ['image/png', 'image/gif', 'image/jpeg', 'image/webp', 'image/tiff', 'image/bmp'];
 // Known media types that VS Code can handle: https://github.com/microsoft/vscode/blob/a64e8e5673a44e5b9c2d493666bde684bd5a135c/src/vs/base/common/mime.ts#L33-L84
 export const KnownMediaExtensions = [
@@ -651,6 +674,7 @@ export enum Schemes {
 	Repo = 'repo', // New issue file for passing data
 	Git = 'git', // File content from the git extension
 	PRQuery = 'prquery', // PR query tree item
+	GitHubCommit = 'githubcommit' // file content from GitHub for a commit
 }
 
 export const COPILOT_QUERY = vscode.Uri.from({ scheme: Schemes.PRQuery, path: 'copilot' });

src/extension.ts

@@ -39,6 +39,7 @@ import { CommentDecorationProvider } from './view/commentDecorationProvider';
 import { CompareChanges } from './view/compareChangesTreeDataProvider';
 import { CreatePullRequestHelper } from './view/createPullRequestHelper';
 import { FileTypeDecorationProvider } from './view/fileTypeDecorationProvider';
+import { GitHubCommitFileSystemProvider } from './view/githubFileContentProvider';
 import { getInMemPRFileSystemProvider } from './view/inMemPRContentProvider';
 import { PullRequestChangesTreeDataProvider } from './view/prChangesTreeDataProvider';
 import { PRNotificationDecorationProvider } from './view/prNotificationDecorationProvider';
@@ -436,6 +437,8 @@ async function deferredActivate(context: vscode.ExtensionContext, showPRControll
 	const readOnlyMessage = new vscode.MarkdownString(vscode.l10n.t('Cannot edit this pull request file. [Check out](command:pr.checkoutFromReadonlyFile) this pull request to edit.'));
 	readOnlyMessage.isTrusted = { enabledCommands: ['pr.checkoutFromReadonlyFile'] };
 	context.subscriptions.push(vscode.workspace.registerFileSystemProvider(Schemes.Pr, inMemPRFileSystemProvider, { isReadonly: readOnlyMessage }));
+	const githubFilesystemProvider = new GitHubCommitFileSystemProvider(reposManager, apiImpl, credentialStore);
+	context.subscriptions.push(vscode.workspace.registerFileSystemProvider(Schemes.GitHubCommit, githubFilesystemProvider, { isReadonly: new vscode.MarkdownString(vscode.l10n.t('GitHub commits cannot be edited')) }));
 
 	await init(context, apiImpl, credentialStore, repositories, prTree, liveshareApiPromise, showPRController, reposManager, createPrHelper, copilotRemoteAgentManager, themeWatcher);
 	return apiImpl;

src/github/githubRepository.ts

@@ -207,7 +207,7 @@ export class GitHubRepository extends Disposable {
 				`github-browse-${this.remote.normalizedHost}-${this.remote.owner}-${this.remote.repositoryName}`,
 				`Pull Request (${this.remote.owner}/${this.remote.repositoryName})`,
 			);
-			this.commentsHandler = new PRCommentControllerRegistry(this.commentsController, this._telemetry);
+			this.commentsHandler = new PRCommentControllerRegistry(this.commentsController, this.telemetry);
 			this._register(this.commentsHandler);
 			this._register(this.commentsController);
 		} catch (e) {
@@ -234,7 +234,7 @@ export class GitHubRepository extends Disposable {
 		public remote: GitHubRemote,
 		public readonly rootUri: vscode.Uri,
 		private readonly _credentialStore: CredentialStore,
-		private readonly _telemetry: ITelemetry,
+		public readonly telemetry: ITelemetry,
 		silent: boolean = false
 	) {
 		super();
@@ -264,7 +264,7 @@ export class GitHubRepository extends Disposable {
 					"action": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth" }
 				}
 			*/
-			this._telemetry.sendTelemetryErrorEvent('pr.codespacesTokenError', {
+			this.telemetry.sendTelemetryErrorEvent('pr.codespacesTokenError', {
 				action: action.context
 			});
 
@@ -566,6 +566,19 @@ export class GitHubRepository extends Disposable {
 		return success;
 	}
 
+	async getCommitParent(ref: string): Promise<string | undefined> {
+		Logger.debug(`Fetch commit for ref ${ref} - enter`, this.id);
+		try {
+			const { octokit, remote } = await this.ensure();
+			const commit = (await octokit.call(octokit.api.repos.getCommit, { owner: remote.owner, repo: remote.repositoryName, ref })).data;
+			return commit.parents[0].sha;
+		} catch (e) {
+			Logger.error(`Fetching commit for ref ${ref} failed: ${e}`, this.id);
+		}
+		Logger.debug(`Fetch commit for ref ${ref} - done`, this.id);
+	}
+
+
 	async getAllPullRequests(page?: number): Promise<PullRequestData | undefined> {
 		let remote: GitHubRemote | undefined;
 		try {
@@ -950,7 +963,7 @@ export class GitHubRepository extends Disposable {
 		if (model) {
 			model.update(pullRequest);
 		} else {
-			model = new PullRequestModel(this._credentialStore, this._telemetry, this, this.remote, pullRequest);
+			model = new PullRequestModel(this._credentialStore, this.telemetry, this, this.remote, pullRequest);
 			const prModel = model;
 			const disposables: vscode.Disposable[] = [];
 			disposables.push(model.onDidChange(() => this._onPullRequestModelChanged(prModel)));

src/github/pullRequestModel.ts

@@ -10,15 +10,15 @@ import gql from 'graphql-tag';
 import * as vscode from 'vscode';
 import { Repository } from '../api/api';
 import { COPILOT_ACCOUNTS, DiffSide, IComment, IReviewThread, SubjectType, ViewedState } from '../common/comment';
-import { getModifiedContentFromDiffHunk, parseDiff } from '../common/diffHunk';
+import { getGitChangeType, getModifiedContentFromDiffHunk, parseDiff } from '../common/diffHunk';
 import { commands } from '../common/executeCommands';
 import { GitChangeType, InMemFileChange, SlimFileChange } from '../common/file';
 import { GitHubRef } from '../common/githubRef';
 import Logger from '../common/logger';
 import { Remote } from '../common/remote';
 import { ITelemetry } from '../common/telemetry';
 import { ClosedEvent, EventType, ReviewEvent, TimelineEvent } from '../common/timelineEvent';
-import { resolvePath, reviewPath, Schemes, toPRUri, toReviewUri } from '../common/uri';
+import { resolvePath, Schemes, toGitHubCommitUri, toPRUri, toReviewUri } from '../common/uri';
 import { formatError, isDescendant } from '../common/utils';
 import { InMemFileChangeModel, RemoteFileChangeModel } from '../view/fileChangeModel';
 import { OctokitCommon } from './common';
@@ -1333,47 +1333,32 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 		return vscode.commands.executeCommand('vscode.changes', vscode.l10n.t('Changes in Pull Request #{0}', pullRequestModel.number), args);
 	}
 
-	static async openCommitChanges(folderManager: FolderRepositoryManager, commitSha: string) {
+	static async openCommitChanges(githubRepository: GitHubRepository, commitSha: string) {
 		try {
-			// Get the repository from the folder manager
-			const repository = folderManager.repository;
-			if (!repository) {
-				vscode.window.showErrorMessage(vscode.l10n.t('No repository found'));
-				return;
-			}
-
-			// Get the commit to find its parent
-			const commit = await repository.getCommit(commitSha);
-			if (!commit.parents || commit.parents.length === 0) {
+			const parentCommit = await githubRepository.getCommitParent(commitSha);
+			if (!parentCommit) {
 				vscode.window.showErrorMessage(vscode.l10n.t('Commit {0} has no parent', commitSha.substring(0, 7)));
 				return;
 			}
-			const parentSha = commit.parents[0];
 
-			// Get the changes between the commit and its parent
-			const changes = await repository.diffBetween(parentSha, commitSha);
-			if (!changes || changes.length === 0) {
+			const changes = await githubRepository.compareCommits(parentCommit, commitSha);
+			if (!changes?.files || changes.files.length === 0) {
 				vscode.window.showInformationMessage(vscode.l10n.t('No changes found in commit {0}', commitSha.substring(0, 7)));
 				return;
 			}
 
 			// Create URI pairs for the multi diff editor using review scheme
 			const args: [vscode.Uri, vscode.Uri | undefined, vscode.Uri | undefined][] = [];
-			for (const change of changes) {
-				const rightRelativePath = path.relative(repository.rootUri.fsPath, change.uri.fsPath);
-				const rightPath = reviewPath(rightRelativePath, commitSha);
-				let rightUri = toReviewUri(rightPath, rightRelativePath, undefined, commitSha, false, { base: false }, repository.rootUri);
-
-				const leftRelativePath = path.relative(repository.rootUri.fsPath, change.originalUri.fsPath);
-				const leftPath = reviewPath(leftRelativePath, parentSha);
-				let leftUri = toReviewUri(leftPath, (change.status === GitChangeType.RENAME) ? path.relative(repository.rootUri.fsPath, change.originalUri.fsPath) : leftRelativePath, undefined, parentSha, false, { base: true }, repository.rootUri);
-
-				if (change.status === GitChangeType.ADD) {
+			for (const change of changes.files) {
+				const rightUri = toGitHubCommitUri(change.filename, { commit: commitSha, owner: githubRepository.remote.owner, repo: githubRepository.remote.repositoryName });
+				const leftUri = toGitHubCommitUri(change.previous_filename ?? change.filename, { commit: parentCommit, owner: githubRepository.remote.owner, repo: githubRepository.remote.repositoryName });
+				const changeType = getGitChangeType(change.status);
+				if (changeType === GitChangeType.ADD) {
 					// For added files, show against empty
 					args.push([rightUri, undefined, rightUri]);
-				} else if (change.status === GitChangeType.DELETE) {
+				} else if (changeType === GitChangeType.DELETE) {
 					// For deleted files, show old version against empty
-					args.push([rightPath, leftUri, undefined]);
+					args.push([rightUri, leftUri, undefined]);
 				} else {
 					args.push([rightUri, leftUri, rightUri]);
 				}
@@ -1382,7 +1367,7 @@ export class PullRequestModel extends IssueModel<PullRequest> implements IPullRe
 			/* __GDPR__
 				"pr.openCommitChanges" : {}
 			*/
-			folderManager.telemetry.sendTelemetryEvent('pr.openCommitChanges');
+			githubRepository.telemetry.sendTelemetryEvent('pr.openCommitChanges');
 
 			return commands.executeCommand('vscode.changes', vscode.l10n.t('Changes in Commit {0}', commitSha.substring(0, 7)), args);
 		} catch (error) {

src/github/pullRequestOverview.ts

@@ -538,7 +538,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
 	private async openCommitChanges(message: IRequestMessage<OpenCommitChangesArgs>): Promise<void> {
 		try {
 			const { commitSha } = message.args;
-			await PullRequestModel.openCommitChanges(this._folderRepositoryManager, commitSha);
+			await PullRequestModel.openCommitChanges(this._item.githubRepository, commitSha);
 		} catch (error) {
 			Logger.error(`Failed to open commit changes: ${formatError(error)}`, PullRequestOverviewPanel.ID);
 			vscode.window.showErrorMessage(vscode.l10n.t('Failed to open commit changes: {0}', formatError(error)));

src/view/githubFileContentProvider.ts

@@ -0,0 +1,39 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as vscode from 'vscode';
+import { GitApiImpl } from '../api/api1';
+import { fromGitHubCommitUri } from '../common/uri';
+import { CredentialStore } from '../github/credentials';
+import { RepositoriesManager } from '../github/repositoriesManager';
+import { RepositoryFileSystemProvider } from './repositoryFileSystemProvider';
+
+export class GitHubCommitFileSystemProvider extends RepositoryFileSystemProvider {
+	constructor(private readonly repos: RepositoriesManager, gitAPI: GitApiImpl, credentialStore: CredentialStore) {
+		super(gitAPI, credentialStore);
+	}
+
+	override async readFile(uri: vscode.Uri): Promise<Uint8Array> {
+		await this.waitForAuth();
+		await this.waitForAnyGitHubRepos(this.repos);
+
+		const params = fromGitHubCommitUri(uri);
+		if (!params) {
+			throw new Error(`Invalid GitHub commit URI: ${uri.toString()}`);
+		}
+
+		const folderManager = this.repos.getManagerForRepository(params.owner, params.repo);
+		if (!folderManager) {
+			throw new Error(`Repository not found for owner: ${params.owner}, repo: ${params.repo}`);
+		}
+
+		const githubRepo = await folderManager.createGitHubRepositoryFromOwnerName(params.owner, params.repo);
+		if (!githubRepo) {
+			throw new Error(`GitHub repository not found for owner: ${params.owner}, repo: ${params.repo}`);
+		}
+
+		return githubRepo.getFile(uri.path, params.commit);
+	}
+}

src/view/repositoryFileSystemProvider.ts

@@ -7,6 +7,7 @@ import * as vscode from 'vscode';
 import { GitApiImpl } from '../api/api1';
 import Logger from '../common/logger';
 import { CredentialStore } from '../github/credentials';
+import { RepositoriesManager } from '../github/repositoriesManager';
 import { ReadonlyFileSystemProvider } from './readonlyFileSystemProvider';
 
 export abstract class RepositoryFileSystemProvider extends ReadonlyFileSystemProvider {
@@ -47,4 +48,20 @@ export abstract class RepositoryFileSystemProvider extends ReadonlyFileSystemPro
 		}
 		return new Promise(resolve => this.credentialStore.onDidGetSession(() => resolve()));
 	}
+
+	protected async waitForAnyGitHubRepos(reposManager: RepositoriesManager): Promise<void> {
+		// Check if any folder manager already has GitHub repositories
+		if (reposManager.folderManagers.some(manager => manager.gitHubRepositories.length > 0)) {
+			return;
+		}
+
+		Logger.appendLine('Waiting for GitHub repositories.', 'RepositoryFileSystemProvider');
+		return new Promise(resolve => {
+			const disposable = reposManager.onDidChangeAnyGitHubRepository(() => {
+				Logger.appendLine('Found GitHub repositories.', 'RepositoryFileSystemProvider');
+				disposable.dispose();
+				resolve();
+			});
+		});
+	}
 }

webviews/components/timeline.tsx

@@ -110,11 +110,8 @@ const CommitEventView = (event: CommitEvent) => {
 	const pr = context.pr;
 
 	const handleCommitClick = (e: React.MouseEvent) => {
-		if (pr.isCurrentlyCheckedOut) {
-			e.preventDefault();
-			context.openCommitChanges(event.sha);
-		}
-		// If not checked out, let the default href behavior proceed
+		e.preventDefault();
+		context.openCommitChanges(event.sha);
 	};
 
 	return (
@@ -129,7 +126,6 @@ const CommitEventView = (event: CommitEvent) => {
 					<a
 						className="message"
 						onClick={handleCommitClick}
-						href={pr.isCurrentlyCheckedOut ? undefined : event.htmlUrl}
 						title={event.htmlUrl}
 					>
 						{event.message.substr(0, event.message.indexOf('\n') > -1 ? event.message.indexOf('\n') : event.message.length)}
@@ -140,7 +136,6 @@ const CommitEventView = (event: CommitEvent) => {
 				<a
 					className="sha"
 					onClick={handleCommitClick}
-					href={pr.isCurrentlyCheckedOut ? undefined : event.htmlUrl}
 					title={event.htmlUrl}
 				>
 					{event.sha.slice(0, 7)}