Skip to content

Commit 94cacf9

Browse files
Daniel-Aaron-BloomDaniel-Aaron-Bloom
authored
refactor based on feedback and add diff links
1 parent 8861224 commit 94cacf9

6 files changed

Lines changed: 252 additions & 184 deletions

File tree

‎common/views.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,4 @@ export interface OpenLocalFileArgs {
186186
endLine: number;
187187
}
188188

189-
export type CheckFilesExistResult = Record<string, boolean>;
190-
191189
// #endregion

‎src/github/issueOverview.ts‎

Lines changed: 63 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55
'use strict';
66

77
import * as vscode from 'vscode';
8-
import { CheckFilesExistResult, CloseResult, OpenLocalFileArgs } from '../../common/views';
8+
import { CloseResult, OpenLocalFileArgs } from '../../common/views';
99
import { openPullRequestOnGitHub } from '../commands';
1010
import { FolderRepositoryManager } from './folderRepositoryManager';
1111
import { GithubItemStateEnum, IAccount, IMilestone, IProject, IProjectItem, RepoAccessAndMergeMethods } from './interface';
1212
import { IssueModel } from './issueModel';
1313
import { getAssigneesQuickPickItems, getLabelOptions, getMilestoneFromQuickPick, getProjectFromQuickPick } from './quickPicks';
14-
import { isInCodespaces, vscodeDevPrLink } from './utils';
14+
import { isInCodespaces, processPermalinks, vscodeDevPrLink } from './utils';
1515
import { ChangeAssigneesReply, DisplayLabel, Issue, ProjectItemsReply, SubmitReviewReply, UnresolvedIdentity } from './views';
1616
import { COPILOT_ACCOUNTS, IComment } from '../common/comment';
1717
import { emojify, ensureEmojis } from '../common/emoji';
@@ -321,10 +321,13 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
321321
this._item = issue as TItem;
322322
this.setPanelTitle(this.buildPanelTitle(issueModel.number, issueModel.title));
323323

324+
// Process permalinks in bodyHTML before sending to webview
325+
issue.bodyHTML = await this.processLinksInBodyHtml(issue.bodyHTML);
326+
324327
Logger.debug('pr.initialize', IssueOverviewPanel.ID);
325328
this._postMessage({
326329
command: 'pr.initialize',
327-
pullrequest: this.getInitializeContext(currentUser, issue, timelineEvents, repositoryAccess, viewerCanEdit, assignableUsers[this._item.remote.remoteName] ?? []),
330+
pullrequest: this.getInitializeContext(currentUser, issue, await this.processTimelineEvents(timelineEvents), repositoryAccess, viewerCanEdit, assignableUsers[this._item.remote.remoteName] ?? []),
328331
});
329332

330333
} catch (e) {
@@ -447,8 +450,6 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
447450
return openPullRequestOnGitHub(this._item, this._telemetry);
448451
case 'pr.open-local-file':
449452
return this.openLocalFile(message);
450-
case 'pr.check-files-exist':
451-
return this.checkFilesExist(message);
452453
case 'pr.debug':
453454
return this.webviewDebug(message);
454455
default:
@@ -572,16 +573,51 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
572573
Logger.debug(message.args, IssueOverviewPanel.ID);
573574
}
574575

