-
Notifications
You must be signed in to change notification settings - Fork 624
Fix commit links in PR description to open multi diff editor instead of browser #7217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: alexr00 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot, some changes:
src/commands.ts
Outdated
function isSourceControl(x: any): x is any { | ||
return !!x?.rootUri; | ||
} | ||
|
||
context.subscriptions.push( | ||
vscode.commands.registerCommand( | ||
'pr.create', | ||
async (args?: { repoPath: string; compareBranch: string } | Repository) => { | ||
async (args?: { repoPath: string; compareBranch: string } | any) => { | ||
// The arguments this is called with are either from the SCM view, or manually passed. | ||
if (isSourceControl(args)) { | ||
(await chooseReviewManager(args.rootUri.fsPath))?.createPullRequest(); | ||
} else { | ||
(await chooseReviewManager(args?.repoPath))?.createPullRequest(args?.compareBranch); | ||
(await chooseReviewManager((args as any)?.repoPath))?.createPullRequest((args as any)?.compareBranch); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes aren't related to the requested feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed by reverting the unrelated changes to the pr.create
command. Commit 81b141c.
src/commands.ts
Outdated
@@ -654,6 +653,79 @@ export function registerCommands( | |||
}), | |||
); | |||
|
|||
context.subscriptions.push( | |||
vscode.commands.registerCommand('pr.openCommitChanges', async (commitShaOrUrl: string, folderManager?: FolderRepositoryManager) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of registering a new command, we should have a static method on the PullRequestModel
which does this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented as a static method PullRequestModel.openCommitChanges()
instead of a registered command. The command now just calls this static method. Commit 81b141c.
webviews/components/timeline.tsx
Outdated
@@ -300,7 +309,12 @@ function AddReviewSummaryComment() { | |||
const CommentEventView = (event: CommentEvent) => <CommentView headerInEditMode comment={event} />; | |||
|
|||
const MergedEventView = (event: MergedEvent) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leave the MergedEventView
with the old behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restored the original MergedEventView
behavior with href
navigation instead of the custom click handler. Commit 81b141c.
…naming Co-authored-by: alexr00 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/commands.ts
Outdated
|
||
context.subscriptions.push( | ||
vscode.commands.registerCommand('pr.openCommitChanges', async (commitSha: string, folderManager?: FolderRepositoryManager) => { | ||
if (!folderManager) { | ||
folderManager = reposManager.folderManagers[0]; | ||
} | ||
if (!folderManager) { | ||
vscode.window.showErrorMessage(vscode.l10n.t('No repository found')); | ||
return; | ||
} | ||
return PullRequestModel.openCommitChanges(folderManager, commitSha); | ||
}), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't register a new command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the command registration. The static method is now called directly from the pullRequestOverview. Commit 1f793fd.
src/github/pullRequestOverview.ts
Outdated
private async openCommitChanges(message: IRequestMessage<{ commitSha: string }>): Promise<void> { | ||
try { | ||
const { commitSha } = message.args; | ||
await vscode.commands.executeCommand('pr.openCommitChanges', commitSha, this._folderRepositoryManager); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call the static method in PullRequestModel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to call PullRequestModel.openCommitChanges()
static method directly instead of using command execution. Commit 1f793fd.
Co-authored-by: alexr00 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
webviews/components/timeline.tsx
Outdated
const context = useContext(PullRequestContext); | ||
|
||
const handleCommitClick = (e: React.MouseEvent) => { | ||
e.preventDefault(); | ||
context.openCommitChanges(event.sha); | ||
}; | ||
|
||
return ( | ||
<div className="comment-container commit"> | ||
<div className="commit-message"> | ||
{commitIcon} | ||
{nbsp} | ||
<div className="avatar-container"> | ||
<Avatar for={event.author} /> | ||
</div> | ||
<div className="message-container"> | ||
<a className="message" onClick={handleCommitClick} style={{ cursor: 'pointer' }} title={event.htmlUrl}> | ||
{event.message.substr(0, event.message.indexOf('\n') > -1 ? event.message.indexOf('\n') : event.message.length)} | ||
</a> | ||
</div> | ||
</div> | ||
<div className="message-container"> | ||
<a className="message" href={event.htmlUrl} title={event.htmlUrl}> | ||
{event.message.substr(0, event.message.indexOf('\n') > -1 ? event.message.indexOf('\n') : event.message.length)} | ||
<div className="timeline-detail"> | ||
<a className="sha" onClick={handleCommitClick} style={{ cursor: 'pointer' }} title={event.htmlUrl}> | ||
{event.sha.slice(0, 7)} | ||
</a> | ||
<Timestamp date={event.committedDate} /> | ||
</div> | ||
</div> | ||
<div className="timeline-detail"> | ||
<a className="sha" href={event.htmlUrl} title={event.htmlUrl}> | ||
{event.sha.slice(0, 7)} | ||
</a> | ||
<Timestamp date={event.committedDate} /> | ||
</div> | ||
</div> | ||
); | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the PR is not checked out, just use the original href.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented conditional behavior: commit links now use href to open in browser when PR is not checked out, and use custom handler to open in VS Code when PR is checked out. Commit dba37b2.
Co-authored-by: alexr00 <[email protected]>
This PR addresses an issue where clicking commit links in PR descriptions would open GitHub in the browser instead of staying within VS Code to show the changes locally.
Problem
When users clicked on commit links in the PR timeline (both regular commits and merge commits), they were taken to GitHub.com in their browser. This interrupted the workflow and didn't provide the familiar VS Code diff viewing experience.
Solution
The implementation intercepts commit link clicks and opens the changes locally using VS Code's multi diff editor:
Backend: Added new
pr.openCommitChanges
command that:repository.diffBetween()
to get file changes between commit and its parentvscode.changes
commandFrontend: Modified timeline components to:
href
navigation withonClick
handlers for commit linksCommitEvent
andMergedEvent
timeline entriesCommunication: Added seamless message passing between webview and extension
Key Features
Testing
Users can now click commit links in PR descriptions and immediately see the changes locally in VS Code's familiar multi diff interface, improving the code review workflow.
Fixes #7183.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.