Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions common/views.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,4 +180,10 @@ export interface OpenCommitChangesArgs {
commitSha: string;
}

export interface OpenLocalFileArgs {
file: string;
startLine: number;
endLine: number;
}

// #endregion
2 changes: 2 additions & 0 deletions src/common/webview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export class WebviewBase extends Disposable {
seq: originalMessage.req,
res: message,
};
await this._waitForReady;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wait here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the same reason we wait in _postMessage above

protected async _postMessage(message: any) {
 	// Without the following ready check, we can end up in a state where the message handler in the webview
 	// isn't ready for any of the messages we post.
 	await this._waitForReady;
 	this._webview?.postMessage({
 		res: message,
 	});
 }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice without this, the extension simply fails to work about half the time in a dev container (never got a failure outside of the container). The await calls to get the file mapping simply never resolve (presumably because the message got dropped)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the refactor, this became less of an issue, but it still seems like the right thing to do.

this._webview?.postMessage(reply);
}

Expand All @@ -82,6 +83,7 @@ export class WebviewBase extends Disposable {
seq: originalMessage?.req,
err: error,
};
await this._waitForReady;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wait here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

this._webview?.postMessage(reply);
}
}
Expand Down
115 changes: 87 additions & 28 deletions src/github/issueOverview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
'use strict';

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

// Process permalinks in bodyHTML before sending to webview
issue.bodyHTML = await this.processLinksInBodyHtml(issue.bodyHTML);

Logger.debug('pr.initialize', IssueOverviewPanel.ID);
this._postMessage({
command: 'pr.initialize',
pullrequest: this.getInitializeContext(currentUser, issue, timelineEvents, repositoryAccess, viewerCanEdit, assignableUsers[this._item.remote.remoteName] ?? []),
pullrequest: this.getInitializeContext(currentUser, issue, await this.processTimelineEvents(timelineEvents), repositoryAccess, viewerCanEdit, assignableUsers[this._item.remote.remoteName] ?? []),
});

} catch (e) {
Expand Down Expand Up @@ -445,6 +448,8 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
return this.copyVscodeDevLink();
case 'pr.openOnGitHub':
return openPullRequestOnGitHub(this._item, this._telemetry);
case 'pr.open-local-file':
return this.openLocalFile(message);
case 'pr.debug':
return this.webviewDebug(message);
default:
Expand Down Expand Up @@ -568,16 +573,51 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
Logger.debug(message.args, IssueOverviewPanel.ID);
}