575-
private editDescription(message: IRequestMessage<{ text: string }>) {
576-
this._item
577-
.edit({ body: message.args.text })
578-
.then(result => {
579-
this._replyMessage(message, { body: result.body, bodyHTML: result.bodyHTML });
580-
})
581-
.catch(e => {
582-
this._throwError(message, e);
583-
vscode.window.showErrorMessage(`Editing description failed: ${formatError(e)}`);
584-
});
576+
/**
577+
* Process permalinks in bodyHTML. Can be overridden by subclasses (e.g., PullRequestOverviewPanel)
578+
* to provide custom processing logic for different item types.
579+
* Returns undefined if bodyHTML is undefined.
580+
*/
581+
protected async processLinksInBodyHtml(bodyHTML: string | undefined): Promise<string | undefined> {
582+
if (!bodyHTML) {
583+
return bodyHTML;
584+
}
585+
return processPermalinks(
586+
bodyHTML,
587+
this._item.githubRepository,
588+
this._item.githubRepository.rootUri
589+
);
590+
}
591+
592+
/**
593+
* Process permalinks in timeline events (comments, reviews, commits).
594+
* Updates bodyHTML fields for all events that contain them.
595+
*/
596+
protected async processTimelineEvents(events: TimelineEvent[]): Promise<TimelineEvent[]> {
597+
return Promise.all(events.map(async (event) => {
598+
if (event.event === EventType.Commented || event.event === EventType.Reviewed || event.event === EventType.Committed) {
599+
event.bodyHTML = await this.processLinksInBodyHtml(event.bodyHTML);
600+
// ReviewEvent also has comments array
601+
if (event.event === EventType.Reviewed && event.comments) {
602+
event.comments = await Promise.all(event.comments.map(async (comment: IComment) => {
603+
comment.bodyHTML = await this.processLinksInBodyHtml(comment.bodyHTML);
604+
return comment;
605+
}));
606+
}
607+
}
608+
return event;
609+
}));
610+
}
611+
612+
private async editDescription(message: IRequestMessage<{ text: string }>) {
613+
try {
614+
const result = await this._item.edit({ body: message.args.text });
615+
const bodyHTML = await this.processLinksInBodyHtml(result.bodyHTML);
616+
this._replyMessage(message, { body: result.body, bodyHTML });
617+
} catch (e) {
618+
this._throwError(message, e);
619+
vscode.window.showErrorMessage(`Editing description failed: ${formatError(e)}`);
620+
}
585621
}
586622
private editTitle(message: IRequestMessage<{ text: string }>) {
587623
return this._item
@@ -622,7 +658,7 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
622658
if (allAssignees) {
623659
const newAssignees: IAccount[] = allAssignees.map(item => item.user);
624660
await this._item.replaceAssignees(newAssignees);
625-
const events = await this._getTimeline();
661+
const events = await this.processTimelineEvents(await this._getTimeline());
626662
const reply: ChangeAssigneesReply = {
627663
assignees: newAssignees,
628664
events
@@ -689,7 +725,7 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
689725
const newAssignees = (this._item.assignees ?? []).concat(currentUser);
690726
await this._item.replaceAssignees(newAssignees);
691727
}
692-
const events = await this._getTimeline();
728+
const events = await this.processTimelineEvents(await this._getTimeline());
693729
const reply: ChangeAssigneesReply = {
694730
assignees: this._item.assignees ?? [],
695731
events
@@ -707,7 +743,7 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
707743
const newAssignees = (this._item.assignees ?? []).concat(copilotUser);
708744
await this._item.replaceAssignees(newAssignees);
709745
}
710-
const events = await this._getTimeline();
746+
const events = await this.processTimelineEvents(await this._getTimeline());
711747
const reply: ChangeAssigneesReply = {
712748
assignees: this._item.assignees ?? [],
713749
events
@@ -730,18 +766,15 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
730766
return this._item.editIssueComment(comment, text);
731767
}
732768

733-
private editComment(message: IRequestMessage<{ comment: IComment; text: string }>) {
734-
this.editCommentPromise(message.args.comment, message.args.text)
735-
.then(result => {
736-
this._replyMessage(message, {
737-
body: result.body,
738-
bodyHTML: result.bodyHTML,
739-
});
740-
})
741-
.catch(e => {
742-
this._throwError(message, e);
743-
vscode.window.showErrorMessage(formatError(e));
744-
});
769+
private async editComment(message: IRequestMessage<{ comment: IComment; text: string }>) {
770+
try {
771+
const result = await this.editCommentPromise(message.args.comment, message.args.text);
772+
const bodyHTML = await this.processLinksInBodyHtml(result.bodyHTML);
773+
this._replyMessage(message, { body: result.body, bodyHTML });
774+
} catch (e) {
775+
this._throwError(message, e);
776+
vscode.window.showErrorMessage(formatError(e));
777+
}
745778
}
746779

747780
protected deleteCommentPromise(comment: IComment): Promise<void> {
@@ -787,26 +820,6 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
787820
}
788821
}
789822

790-
private async checkFilesExist(message: IRequestMessage<string[]>): Promise<void> {
791-
const files = message.args;
792-
const results: CheckFilesExistResult = {};
793-
794-
await Promise.all(files.map(async (relativePath) => {
795-
const localFile = vscode.Uri.joinPath(
796-
this._item.githubRepository.rootUri,
797-
relativePath
798-
);
799-
try {
800-
const stat = await vscode.workspace.fs.stat(localFile);
801-
results[relativePath] = stat.type === vscode.FileType.File;
802-
} catch (e) {
803-
results[relativePath] = false;
804-
}
805-
}));
806-
807-
return this._replyMessage(message, results);
808-
}
809-
810823
protected async close(message: IRequestMessage<string>) {
811824
let comment: IComment | undefined;
812825
if (message.args) {

‎src/github/pullRequestOverview.ts‎

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55
'use strict';
66

7+
import * as crypto from 'crypto';
78
import * as vscode from 'vscode';
89
import { OpenCommitChangesArgs } from '../../common/views';
910
import { openPullRequestOnGitHub } from '../commands';
@@ -26,7 +27,7 @@ import { IssueOverviewPanel, panelKey } from './issueOverview';
2627
import { isCopilotOnMyBehalf, PullRequestModel } from './pullRequestModel';
2728
import { PullRequestReviewCommon, ReviewContext } from './pullRequestReviewCommon';
2829
import { branchPicks, pickEmail, reviewersQuickPick } from './quickPicks';
29-
import { parseReviewers } from './utils';
30+
import { parseReviewers, processDiffLinks, processPermalinks } from './utils';
3031
import { CancelCodingAgentReply, ChangeBaseReply, ChangeReviewersReply, DeleteReviewResult, MergeArguments, MergeResult, PullRequest, ReadyForReviewAndMergeContext, ReadyForReviewContext, ReviewCommentContext, ReviewType, UnresolvedIdentity } from './views';
3132
import { debounce } from '../common/async';
3233
import { COPILOT_ACCOUNTS, IComment } from '../common/comment';
@@ -233,6 +234,38 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
233234
}
234235
}
235236

237+
/**
238+
* Override to process permalinks with PR-specific logic (including diff links).
239+
* Returns undefined if bodyHTML is undefined.
240+
*/
241+
protected override async processLinksInBodyHtml(bodyHTML: string | undefined): Promise<string | undefined> {
242+
if (!bodyHTML) {
243+
return bodyHTML;
244+
}
245+
// Check cache first, otherwise fetch raw file changes
246+
const rawFileChanges = this._item.rawFileChanges ?? await this._item.getRawFileChangesInfo();
247+
248+
// Create hash-to-filename mapping for diff links
249+
const hashMap: Record<string, string> = {};
250+
rawFileChanges.forEach(file => {
251+
const hash = crypto.createHash('sha256').update(file.filename).digest('hex');
252+
hashMap[hash] = file.filename;
253+
});
254+
255+
let result = await processPermalinks(
256+
bodyHTML,
257+
this._item.githubRepository,
258+
this._item.githubRepository.rootUri
259+
);
260+
result = await processDiffLinks(
261+
result,
262+
this._item.githubRepository,
263+
hashMap,
264+
this._item.number
265+
);
266+
return result;
267+
}
268+
236269
protected override onDidChangeViewState(e: vscode.WebviewPanelOnDidChangeViewStateEvent): void {
237270
super.onDidChangeViewState(e);
238271
this.setVisibilityContext();
@@ -370,6 +403,9 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
370403
this._assignableUsers = assignableUsers;
371404
this.setPanelTitle(this.buildPanelTitle(pullRequestModel.number, pullRequestModel.title));
372405

406+
// Process permalinks in bodyHTML before sending to webview
407+
pullRequest.bodyHTML = await this.processLinksInBodyHtml(pullRequest.bodyHTML);
408+
373409
const isCurrentlyCheckedOut = pullRequestModel.equals(this._folderRepositoryManager.activePullRequest);
374410
const mergeMethodsAvailability = repositoryAccess!.mergeMethodsAvailability;
375411

@@ -383,7 +419,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
383419
const users = this._assignableUsers[pullRequestModel.remote.remoteName] ?? [];
384420
const copilotUser = users.find(user => COPILOT_ACCOUNTS[user.login]);
385421
const isCopilotAlreadyReviewer = this._existingReviewers.some(reviewer => !isITeam(reviewer.reviewer) && reviewer.reviewer.login === COPILOT_REVIEWER);
386-
const baseContext = this.getInitializeContext(currentUser, pullRequest, timelineEvents, repositoryAccess, viewerCanEdit, users);
422+
const baseContext = this.getInitializeContext(currentUser, pullRequest, await this.processTimelineEvents(timelineEvents), repositoryAccess, viewerCanEdit, users);
387423

388424
this.preLoadInfoNotRequiredForOverview(pullRequest);
389425

@@ -535,6 +571,8 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
535571
return this.cancelGenerateDescription();
536572
case 'pr.change-base-branch':
537573
return this.changeBaseBranch(message);
574+
case 'pr.open-diff-from-link':
575+
return this.openDiffFromLink(message);
538576
}
539577
}
540578

@@ -638,6 +676,33 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
638676
}
639677
}
640678

