diff --git a/resources/icons/codicons/diff-multiple.svg b/resources/icons/codicons/diff-multiple.svg new file mode 100644 index 0000000000..0f71971ee1 --- /dev/null +++ b/resources/icons/codicons/diff-multiple.svg @@ -0,0 +1 @@ + diff --git a/src/github/pullRequestModel.ts b/src/github/pullRequestModel.ts index 9d13bc4adf..c18fad70b5 100644 --- a/src/github/pullRequestModel.ts +++ b/src/github/pullRequestModel.ts @@ -86,7 +86,8 @@ import { RestAccount, restPaginate, } from './utils'; -import { Repository } from '../api/api'; +import { Change, Repository } from '../api/api'; +import { Status } from '../api/api1'; import { COPILOT_ACCOUNTS, DiffSide, IComment, IReviewThread, SubjectType, ViewedState } from '../common/comment'; import { getGitChangeType, getModifiedContentFromDiffHunk, parseDiff } from '../common/diffHunk'; import { commands } from '../common/executeCommands'; @@ -1642,6 +1643,116 @@ export class PullRequestModel extends IssueModel implements IPullRe return vscode.commands.executeCommand('vscode.changes', vscode.l10n.t('Changes in Pull Request #{0}', pullRequestModel.number), args); } + static async openReadonlyChanges(folderManager: FolderRepositoryManager, pullRequestModel: PullRequestModel): Promise { + const headCommit = pullRequestModel.head?.sha; + if (!headCommit) { + throw new Error(`Pull request #${pullRequestModel.number} has no head commit.`); + } + + let startCommit = pullRequestModel.item.merged + ? pullRequestModel.base.sha + : await this.getLocalMergeBase(folderManager.repository, pullRequestModel.base.sha, headCommit); + let localChanges: Change[] | undefined; + if (startCommit && await this.commitsExistLocally(folderManager.repository, startCommit, headCommit)) { + localChanges = await folderManager.repository.diffBetween(startCommit, headCommit); + } + + let remoteChanges: (InMemFileChange | SlimFileChange)[] | undefined; + if (!localChanges) { + const allChanges = await pullRequestModel.getAllFileChangesInfo(); + remoteChanges = allChanges.changes; + startCommit = allChanges.mergeBase; + if (await this.commitsExistLocally(folderManager.repository, startCommit, headCommit)) { + localChanges = await folderManager.repository.diffBetween(startCommit, headCommit); + } + } + + if (!startCommit) { + throw new Error(`Pull request #${pullRequestModel.number} has no base commit.`); + } + + let args: [vscode.Uri, vscode.Uri | undefined, vscode.Uri | undefined][]; + if (localChanges) { + args = localChanges.map(change => this.localChangeToMultiDiffEntry(change, startCommit, headCommit)); + } else { + if (!remoteChanges) { + throw new Error(`Pull request #${pullRequestModel.number} has no file changes.`); + } + const remote = pullRequestModel.githubRepository.remote; + args = remoteChanges.map((change): [vscode.Uri, vscode.Uri | undefined, vscode.Uri | undefined] => { + const rightUri = toGitHubCommitUri(change.fileName, { commit: headCommit, owner: remote.owner, repo: remote.repositoryName }); + const parentFileName = change.status === GitChangeType.RENAME ? change.previousFileName ?? change.fileName : change.fileName; + const leftUri = toGitHubCommitUri(parentFileName, { commit: startCommit, owner: remote.owner, repo: remote.repositoryName }); + if (change.status === GitChangeType.ADD) { + return [rightUri, undefined, rightUri]; + } else if (change.status === GitChangeType.DELETE) { + return [rightUri, leftUri, undefined]; + } + return [rightUri, leftUri, rightUri]; + }); + } + + /* __GDPR__ + "pr.viewChanges" : { + "source" : { "classification": "SystemMetaData", "purpose": "FeatureInsight" } + } + */ + folderManager.telemetry.sendTelemetryEvent('pr.viewChanges', { source: localChanges ? 'git' : 'github' }); + return vscode.commands.executeCommand('vscode.changes', vscode.l10n.t('Changes in Pull Request #{0}', pullRequestModel.number), args); + } + + private static async getLocalMergeBase(repository: Repository, baseCommit: string, headCommit: string): Promise { + if (!await this.commitsExistLocally(repository, baseCommit, headCommit)) { + return; + } + try { + return await repository.getMergeBase(baseCommit, headCommit); + } catch (error) { + Logger.debug(`Using GitHub to determine the pull request merge base: ${formatError(error)}`, PullRequestModel.ID); + return; + } + } + + private static async commitsExistLocally(repository: Repository, startCommit: string, endCommit: string): Promise { + try { + await Promise.all([repository.getCommit(startCommit), repository.getCommit(endCommit)]); + return true; + } catch (error) { + Logger.debug(`Using GitHub content because the pull request commit range is not available locally: ${formatError(error)}`, PullRequestModel.ID); + return false; + } + } + + private static localChangeToMultiDiffEntry(change: Change, startCommit: string, endCommit: string): [vscode.Uri, vscode.Uri | undefined, vscode.Uri | undefined] { + const rightFileUri = change.renameUri ?? change.uri; + const leftFileUri = change.originalUri; + const rightUri = this.toGitCommitUri(rightFileUri, endCommit); + const leftUri = this.toGitCommitUri(leftFileUri, startCommit); + + switch (change.status) { + case Status.INDEX_ADDED: + case Status.ADDED_BY_US: + case Status.ADDED_BY_THEM: + case Status.UNTRACKED: + case Status.INTENT_TO_ADD: + return [rightUri, undefined, rightUri]; + case Status.INDEX_DELETED: + case Status.DELETED: + case Status.DELETED_BY_US: + case Status.DELETED_BY_THEM: + return [rightUri, leftUri, undefined]; + default: + return [rightUri, leftUri, rightUri]; + } + } + + private static toGitCommitUri(fileUri: vscode.Uri, commit: string): vscode.Uri { + return fileUri.with({ + scheme: Schemes.Git, + query: JSON.stringify({ path: fileUri.fsPath, ref: commit }), + }); + } + static async openCommitChanges(extensionUri: vscode.Uri, githubRepository: GitHubRepository, commitSha: string) { try { const parentCommit = await githubRepository.getCommitParent(commitSha); @@ -1806,6 +1917,25 @@ export class PullRequestModel extends IssueModel implements IPullRe return parsed; } + async getAllFileChangesInfo(): Promise<{ changes: (InMemFileChange | SlimFileChange)[], mergeBase: string }> { + const githubRepository = this.githubRepository; + const { octokit, remote } = await githubRepository.ensure(); + if (this.item.merged) { + const files = await restPaginate(octokit.api.pulls.listFiles, { + repo: remote.repositoryName, + owner: remote.owner, + pull_number: this.number, + }); + return { changes: await parseDiff(files, this.base.sha), mergeBase: this.base.sha }; + } + + if (!this.head) { + throw new Error(`Pull request #${this.number} has no head commit.`); + } + const { files, mergeBaseSha } = await compareCommits(remote, octokit, this.base, this.head, this.base.sha, this.number, PullRequestModel.ID); + return { changes: await parseDiff(files, mergeBaseSha), mergeBase: mergeBaseSha }; + } + async getPatch(): Promise { const githubRepository = this.githubRepository; const { octokit, remote } = await githubRepository.ensure(); diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index e5f613c559..baae3e5132 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -500,6 +500,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel): Promise { + const fileSystemProvider = getGitHubCommitFileSystemProvider(); + if (!fileSystemProvider) { + throw new Error('GitHub commit file system provider is not initialized.'); + } + fileSystemProvider.registerGitHubRepository(this._item.githubRepository); + await PullRequestModel.openReadonlyChanges(this._folderRepositoryManager, this._item); + await this._replyMessage(message, {}); + } + private resolveCommentThread(message: IRequestMessage<{ threadId: string, toResolve: boolean, thread: IComment[] }>) { // Serialize resolve/unresolve operations so that concurrent calls don't race. // Each call fetches the full timeline after its mutation, and without serialization diff --git a/src/github/views.ts b/src/github/views.ts index 0b40604f74..3ad50bb543 100644 --- a/src/github/views.ts +++ b/src/github/views.ts @@ -75,6 +75,7 @@ export interface Issue { export interface PullRequest extends Issue { isCopilotOnMyBehalf: boolean; + isAgentSessionsWorkspace: boolean; isCurrentlyCheckedOut: boolean; isRemoteBaseDeleted?: boolean; base: string; diff --git a/src/test/github/pullRequestModel.test.ts b/src/test/github/pullRequestModel.test.ts index 5084d1b0ee..17dc84d257 100644 --- a/src/test/github/pullRequestModel.test.ts +++ b/src/test/github/pullRequestModel.test.ts @@ -4,8 +4,12 @@ *--------------------------------------------------------------------------------------------*/ import { default as assert } from 'assert'; +import * as vscode from 'vscode'; import { MockCommandRegistry } from '../mocks/mockCommandRegistry'; +import { Status } from '../../api/api1'; +import { GitChangeType, SlimFileChange } from '../../common/file'; import { CredentialStore } from '../../github/credentials'; +import { FolderRepositoryManager } from '../../github/folderRepositoryManager'; import { PullRequestModel } from '../../github/pullRequestModel'; import { GithubItemStateEnum } from '../../github/interface'; import { Protocol } from '../../common/protocol'; @@ -98,6 +102,86 @@ describe('PullRequestModel', function () { assert.strictEqual(open.state, GithubItemStateEnum.Merged); }); + describe('openReadonlyChanges', function () { + const baseCommit = '1111111111111111111111111111111111111111'; + const mergeBase = '2222222222222222222222222222222222222222'; + const headCommit = '3333333333333333333333333333333333333333'; + + function createPullRequestModel(): PullRequestModel { + const pr = new PullRequestBuilder() + .base(base => base.sha(baseCommit)) + .head(head => head.sha(headCommit)) + .build(); + return new PullRequestModel(credentials, telemetry, repo, remote, convertRESTPullRequestToRawPullRequest(pr, repo)); + } + + it('uses the git filesystem when the commit range is available locally', async function () { + const model = createPullRequestModel(); + const oldUri = vscode.Uri.file('C:\\users\\test\\repo\\old.ts'); + const newUri = vscode.Uri.file('C:\\users\\test\\repo\\new.ts'); + const getCommit = sinon.stub().resolves({ hash: '', message: '', parents: [] }); + const getMergeBase = sinon.stub().resolves(mergeBase); + const diffBetween = sinon.stub().resolves([{ + uri: newUri, + originalUri: oldUri, + renameUri: newUri, + status: Status.INDEX_RENAMED, + }]); + const executeCommand = sinon.stub(vscode.commands, 'executeCommand').resolves(); + const folderManager = { + repository: { getCommit, getMergeBase, diffBetween }, + telemetry, + } as unknown as FolderRepositoryManager; + + await PullRequestModel.openReadonlyChanges(folderManager, model); + + assert(getMergeBase.calledOnceWithExactly(baseCommit, headCommit)); + assert(diffBetween.calledOnceWithExactly(mergeBase, headCommit)); + const [command, , entries] = executeCommand.firstCall.args; + assert.strictEqual(command, 'vscode.changes'); + assert.strictEqual(entries.length, 1); + const [resourceUri, originalUri, modifiedUri] = entries[0]; + assert.strictEqual(resourceUri.scheme, 'git'); + assert.strictEqual(originalUri.scheme, 'git'); + assert.strictEqual(modifiedUri.scheme, 'git'); + assert.deepStrictEqual(JSON.parse(originalUri.query), { path: oldUri.fsPath, ref: mergeBase }); + assert.deepStrictEqual(JSON.parse(modifiedUri.query), { path: newUri.fsPath, ref: headCommit }); + }); + + it('uses GitHub when the commit range is not available locally', async function () { + const model = createPullRequestModel(); + const getCommit = sinon.stub().rejects(new Error('Unknown commit')); + const getAllFileChangesInfo = sinon.stub(model, 'getAllFileChangesInfo').resolves({ + changes: [new SlimFileChange(mergeBase, '', GitChangeType.RENAME, 'new.ts', 'old.ts')], + mergeBase, + }); + const executeCommand = sinon.stub(vscode.commands, 'executeCommand').resolves(); + const folderManager = { + repository: { getCommit }, + telemetry, + } as unknown as FolderRepositoryManager; + + await PullRequestModel.openReadonlyChanges(folderManager, model); + + assert(getAllFileChangesInfo.calledOnce); + const [, , entries] = executeCommand.firstCall.args; + const [resourceUri, originalUri, modifiedUri] = entries[0]; + assert.strictEqual(resourceUri.scheme, 'githubcommit'); + assert.strictEqual(originalUri.scheme, 'githubcommit'); + assert.strictEqual(modifiedUri.scheme, 'githubcommit'); + assert.deepStrictEqual(JSON.parse(originalUri.query), { + commit: mergeBase, + owner: repo.remote.owner, + repo: repo.remote.repositoryName, + }); + assert.deepStrictEqual(JSON.parse(modifiedUri.query), { + commit: headCommit, + owner: repo.remote.owner, + repo: repo.remote.repositoryName, + }); + }); + }); + describe('reviewThreadCache', function () { function page(id: string, endCursor: string | null) { return { diff --git a/webviews/common/context.tsx b/webviews/common/context.tsx index cc7e617b8e..ee405d1284 100644 --- a/webviews/common/context.tsx +++ b/webviews/common/context.tsx @@ -54,6 +54,8 @@ export class PRContext { public openChanges = (openToTheSide?: boolean) => this.postMessage({ command: 'pr.open-changes', args: { openToTheSide } }); + public viewChanges = () => this.postMessage({ command: 'pr.view-changes' }); + public copyPrLink = () => this.postMessage({ command: 'pr.copy-prlink' }); public copyVscodeDevLink = () => this.postMessage({ command: 'pr.copy-vscodedevlink' }); diff --git a/webviews/components/header.tsx b/webviews/components/header.tsx index bd61fe0ecb..9e04929ead 100644 --- a/webviews/components/header.tsx +++ b/webviews/components/header.tsx @@ -5,7 +5,7 @@ import React, { useContext, useState } from 'react'; import { ContextDropdown } from './contextDropdown'; -import { copilotErrorIcon, copilotInProgressIcon, copilotSuccessIcon, copyIcon, editIcon, gitMergeIcon, gitPullRequestClosedIcon, gitPullRequestDraftIcon, gitPullRequestIcon, issuescon, loadingIcon, passIcon } from './icon'; +import { copilotErrorIcon, copilotInProgressIcon, copilotSuccessIcon, copyIcon, diffMultipleIcon, editIcon, gitMergeIcon, gitPullRequestClosedIcon, gitPullRequestDraftIcon, gitPullRequestIcon, issuescon, loadingIcon, passIcon } from './icon'; import { AuthorLink, Avatar } from './user'; import { copilotEventToStatus, CopilotPRStatus, mostRecentCopilotEvent } from '../../src/common/copilot'; import { CopilotStartedEvent, TimelineEvent } from '../../src/common/timelineEvent'; @@ -28,6 +28,7 @@ export function Header({ isCurrentlyCheckedOut, isDraft, isIssue, + isAgentSessionsWorkspace, doneCheckoutBranch, events, owner, @@ -50,6 +51,8 @@ export function Header({ setEditMode={setEditMode} setCurrentTitle={setCurrentTitle} canEdit={canEdit} + isIssue={isIssue} + isAgentSessionsWorkspace={isAgentSessionsWorkspace} owner={owner} repo={repo} /> @@ -79,11 +82,13 @@ interface TitleProps { setEditMode: React.Dispatch>; setCurrentTitle: React.Dispatch>; canEdit: boolean; + isIssue: boolean; + isAgentSessionsWorkspace: boolean; owner: string; repo: string; } -function Title({ title, titleHTML, number, url, inEditMode, setEditMode, setCurrentTitle, canEdit, owner, repo }: TitleProps): JSX.Element { +function Title({ title, titleHTML, number, url, inEditMode, setEditMode, setCurrentTitle, canEdit, isIssue, isAgentSessionsWorkspace, owner, repo }: TitleProps): JSX.Element { const { setTitle, copyPrLink, openOnGitHub } = useContext(PullRequestContext); const titleForm = ( @@ -146,6 +151,7 @@ function Title({ title, titleHTML, number, url, inEditMode, setEditMode, setCurr + {!isIssue && isAgentSessionsWorkspace ? : null} ); @@ -153,6 +159,15 @@ function Title({ title, titleHTML, number, url, inEditMode, setEditMode, setCurr return editableTitle; } +export function ViewChangesButton(): JSX.Element { + const { viewChanges } = useContext(PullRequestContext); + return ( + + ); +} + interface ButtonGroupProps { isCurrentlyCheckedOut: boolean; isIssue: boolean; diff --git a/webviews/components/icon.tsx b/webviews/components/icon.tsx index daca557f3a..b7a1b3f26a 100644 --- a/webviews/components/icon.tsx +++ b/webviews/components/icon.tsx @@ -23,6 +23,7 @@ export const cloudUploadIcon = ; export const copilotIcon = ; export const copyIcon = ; +export const diffMultipleIcon = ; export const editIcon = ; export const errorIcon = ; export const feedbackIcon = ; diff --git a/webviews/components/stickyHeader.tsx b/webviews/components/stickyHeader.tsx index bead62d6fa..dc6500bdda 100644 --- a/webviews/components/stickyHeader.tsx +++ b/webviews/components/stickyHeader.tsx @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as React from 'react'; -import { getStatus } from './header'; +import { getStatus, ViewChangesButton } from './header'; import { copyIcon } from './icon'; import { PullRequest } from '../../src/github/views'; import PullRequestContext from '../common/context'; @@ -74,6 +74,7 @@ export function StickyHeader({ pr, visible }: { pr: PullRequest; visible: boolea + {!pr.isIssue && pr.isAgentSessionsWorkspace ? : null} ); diff --git a/webviews/editorWebview/test/builder/pullRequest.ts b/webviews/editorWebview/test/builder/pullRequest.ts index 0847efe891..265316eec6 100644 --- a/webviews/editorWebview/test/builder/pullRequest.ts +++ b/webviews/editorWebview/test/builder/pullRequest.ts @@ -66,5 +66,6 @@ export const PullRequestBuilder = createBuilderClass()({ canAssignCopilot: { default: false }, canRequestCopilotReview: { default: false }, isCopilotOnMyBehalf: { default: false }, + isAgentSessionsWorkspace: { default: false }, reactions: { default: [] }, }); diff --git a/webviews/editorWebview/test/overview.test.tsx b/webviews/editorWebview/test/overview.test.tsx index 2b14eac345..6a23cae97d 100644 --- a/webviews/editorWebview/test/overview.test.tsx +++ b/webviews/editorWebview/test/overview.test.tsx @@ -59,6 +59,39 @@ describe('Overview', function () { assert.strictEqual(openOnGitHub.callCount, 2); }); + it('shows view changes in both headers', function () { + const pr = new PullRequestBuilder().isAgentSessionsWorkspace(true).build(); + const context = new PRContext(pr); + const viewChanges = sinon.stub(context, 'viewChanges'); + + const out = render( + + + , + ); + + const viewChangesButtons = out.container.querySelectorAll('[aria-label="View Pull Request Changes"]'); + assert.strictEqual(viewChangesButtons.length, 2); + viewChangesButtons.forEach(button => { + assert.strictEqual(button.parentElement?.lastElementChild, button); + fireEvent.click(button); + }); + assert.strictEqual(viewChanges.callCount, 2); + }); + + it('does not show view changes outside the agents window', function () { + const pr = new PullRequestBuilder().isAgentSessionsWorkspace(false).build(); + const context = new PRContext(pr); + + const out = render( + + + , + ); + + assert.strictEqual(out.container.querySelector('[aria-label="View Pull Request Changes"]'), null); + }); + it('applies sticky class when scrolled', function () { const pr = new PullRequestBuilder().build(); const context = new PRContext(pr);