From 1aa080576a722f07d77ac01728058f0590b4f781 Mon Sep 17 00:00:00 2001 From: Daniel Bloom Date: Mon, 9 Mar 2026 01:49:36 -0700 Subject: [PATCH 1/8] Link to local file for permalinks in webview fixes #8571 --- common/views.ts | 8 ++ src/github/issueOverview.ts | 48 ++++++++++- webviews/common/context.tsx | 6 ++ webviews/editorWebview/app.tsx | 146 +++++++++++++++++++++++++++++++++ 4 files changed, 207 insertions(+), 1 deletion(-) diff --git a/common/views.ts b/common/views.ts index 5682e9b011..d0a9045011 100644 --- a/common/views.ts +++ b/common/views.ts @@ -180,4 +180,12 @@ export interface OpenCommitChangesArgs { commitSha: string; } +export interface OpenLocalFileArgs { + file: string; + startLine: number; + endLine: number; +} + +export type CheckFilesExistResult = Record; + // #endregion \ No newline at end of file diff --git a/src/github/issueOverview.ts b/src/github/issueOverview.ts index 67f3aef622..5b2902452a 100644 --- a/src/github/issueOverview.ts +++ b/src/github/issueOverview.ts @@ -5,7 +5,7 @@ 'use strict'; import * as vscode from 'vscode'; -import { CloseResult } from '../../common/views'; +import { CheckFilesExistResult, CloseResult, OpenLocalFileArgs } from '../../common/views'; import { openPullRequestOnGitHub } from '../commands'; import { FolderRepositoryManager } from './folderRepositoryManager'; import { GithubItemStateEnum, IAccount, IMilestone, IProject, IProjectItem, RepoAccessAndMergeMethods } from './interface'; @@ -445,6 +445,10 @@ export class IssueOverviewPanel 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.check-files-exist': + return this.checkFilesExist(message); case 'pr.debug': return this.webviewDebug(message); default: @@ -761,6 +765,48 @@ export class IssueOverviewPanel extends W }); } + protected async openLocalFile(message: IRequestMessage): Promise { + 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); + } + } + + private async checkFilesExist(message: IRequestMessage): Promise { + const files = message.args; + const results: CheckFilesExistResult = {}; + + await Promise.all(files.map(async (relativePath) => { + const localFile = vscode.Uri.joinPath( + this._item.githubRepository.rootUri, + relativePath + ); + try { + const stat = await vscode.workspace.fs.stat(localFile); + results[relativePath] = stat.type === vscode.FileType.File; + } catch (e) { + results[relativePath] = false; + } + })); + + return this._replyMessage(message, results); + } + protected async close(message: IRequestMessage) { let comment: IComment | undefined; if (message.args) { diff --git a/webviews/common/context.tsx b/webviews/common/context.tsx index 55cb61836f..93feef0d84 100644 --- a/webviews/common/context.tsx +++ b/webviews/common/context.tsx @@ -361,6 +361,12 @@ export class PRContext { public openSessionLog = (link: SessionLinkInfo) => this.postMessage({ command: 'pr.open-session-log', args: { link } }); + public openLocalFile = (file: string, startLine: number, endLine: number) => + this.postMessage({ command: 'pr.open-local-file', args: { file, startLine, endLine } }); + + public checkFilesExist = (files: string[]): Promise> => + this.postMessage({ command: 'pr.check-files-exist', args: files }); + public viewCheckLogs = (status: PullRequestCheckStatus) => this.postMessage({ command: 'pr.view-check-logs', args: { status } }); public openCommitChanges = async (commitSha: string) => { diff --git a/webviews/editorWebview/app.tsx b/webviews/editorWebview/app.tsx index 3a3b7691d4..589d3ee6be 100644 --- a/webviews/editorWebview/app.tsx +++ b/webviews/editorWebview/app.tsx @@ -11,6 +11,86 @@ import { PullRequest } from '../../src/github/views'; import { COMMENT_TEXTAREA_ID } from '../common/constants'; import PullRequestContext from '../common/context'; +const PROCESSED_MARKER = 'data-permalink-processed'; + +interface PermalinkAnchor { + element: HTMLAnchorElement; + url: string; + file: string; + startLine: number; + endLine: number; +} + +function findUnprocessedPermalinks( + root: Document | Element, + repoName: string, +): PermalinkAnchor[] { + const anchors: PermalinkAnchor[] = []; + const urlPattern = new RegExp( + `^https://github\\.com/[^/]+/${repoName}/blob/[0-9a-f]{40}/([^#]+)#L([0-9]+)(?:-L([0-9]+))?$`, + ); + + // Find all unprocessed anchor elements + const allAnchors = root.querySelectorAll( + `a[href^="https://github.com/"]:not([${PROCESSED_MARKER}])`, + ); + + allAnchors.forEach((anchor: Element) => { + const htmlAnchor = anchor as HTMLAnchorElement; + + const href = htmlAnchor.getAttribute('href'); + if (!href) return; + + const match = href.match(urlPattern); + if (match) { + const file = match[1]; + const startLine = parseInt(match[2]); + const endLine = match[3] ? parseInt(match[3]) : startLine; + + anchors.push({ + element: htmlAnchor, + url: href, + file, + startLine, + endLine, + }); + } + }); + + return anchors; +} + + +function updatePermalinks( + anchors: PermalinkAnchor[], + fileExistenceMap: Record, +): void { + anchors.forEach(({ element, url, file, startLine, endLine }) => { + const exists = fileExistenceMap[file]; + if (!exists) { + return; + } + + element.setAttribute('data-local-file', file); + element.setAttribute('data-start-line', startLine.toString()); + element.setAttribute('data-end-line', endLine.toString()); + + // Add "(view on GitHub)" link after this anchor + const githubLink = document.createElement('a'); + githubLink.href = url; + githubLink.textContent = 'view on GitHub'; + githubLink.setAttribute(PROCESSED_MARKER, 'true'); + if (element.className) { + githubLink.className = element.className; + } + element.after( + document.createTextNode(' ('), + githubLink, + document.createTextNode(')'), + ); + }); +} + export function main() { render({pr => }, document.getElementById('app')); } @@ -41,6 +121,72 @@ export function Root({ children }) { return () => window.removeEventListener('focus', handleWindowFocus); }, []); + useEffect(() => { + const handleLinkClick = (event: MouseEvent) => { + const target = event.target as HTMLElement; + const anchor = target.closest('a[data-local-file]'); + if (anchor) { + const file = anchor.getAttribute('data-local-file'); + const startLine = anchor.getAttribute('data-start-line'); + const endLine = anchor.getAttribute('data-end-line'); + if (file && startLine && endLine) { + // Swallow the event and open the file + event.preventDefault(); + event.stopPropagation(); + ctx.openLocalFile(file, parseInt(startLine), parseInt(endLine)); + } + } + }; + + document.addEventListener('click', handleLinkClick, true); + return () => document.removeEventListener('click', handleLinkClick, true); + }, [ctx]); + + // Process GitHub permalinks + useEffect(() => { + if (!pr) return; + + const processPermalinks = debounce(async () => { + try { + const anchors = findUnprocessedPermalinks(document.body, pr.repo); + anchors.forEach(({ element }) => { + element.setAttribute(PROCESSED_MARKER, 'true'); + }); + + if (anchors.length > 0) { + const uniqueFiles = Array.from(new Set(anchors.map((a) => a.file))); + const fileExistenceMap = await ctx.checkFilesExist(uniqueFiles); + updatePermalinks(anchors, fileExistenceMap); + } + } catch (error) { + console.error('Error processing permalinks:', error); + } + }, 100); + + // Start observing the document body for changes + const observer = new MutationObserver((mutations) => { + const hasNewNodes = mutations.some( + ({ addedNodes }) => addedNodes.length > 0, + ); + + if (hasNewNodes) { + processPermalinks(); + } + }); + observer.observe(document.body, { + childList: true, + subtree: true, + }); + + // Process the initial set of links + processPermalinks(); + + return () => { + observer.disconnect(); + processPermalinks.clear(); + }; + }, [pr, ctx]); + window.onscroll = debounce(() => { ctx.postMessage({ command: 'scroll', From 88612245064f1fb1a7ab358370e8b23c7022e7b6 Mon Sep 17 00:00:00 2001 From: Daniel Bloom Date: Tue, 10 Mar 2026 14:21:30 -0700 Subject: [PATCH 2/8] Fix _waitForReady --- src/common/webview.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/common/webview.ts b/src/common/webview.ts index f887fd349b..a41fab61e4 100644 --- a/src/common/webview.ts +++ b/src/common/webview.ts @@ -74,6 +74,7 @@ export class WebviewBase extends Disposable { seq: originalMessage.req, res: message, }; + await this._waitForReady; this._webview?.postMessage(reply); } @@ -82,6 +83,7 @@ export class WebviewBase extends Disposable { seq: originalMessage?.req, err: error, }; + await this._waitForReady; this._webview?.postMessage(reply); } } From 94cacf939570027fdbe00a2f0d8edbdecc1dbd37 Mon Sep 17 00:00:00 2001 From: Daniel-Aaron-Bloom <76709210+Daniel-Aaron-Bloom@users.noreply.github.com> Date: Thu, 12 Mar 2026 17:39:41 -0700 Subject: [PATCH 3/8] refactor based on feedback and add diff links --- common/views.ts | 2 - src/github/issueOverview.ts | 113 ++++++++++++++----------- src/github/pullRequestOverview.ts | 71 +++++++++++++++- src/github/utils.ts | 110 ++++++++++++++++++++++++ webviews/common/context.tsx | 4 +- webviews/editorWebview/app.tsx | 136 ++---------------------------- 6 files changed, 252 insertions(+), 184 deletions(-) diff --git a/common/views.ts b/common/views.ts index d0a9045011..5ea4602411 100644 --- a/common/views.ts +++ b/common/views.ts @@ -186,6 +186,4 @@ export interface OpenLocalFileArgs { endLine: number; } -export type CheckFilesExistResult = Record; - // #endregion \ No newline at end of file diff --git a/src/github/issueOverview.ts b/src/github/issueOverview.ts index 5b2902452a..103a8c7e86 100644 --- a/src/github/issueOverview.ts +++ b/src/github/issueOverview.ts @@ -5,13 +5,13 @@ 'use strict'; import * as vscode from 'vscode'; -import { CheckFilesExistResult, CloseResult, OpenLocalFileArgs } 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'; @@ -321,10 +321,13 @@ export class IssueOverviewPanel 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) { @@ -447,8 +450,6 @@ export class IssueOverviewPanel extends W return openPullRequestOnGitHub(this._item, this._telemetry); case 'pr.open-local-file': return this.openLocalFile(message); - case 'pr.check-files-exist': - return this.checkFilesExist(message); case 'pr.debug': return this.webviewDebug(message); default: @@ -572,16 +573,51 @@ export class IssueOverviewPanel 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 { + 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 { + 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 @@ -622,7 +658,7 @@ export class IssueOverviewPanel 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 @@ -689,7 +725,7 @@ export class IssueOverviewPanel 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 @@ -707,7 +743,7 @@ export class IssueOverviewPanel 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 @@ -730,18 +766,15 @@ export class IssueOverviewPanel 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 { @@ -787,26 +820,6 @@ export class IssueOverviewPanel extends W } } - private async checkFilesExist(message: IRequestMessage): Promise { - const files = message.args; - const results: CheckFilesExistResult = {}; - - await Promise.all(files.map(async (relativePath) => { - const localFile = vscode.Uri.joinPath( - this._item.githubRepository.rootUri, - relativePath - ); - try { - const stat = await vscode.workspace.fs.stat(localFile); - results[relativePath] = stat.type === vscode.FileType.File; - } catch (e) { - results[relativePath] = false; - } - })); - - return this._replyMessage(message, results); - } - protected async close(message: IRequestMessage) { let comment: IComment | undefined; if (message.args) { diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index b47e103f91..22a24dfedb 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -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'; @@ -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'; @@ -233,6 +234,38 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { + 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 = {}; + 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(); @@ -370,6 +403,9 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel 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); @@ -535,6 +571,8 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel): Promise { + 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 { try { const resource = SessionIdForPr.getResource(this._item.number, message.args.link.sessionIndex); @@ -728,7 +793,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { + try { + const repoName = escapeRegExp(githubRepository.remote.repositoryName); + const repoOwner = escapeRegExp(githubRepository.remote.owner); + const authority = escapeRegExp(githubRepository.remote.gitProtocol.url.authority); + + // Process blob permalinks (exclude already processed links) + const blobPattern = new RegExp( + `]*data-permalink-processed)([^>]*?href="https?:\/\/${authority}\/${repoOwner}\/${repoName}\/blob\/[0-9a-f]{40}\/([^"#]+)#L(\\d+)(?:-L(\\d+))?"[^>]*?)>([^<]*?)<\/a>`, + 'g' + ); + + return await stringReplaceAsync(bodyHTML, blobPattern, async ( + fullMatch: string, + attributes: string, + filePath: string, + startLine: string, + endLine: string | undefined, + linkText: string + ) => { + try { + // Extract the original URL from attributes + const hrefMatch = attributes.match(/href="([^"]+)"/); + const originalUrl = hrefMatch ? hrefMatch[1] : ''; + + // Check if file exists locally + const localFileUri = vscode.Uri.joinPath(rootUri, filePath); + try { + const stat = await vscode.workspace.fs.stat(localFileUri); + if (stat.type === vscode.FileType.File) { + // File exists - add data attributes for local handling and "(view on GitHub)" suffix + const endLineValue = endLine || startLine; + return `${linkText} (view on GitHub)`; + } + } catch { + // File doesn't exist - keep original link + } + } catch (error) { + Logger.warn(`Failed to process blob permalink: ${error}`, 'processPermalinks'); + } + return fullMatch; + }); + } catch (error) { + Logger.error(`Failed to process blob permalinks in HTML: ${error}`, 'processPermalinks'); + return bodyHTML; // Return original HTML if processing fails + } +} + +/** + * Process GitHub diff permalinks in HTML and add data attributes for local file handling. + * Finds diff permalinks (e.g., /pull/123/files#diff-[hash]R10), maps hashes to filenames, + * and adds data attributes to enable clicking to open diff views. + */ +export async function processDiffLinks( + bodyHTML: string, + githubRepository: GitHubRepository, + hashMap: Record, + prNumber: number +): Promise { + try { + const repoName = escapeRegExp(githubRepository.remote.repositoryName); + const repoOwner = escapeRegExp(githubRepository.remote.owner); + const authority = escapeRegExp(githubRepository.remote.gitProtocol.url.authority); + + const diffPattern = new RegExp( + `]*data-permalink-processed)([^>]*?href="https?:\/\/${authority}\/${repoOwner}\/${repoName}\/pull\/${prNumber}\/(?:files|changes)#diff-([a-f0-9]{64})(?:R(\\d+)(?:-R(\\d+))?)?"[^>]*?)>([^<]*?)<\/a>`, + 'g' + ); + + return await stringReplaceAsync(bodyHTML, diffPattern, async ( + fullMatch: string, + attributes: string, + diffHash: string, + startLine: string | undefined, + endLine: string | undefined, + linkText: string + ) => { + try { + // Extract the original URL from attributes + const hrefMatch = attributes.match(/href="([^"]+)"/); + const originalUrl = hrefMatch ? hrefMatch[1] : ''; + + // Look up filename from hash + const fileName = hashMap[diffHash]; + if (fileName) { + // Hash found - add data attributes for diff handling and "(view on GitHub)" suffix + const startLineValue = startLine || '1'; + const endLineValue = endLine || startLineValue; + return `${linkText} (view on GitHub)`; + } + } catch (error) { + Logger.warn(`Failed to process diff permalink: ${error}`, 'processDiffLinks'); + } + return fullMatch; + }); + } catch (error) { + Logger.error(`Failed to process diff permalinks in HTML: ${error}`, 'processDiffLinks'); + return bodyHTML; // Return original HTML if processing fails + } +} + export function convertRESTPullRequestToRawPullRequest( pullRequest: | OctokitCommon.PullsGetResponseData diff --git a/webviews/common/context.tsx b/webviews/common/context.tsx index 93feef0d84..a34d19f08c 100644 --- a/webviews/common/context.tsx +++ b/webviews/common/context.tsx @@ -364,8 +364,8 @@ export class PRContext { public openLocalFile = (file: string, startLine: number, endLine: number) => this.postMessage({ command: 'pr.open-local-file', args: { file, startLine, endLine } }); - public checkFilesExist = (files: string[]): Promise> => - this.postMessage({ command: 'pr.check-files-exist', args: files }); + public openDiffFromLink = (file: string, startLine: number, endLine: number) => + this.postMessage({ command: 'pr.open-diff-from-link', args: { file, startLine, endLine } }); public viewCheckLogs = (status: PullRequestCheckStatus) => this.postMessage({ command: 'pr.view-check-logs', args: { status } }); diff --git a/webviews/editorWebview/app.tsx b/webviews/editorWebview/app.tsx index 589d3ee6be..3d57c887c9 100644 --- a/webviews/editorWebview/app.tsx +++ b/webviews/editorWebview/app.tsx @@ -11,86 +11,6 @@ import { PullRequest } from '../../src/github/views'; import { COMMENT_TEXTAREA_ID } from '../common/constants'; import PullRequestContext from '../common/context'; -const PROCESSED_MARKER = 'data-permalink-processed'; - -interface PermalinkAnchor { - element: HTMLAnchorElement; - url: string; - file: string; - startLine: number; - endLine: number; -} - -function findUnprocessedPermalinks( - root: Document | Element, - repoName: string, -): PermalinkAnchor[] { - const anchors: PermalinkAnchor[] = []; - const urlPattern = new RegExp( - `^https://github\\.com/[^/]+/${repoName}/blob/[0-9a-f]{40}/([^#]+)#L([0-9]+)(?:-L([0-9]+))?$`, - ); - - // Find all unprocessed anchor elements - const allAnchors = root.querySelectorAll( - `a[href^="https://github.com/"]:not([${PROCESSED_MARKER}])`, - ); - - allAnchors.forEach((anchor: Element) => { - const htmlAnchor = anchor as HTMLAnchorElement; - - const href = htmlAnchor.getAttribute('href'); - if (!href) return; - - const match = href.match(urlPattern); - if (match) { - const file = match[1]; - const startLine = parseInt(match[2]); - const endLine = match[3] ? parseInt(match[3]) : startLine; - - anchors.push({ - element: htmlAnchor, - url: href, - file, - startLine, - endLine, - }); - } - }); - - return anchors; -} - - -function updatePermalinks( - anchors: PermalinkAnchor[], - fileExistenceMap: Record, -): void { - anchors.forEach(({ element, url, file, startLine, endLine }) => { - const exists = fileExistenceMap[file]; - if (!exists) { - return; - } - - element.setAttribute('data-local-file', file); - element.setAttribute('data-start-line', startLine.toString()); - element.setAttribute('data-end-line', endLine.toString()); - - // Add "(view on GitHub)" link after this anchor - const githubLink = document.createElement('a'); - githubLink.href = url; - githubLink.textContent = 'view on GitHub'; - githubLink.setAttribute(PROCESSED_MARKER, 'true'); - if (element.className) { - githubLink.className = element.className; - } - element.after( - document.createTextNode(' ('), - githubLink, - document.createTextNode(')'), - ); - }); -} - export function main() { render({pr => }, document.getElementById('app')); } @@ -129,11 +49,18 @@ export function Root({ children }) { const file = anchor.getAttribute('data-local-file'); const startLine = anchor.getAttribute('data-start-line'); const endLine = anchor.getAttribute('data-end-line'); + const linkType = anchor.getAttribute('data-link-type'); if (file && startLine && endLine) { - // Swallow the event and open the file + // Swallow the event event.preventDefault(); event.stopPropagation(); - ctx.openLocalFile(file, parseInt(startLine), parseInt(endLine)); + + // Open diff view for diff links, local file for blob permalinks + if (linkType === 'diff') { + ctx.openDiffFromLink(file, parseInt(startLine), parseInt(endLine)); + } else { + ctx.openLocalFile(file, parseInt(startLine), parseInt(endLine)); + } } } }; @@ -142,51 +69,6 @@ export function Root({ children }) { return () => document.removeEventListener('click', handleLinkClick, true); }, [ctx]); - // Process GitHub permalinks - useEffect(() => { - if (!pr) return; - - const processPermalinks = debounce(async () => { - try { - const anchors = findUnprocessedPermalinks(document.body, pr.repo); - anchors.forEach(({ element }) => { - element.setAttribute(PROCESSED_MARKER, 'true'); - }); - - if (anchors.length > 0) { - const uniqueFiles = Array.from(new Set(anchors.map((a) => a.file))); - const fileExistenceMap = await ctx.checkFilesExist(uniqueFiles); - updatePermalinks(anchors, fileExistenceMap); - } - } catch (error) { - console.error('Error processing permalinks:', error); - } - }, 100); - - // Start observing the document body for changes - const observer = new MutationObserver((mutations) => { - const hasNewNodes = mutations.some( - ({ addedNodes }) => addedNodes.length > 0, - ); - - if (hasNewNodes) { - processPermalinks(); - } - }); - observer.observe(document.body, { - childList: true, - subtree: true, - }); - - // Process the initial set of links - processPermalinks(); - - return () => { - observer.disconnect(); - processPermalinks.clear(); - }; - }, [pr, ctx]); - window.onscroll = debounce(() => { ctx.postMessage({ command: 'scroll', From 8304066af160be36943ba4caec6eaad34c46e1fb Mon Sep 17 00:00:00 2001 From: Daniel Bloom Date: Mon, 16 Mar 2026 17:23:42 -0700 Subject: [PATCH 4/8] refactor based on feedback --- common/views.ts | 1 + src/common/utils.ts | 161 ++++++++++++++++++++++++++++++ src/github/issueOverview.ts | 43 ++++---- src/github/pullRequestOverview.ts | 40 ++++---- src/github/utils.ts | 94 ++++------------- webviews/common/context.tsx | 8 +- webviews/editorWebview/app.tsx | 16 ++- 7 files changed, 242 insertions(+), 121 deletions(-) diff --git a/common/views.ts b/common/views.ts index 5ea4602411..9e2f35fdf2 100644 --- a/common/views.ts +++ b/common/views.ts @@ -184,6 +184,7 @@ export interface OpenLocalFileArgs { file: string; startLine: number; endLine: number; + href: string; } // #endregion \ No newline at end of file diff --git a/src/common/utils.ts b/src/common/utils.ts index 0f36f1202b..8eff714cd8 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -1014,4 +1014,165 @@ export function truncate(value: string, maxLength: number, suffix = '...'): stri return value; } return `${value.substr(0, maxLength)}${suffix}`; +} + +/** + * Metadata extracted from code reference link data attributes. + * This interface defines the contract between the extension (which creates the attributes) + * and the webview (which reads them). + */ +export interface CodeReferenceLinkMetadata { + localFile: string; + startLine: number; + endLine: number; + linkType: 'blob' | 'diff'; + href: string; +} + +/** + * Extracts code reference link metadata from an anchor element's data attributes. + * Returns null if any required attributes are missing. + */ +export function extractCodeReferenceLinkMetadata(anchor: Element): CodeReferenceLinkMetadata | null { + const localFile = anchor.getAttribute('data-local-file'); + const startLine = anchor.getAttribute('data-start-line'); + const endLine = anchor.getAttribute('data-end-line'); + const linkType = anchor.getAttribute('data-link-type'); + const href = anchor.getAttribute('href'); + + if (!localFile || !startLine || !endLine || !linkType || !href) { + return null; + } + + return { + localFile, + startLine: parseInt(startLine), + endLine: parseInt(endLine), + linkType: linkType as 'blob' | 'diff', + href + }; +} + +/** + * Process GitHub blob permalinks in HTML and add data attributes for local file handling. + * Finds blob permalinks (e.g., /blob/[sha]/file.ts#L10), checks if files exist locally, + * and adds data attributes to enable clicking to open local files. + * + * @param bodyHTML - The HTML content to process + * @param repoOwner - GitHub repository owner + * @param repoName - GitHub repository name + * @param authority - Git protocol URL authority (e.g., 'github.com') + * @param fileExistsCheck - Async function that checks if a file exists locally given its relative path + * @returns Promise resolving to processed HTML + */ +export async function processPermalinks( + bodyHTML: string, + repoOwner: string, + repoName: string, + authority: string, + fileExistsCheck: (filePath: string) => Promise +): Promise { + try { + const escapedRepoName = escapeRegExp(repoName); + const escapedRepoOwner = escapeRegExp(repoOwner); + const escapedAuthority = escapeRegExp(authority); + + // Process blob permalinks (exclude already processed links) + const blobPattern = new RegExp( + `]*data-permalink-processed)([^>]*?href="https?:\/\/${escapedAuthority}\/${escapedRepoOwner}\/${escapedRepoName}\/blob\/[0-9a-f]{40}\/(?[^"#]+)#L(?\\d+)(?:-L(?\\d+))?"[^>]*?)>(?[^<]*?)<\/a>`, + 'g' + ); + + return await stringReplaceAsync(bodyHTML, blobPattern, async ( + fullMatch: string, + attributes: string, + filePath: string, + startLine: string, + endLine: string | undefined, + linkText: string + ) => { + try { + // Extract the original URL from attributes + const hrefMatch = attributes.match(/ href="([^"]+)"/); + const originalUrl = hrefMatch ? hrefMatch[1] : ''; + + // Check if file exists locally + const exists = await fileExistsCheck(filePath); + if (exists) { + // File exists - add data attributes for local handling and "(view on GitHub)" suffix + const endLineValue = endLine || startLine; + return `${linkText} (view on GitHub)`; + } + } catch (error) { + // File doesn't exist or check failed - keep original link + } + return fullMatch; + }); + } catch (error) { + // Return original HTML if processing fails + return bodyHTML; + } +} + +/** + * Process GitHub diff permalinks in HTML and add data attributes for local file handling. + * Finds diff permalinks (e.g., /pull/123/files#diff-[hash]R10), maps hashes to filenames, + * and adds data attributes to enable clicking to open diff views. + * + * @param bodyHTML - The HTML content to process + * @param repoOwner - GitHub repository owner + * @param repoName - GitHub repository name + * @param authority - Git protocol URL authority (e.g., 'github.com') + * @param hashMap - Map of diff hashes to file paths + * @param prNumber - Pull request number + * @returns Promise resolving to processed HTML + */ +export async function processDiffLinks( + bodyHTML: string, + repoOwner: string, + repoName: string, + authority: string, + hashMap: Record, + prNumber: number +): Promise { + try { + const escapedRepoName = escapeRegExp(repoName); + const escapedRepoOwner = escapeRegExp(repoOwner); + const escapedAuthority = escapeRegExp(authority); + + const diffPattern = new RegExp( + `]*data-permalink-processed)([^>]*?href="https?:\/\/${escapedAuthority}\/${escapedRepoOwner}\/${escapedRepoName}\/pull\/${prNumber}\/(?:files|changes)#diff-(?[a-f0-9]{64})(?:R(?\\d+)(?:-R(?\\d+))?)?"[^>]*?)>(?[^<]*?)<\/a>`, + 'g' + ); + + return await stringReplaceAsync(bodyHTML, diffPattern, async ( + fullMatch: string, + attributes: string, + diffHash: string, + startLine: string | undefined, + endLine: string | undefined, + linkText: string + ) => { + try { + // Extract the original URL from attributes + const hrefMatch = attributes.match(/ href="([^"]+)"/); + const originalUrl = hrefMatch ? hrefMatch[1] : ''; + + // Look up filename from hash + const fileName = hashMap[diffHash]; + if (fileName) { + // Hash found - add data attributes for diff handling and "(view on GitHub)" suffix + const startLineValue = startLine || '1'; + const endLineValue = endLine || startLineValue; + return `${linkText} (view on GitHub)`; + } + } catch (error) { + // Failed to process - keep original link + } + return fullMatch; + }); + } catch (error) { + // Return original HTML if processing fails + return bodyHTML; + } } \ No newline at end of file diff --git a/src/github/issueOverview.ts b/src/github/issueOverview.ts index 103a8c7e86..07d6384999 100644 --- a/src/github/issueOverview.ts +++ b/src/github/issueOverview.ts @@ -322,12 +322,15 @@ export class IssueOverviewPanel extends W this.setPanelTitle(this.buildPanelTitle(issueModel.number, issueModel.title)); // Process permalinks in bodyHTML before sending to webview - issue.bodyHTML = await this.processLinksInBodyHtml(issue.bodyHTML); + const processedBodyHTML = await this.processLinksInBodyHtml(issue.bodyHTML); + const context = this.getInitializeContext(currentUser, issue, await this.processTimelineEvents(timelineEvents), repositoryAccess, viewerCanEdit, assignableUsers[this._item.remote.remoteName] ?? []); + // Override bodyHTML with processed version without mutating original issue + context.bodyHTML = processedBodyHTML; Logger.debug('pr.initialize', IssueOverviewPanel.ID); this._postMessage({ command: 'pr.initialize', - pullrequest: this.getInitializeContext(currentUser, issue, await this.processTimelineEvents(timelineEvents), repositoryAccess, viewerCanEdit, assignableUsers[this._item.remote.remoteName] ?? []), + pullrequest: context, }); } catch (e) { @@ -574,7 +577,7 @@ export class IssueOverviewPanel extends W } /** - * Process permalinks in bodyHTML. Can be overridden by subclasses (e.g., PullRequestOverviewPanel) + * Process code reference links 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. */ @@ -590,22 +593,25 @@ export class IssueOverviewPanel extends W } /** - * Process permalinks in timeline events (comments, reviews, commits). + * Process code reference links in timeline events (comments, reviews, commits). * Updates bodyHTML fields for all events that contain them. */ protected async processTimelineEvents(events: TimelineEvent[]): Promise { 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); + // Create a shallow copy to avoid mutating the original + const processedEvent = { ...event }; + + if (processedEvent.event === EventType.Commented || processedEvent.event === EventType.Reviewed || processedEvent.event === EventType.Committed) { + processedEvent.bodyHTML = await this.processLinksInBodyHtml(processedEvent.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; - })); + if (processedEvent.event === EventType.Reviewed && processedEvent.comments) { + processedEvent.comments = await Promise.all(processedEvent.comments.map(async (comment: IComment) => ({ + ...comment, + bodyHTML: await this.processLinksInBodyHtml(comment.bodyHTML) + }))); } } - return event; + return processedEvent; })); } @@ -631,8 +637,9 @@ export class IssueOverviewPanel extends W }); } - protected _getTimeline(): Promise { - return this._item.getIssueTimelineEvents(); + protected async _getTimeline(): Promise { + const events = await this._item.getIssueTimelineEvents(); + return this.processTimelineEvents(events); } private async changeAssignees(message: IRequestMessage): Promise { @@ -658,7 +665,7 @@ export class IssueOverviewPanel extends W if (allAssignees) { const newAssignees: IAccount[] = allAssignees.map(item => item.user); await this._item.replaceAssignees(newAssignees); - const events = await this.processTimelineEvents(await this._getTimeline()); + const events = await this._getTimeline(); const reply: ChangeAssigneesReply = { assignees: newAssignees, events @@ -725,7 +732,7 @@ export class IssueOverviewPanel extends W const newAssignees = (this._item.assignees ?? []).concat(currentUser); await this._item.replaceAssignees(newAssignees); } - const events = await this.processTimelineEvents(await this._getTimeline()); + const events = await this._getTimeline(); const reply: ChangeAssigneesReply = { assignees: this._item.assignees ?? [], events @@ -743,7 +750,7 @@ export class IssueOverviewPanel extends W const newAssignees = (this._item.assignees ?? []).concat(copilotUser); await this._item.replaceAssignees(newAssignees); } - const events = await this.processTimelineEvents(await this._getTimeline()); + const events = await this._getTimeline(); const reply: ChangeAssigneesReply = { assignees: this._item.assignees ?? [], events @@ -817,6 +824,8 @@ export class IssueOverviewPanel extends W }); } catch (e) { Logger.error(`Open local file failed: ${formatError(e)}`, IssueOverviewPanel.ID); + // Fallback to opening external URL + await vscode.env.openExternal(vscode.Uri.parse(message.args.href)); } } diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index 22a24dfedb..2139f5bb28 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -404,7 +404,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel COPILOT_ACCOUNTS[user.login]); const isCopilotAlreadyReviewer = this._existingReviewers.some(reviewer => !isITeam(reviewer.reviewer) && reviewer.reviewer.login === COPILOT_REVIEWER); const baseContext = this.getInitializeContext(currentUser, pullRequest, await this.processTimelineEvents(timelineEvents), repositoryAccess, viewerCanEdit, users); + // Override bodyHTML with processed version without mutating original pullRequest + baseContext.bodyHTML = processedBodyHTML; this.preLoadInfoNotRequiredForOverview(pullRequest); @@ -663,8 +665,9 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { - return this._item.getTimelineEvents(); + protected override async _getTimeline(): Promise { + const events = await this._item.getTimelineEvents(); + return this.processTimelineEvents(events); } private async openDiff(message: IRequestMessage<{ comment: IComment }>): Promise { @@ -676,7 +679,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel): Promise { + private async openDiffFromLink(message: IRequestMessage<{ file: string; startLine: number; endLine: number; href: string }>): Promise { try { const { file, startLine } = message.args; const fileChanges = await this._item.getFileChangesInfo(); @@ -684,23 +687,26 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel fileChange.fileName === file || fileChange.previousFileName === file, ); - if (!change) { - Logger.warn(`Could not find file ${file} in PR changes`, PullRequestOverviewPanel.ID); + if (change) { + + const pathSegments = file.split('/'); + // GitHub line numbers are 1-indexed, VSCode selection API is 0-indexed + await PullRequestModel.openDiff( + this._folderRepositoryManager, + this._item, + change, + pathSegments[pathSegments.length - 1], + startLine - 1, + ); 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, - ); + Logger.warn(`Could not find file ${file} in PR changes`, PullRequestOverviewPanel.ID); } catch (e) { Logger.error(`Open diff from link failed: ${formatError(e)}`, PullRequestOverviewPanel.ID); } + + // Fallback to opening external URL + await vscode.env.openExternal(vscode.Uri.parse(message.args.href)); } private async openSessionLog(message: IRequestMessage<{ link: SessionLinkInfo }>): Promise { @@ -793,7 +799,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel { try { - const repoName = escapeRegExp(githubRepository.remote.repositoryName); - const repoOwner = escapeRegExp(githubRepository.remote.owner); - const authority = escapeRegExp(githubRepository.remote.gitProtocol.url.authority); - - // Process blob permalinks (exclude already processed links) - const blobPattern = new RegExp( - `]*data-permalink-processed)([^>]*?href="https?:\/\/${authority}\/${repoOwner}\/${repoName}\/blob\/[0-9a-f]{40}\/([^"#]+)#L(\\d+)(?:-L(\\d+))?"[^>]*?)>([^<]*?)<\/a>`, - 'g' - ); + const repoName = githubRepository.remote.repositoryName; + const repoOwner = githubRepository.remote.owner; + const authority = githubRepository.remote.gitProtocol.url.authority; - return await stringReplaceAsync(bodyHTML, blobPattern, async ( - fullMatch: string, - attributes: string, - filePath: string, - startLine: string, - endLine: string | undefined, - linkText: string - ) => { + // Create file existence check callback + const fileExistsCheck = async (filePath: string): Promise => { try { - // Extract the original URL from attributes - const hrefMatch = attributes.match(/href="([^"]+)"/); - const originalUrl = hrefMatch ? hrefMatch[1] : ''; - - // Check if file exists locally const localFileUri = vscode.Uri.joinPath(rootUri, filePath); - try { - const stat = await vscode.workspace.fs.stat(localFileUri); - if (stat.type === vscode.FileType.File) { - // File exists - add data attributes for local handling and "(view on GitHub)" suffix - const endLineValue = endLine || startLine; - return `${linkText} (view on GitHub)`; - } - } catch { - // File doesn't exist - keep original link - } - } catch (error) { - Logger.warn(`Failed to process blob permalink: ${error}`, 'processPermalinks'); + const stat = await vscode.workspace.fs.stat(localFileUri); + return stat.type === vscode.FileType.File; + } catch { + return false; } - return fullMatch; - }); + }; + + return await processPermalinksCore(bodyHTML, repoOwner, repoName, authority, fileExistsCheck); } catch (error) { Logger.error(`Failed to process blob permalinks in HTML: ${error}`, 'processPermalinks'); - return bodyHTML; // Return original HTML if processing fails + return bodyHTML; } } @@ -403,44 +379,14 @@ export async function processDiffLinks( prNumber: number ): Promise { try { - const repoName = escapeRegExp(githubRepository.remote.repositoryName); - const repoOwner = escapeRegExp(githubRepository.remote.owner); - const authority = escapeRegExp(githubRepository.remote.gitProtocol.url.authority); + const repoName = githubRepository.remote.repositoryName; + const repoOwner = githubRepository.remote.owner; + const authority = githubRepository.remote.gitProtocol.url.authority; - const diffPattern = new RegExp( - `]*data-permalink-processed)([^>]*?href="https?:\/\/${authority}\/${repoOwner}\/${repoName}\/pull\/${prNumber}\/(?:files|changes)#diff-([a-f0-9]{64})(?:R(\\d+)(?:-R(\\d+))?)?"[^>]*?)>([^<]*?)<\/a>`, - 'g' - ); - - return await stringReplaceAsync(bodyHTML, diffPattern, async ( - fullMatch: string, - attributes: string, - diffHash: string, - startLine: string | undefined, - endLine: string | undefined, - linkText: string - ) => { - try { - // Extract the original URL from attributes - const hrefMatch = attributes.match(/href="([^"]+)"/); - const originalUrl = hrefMatch ? hrefMatch[1] : ''; - - // Look up filename from hash - const fileName = hashMap[diffHash]; - if (fileName) { - // Hash found - add data attributes for diff handling and "(view on GitHub)" suffix - const startLineValue = startLine || '1'; - const endLineValue = endLine || startLineValue; - return `${linkText} (view on GitHub)`; - } - } catch (error) { - Logger.warn(`Failed to process diff permalink: ${error}`, 'processDiffLinks'); - } - return fullMatch; - }); + return await processDiffLinksCore(bodyHTML, repoOwner, repoName, authority, hashMap, prNumber); } catch (error) { Logger.error(`Failed to process diff permalinks in HTML: ${error}`, 'processDiffLinks'); - return bodyHTML; // Return original HTML if processing fails + return bodyHTML; } } @@ -1996,4 +1942,4 @@ export async function extractRepoFromQuery(folderManager: FolderRepositoryManage } return undefined; -} \ No newline at end of file +} diff --git a/webviews/common/context.tsx b/webviews/common/context.tsx index a34d19f08c..7013d324c9 100644 --- a/webviews/common/context.tsx +++ b/webviews/common/context.tsx @@ -361,11 +361,11 @@ export class PRContext { public openSessionLog = (link: SessionLinkInfo) => this.postMessage({ command: 'pr.open-session-log', args: { link } }); - public openLocalFile = (file: string, startLine: number, endLine: number) => - this.postMessage({ command: 'pr.open-local-file', args: { file, startLine, endLine } }); + public openLocalFile = (file: string, startLine: number, endLine: number, href: string) => + this.postMessage({ command: 'pr.open-local-file', args: { file, startLine, endLine, href } }); - public openDiffFromLink = (file: string, startLine: number, endLine: number) => - this.postMessage({ command: 'pr.open-diff-from-link', args: { file, startLine, endLine } }); + public openDiffFromLink = (file: string, startLine: number, endLine: number, href: string) => + this.postMessage({ command: 'pr.open-diff-from-link', args: { file, startLine, endLine, href } }); public viewCheckLogs = (status: PullRequestCheckStatus) => this.postMessage({ command: 'pr.view-check-logs', args: { status } }); diff --git a/webviews/editorWebview/app.tsx b/webviews/editorWebview/app.tsx index 3d57c887c9..292438a3b6 100644 --- a/webviews/editorWebview/app.tsx +++ b/webviews/editorWebview/app.tsx @@ -7,6 +7,7 @@ import * as debounce from 'debounce'; import React, { useContext, useEffect, useState } from 'react'; import { render } from 'react-dom'; import { Overview } from './overview'; +import { extractCodeReferenceLinkMetadata } from '../../src/common/utils'; import { PullRequest } from '../../src/github/views'; import { COMMENT_TEXTAREA_ID } from '../common/constants'; import PullRequestContext from '../common/context'; @@ -46,20 +47,17 @@ export function Root({ children }) { const target = event.target as HTMLElement; const anchor = target.closest('a[data-local-file]'); if (anchor) { - const file = anchor.getAttribute('data-local-file'); - const startLine = anchor.getAttribute('data-start-line'); - const endLine = anchor.getAttribute('data-end-line'); - const linkType = anchor.getAttribute('data-link-type'); - if (file && startLine && endLine) { - // Swallow the event + const metadata = extractCodeReferenceLinkMetadata(anchor); + if (metadata) { + // Prevent default navigation, handlers will fallback to opening the link externally if they fail event.preventDefault(); event.stopPropagation(); // Open diff view for diff links, local file for blob permalinks - if (linkType === 'diff') { - ctx.openDiffFromLink(file, parseInt(startLine), parseInt(endLine)); + if (metadata.linkType === 'diff') { + ctx.openDiffFromLink(metadata.localFile, metadata.startLine, metadata.endLine, metadata.href); } else { - ctx.openLocalFile(file, parseInt(startLine), parseInt(endLine)); + ctx.openLocalFile(metadata.localFile, metadata.startLine, metadata.endLine, metadata.href); } } } From 678912964e2675d2011251965e083214757b742a Mon Sep 17 00:00:00 2001 From: Daniel Bloom Date: Mon, 16 Mar 2026 17:41:34 -0700 Subject: [PATCH 5/8] process inside getInitializeContext --- src/github/issueOverview.ts | 11 ++++------- src/github/pullRequestOverview.ts | 7 +------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/github/issueOverview.ts b/src/github/issueOverview.ts index 07d6384999..d15cacfe18 100644 --- a/src/github/issueOverview.ts +++ b/src/github/issueOverview.ts @@ -249,7 +249,7 @@ export class IssueOverviewPanel extends W return isInCodespaces(); } - protected getInitializeContext(currentUser: IAccount, issue: IssueModel, timelineEvents: TimelineEvent[], repositoryAccess: RepoAccessAndMergeMethods, viewerCanEdit: boolean, assignableUsers: IAccount[]): Issue { + protected async getInitializeContext(currentUser: IAccount, issue: IssueModel, timelineEvents: TimelineEvent[], repositoryAccess: RepoAccessAndMergeMethods, viewerCanEdit: boolean, assignableUsers: IAccount[]): Promise { const hasWritePermission = repositoryAccess.hasWritePermission; const canEdit = hasWritePermission || viewerCanEdit; const labels = issue.item.labels.map(label => ({ @@ -266,12 +266,12 @@ export class IssueOverviewPanel extends W url: issue.html_url, createdAt: issue.createdAt, body: issue.body, - bodyHTML: issue.bodyHTML, + bodyHTML: await this.processLinksInBodyHtml(issue.bodyHTML), labels: labels, author: issue.author, state: issue.state, stateReason: issue.stateReason, - events: timelineEvents, + events: await this.processTimelineEvents(timelineEvents), continueOnGitHub: this.continueOnGitHub(), canEdit, hasWritePermission, @@ -322,10 +322,7 @@ export class IssueOverviewPanel extends W this.setPanelTitle(this.buildPanelTitle(issueModel.number, issueModel.title)); // Process permalinks in bodyHTML before sending to webview - const processedBodyHTML = await this.processLinksInBodyHtml(issue.bodyHTML); - const context = this.getInitializeContext(currentUser, issue, await this.processTimelineEvents(timelineEvents), repositoryAccess, viewerCanEdit, assignableUsers[this._item.remote.remoteName] ?? []); - // Override bodyHTML with processed version without mutating original issue - context.bodyHTML = processedBodyHTML; + const context = await this.getInitializeContext(currentUser, issue, timelineEvents, repositoryAccess, viewerCanEdit, assignableUsers[this._item.remote.remoteName] ?? []); Logger.debug('pr.initialize', IssueOverviewPanel.ID); this._postMessage({ diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index 2139f5bb28..50f86367b0 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -403,9 +403,6 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel COPILOT_ACCOUNTS[user.login]); const isCopilotAlreadyReviewer = this._existingReviewers.some(reviewer => !isITeam(reviewer.reviewer) && reviewer.reviewer.login === COPILOT_REVIEWER); - const baseContext = this.getInitializeContext(currentUser, pullRequest, await this.processTimelineEvents(timelineEvents), repositoryAccess, viewerCanEdit, users); - // Override bodyHTML with processed version without mutating original pullRequest - baseContext.bodyHTML = processedBodyHTML; + const baseContext = await this.getInitializeContext(currentUser, pullRequest, timelineEvents, repositoryAccess, viewerCanEdit, users); this.preLoadInfoNotRequiredForOverview(pullRequest); From 6e5142daeddb9c24a9bd503242cefd835c1c22d3 Mon Sep 17 00:00:00 2001 From: Daniel Bloom Date: Mon, 16 Mar 2026 18:57:24 -0700 Subject: [PATCH 6/8] extra space --- src/common/utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/common/utils.ts b/src/common/utils.ts index 8eff714cd8..e35a3d6379 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -1093,7 +1093,7 @@ export async function processPermalinks( ) => { try { // Extract the original URL from attributes - const hrefMatch = attributes.match(/ href="([^"]+)"/); + const hrefMatch = attributes.match(/href="([^"]+)"/); const originalUrl = hrefMatch ? hrefMatch[1] : ''; // Check if file exists locally @@ -1155,7 +1155,7 @@ export async function processDiffLinks( ) => { try { // Extract the original URL from attributes - const hrefMatch = attributes.match(/ href="([^"]+)"/); + const hrefMatch = attributes.match(/href="([^"]+)"/); const originalUrl = hrefMatch ? hrefMatch[1] : ''; // Look up filename from hash @@ -1175,4 +1175,4 @@ export async function processDiffLinks( // Return original HTML if processing fails return bodyHTML; } -} \ No newline at end of file +} From 854b78ed243e91c2e431dca323c33a06c2fe343e Mon Sep 17 00:00:00 2001 From: Alex Ross <38270282+alexr00@users.noreply.github.com> Date: Wed, 18 Mar 2026 15:41:39 +0100 Subject: [PATCH 7/8] nits and generated tests --- src/github/issueOverview.ts | 3 +- src/github/pullRequestOverview.ts | 4 +- src/test/common/utils.test.ts | 184 ++++++++++++++++++++++++++++++ webviews/common/context.tsx | 16 ++- 4 files changed, 197 insertions(+), 10 deletions(-) diff --git a/src/github/issueOverview.ts b/src/github/issueOverview.ts index d15cacfe18..17f8107ad4 100644 --- a/src/github/issueOverview.ts +++ b/src/github/issueOverview.ts @@ -814,8 +814,7 @@ export class IssueOverviewPanel extends W 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, { + await vscode.window.showTextDocument(fileUri, { selection, viewColumn: vscode.ViewColumn.One }); diff --git a/src/github/pullRequestOverview.ts b/src/github/pullRequestOverview.ts index 50f86367b0..d1863282af 100644 --- a/src/github/pullRequestOverview.ts +++ b/src/github/pullRequestOverview.ts @@ -6,7 +6,7 @@ import * as crypto from 'crypto'; import * as vscode from 'vscode'; -import { OpenCommitChangesArgs } from '../../common/views'; +import { OpenCommitChangesArgs, OpenLocalFileArgs } from '../../common/views'; import { openPullRequestOnGitHub } from '../commands'; import { getCopilotApi } from './copilotApi'; import { SessionIdForPr } from './copilotRemoteAgent'; @@ -674,7 +674,7 @@ export class PullRequestOverviewPanel extends IssueOverviewPanel): Promise { + private async openDiffFromLink(message: IRequestMessage): Promise { try { const { file, startLine } = message.args; const fileChanges = await this._item.getFileChangesInfo(); diff --git a/src/test/common/utils.test.ts b/src/test/common/utils.test.ts index aabe5b77d3..2b8e5585b2 100644 --- a/src/test/common/utils.test.ts +++ b/src/test/common/utils.test.ts @@ -51,4 +51,188 @@ describe('utils', () => { assert.strictEqual(utils.formatError(error), 'Cannot push to this repo'); }); }); + + describe('processPermalinks', () => { + const repoOwner = 'microsoft'; + const repoName = 'vscode'; + const authority = 'github.com'; + const sha = 'a'.repeat(40); + + function makePermalink(filePath: string, startLine: number, endLine?: number): string { + const lineRef = endLine ? `#L${startLine}-L${endLine}` : `#L${startLine}`; + return `link text`; + } + + it('should add data attributes when file exists locally', async () => { + const html = makePermalink('src/file.ts', 10); + const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => true); + + assert(result.includes('data-local-file="src/file.ts"')); + assert(result.includes('data-start-line="10"')); + assert(result.includes('data-end-line="10"')); + assert(result.includes('data-link-type="blob"')); + assert(result.includes('data-permalink-processed="true"')); + assert(result.includes('view on GitHub')); + }); + + it('should set end line when range is specified', async () => { + const html = makePermalink('src/file.ts', 10, 20); + const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => true); + + assert(result.includes('data-start-line="10"')); + assert(result.includes('data-end-line="20"')); + }); + + it('should not modify links when file does not exist locally', async () => { + const html = makePermalink('src/file.ts', 10); + const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => false); + + assert.strictEqual(result, html); + }); + + it('should not modify non-permalink links', async () => { + const html = 'example'; + const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => true); + + assert.strictEqual(result, html); + }); + + it('should not modify links to a different repo', async () => { + const html = `link`; + const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => true); + + assert.strictEqual(result, html); + }); + + it('should skip already processed links', async () => { + const html = `link`; + const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => true); + + assert.strictEqual(result, html); + }); + + it('should process multiple links independently', async () => { + const html = makePermalink('src/exists.ts', 1) + makePermalink('src/missing.ts', 2); + const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async (path) => path === 'src/exists.ts'); + + assert(result.includes('data-local-file="src/exists.ts"')); + assert(!result.includes('data-local-file="src/missing.ts"')); + }); + + it('should return original HTML when fileExistsCheck throws', async () => { + const html = makePermalink('src/file.ts', 10); + const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => { throw new Error('fail'); }); + + assert.strictEqual(result, html); + }); + + it('should handle links without surrounding text', async () => { + const html = makePermalink('src/file.ts', 5); + const result = await utils.processPermalinks(html, repoOwner, repoName, authority, async () => true); + + assert(result.includes('link text')); + assert(result.includes('data-local-file="src/file.ts"')); + }); + }); + + describe('processDiffLinks', () => { + const repoOwner = 'microsoft'; + const repoName = 'vscode'; + const authority = 'github.com'; + const prNumber = 123; + const diffHash = 'a'.repeat(64); + + function makeDiffLink(hash: string, startLine?: number, endLine?: number, variant: 'files' | 'changes' = 'files'): string { + let fragment = `diff-${hash}`; + if (startLine !== undefined) { + fragment += `R${startLine}`; + if (endLine !== undefined) { + fragment += `-R${endLine}`; + } + } + return `link text`; + } + + it('should add data attributes when hash maps to a file', async () => { + const hashMap: Record = { [diffHash]: 'src/file.ts' }; + const html = makeDiffLink(diffHash, 10); + const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber); + + assert(result.includes('data-local-file="src/file.ts"')); + assert(result.includes('data-start-line="10"')); + assert(result.includes('data-end-line="10"')); + assert(result.includes('data-link-type="diff"')); + assert(result.includes('data-permalink-processed="true"')); + assert(result.includes('view on GitHub')); + }); + + it('should set end line when range is specified', async () => { + const hashMap: Record = { [diffHash]: 'src/file.ts' }; + const html = makeDiffLink(diffHash, 10, 20); + const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber); + + assert(result.includes('data-start-line="10"')); + assert(result.includes('data-end-line="20"')); + }); + + it('should default start line to 1 when no line is specified', async () => { + const hashMap: Record = { [diffHash]: 'src/file.ts' }; + const html = makeDiffLink(diffHash); + const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber); + + assert(result.includes('data-start-line="1"')); + assert(result.includes('data-end-line="1"')); + }); + + it('should not modify links when hash is not in the map', async () => { + const hashMap: Record = {}; + const html = makeDiffLink(diffHash, 10); + const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber); + + assert.strictEqual(result, html); + }); + + it('should not modify non-diff links', async () => { + const hashMap: Record = { [diffHash]: 'src/file.ts' }; + const html = 'example'; + const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber); + + assert.strictEqual(result, html); + }); + + it('should not modify links to a different repo', async () => { + const hashMap: Record = { [diffHash]: 'src/file.ts' }; + const html = `link`; + const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber); + + assert.strictEqual(result, html); + }); + + it('should skip already processed links', async () => { + const hashMap: Record = { [diffHash]: 'src/file.ts' }; + const html = `link`; + const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber); + + assert.strictEqual(result, html); + }); + + it('should match links using changes variant', async () => { + const hashMap: Record = { [diffHash]: 'src/file.ts' }; + const html = makeDiffLink(diffHash, 5, undefined, 'changes'); + const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber); + + assert(result.includes('data-local-file="src/file.ts"')); + assert(result.includes('data-start-line="5"')); + }); + + it('should process multiple links independently', async () => { + const otherHash = 'b'.repeat(64); + const hashMap: Record = { [diffHash]: 'src/found.ts' }; + const html = makeDiffLink(diffHash, 1) + makeDiffLink(otherHash, 2); + const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, hashMap, prNumber); + + assert(result.includes('data-local-file="src/found.ts"')); + assert(!result.includes('data-local-file="src/other.ts"')); + }); + }); }); diff --git a/webviews/common/context.tsx b/webviews/common/context.tsx index 7013d324c9..9f4d8dd948 100644 --- a/webviews/common/context.tsx +++ b/webviews/common/context.tsx @@ -7,7 +7,7 @@ import { createContext } from 'react'; import { getState, setState, updateState } from './cache'; import { COMMENT_TEXTAREA_ID } from './constants'; import { getMessageHandler, MessageHandler } from './message'; -import { CloseResult, DescriptionResult, OpenCommitChangesArgs } from '../../common/views'; +import { CloseResult, DescriptionResult, OpenCommitChangesArgs, OpenLocalFileArgs } from '../../common/views'; import { IComment } from '../../src/common/comment'; import { EventType, ReviewEvent, SessionLinkInfo, TimelineEvent } from '../../src/common/timelineEvent'; import { IProjectItem, MergeMethod, PullRequestCheckStatus, ReadyForReview } from '../../src/github/interface'; @@ -361,12 +361,16 @@ export class PRContext { public openSessionLog = (link: SessionLinkInfo) => this.postMessage({ command: 'pr.open-session-log', args: { link } }); - public openLocalFile = (file: string, startLine: number, endLine: number, href: string) => - this.postMessage({ command: 'pr.open-local-file', args: { file, startLine, endLine, href } }); - - public openDiffFromLink = (file: string, startLine: number, endLine: number, href: string) => - this.postMessage({ command: 'pr.open-diff-from-link', args: { file, startLine, endLine, href } }); + public openLocalFile = (file: string, startLine: number, endLine: number, href: string) => { + const args: OpenLocalFileArgs = { file, startLine, endLine, href }; + this.postMessage({ command: 'pr.open-local-file', args }); + }; + public openDiffFromLink = (file: string, startLine: number, endLine: number, href: string) => { + const args: OpenLocalFileArgs = { file, startLine, endLine, href }; + this.postMessage({ command: 'pr.open-diff-from-link', args }); + }; + public viewCheckLogs = (status: PullRequestCheckStatus) => this.postMessage({ command: 'pr.view-check-logs', args: { status } }); public openCommitChanges = async (commitSha: string) => { From ff286cb80d202ece4c01688b7cb8e556530ac17f Mon Sep 17 00:00:00 2001 From: Daniel Bloom Date: Wed, 18 Mar 2026 08:56:57 -0700 Subject: [PATCH 8/8] fix: remove repoOwner so it works across forks --- src/common/utils.ts | 7 +++---- src/github/utils.ts | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/common/utils.ts b/src/common/utils.ts index e35a3d6379..4df10e850f 100644 --- a/src/common/utils.ts +++ b/src/common/utils.ts @@ -1057,9 +1057,9 @@ export function extractCodeReferenceLinkMetadata(anchor: Element): CodeReference * Process GitHub blob permalinks in HTML and add data attributes for local file handling. * Finds blob permalinks (e.g., /blob/[sha]/file.ts#L10), checks if files exist locally, * and adds data attributes to enable clicking to open local files. + * Supports links from any repository owner to work across forks. * * @param bodyHTML - The HTML content to process - * @param repoOwner - GitHub repository owner * @param repoName - GitHub repository name * @param authority - Git protocol URL authority (e.g., 'github.com') * @param fileExistsCheck - Async function that checks if a file exists locally given its relative path @@ -1067,19 +1067,18 @@ export function extractCodeReferenceLinkMetadata(anchor: Element): CodeReference */ export async function processPermalinks( bodyHTML: string, - repoOwner: string, repoName: string, authority: string, fileExistsCheck: (filePath: string) => Promise ): Promise { try { const escapedRepoName = escapeRegExp(repoName); - const escapedRepoOwner = escapeRegExp(repoOwner); const escapedAuthority = escapeRegExp(authority); // Process blob permalinks (exclude already processed links) + // Allow any owner to support links across forks const blobPattern = new RegExp( - `]*data-permalink-processed)([^>]*?href="https?:\/\/${escapedAuthority}\/${escapedRepoOwner}\/${escapedRepoName}\/blob\/[0-9a-f]{40}\/(?[^"#]+)#L(?\\d+)(?:-L(?\\d+))?"[^>]*?)>(?[^<]*?)<\/a>`, + `]*data-permalink-processed)([^>]*?href="https?:\/\/${escapedAuthority}\/[^\/]+\/${escapedRepoName}\/blob\/[0-9a-f]{40}\/(?[^"#]+)#L(?\\d+)(?:-L(?\\d+))?"[^>]*?)>(?[^<]*?)<\/a>`, 'g' ); diff --git a/src/github/utils.ts b/src/github/utils.ts index 4cbda51651..62035f7119 100644 --- a/src/github/utils.ts +++ b/src/github/utils.ts @@ -346,7 +346,6 @@ export async function processPermalinks( ): Promise { try { const repoName = githubRepository.remote.repositoryName; - const repoOwner = githubRepository.remote.owner; const authority = githubRepository.remote.gitProtocol.url.authority; // Create file existence check callback @@ -360,7 +359,7 @@ export async function processPermalinks( } }; - return await processPermalinksCore(bodyHTML, repoOwner, repoName, authority, fileExistsCheck); + return await processPermalinksCore(bodyHTML, repoName, authority, fileExistsCheck); } catch (error) { Logger.error(`Failed to process blob permalinks in HTML: ${error}`, 'processPermalinks'); return bodyHTML;