private editDescription(message: IRequestMessage<{ text: string }>) {
this._item
.edit({ body: message.args.text })
.then(result => {
this._replyMessage(message, { body: result.body, bodyHTML: result.bodyHTML });
})
.catch(e => {
this._throwError(message, e);
vscode.window.showErrorMessage(`Editing description failed: ${formatError(e)}`);
});
/**
* Process permalinks in bodyHTML. Can be overridden by subclasses (e.g., PullRequestOverviewPanel)
* to provide custom processing logic for different item types.
* Returns undefined if bodyHTML is undefined.
*/
protected async processLinksInBodyHtml(bodyHTML: string | undefined): Promise<string | undefined> {
if (!bodyHTML) {
return bodyHTML;
}
return processPermalinks(
bodyHTML,
this._item.githubRepository,
this._item.githubRepository.rootUri
);
}

/**
* Process permalinks in timeline events (comments, reviews, commits).
* Updates bodyHTML fields for all events that contain them.
*/
protected async processTimelineEvents(events: TimelineEvent[]): Promise<TimelineEvent[]> {
return Promise.all(events.map(async (event) => {
if (event.event === EventType.Commented || event.event === EventType.Reviewed || event.event === EventType.Committed) {
event.bodyHTML = await this.processLinksInBodyHtml(event.bodyHTML);
// ReviewEvent also has comments array
if (event.event === EventType.Reviewed && event.comments) {
event.comments = await Promise.all(event.comments.map(async (comment: IComment) => {
comment.bodyHTML = await this.processLinksInBodyHtml(comment.bodyHTML);
return comment;
}));
}
}
return event;
}));
}

private async editDescription(message: IRequestMessage<{ text: string }>) {
try {
const result = await this._item.edit({ body: message.args.text });
const bodyHTML = await this.processLinksInBodyHtml(result.bodyHTML);
this._replyMessage(message, { body: result.body, bodyHTML });
} catch (e) {
this._throwError(message, e);
vscode.window.showErrorMessage(`Editing description failed: ${formatError(e)}`);
}
}
private editTitle(message: IRequestMessage<{ text: string }>) {
return this._item
Expand Down Expand Up @@ -618,7 +658,7 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
if (allAssignees) {
const newAssignees: IAccount[] = allAssignees.map(item => item.user);
await this._item.replaceAssignees(newAssignees);
const events = await this._getTimeline();
const events = await this.processTimelineEvents(await this._getTimeline());
const reply: ChangeAssigneesReply = {
assignees: newAssignees,
events
Expand Down Expand Up @@ -685,7 +725,7 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
const newAssignees = (this._item.assignees ?? []).concat(currentUser);
await this._item.replaceAssignees(newAssignees);
}
const events = await this._getTimeline();
const events = await this.processTimelineEvents(await this._getTimeline());
const reply: ChangeAssigneesReply = {
assignees: this._item.assignees ?? [],
events
Expand All @@ -703,7 +743,7 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
const newAssignees = (this._item.assignees ?? []).concat(copilotUser);
await this._item.replaceAssignees(newAssignees);
}
const events = await this._getTimeline();
const events = await this.processTimelineEvents(await this._getTimeline());
const reply: ChangeAssigneesReply = {
assignees: this._item.assignees ?? [],
events
Expand All @@ -726,18 +766,15 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
return this._item.editIssueComment(comment, text);
}

private editComment(message: IRequestMessage<{ comment: IComment; text: string }>) {
this.editCommentPromise(message.args.comment, message.args.text)
.then(result => {
this._replyMessage(message, {
body: result.body,
bodyHTML: result.bodyHTML,
});
})
.catch(e => {
this._throwError(message, e);
vscode.window.showErrorMessage(formatError(e));
});
private async editComment(message: IRequestMessage<{ comment: IComment; text: string }>) {
try {
const result = await this.editCommentPromise(message.args.comment, message.args.text);
const bodyHTML = await this.processLinksInBodyHtml(result.bodyHTML);
this._replyMessage(message, { body: result.body, bodyHTML });
} catch (e) {
this._throwError(message, e);
vscode.window.showErrorMessage(formatError(e));
}
}

protected deleteCommentPromise(comment: IComment): Promise<void> {
Expand All @@ -761,6 +798,28 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
});
}

protected async openLocalFile(message: IRequestMessage<OpenLocalFileArgs>): Promise<void> {
try {
const { file, startLine, endLine } = message.args;
// Resolve relative path to absolute using repository root
const fileUri = vscode.Uri.joinPath(
this._item.githubRepository.rootUri,
file
);
const selection = new vscode.Range(
new vscode.Position(startLine - 1, 0),
new vscode.Position(endLine - 1, Number.MAX_SAFE_INTEGER)
);
const document = await vscode.workspace.openTextDocument(fileUri);
await vscode.window.showTextDocument(document, {
selection,
viewColumn: vscode.ViewColumn.One
});
} catch (e) {
Logger.error(`Open local file failed: ${formatError(e)}`, IssueOverviewPanel.ID);
}
}

protected async close(message: IRequestMessage<string>) {
let comment: IComment | undefined;
if (message.args) {
Expand Down
71 changes: 68 additions & 3 deletions src/github/pullRequestOverview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*--------------------------------------------------------------------------------------------*/
'use strict';

import * as crypto from 'crypto';
import * as vscode from 'vscode';
import { OpenCommitChangesArgs } from '../../common/views';
import { openPullRequestOnGitHub } from '../commands';
Expand All @@ -26,7 +27,7 @@ import { IssueOverviewPanel, panelKey } from './issueOverview';
import { isCopilotOnMyBehalf, PullRequestModel } from './pullRequestModel';
import { PullRequestReviewCommon, ReviewContext } from './pullRequestReviewCommon';
import { branchPicks, pickEmail, reviewersQuickPick } from './quickPicks';
import { parseReviewers } from './utils';
import { parseReviewers, processDiffLinks, processPermalinks } from './utils';
import { CancelCodingAgentReply, ChangeBaseReply, ChangeReviewersReply, DeleteReviewResult, MergeArguments, MergeResult, PullRequest, ReadyForReviewAndMergeContext, ReadyForReviewContext, ReviewCommentContext, ReviewType, UnresolvedIdentity } from './views';
import { debounce } from '../common/async';
import { COPILOT_ACCOUNTS, IComment } from '../common/comment';
Expand Down Expand Up @@ -233,6 +234,38 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
}
}

/**
* Override to process permalinks with PR-specific logic (including diff links).
* Returns undefined if bodyHTML is undefined.
*/
protected override async processLinksInBodyHtml(bodyHTML: string | undefined): Promise<string | undefined> {
if (!bodyHTML) {
return bodyHTML;
}
// Check cache first, otherwise fetch raw file changes
const rawFileChanges = this._item.rawFileChanges ?? await this._item.getRawFileChangesInfo();

// Create hash-to-filename mapping for diff links
const hashMap: Record<string, string> = {};
rawFileChanges.forEach(file => {
const hash = crypto.createHash('sha256').update(file.filename).digest('hex');
hashMap[hash] = file.filename;
});

let result = await processPermalinks(
bodyHTML,
this._item.githubRepository,
this._item.githubRepository.rootUri
);
result = await processDiffLinks(
result,
this._item.githubRepository,
hashMap,
this._item.number
);
return result;
}

protected override onDidChangeViewState(e: vscode.WebviewPanelOnDidChangeViewStateEvent): void {
super.onDidChangeViewState(e);
this.setVisibilityContext();
Expand Down Expand Up @@ -370,6 +403,9 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
this._assignableUsers = assignableUsers;
this.setPanelTitle(this.buildPanelTitle(pullRequestModel.number, pullRequestModel.title));

// Process permalinks in bodyHTML before sending to webview
pullRequest.bodyHTML = await this.processLinksInBodyHtml(pullRequest.bodyHTML);

const isCurrentlyCheckedOut = pullRequestModel.equals(this._folderRepositoryManager.activePullRequest);
const mergeMethodsAvailability = repositoryAccess!.mergeMethodsAvailability;

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

this.preLoadInfoNotRequiredForOverview(pullRequest);

Expand Down Expand Up @@ -535,6 +571,8 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
return this.cancelGenerateDescription();
case 'pr.change-base-branch':
return this.changeBaseBranch(message);
case 'pr.open-diff-from-link':
return this.openDiffFromLink(message);
}
}

Expand Down Expand Up @@ -638,6 +676,33 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
}
}

