Skip to content

Commit 807662d

Browse files
roblourensroblourens
committed
Update Agent Host branch picker filtering
(Written by Copilot)
1 parent 8f76635 commit 807662d

5 files changed

Lines changed: 206 additions & 87 deletions

File tree

‎src/vs/platform/actionWidget/browser/actionList.ts‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export const previewSelectedActionCommand = 'previewSelectedCodeAction';
4040
export interface IActionListDelegate<T> {
4141
onHide(didCancel?: boolean): void;
4242
onSelect(action: T, preview?: boolean): void;
43+
onFilter?(filter: string, cancellationToken: CancellationToken): Promise<readonly IActionListItem<T>[]>;
4344
onHover?(action: T, cancellationToken: CancellationToken): Promise<{ canPreview: boolean } | void>;
4445
onFocus?(action: T | undefined): void;
4546
}
@@ -488,6 +489,7 @@ export class ActionListWidget<T> extends Disposable {
488489
private _hasLaidOut = false;
489490
private readonly _filterInput: HTMLInputElement | undefined;
490491
private readonly _filterContainer: HTMLElement | undefined;
492+
private readonly _filterCts = this._register(new MutableDisposable<CancellationTokenSource>());
491493

492494
private readonly _onDidRequestLayout = this._register(new Emitter<void>());
493495

@@ -622,7 +624,7 @@ export class ActionListWidget<T> extends Disposable {
622624

623625
this._register(dom.addDisposableListener(this._filterInput, 'input', () => {
624626
this._filterText = this._filterInput!.value;
625-
this._applyFilter();
627+
this._applyOrUpdateFilter();
626628
}));
627629
}
628630

@@ -659,7 +661,7 @@ export class ActionListWidget<T> extends Disposable {
659661
this._filterInput.focus();
660662
this._filterInput.value = e.key;
661663
this._filterText = e.key;
662-
this._applyFilter();
664+
this._applyOrUpdateFilter();
663665
e.preventDefault();
664666
e.stopPropagation();
665667
}
@@ -677,6 +679,25 @@ export class ActionListWidget<T> extends Disposable {
677679
this._applyFilter();
678680
}
679681

682+
private _applyOrUpdateFilter(): void {
683+
if (!this._delegate.onFilter) {
684+
this._applyFilter();
685+
return;
686+
}
687+
688+
const filterText = this._filterText;
689+
this._filterCts.value?.cancel();
690+
const cts = new CancellationTokenSource();
691+
this._filterCts.value = cts;
692+
this._delegate.onFilter(filterText, cts.token).then(items => {
693+
if (cts.token.isCancellationRequested) {
694+
return;
695+
}
696+
this._allMenuItems = [...items];
697+
this._applyFilter();
698+
}).catch(() => { /* best-effort */ });
699+
}
700+
680701
private _applyFilter(): void {
681702
const filterLower = this._filterText.toLowerCase();
682703
const isFiltering = filterLower.length > 0;
@@ -841,6 +862,8 @@ export class ActionListWidget<T> extends Disposable {
841862
hide(didCancel?: boolean): void {
842863
this._delegate.onHide(didCancel);
843864
this.cts.cancel();
865+
this._filterCts.value?.cancel();
866+
this._filterCts.clear();
844867
this._hover.clear();
845868
this._hideSubmenu();
846869
}
@@ -849,7 +872,7 @@ export class ActionListWidget<T> extends Disposable {
849872
if (this._filterInput && this._filterText) {
850873
this._filterInput.value = '';
851874
this._filterText = '';
852-
this._applyFilter();
875+
this._applyOrUpdateFilter();
853876
return true;
854877
}
855878
return false;

‎src/vs/platform/actionWidget/test/browser/actionList.test.ts‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import assert from 'assert';
7+
import { DeferredPromise, timeout } from '../../../../base/common/async.js';
8+
import { CancellationToken } from '../../../../base/common/cancellation.js';
9+
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js';
10+
import { runWithFakedTimers } from '../../../../base/test/common/timeTravelScheduler.js';
11+
import { IHoverService } from '../../../hover/browser/hover.js';
12+
import { NullHoverService } from '../../../hover/test/browser/nullHoverService.js';
13+
import { TestInstantiationService } from '../../../instantiation/test/common/instantiationServiceMock.js';
14+
import { MockKeybindingService } from '../../../keybinding/test/common/mockKeybindingService.js';
15+
import { IKeybindingService } from '../../../keybinding/common/keybinding.js';
16+
import { IOpenerService } from '../../../opener/common/opener.js';
17+
import { NullOpenerService } from '../../../opener/test/common/nullOpenerService.js';
18+
import { ActionListItemKind, ActionListWidget, IActionListItem } from '../../browser/actionList.js';
19+
20+
interface ITestActionItem {
21+
readonly id: string;
22+
}
23+
24+
function action(id: string): IActionListItem<ITestActionItem> {
25+
return { kind: ActionListItemKind.Action, label: id, item: { id } };
26+
}
27+
28+
function createActionListWidget(disposables: ReturnType<typeof ensureNoDisposablesAreLeakedInTestSuite>, options: {
29+
readonly onFilter: (filter: string, cancellationToken: CancellationToken) => Promise<readonly IActionListItem<ITestActionItem>[]>;
30+
}): ActionListWidget<ITestActionItem> {
31+
const instantiationService = disposables.add(new TestInstantiationService());
32+
instantiationService.set(IKeybindingService, new MockKeybindingService());
33+
instantiationService.set(IHoverService, NullHoverService);
34+
instantiationService.set(IOpenerService, NullOpenerService);
35+
36+
const widget = disposables.add(instantiationService.createInstance(
37+
ActionListWidget<ITestActionItem>,
38+
'testActionList',
39+
false,
40+
[action('initial')],
41+
{
42+
onHide: () => { },
43+
onSelect: () => { },
44+
onFilter: options.onFilter,
45+
},
46+
undefined,
47+
{ showFilter: true },
48+
));
49+
50+
if (widget.filterContainer) {
51+
document.body.appendChild(widget.filterContainer);
52+
disposables.add({ dispose: () => widget.filterContainer?.remove() });
53+
}
54+
document.body.appendChild(widget.domNode);
55+
disposables.add({ dispose: () => widget.domNode.remove() });
56+
widget.layout(200, 200);
57+
58+
return widget;
59+
}
60+
61+
function typeFilter(widget: ActionListWidget<ITestActionItem>, value: string): void {
62+
assert.ok(widget.filterInput);
63+
widget.filterInput.value = value;
64+
widget.filterInput.dispatchEvent(new Event('input'));
65+
}
66+
67+
suite('ActionListWidget', () => {
68+
const disposables = ensureNoDisposablesAreLeakedInTestSuite();
69+
70+
test('runs dynamic filter updates immediately', () => runWithFakedTimers({ useFakeTimers: true }, async () => {
71+
const filters: string[] = [];
72+
const widget = createActionListWidget(disposables, {
73+
onFilter: async filter => {
74+
filters.push(filter);
75+
return [action(`${filter}-result`)];
76+
},
77+
});
78+
79+
typeFilter(widget, 'm');
80+
typeFilter(widget, 'ma');
81+
assert.deepStrictEqual(filters, ['m', 'ma']);
82+
await timeout(0);
83+
assert.ok(widget.domNode.textContent?.includes('ma-result'));
84+
}));
85+
86+
test('ignores stale dynamic filter results', async () => {
87+
const firstResult = new DeferredPromise<readonly IActionListItem<ITestActionItem>[]>();
88+
const secondResult = new DeferredPromise<readonly IActionListItem<ITestActionItem>[]>();
89+
const filters: string[] = [];
90+
const widget = createActionListWidget(disposables, {
91+
onFilter: filter => {
92+
filters.push(filter);
93+
return filter === 'm' ? firstResult.p : secondResult.p;
94+
},
95+
});
96+
97+
typeFilter(widget, 'm');
98+
typeFilter(widget, 'ma');
99+
assert.deepStrictEqual(filters, ['m', 'ma']);
100+
101+
firstResult.complete([action('ma-stale-result')]);
102+
await timeout(0);
103+
assert.ok(!widget.domNode.textContent?.includes('ma-stale-result'));
104+
105+
secondResult.complete([action('ma-fresh-result')]);
106+
await timeout(0);
107+
assert.ok(widget.domNode.textContent?.includes('ma-fresh-result'));
108+
});
109+
});

‎src/vs/platform/agentHost/node/agentHostGitService.ts‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,26 @@ export interface IAgentHostGitService {
2020
removeWorktree(repositoryRoot: URI, worktree: URI): Promise<void>;
2121
}
2222

23+
function getCommonBranchPriority(branch: string): number {
24+
if (branch === 'main') {
25+
return 0;
26+
}
27+
if (branch === 'master') {
28+
return 1;
29+
}
30+
return 2;
31+
}
32+
33+
export function getBranchCompletions(branches: readonly string[], options?: { readonly query?: string; readonly limit?: number }): string[] {
34+
const normalizedQuery = options?.query?.toLowerCase();
35+
const filtered = normalizedQuery
36+
? branches.filter(branch => branch.toLowerCase().includes(normalizedQuery))
37+
: [...branches];
38+
39+
filtered.sort((a, b) => getCommonBranchPriority(a) - getCommonBranchPriority(b));
40+
return options?.limit ? filtered.slice(0, options.limit) : filtered;
41+
}
42+
2343
export class AgentHostGitService implements IAgentHostGitService {
2444
declare readonly _serviceBrand: undefined;
2545

@@ -35,21 +55,14 @@ export class AgentHostGitService implements IAgentHostGitService {
3555

3656
async getBranches(workingDirectory: URI, options?: { readonly query?: string; readonly limit?: number }): Promise<string[]> {
3757
const args = ['for-each-ref', '--format=%(refname:short)', '--sort=-committerdate'];
38-
if (options?.limit && !options.query) {
39-
args.push(`--count=${options.limit}`);
40-
}
4158
args.push('refs/heads');
4259

4360
const output = await this._runGit(workingDirectory, args);
4461
if (!output) {
4562
return [];
4663
}
4764
const branches = output.split(/\r?\n/g).map(line => line.trim()).filter(branch => branch.length > 0);
48-
const normalizedQuery = options?.query?.toLowerCase();
49-
const filtered = normalizedQuery
50-
? branches.filter(branch => branch.toLowerCase().includes(normalizedQuery))
51-
: branches;
52-
return options?.limit ? filtered.slice(0, options.limit) : filtered;
65+
return getBranchCompletions(branches, options);
5366
}
5467

5568
async getRepositoryRoot(workingDirectory: URI): Promise<URI | undefined> {

‎src/vs/platform/agentHost/test/node/agentHostGitService.test.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import assert from 'assert';
7+
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js';
8+
import { getBranchCompletions } from '../../node/agentHostGitService.js';
9+
10+
suite('AgentHostGitService', () => {
11+
ensureNoDisposablesAreLeakedInTestSuite();
12+
13+
test('sorts common branch names to the top before applying limit', () => {
14+
assert.deepStrictEqual(
15+
getBranchCompletions(['feature/recent', 'release', 'master', 'main', 'feature/older'], { limit: 3 }),
16+
['main', 'master', 'feature/recent'],
17+
);
18+
});
19+
20+
test('preserves git order for non-common branches', () => {
21+
assert.deepStrictEqual(
22+
getBranchCompletions(['feature/recent', 'release', 'feature/older']),
23+
['feature/recent', 'release', 'feature/older'],
24+
);
25+
});
26+
27+
test('filters before sorting common branch names', () => {
28+
assert.deepStrictEqual(
29+
getBranchCompletions(['feature/recent', 'master', 'main', 'maintenance'], { query: 'ma' }),
30+
['main', 'master', 'maintenance'],
31+
);
32+
});
33+
});

0 commit comments

Comments
 (0)