Skip to content

make it more stable #298691

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.

Sign up for GitHub

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

Merged
Sandeep Somavarapu (sandy081) merged 1 commit into from
Mar 2, 2026
Merged

make it more stable #298691

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 12 additions & 19 deletions src/vs/sessions/contrib/chat/browser/folderPicker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as dom from '../../../../base/browser/dom.js';
import { Codicon } from '../../../../base/common/codicons.js';
import { Emitter, Event } from '../../../../base/common/event.js';
import { Disposable, DisposableStore } from '../../../../base/common/lifecycle.js';
import { basename, isEqual } from '../../../../base/common/resources.js';
import { basename, extUriBiasedIgnorePathCase, isEqual } from '../../../../base/common/resources.js';
import { URI } from '../../../../base/common/uri.js';
import { localize } from '../../../../nls.js';
import { IActionWidgetService } from '../../../../platform/actionWidget/browser/actionWidget.js';
Expand All @@ -27,6 +27,7 @@ const FILTER_THRESHOLD = 10;
interface IFolderItem {
readonly uri: URI;
readonly label: string;
readonly checked?: boolean;
}

/**
Expand Down Expand Up @@ -218,40 +219,32 @@ export class FolderPicker extends Disposable {

private _buildItems(currentFolderUri: URI | undefined): IActionListItem<IFolderItem>[] {
const seenUris = new Set<string>();
if (currentFolderUri) {
seenUris.add(currentFolderUri.toString());
}

const items: IActionListItem<IFolderItem>[] = [];

// Currently selected folder (shown first, checked)
// Collect all folders (current + recently picked), deduplicated and sorted by name
const allFolders: { uri: URI; label: string }[] = [];
if (currentFolderUri) {
items.push({
kind: ActionListItemKind.Action,
label: basename(currentFolderUri),
group: { title: '', icon: Codicon.folder },
item: { uri: currentFolderUri, label: basename(currentFolderUri) },
});
seenUris.add(currentFolderUri.toString());
allFolders.push({ uri: currentFolderUri, label: basename(currentFolderUri) });
}

// Recently picked folders (sorted by name)
const dedupedFolders: { uri: URI; label: string }[] = [];
for (const folderUri of this._recentlyPickedFolders) {
const key = folderUri.toString();
if (seenUris.has(key)) {
continue;
}
seenUris.add(key);
dedupedFolders.push({ uri: folderUri, label: basename(folderUri) });
allFolders.push({ uri: folderUri, label: basename(folderUri) });
}
dedupedFolders.sort((a, b) => a.label.localeCompare(b.label));
for (const folder of dedupedFolders) {
allFolders.sort((a, b) => extUriBiasedIgnorePathCase.compare(a.uri, b.uri));

Copilot AI Mar 2, 2026

Copy link

Choose a reason for hiding this comment

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

The comment says the folders are “sorted by name”, but the actual sort uses extUriBiasedIgnorePathCase.compare(a.uri, b.uri), which orders by full URI (including parent path) rather than the displayed label (basename). This can make the list appear unsorted to users when parent paths differ. Consider sorting primarily by label (ideally with a deterministic, locale-independent comparator) and using URI comparison only as a tie-breaker, or update the comment if URI ordering is intended.

Suggested change
allFolders.sort((a, b) => extUriBiasedIgnorePathCase.compare(a.uri, b.uri));
allFolders.sort((a, b) => {
if (a.label < b.label) {
return -1;
}
if (a.label > b.label) {
return 1;
}
return extUriBiasedIgnorePathCase.compare(a.uri, b.uri);
});

Copilot uses AI. Check for mistakes.
for (const folder of allFolders) {
const isCurrent = currentFolderUri && isEqual(folder.uri, currentFolderUri);
items.push({
kind: ActionListItemKind.Action,
label: folder.label,
group: { title: '', icon: Codicon.folder },
item: { uri: folder.uri, label: folder.label },
onRemove: () => this._removeFolder(folder.uri),
item: { uri: folder.uri, label: folder.label, checked: isCurrent || false },
...(!isCurrent ? { onRemove: () => this._removeFolder(folder.uri) } : {}),
});
}

Expand Down
Loading