Skip to content

Commit c45e36d

Browse files
alexr00alexr00
authored
Coding Agent: Unclear which PR the number indicator refers to (#7203)
Fixes #7169
1 parent 72674ae commit c45e36d

File tree

6 files changed

+73
-25
lines changed

6 files changed

+73
-25
lines changed

‎src/common/uri.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,20 @@ export function createPRNodeIdentifier(pullRequest: PullRequestModel | { remote:
426426
return identifier;
427427
}
428428

429+
export function parsePRNodeIdentifier(identifier: string): { remote: string, prNumber: number } | undefined {
430+
const lastColon = identifier.lastIndexOf(':');
431+
if (lastColon === -1) {
432+
return undefined;
433+
}
434+
const remote = identifier.substring(0, lastColon);
435+
const prNumberStr = identifier.substring(lastColon + 1);
436+
const prNumber = Number(prNumberStr);
437+
if (!remote || isNaN(prNumber) || prNumber <= 0) {
438+
return undefined;
439+
}
440+
return { remote, prNumber };
441+
}
442+
429443
export function createPRNodeUri(
430444
pullRequest: PullRequestModel | { remote: string, prNumber: number } | string
431445
): vscode.Uri {

‎src/github/copilotPrWatcher.ts‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { COPILOT_LOGINS, copilotEventToStatus, CopilotPRStatus } from '../common
88
import { Disposable } from '../common/lifecycle';
99
import { PR_SETTINGS_NAMESPACE, QUERIES } from '../common/settingKeys';
1010
import { FolderRepositoryManager } from './folderRepositoryManager';
11+
import { PullRequestModel } from './pullRequestModel';
1112
import { RepositoriesManager } from './repositoriesManager';
1213
import { variableSubstitution } from './utils';
1314

@@ -18,11 +19,11 @@ export function isCopilotQuery(query: string): boolean {
1819

1920
export class CopilotStateModel extends Disposable {
2021
private _isInitialized = false;
21-
private readonly _states: Map<string, CopilotPRStatus> = new Map();
22+
private readonly _states: Map<string, { item: PullRequestModel, status: CopilotPRStatus }> = new Map();
2223
private readonly _showNotification: Set<string> = new Set();
2324
private readonly _onDidChangeStates = this._register(new vscode.EventEmitter<void>());
2425
readonly onDidChangeStates = this._onDidChangeStates.event;
25-
private readonly _onDidChangeNotifications = this._register(new vscode.EventEmitter<void>());
26+
private readonly _onDidChangeNotifications = this._register(new vscode.EventEmitter<PullRequestModel[]>());
2627
readonly onDidChangeNotifications = this._onDidChangeNotifications.event;
2728

2829
makeKey(owner: string, repo: string, prNumber: number): string {
@@ -37,39 +38,41 @@ export class CopilotStateModel extends Disposable {
3738
if (this._states.has(key)) {
3839
this._states.delete(key);
3940
if (this._showNotification.has(key)) {
41+
const item = this._states.get(key)!;
4042
this._showNotification.delete(key);
41-
this._onDidChangeNotifications.fire();
43+
this._onDidChangeNotifications.fire([item.item]);
4244
}
4345
this._onDidChangeStates.fire();
4446
}
4547
}
4648

47-
set(owner: string, repo: string, prNumber: number, status: CopilotPRStatus): void {
48-
const key = this.makeKey(owner, repo, prNumber);
49+
set(pullRequestModel: PullRequestModel, status: CopilotPRStatus): void {
50+
const key = this.makeKey(pullRequestModel.remote.owner, pullRequestModel.remote.repositoryName, pullRequestModel.number);
4951
const currentStatus = this._states.get(key);
50-
if (currentStatus === status) {
52+
if (currentStatus?.status === status) {
5153
return;
5254
}
53-
this._states.set(key, status);
55+
this._states.set(key, { item: pullRequestModel, status });
5456
if (this._isInitialized) {
5557
this._showNotification.add(key);
56-
this._onDidChangeNotifications.fire();
58+
this._onDidChangeNotifications.fire(pullRequestModel ? [pullRequestModel] : []);
5759
}
5860
this._onDidChangeStates.fire();
5961
}
6062

6163
get(owner: string, repo: string, prNumber: number): CopilotPRStatus {
6264
const key = this.makeKey(owner, repo, prNumber);
63-
return this._states.get(key) ?? CopilotPRStatus.None;
65+
return this._states.get(key)?.status ?? CopilotPRStatus.None;
6466
}
6567

6668
keys(): string[] {
6769
return Array.from(this._states.keys());
6870
}
6971

7072
clearNotifications(): void {
73+
const items = Array.from(this._showNotification).map(key => this._states.get(key)?.item).filter((item): item is PullRequestModel => !!item);
7174
this._showNotification.clear();
72-
this._onDidChangeNotifications.fire();
75+
this._onDidChangeNotifications.fire(items);
7376
}
7477

7578
get notifications(): ReadonlySet<string> {
@@ -158,7 +161,7 @@ export class CopilotPRWatcher extends Disposable {
158161
prNumber: pr.number,
159162
status: latestEvent
160163
});
161-
this._model.set(pr.remote.owner, pr.remote.repositoryName, pr.number, latestEvent);
164+
this._model.set(pr, latestEvent);
162165
}
163166
}
164167

‎src/github/copilotRemoteAgent.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export class CopilotRemoteAgentManager extends Disposable {
5454
private readonly _stateModel: CopilotStateModel;
5555
private readonly _onDidChangeStates = this._register(new vscode.EventEmitter<void>());
5656
readonly onDidChangeStates = this._onDidChangeStates.event;
57-
private readonly _onDidChangeNotifications = this._register(new vscode.EventEmitter<void>());
57+
private readonly _onDidChangeNotifications = this._register(new vscode.EventEmitter<PullRequestModel[]>());
5858
readonly onDidChangeNotifications = this._onDidChangeNotifications.event;
5959
private readonly _onDidCreatePullRequest = this._register(new vscode.EventEmitter<number>());
6060
readonly onDidCreatePullRequest = this._onDidCreatePullRequest.event;
@@ -70,7 +70,7 @@ export class CopilotRemoteAgentManager extends Disposable {
7070
this._stateModel = new CopilotStateModel();
7171
this._register(new CopilotPRWatcher(this.repositoriesManager, this._stateModel));
7272
this._register(this._stateModel.onDidChangeStates(() => this._onDidChangeStates.fire()));
73-
this._register(this._stateModel.onDidChangeNotifications(() => this._onDidChangeNotifications.fire()));
73+
this._register(this._stateModel.onDidChangeNotifications(items => this._onDidChangeNotifications.fire(items)));
7474

7575
this._register(this.repositoriesManager.onDidChangeFolderRepositories((event) => {
7676
if (event.added) {
@@ -627,7 +627,12 @@ export class CopilotRemoteAgentManager extends Disposable {
627627
this._stateModel.clearNotifications();
628628
}
629629

630-
get notifications(): ReadonlySet<string> {
631-
return this._stateModel.notifications;
630+
get notificationsCount(): number {
631+
return this._stateModel.notifications.size;
632+
}
633+
634+
hasNotification(owner: string, repo: string, pullRequestNumber: number): boolean {
635+
const key = this._stateModel.makeKey(owner, repo, pullRequestNumber);
636+
return this._stateModel.notifications.has(key);
632637
}
633638
}

‎src/view/prStatusDecorationProvider.ts‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55

66
import * as vscode from 'vscode';
77
import { Disposable } from '../common/lifecycle';
8-
import { COPILOT_QUERY, createPRNodeUri, fromPRNodeUri, Schemes } from '../common/uri';
8+
import { Protocol } from '../common/protocol';
9+
import { COPILOT_QUERY, createPRNodeUri, fromPRNodeUri, parsePRNodeIdentifier, PRNodeUriParams, Schemes } from '../common/uri';
910
import { CopilotRemoteAgentManager } from '../github/copilotRemoteAgent';
1011
import { getStatusDecoration } from '../github/markdownUtils';
1112
import { PrsTreeModel } from './prsTreeModel';
@@ -26,8 +27,12 @@ export class PRStatusDecorationProvider extends Disposable implements vscode.Fil
2627
})
2728
);
2829

29-
this._register(this._copilotManager.onDidChangeNotifications(() => {
30-
this._onDidChangeFileDecorations.fire(COPILOT_QUERY);
30+
this._register(this._copilotManager.onDidChangeNotifications(items => {
31+
const uris = [COPILOT_QUERY];
32+
for (const item of items) {
33+
uris.push(createPRNodeUri(item));
34+
}
35+
this._onDidChangeFileDecorations.fire(uris);
3136
}));
3237
}
3338

@@ -46,19 +51,40 @@ export class PRStatusDecorationProvider extends Disposable implements vscode.Fil
4651
if (!params) {
4752
return;
4853
}
54+
55+
const copilotDecoration = this._getCopilotDecoration(params);
56+
if (copilotDecoration) {
57+
return copilotDecoration;
58+
}
59+
4960
const status = this._prsTreeModel.cachedPRStatus(params.prIdentifier);
5061
if (!status) {
5162
return;
5263
}
5364

54-
return getStatusDecoration(status.status) as vscode.FileDecoration;
65+
const decoration = getStatusDecoration(status.status) as vscode.FileDecoration;
66+
return decoration;
67+
}
68+
69+
private _getCopilotDecoration(params: PRNodeUriParams): vscode.FileDecoration | undefined {
70+
const idParts = parsePRNodeIdentifier(params.prIdentifier);
71+
if (!idParts) {
72+
return;
73+
}
74+
const protocol = new Protocol(idParts.remote);
75+
if (this._copilotManager.hasNotification(protocol.owner, protocol.repositoryName, idParts.prNumber)) {
76+
return {
77+
badge: new vscode.ThemeIcon('copilot') as any,
78+
color: new vscode.ThemeColor('pullRequests.notification')
79+
};
80+
}
5581
}
5682

5783
private _queryDecoration(uri: vscode.Uri): vscode.ProviderResult<vscode.FileDecoration> {
5884
if (uri.path === 'copilot') {
59-
if (this._copilotManager.notifications.size > 0) {
85+
if (this._copilotManager.notificationsCount > 0) {
6086
return {
61-
tooltip: vscode.l10n.t('Coding agent has made changes', this._copilotManager.notifications.size),
87+
tooltip: vscode.l10n.t('Coding agent has made changes', this._copilotManager.notificationsCount),
6288
badge: new vscode.ThemeIcon('copilot') as any,
6389
color: new vscode.ThemeColor('pullRequests.notification'),
6490
};

‎src/view/prsTreeDataProvider.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
9393
});
9494

9595
this._register(this._copilotManager.onDidChangeStates(() => {
96-
if (this._copilotManager.notifications.size > 0) {
96+
if (this._copilotManager.notificationsCount > 0) {
9797
this._view.badge = {
98-
tooltip: this._copilotManager.notifications.size === 1 ? vscode.l10n.t('Coding agent has 1 pull request to view') : vscode.l10n.t('Coding agent has {0} pull requests to view', this._copilotManager.notifications.size),
99-
value: this._copilotManager.notifications.size
98+
tooltip: this._copilotManager.notificationsCount === 1 ? vscode.l10n.t('Coding agent has 1 pull request to view') : vscode.l10n.t('Coding agent has {0} pull requests to view', this._copilotManager.notificationsCount),
99+
value: this._copilotManager.notificationsCount
100100
};
101101
this.refresh();
102102
} else {

‎src/view/treeNodes/categoryNode.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ export class CategoryTreeNode extends TreeNode implements vscode.TreeItem {
146146

147147
this.prs = new Map();
148148

149-
const hasCopilotChanges = _categoryQuery && isCopilotQuery(_categoryQuery) && this._copilotManager.notifications.size > 0;
149+
const hasCopilotChanges = _categoryQuery && isCopilotQuery(_categoryQuery) && this._copilotManager.notificationsCount > 0;
150150

151151
switch (this.type) {
152152
case PRType.All:

0 commit comments

Comments
 (0)