private async openDiffFromLink(message: IRequestMessage<{ file: string; startLine: number; endLine: number }>): Promise<void> {
try {
const { file, startLine } = message.args;
const fileChanges = await this._item.getFileChangesInfo();
const change = fileChanges.find(
fileChange => fileChange.fileName === file || fileChange.previousFileName === file,
);

if (!change) {
Logger.warn(`Could not find file ${file} in PR changes`, PullRequestOverviewPanel.ID);
return;
}

const pathSegments = file.split('/');
// GitHub line numbers are 1-indexed, VSCode selection API is 0-indexed
return PullRequestModel.openDiff(
this._folderRepositoryManager,
this._item,
change,
pathSegments[pathSegments.length - 1],
startLine - 1,
);
} catch (e) {
Logger.error(`Open diff from link failed: ${formatError(e)}`, PullRequestOverviewPanel.ID);
}
}

private async openSessionLog(message: IRequestMessage<{ link: SessionLinkInfo }>): Promise<void> {
try {
const resource = SessionIdForPr.getResource(this._item.number, message.args.link.sessionIndex);
Expand Down Expand Up @@ -728,7 +793,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel<PullRequestMode
await this._item.unresolveReviewThread(message.args.threadId);
}
const timelineEvents = await this._getTimeline();
this._replyMessage(message, timelineEvents);
this._replyMessage(message, await this.processTimelineEvents(timelineEvents));
} catch (e) {
vscode.window.showErrorMessage(e);
this._replyMessage(message, undefined);
Expand Down
Loading