679+
private async openDiffFromLink(message: IRequestMessage<{ file: string; startLine: number; endLine: number }>): Promise<void> {
680+
try {
681+
const { file, startLine } = message.args;
682+
const fileChanges = await this._item.getFileChangesInfo();
683+
const change = fileChanges.find(
684+
fileChange => fileChange.fileName === file || fileChange.previousFileName === file,
685+
);
686+
687+
if (!change) {
688+
Logger.warn(`Could not find file ${file} in PR changes`, PullRequestOverviewPanel.ID);
689+
return;
690+
}
691+
692+
const pathSegments = file.split('/');
693+
// GitHub line numbers are 1-indexed, VSCode selection API is 0-indexed
694+
return PullRequestModel.openDiff(
695+
this._folderRepositoryManager,
696+
this._item,
697+
change,
698+
pathSegments[pathSegments.length - 1],
699+
startLine - 1,
700+
);
701+
} catch (e) {
702+
Logger.error(`Open diff from link failed: ${formatError(e)}`, PullRequestOverviewPanel.ID);
703+
}
704+
}
705+
641706
private async openSessionLog(message: IRequestMessage<{ link: SessionLinkInfo }>): Promise<void> {
642707
try {
643708
const resource = SessionIdForPr.getResource(this._item.number, message.args.link.sessionIndex);
@@ -728,7 +793,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
728793
await this._item.unresolveReviewThread(message.args.threadId);
729794
}
730795
const timelineEvents = await this._getTimeline();
731-
this._replyMessage(message, timelineEvents);
796+
this._replyMessage(message, await this.processTimelineEvents(timelineEvents));
732797
} catch (e) {
733798
vscode.window.showErrorMessage(e);
734799
this._replyMessage(message, undefined);

0 commit comments

Comments
 (0)