mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
fix(file-explorer): sort numbered file names naturally across every listing surface (#11576)
* fix(file-explorer): sort numbered file names naturally The File Explorer compared names with bare localeCompare, so numbered files listed 100, 200 before 99. Hoist the numeric collator Source Control file rows already use (#10850) into src/shared and apply it to the local and runtime directory listings, the name-filtered view, and Source Control directory nodes, which were inconsistent with the file rows one line below (#11426). * fix(file-explorer): natural sort on SSH funnels, relay, and pickers Adversarial-review round 1 rework: - Both readDir funnels short-circuited to the SSH filesystem provider before the patched sort, so SSH workspaces kept lexicographic order; re-sort locally after the provider returns (the remote relay may be an older build), and fix the relay's own comparator for relay-native consumers. - sortDirEntries (shared, unit-tested) owns the directories-first + natural-order listing contract used by every funnel. - compareFileNames breaks numeric-collation ties ('2' vs '02') by code units so sibling order stays total instead of readdir order, and pins the collator locale to 'en' so every host produces one order. - The SSH folder browser and runtime server dir picker now match the Explorer they browse into. - Ordering pinned by tests at the relay, source-control tree, and shared helper. * fix(mobile): natural sort in the mobile file explorer Mobile re-sorted host readDir results with bare localeCompare, undoing the host funnel's natural order (round-2 review). Reuse the shared comparator and pin the order in the mobile suite. * fix(file-explorer): natural sort at the renderer choke point and remaining ties Round-3 review: the remote-runtime RPC and paired-web routes return the host's order verbatim, so re-sort in readFileExplorerDirectory where every desktop route converges; pin the SSH funnel with a handler-level test; and route Source Control path compares through compareFileNames so numeric-collation ties share one total order with the Explorer. * docs(file-name-sort): state the real perf baseline in the hoist comment * refactor(source-control): drop the dead collator export; pin the test oracle locale * fix(file-listings): cover remaining natural-sort surfaces
This commit is contained in:
@@ -34,6 +34,18 @@ describe('file-tree', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('orders numbered names naturally, matching the desktop explorer', () => {
|
||||
const cache: DirectoryCache = {
|
||||
'': { entries: [entry('100 - b.txt'), entry('9 - c.txt'), entry('99 - a.txt')] }
|
||||
}
|
||||
|
||||
expect(flattenDirectoryCache(cache, new Set()).map((row) => row.id)).toEqual([
|
||||
'file:9 - c.txt',
|
||||
'file:99 - a.txt',
|
||||
'file:100 - b.txt'
|
||||
])
|
||||
})
|
||||
|
||||
it('mirrors desktop default browse exclusions while keeping dotfiles visible', () => {
|
||||
const cache: DirectoryCache = {
|
||||
'': {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
// Pure tree projection for the mobile file explorer. Mobile mirrors desktop
|
||||
// browse semantics by flattening cached files.readDir results as folders open.
|
||||
import { compareFileNames } from '../../../src/shared/file-name-sort'
|
||||
|
||||
export type MobileDirEntry = {
|
||||
name: string
|
||||
@@ -118,7 +119,7 @@ function compareDirectoryEntries(a: MobileDirEntry, b: MobileDirEntry): number {
|
||||
if (a.isDirectory !== b.isDirectory) {
|
||||
return a.isDirectory ? -1 : 1
|
||||
}
|
||||
return a.name.localeCompare(b.name)
|
||||
return compareFileNames(a.name, b.name)
|
||||
}
|
||||
|
||||
export function shouldIncludeMobileFileExplorerEntry(entry: MobileDirEntry): boolean {
|
||||
|
||||
@@ -366,6 +366,30 @@ describe('registerFilesystemHandlers', () => {
|
||||
lstatMock.mockRejectedValue(Object.assign(new Error('missing'), { code: 'ENOENT' }))
|
||||
})
|
||||
|
||||
it('re-sorts SSH provider listings directories-first in natural order', async () => {
|
||||
// Why: the remote relay may be an older build that still sorts lexicographically.
|
||||
getSshFilesystemProviderMock.mockReturnValueOnce({
|
||||
readDir: vi.fn().mockResolvedValue([
|
||||
{ name: '100 - b.txt', isDirectory: false, isSymlink: false },
|
||||
{ name: '9 - c.txt', isDirectory: false, isSymlink: false },
|
||||
{ name: '10 - dir', isDirectory: true, isSymlink: false },
|
||||
{ name: '99 - a.txt', isDirectory: false, isSymlink: false }
|
||||
])
|
||||
})
|
||||
registerFilesystemHandlers(store as never)
|
||||
|
||||
const result = (await handlers.get('fs:readDir')!(null, {
|
||||
dirPath: '/remote/repo',
|
||||
connectionId: 'ssh-1'
|
||||
})) as { name: string }[]
|
||||
expect(result.map((e) => e.name)).toEqual([
|
||||
'10 - dir',
|
||||
'9 - c.txt',
|
||||
'99 - a.txt',
|
||||
'100 - b.txt'
|
||||
])
|
||||
})
|
||||
|
||||
it('returns an actionable reconnect error when the SSH filesystem provider is unavailable', async () => {
|
||||
registerFilesystemHandlers(store as never)
|
||||
|
||||
|
||||
@@ -30,6 +30,7 @@ import type {
|
||||
} from '../../shared/types'
|
||||
import type { GitHistoryOptions, GitHistoryResult } from '../../shared/git-history'
|
||||
import type { SshMutationExpectation } from '../../shared/ssh-types'
|
||||
import { sortDirEntries } from '../../shared/file-name-sort'
|
||||
import { assertSshMutationExpectation } from '../ssh/ssh-connection-generation'
|
||||
import {
|
||||
buildRgArgs,
|
||||
@@ -540,7 +541,9 @@ export function registerFilesystemHandlers(
|
||||
if (args.connectionId) {
|
||||
throwSite = 'ssh-provider'
|
||||
const provider = requireSshFilesystemProvider(args.connectionId)
|
||||
return await provider.readDir(args.dirPath)
|
||||
// Why: re-sort locally — the remote relay may be an older build with
|
||||
// lexicographic ordering.
|
||||
return sortDirEntries(await provider.readDir(args.dirPath))
|
||||
}
|
||||
throwSite = 'authorize'
|
||||
const dirPath = await resolveAuthorizedPath(args.dirPath, store)
|
||||
@@ -555,12 +558,7 @@ export function registerFilesystemHandlers(
|
||||
isSymlink: entry.isSymbolicLink()
|
||||
}))
|
||||
)
|
||||
return mapped.sort((a, b) => {
|
||||
if (a.isDirectory !== b.isDirectory) {
|
||||
return a.isDirectory ? -1 : 1
|
||||
}
|
||||
return a.name.localeCompare(b.name)
|
||||
})
|
||||
return sortDirEntries(mapped)
|
||||
} catch (error: unknown) {
|
||||
recordCrashBreadcrumb(
|
||||
'fs_readdir_error',
|
||||
|
||||
@@ -3,6 +3,7 @@ import type { SshConnectionManager } from '../ssh/ssh-connection-manager'
|
||||
import type { SshExecOptions } from '../ssh/ssh-connection-utils'
|
||||
import { powerShellCommand, powerShellLiteral } from '../ssh/ssh-remote-powershell'
|
||||
import type { FilesystemPathFlavor } from '../../shared/types'
|
||||
import { sortDirEntries } from '../../shared/file-name-sort'
|
||||
|
||||
export type RemoteDirEntry = {
|
||||
name: string
|
||||
@@ -220,13 +221,8 @@ async function runBrowseCommand(
|
||||
}
|
||||
}
|
||||
|
||||
// Sort: directories first, then alphabetical
|
||||
entries.sort((a, b) => {
|
||||
if (a.isDirectory !== b.isDirectory) {
|
||||
return a.isDirectory ? -1 : 1
|
||||
}
|
||||
return a.name.localeCompare(b.name)
|
||||
})
|
||||
// Sort: directories first, then natural name order (matches the Explorer)
|
||||
sortDirEntries(entries)
|
||||
|
||||
resolveOnce({ entries, resolvedPath, pathFlavor })
|
||||
}
|
||||
|
||||
@@ -38,6 +38,7 @@ import {
|
||||
resolveRuntimePath
|
||||
} from '../../shared/cross-platform-path'
|
||||
import { PhysicalExitTracker } from '../../shared/physical-exit-tracker'
|
||||
import { sortDirEntries } from '../../shared/file-name-sort'
|
||||
import type {
|
||||
RuntimeFileListResult,
|
||||
RuntimeFileOpenResult,
|
||||
@@ -1292,7 +1293,9 @@ export class RuntimeFileCommands {
|
||||
if (!provider) {
|
||||
throw new Error(SSH_FILESYSTEM_PROVIDER_UNAVAILABLE_MESSAGE)
|
||||
}
|
||||
return provider.readDir(target.path)
|
||||
// Why: re-sort locally — the remote relay may be an older build with
|
||||
// lexicographic ordering.
|
||||
return sortDirEntries(await provider.readDir(target.path))
|
||||
}
|
||||
|
||||
const dirPath = await resolveAuthorizedPath(target.path, this.host.requireStore())
|
||||
@@ -1307,12 +1310,7 @@ export class RuntimeFileCommands {
|
||||
}
|
||||
})
|
||||
)
|
||||
return mapped.sort((a, b) => {
|
||||
if (a.isDirectory !== b.isDirectory) {
|
||||
return a.isDirectory ? -1 : 1
|
||||
}
|
||||
return a.name.localeCompare(b.name)
|
||||
})
|
||||
return sortDirEntries(mapped)
|
||||
}
|
||||
|
||||
async watchFileExplorer(
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
normalizeTerminalTitle
|
||||
} from '../../shared/agent-detection'
|
||||
import { extractOscTitleScanTail } from '../../shared/osc-title-scan-tail'
|
||||
import { sortDirEntries } from '../../shared/file-name-sort'
|
||||
import { isServerDriveListRequest, listWindowsDrives } from './windows-drive-listing'
|
||||
import { extractLastOsc7Uri, extractOscScanTail } from '../daemon/osc7-uri-extraction'
|
||||
import { parseFileUriPathParts } from '../daemon/osc7-file-uri'
|
||||
@@ -18250,12 +18251,7 @@ export class OrcaRuntimeService {
|
||||
isDirectory: entry.isDirectory(),
|
||||
isSymlink: entry.isSymbolicLink()
|
||||
}))
|
||||
mapped.sort((a, b) => {
|
||||
if (a.isDirectory !== b.isDirectory) {
|
||||
return a.isDirectory ? -1 : 1
|
||||
}
|
||||
return a.name.localeCompare(b.name)
|
||||
})
|
||||
sortDirEntries(mapped)
|
||||
return {
|
||||
resolvedPath: dirPath,
|
||||
entries: mapped,
|
||||
|
||||
@@ -164,19 +164,23 @@ describe('FsHandler', () => {
|
||||
await expect(dispatcher.callRequest('fs.tempDir')).resolves.toBe(tmpdir())
|
||||
})
|
||||
|
||||
it('readDir returns sorted entries with directories first', async () => {
|
||||
it('readDir returns entries directories-first in natural name order', async () => {
|
||||
mkdirSync(path.join(tmpDir, 'subdir'))
|
||||
writeFileSync(path.join(tmpDir, 'file.txt'), 'hello')
|
||||
writeFileSync(path.join(tmpDir, 'aaa.txt'), 'world')
|
||||
for (const name of ['file.txt', '100 - b.txt', '99 - a.txt', '9 - c.txt']) {
|
||||
writeFileSync(path.join(tmpDir, name), 'x')
|
||||
}
|
||||
|
||||
const result = (await dispatcher.callRequest('fs.readDir', { dirPath: tmpDir })) as {
|
||||
name: string
|
||||
isDirectory: boolean
|
||||
}[]
|
||||
expect(result[0].name).toBe('subdir')
|
||||
expect(result[0].isDirectory).toBe(true)
|
||||
expect(result.find((e) => e.name === 'file.txt')).toBeDefined()
|
||||
expect(result.find((e) => e.name === 'aaa.txt')).toBeDefined()
|
||||
expect(result[0]).toMatchObject({ name: 'subdir', isDirectory: true })
|
||||
expect(result.slice(1).map((e) => e.name)).toEqual([
|
||||
'9 - c.txt',
|
||||
'99 - a.txt',
|
||||
'100 - b.txt',
|
||||
'file.txt'
|
||||
])
|
||||
})
|
||||
|
||||
it('readDir reports symlinked directories as directories', async () => {
|
||||
|
||||
@@ -5,6 +5,7 @@ import { execFile } from 'node:child_process'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import type { RelayDispatcher, RequestContext } from './dispatcher'
|
||||
import { sortDirEntries } from '../shared/file-name-sort'
|
||||
import type { RelayContext } from './context'
|
||||
// Why: RelayContext is accepted in the constructor for protocol back-compat
|
||||
// (see docs/relay-fs-allowlist-removal.md), but no longer consulted on FS ops.
|
||||
@@ -148,12 +149,7 @@ export class FsHandler {
|
||||
isSymlink: entry.isSymbolicLink()
|
||||
}))
|
||||
)
|
||||
return mapped.sort((a, b) => {
|
||||
if (a.isDirectory !== b.isDirectory) {
|
||||
return a.isDirectory ? -1 : 1
|
||||
}
|
||||
return a.name.localeCompare(b.name)
|
||||
})
|
||||
return sortDirEntries(mapped)
|
||||
}
|
||||
|
||||
private async readFile(params: Record<string, unknown>) {
|
||||
|
||||
@@ -9,11 +9,33 @@ import {
|
||||
} from './quick-open-search'
|
||||
|
||||
describe('quick-open-search', () => {
|
||||
it('returns the first 50 paths with score 0 for an empty query', () => {
|
||||
const files = Array.from({ length: 75 }, (_, index) => `src/file-${index}.ts`)
|
||||
it('orders numbered paths naturally for empty queries and fuzzy-score ties', () => {
|
||||
const files = prepareQuickOpenFiles([
|
||||
'songs/100 - b.txt',
|
||||
'songs/9 - c.txt',
|
||||
'songs/99 - a.txt'
|
||||
])
|
||||
|
||||
expect(rankQuickOpenFiles('', files).map((item) => item.path)).toEqual([
|
||||
'songs/9 - c.txt',
|
||||
'songs/99 - a.txt',
|
||||
'songs/100 - b.txt'
|
||||
])
|
||||
expect(rankQuickOpenFiles('songs', files).map((item) => item.path)).toEqual([
|
||||
'songs/9 - c.txt',
|
||||
'songs/99 - a.txt',
|
||||
'songs/100 - b.txt'
|
||||
])
|
||||
})
|
||||
|
||||
it('returns the first 50 naturally sorted paths with score 0 for an empty query', () => {
|
||||
const files = Array.from({ length: 75 }, (_, index) => `src/file-${74 - index}.ts`)
|
||||
|
||||
expect(rankQuickOpenFiles('', prepareQuickOpenFiles(files))).toEqual(
|
||||
files.slice(0, QUICK_OPEN_RESULT_LIMIT).map((path) => ({ path, score: 0 }))
|
||||
Array.from({ length: QUICK_OPEN_RESULT_LIMIT }, (_, index) => ({
|
||||
path: `src/file-${index}.ts`,
|
||||
score: 0
|
||||
}))
|
||||
)
|
||||
})
|
||||
|
||||
@@ -35,8 +57,8 @@ describe('quick-open-search', () => {
|
||||
).toEqual(['src/components/Button.tsx', 'button-area/deep/path/file.tsx'])
|
||||
})
|
||||
|
||||
it('keeps first-seen order for tie-heavy results at the limit boundary', () => {
|
||||
const files = Array.from({ length: 10 }, (_, index) => `src/path-${index}.bin`)
|
||||
it('uses natural order for tie-heavy results at the limit boundary', () => {
|
||||
const files = Array.from({ length: 10 }, (_, index) => `src/path-${9 - index}.bin`)
|
||||
|
||||
expect(rankQuickOpenFiles('s', prepareQuickOpenFiles(files), 4)).toEqual([
|
||||
{ path: 'src/path-0.bin', score: 0 },
|
||||
@@ -85,9 +107,9 @@ describe('quick-open-search', () => {
|
||||
it('indexes normalized relative paths without changing path semantics', () => {
|
||||
const files = [
|
||||
'src/renderer/src/components/QuickOpen.tsx',
|
||||
'legacy\\provider\\raw-path.ts',
|
||||
'packages/windows-origin/src/App.tsx',
|
||||
'single-file.ts',
|
||||
'legacy\\provider\\raw-path.ts'
|
||||
'single-file.ts'
|
||||
]
|
||||
|
||||
expect(prepareQuickOpenFiles(files)).toEqual([
|
||||
@@ -97,22 +119,22 @@ describe('quick-open-search', () => {
|
||||
lowerFilename: 'quickopen.tsx',
|
||||
inputIndex: 0
|
||||
},
|
||||
{
|
||||
path: 'legacy\\provider\\raw-path.ts',
|
||||
lowerPath: 'legacy/provider/raw-path.ts',
|
||||
lowerFilename: 'raw-path.ts',
|
||||
inputIndex: 1
|
||||
},
|
||||
{
|
||||
path: 'packages/windows-origin/src/App.tsx',
|
||||
lowerPath: 'packages/windows-origin/src/app.tsx',
|
||||
lowerFilename: 'app.tsx',
|
||||
inputIndex: 1
|
||||
inputIndex: 2
|
||||
},
|
||||
{
|
||||
path: 'single-file.ts',
|
||||
lowerPath: 'single-file.ts',
|
||||
lowerFilename: 'single-file.ts',
|
||||
inputIndex: 2
|
||||
},
|
||||
{
|
||||
path: 'legacy\\provider\\raw-path.ts',
|
||||
lowerPath: 'legacy/provider/raw-path.ts',
|
||||
lowerFilename: 'raw-path.ts',
|
||||
inputIndex: 3
|
||||
}
|
||||
])
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { isClipboardTextByteLengthOverLimit } from '../../../shared/clipboard-text'
|
||||
import { compareFileNames } from '../../../shared/file-name-sort'
|
||||
|
||||
export const QUICK_OPEN_RESULT_LIMIT = 50
|
||||
export const QUICK_OPEN_QUERY_MAX_BYTES = 2 * 1024
|
||||
@@ -51,7 +52,11 @@ export function rankQuickOpenFiles(
|
||||
// still naturally type backslashes in path queries.
|
||||
const normalizedQuery = query.trim().replace(/\\/g, '/').toLowerCase()
|
||||
if (!normalizedQuery) {
|
||||
return files.slice(0, limit).map((file) => ({ path: file.path, score: 0 }))
|
||||
const results: QuickOpenRankedResult[] = []
|
||||
for (const file of files) {
|
||||
retainTopResult(results, { path: file.path, score: 0, inputIndex: file.inputIndex }, limit)
|
||||
}
|
||||
return finalizeResults(results)
|
||||
}
|
||||
|
||||
const results: QuickOpenRankedResult[] = []
|
||||
@@ -61,10 +66,10 @@ export function rankQuickOpenFiles(
|
||||
continue
|
||||
}
|
||||
|
||||
insertTopResult(results, { path: file.path, score, inputIndex: file.inputIndex }, limit)
|
||||
retainTopResult(results, { path: file.path, score, inputIndex: file.inputIndex }, limit)
|
||||
}
|
||||
|
||||
return results.map(({ path, score }) => ({ path, score }))
|
||||
return finalizeResults(results)
|
||||
}
|
||||
|
||||
function fuzzyMatchIndexedFile(query: string, file: QuickOpenIndexedFile): number {
|
||||
@@ -106,42 +111,63 @@ type QuickOpenRankedResult = QuickOpenSearchResult & {
|
||||
inputIndex: number
|
||||
}
|
||||
|
||||
function insertTopResult(
|
||||
results: QuickOpenRankedResult[],
|
||||
function retainTopResult(
|
||||
heap: QuickOpenRankedResult[],
|
||||
candidate: QuickOpenRankedResult,
|
||||
limit: number
|
||||
): void {
|
||||
const worst = results.at(-1)
|
||||
if (results.length === limit && worst && compareRankedResult(candidate, worst) >= 0) {
|
||||
if (heap.length === limit && compareRankedResult(candidate, heap[0]) >= 0) {
|
||||
return
|
||||
}
|
||||
|
||||
const insertAt = findInsertionIndex(results, candidate)
|
||||
results.splice(insertAt, 0, candidate)
|
||||
if (results.length > limit) {
|
||||
results.pop()
|
||||
if (heap.length < limit) {
|
||||
heap.push(candidate)
|
||||
siftResultUp(heap, heap.length - 1)
|
||||
return
|
||||
}
|
||||
|
||||
heap[0] = candidate
|
||||
siftResultDown(heap)
|
||||
}
|
||||
|
||||
function siftResultUp(heap: QuickOpenRankedResult[], startIndex: number): void {
|
||||
let index = startIndex
|
||||
while (index > 0) {
|
||||
const parentIndex = Math.floor((index - 1) / 2)
|
||||
if (compareRankedResult(heap[index], heap[parentIndex]) <= 0) {
|
||||
return
|
||||
}
|
||||
;[heap[index], heap[parentIndex]] = [heap[parentIndex], heap[index]]
|
||||
index = parentIndex
|
||||
}
|
||||
}
|
||||
|
||||
function findInsertionIndex(
|
||||
results: readonly QuickOpenRankedResult[],
|
||||
candidate: QuickOpenRankedResult
|
||||
): number {
|
||||
let low = 0
|
||||
let high = results.length
|
||||
|
||||
while (low < high) {
|
||||
const mid = Math.floor((low + high) / 2)
|
||||
if (compareRankedResult(candidate, results[mid]) < 0) {
|
||||
high = mid
|
||||
} else {
|
||||
low = mid + 1
|
||||
function siftResultDown(heap: QuickOpenRankedResult[]): void {
|
||||
let index = 0
|
||||
while (true) {
|
||||
const leftIndex = index * 2 + 1
|
||||
if (leftIndex >= heap.length) {
|
||||
return
|
||||
}
|
||||
const rightIndex = leftIndex + 1
|
||||
const worseChildIndex =
|
||||
rightIndex < heap.length && compareRankedResult(heap[rightIndex], heap[leftIndex]) > 0
|
||||
? rightIndex
|
||||
: leftIndex
|
||||
if (compareRankedResult(heap[worseChildIndex], heap[index]) <= 0) {
|
||||
return
|
||||
}
|
||||
;[heap[index], heap[worseChildIndex]] = [heap[worseChildIndex], heap[index]]
|
||||
index = worseChildIndex
|
||||
}
|
||||
}
|
||||
|
||||
return low
|
||||
function finalizeResults(results: QuickOpenRankedResult[]): QuickOpenSearchResult[] {
|
||||
return results
|
||||
.sort(compareRankedResult)
|
||||
.map(({ path, score }): QuickOpenSearchResult => ({ path, score }))
|
||||
}
|
||||
|
||||
function compareRankedResult(a: QuickOpenRankedResult, b: QuickOpenRankedResult): number {
|
||||
return a.score - b.score || a.inputIndex - b.inputIndex
|
||||
return a.score - b.score || compareFileNames(a.path, b.path) || a.inputIndex - b.inputIndex
|
||||
}
|
||||
|
||||
@@ -125,8 +125,8 @@ import {
|
||||
buildActiveOpenRowKeys
|
||||
} from './source-control-active-open-file-keys'
|
||||
import {
|
||||
filterAndSortSourceControlPathEntries,
|
||||
filterSourceControlGroupedPathEntries,
|
||||
filterSourceControlPathEntries,
|
||||
getSourceControlFileFilterState
|
||||
} from './source-control-file-filter'
|
||||
import { getCommitMessageTextareaRows } from './source-control-commit-message-rows'
|
||||
@@ -1822,7 +1822,7 @@ function SourceControlInner(): React.JSX.Element {
|
||||
)
|
||||
|
||||
const filteredBranchEntries = useMemo(
|
||||
() => filterSourceControlPathEntries(branchEntries, fileFilterState),
|
||||
() => filterAndSortSourceControlPathEntries(branchEntries, fileFilterState),
|
||||
[branchEntries, fileFilterState]
|
||||
)
|
||||
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const readRuntimeDirectory = vi.fn()
|
||||
vi.mock('@/runtime/runtime-file-client', () => ({
|
||||
readRuntimeDirectory: (...args: unknown[]) => readRuntimeDirectory(...args)
|
||||
}))
|
||||
vi.mock('./file-explorer-operation-owner', () => ({
|
||||
getFileExplorerOperationOwner: () => ({ kind: 'local' }),
|
||||
getFileExplorerOperationRoute: () => ({ settings: null, connectionId: null }),
|
||||
getFileExplorerOwnerUnresolvedMessage: () => 'unresolved'
|
||||
}))
|
||||
|
||||
import { readFileExplorerDirectory } from './file-explorer-directory-listing'
|
||||
|
||||
describe('readFileExplorerDirectory', () => {
|
||||
it('re-sorts backend order — remote-runtime and paired-web routes return the host order verbatim', async () => {
|
||||
readRuntimeDirectory.mockResolvedValueOnce([
|
||||
{ name: '100 - b.txt', isDirectory: false, isSymlink: false },
|
||||
{ name: '9 - c.txt', isDirectory: false, isSymlink: false },
|
||||
{ name: '10 - dir', isDirectory: true, isSymlink: false },
|
||||
{ name: '99 - a.txt', isDirectory: false, isSymlink: false }
|
||||
])
|
||||
|
||||
const { entries } = await readFileExplorerDirectory('wt-1', '/w', '/w/dir')
|
||||
expect(entries.map((e) => e.name)).toEqual([
|
||||
'10 - dir',
|
||||
'9 - c.txt',
|
||||
'99 - a.txt',
|
||||
'100 - b.txt'
|
||||
])
|
||||
})
|
||||
})
|
||||
@@ -1,5 +1,6 @@
|
||||
import { joinPath, normalizeRelativePath } from '@/lib/path'
|
||||
import type { DirEntry } from '../../../../shared/types'
|
||||
import { sortDirEntries } from '../../../../shared/file-name-sort'
|
||||
import { readRuntimeDirectory } from '@/runtime/runtime-file-client'
|
||||
import type { FileExplorerOperationOwner, TreeNode } from './file-explorer-types'
|
||||
import { shouldIncludeFileExplorerEntry } from './file-explorer-entries'
|
||||
@@ -56,5 +57,8 @@ export async function readFileExplorerDirectory(
|
||||
},
|
||||
dirPath
|
||||
)
|
||||
return { entries, operationOwner }
|
||||
// Why: remote-runtime and paired-web routes return the host's order verbatim,
|
||||
// and an older host may still sort lexicographically; re-sorting an already
|
||||
// sorted local listing is near-free (adaptive sort).
|
||||
return { entries: sortDirEntries(entries), operationOwner }
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { joinPath, normalizeRelativePath } from '@/lib/path'
|
||||
import { isClipboardTextByteLengthOverLimit } from '../../../../shared/clipboard-text'
|
||||
import { compareFileNames } from '../../../../shared/file-name-sort'
|
||||
import type { FileExplorerOperationOwner, TreeNode } from './file-explorer-types'
|
||||
import {
|
||||
createFileExplorerRowProjectionFromParts,
|
||||
@@ -232,7 +233,7 @@ function appendNameFilteredEntries(
|
||||
if (a.node.isDirectory !== b.node.isDirectory) {
|
||||
return a.node.isDirectory ? -1 : 1
|
||||
}
|
||||
return a.node.name.localeCompare(b.node.name)
|
||||
return compareFileNames(a.node.name, b.node.name)
|
||||
})
|
||||
for (const entry of sortedEntries) {
|
||||
visibleFlatRows.push(entry.node)
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
SOURCE_CONTROL_FILE_FILTER_QUERY_MAX_BYTES,
|
||||
filterAndSortSourceControlPathEntries,
|
||||
filterSourceControlGroupedPathEntries,
|
||||
filterSourceControlPathEntries,
|
||||
getSourceControlFileFilterState,
|
||||
@@ -9,6 +10,26 @@ import {
|
||||
} from './source-control-file-filter'
|
||||
|
||||
describe('source-control-file-filter', () => {
|
||||
it('naturally orders committed branch rows without mutating store input', () => {
|
||||
const entries = [
|
||||
{ path: 'migrations/100.sql' },
|
||||
{ path: 'migrations/9.sql' },
|
||||
{ path: 'migrations/99.sql' }
|
||||
]
|
||||
|
||||
expect(
|
||||
filterAndSortSourceControlPathEntries(entries, {
|
||||
normalizedFilter: '',
|
||||
tooLarge: false
|
||||
}).map((entry) => entry.path)
|
||||
).toEqual(['migrations/9.sql', 'migrations/99.sql', 'migrations/100.sql'])
|
||||
expect(entries.map((entry) => entry.path)).toEqual([
|
||||
'migrations/100.sql',
|
||||
'migrations/9.sql',
|
||||
'migrations/99.sql'
|
||||
])
|
||||
})
|
||||
|
||||
it('normalizes bounded queries and filters entries by path', () => {
|
||||
const filter = getSourceControlFileFilterState(' SRC/button ')
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { isClipboardTextByteLengthOverLimit } from '../../../../shared/clipboard-text'
|
||||
import { compareFileNames } from '../../../../shared/file-name-sort'
|
||||
|
||||
export const SOURCE_CONTROL_FILE_FILTER_QUERY_MAX_BYTES = 2 * 1024
|
||||
|
||||
@@ -48,6 +49,15 @@ export function filterSourceControlPathEntries<T extends SourceControlPathEntry>
|
||||
return entries.filter((entry) => entry.path.toLowerCase().includes(filter.normalizedFilter))
|
||||
}
|
||||
|
||||
export function filterAndSortSourceControlPathEntries<T extends SourceControlPathEntry>(
|
||||
entries: T[],
|
||||
filter: SourceControlFileFilterState
|
||||
): T[] {
|
||||
return [...filterSourceControlPathEntries(entries, filter)].sort((a, b) =>
|
||||
compareFileNames(a.path, b.path)
|
||||
)
|
||||
}
|
||||
|
||||
export function filterSourceControlGroupedPathEntries<T extends SourceControlPathEntry>(
|
||||
grouped: SourceControlGroupedPathEntries<T>,
|
||||
filter: SourceControlFileFilterState
|
||||
|
||||
@@ -22,7 +22,9 @@ function referenceCompare(a: GitStatusEntry, b: GitStatusEntry): number {
|
||||
}
|
||||
return 2
|
||||
}
|
||||
return rank(a) - rank(b) || a.path.localeCompare(b.path, undefined, { numeric: true })
|
||||
// 'en' pinned to match the shared collator — an undefined locale would make
|
||||
// this oracle environment-dependent (sv/cs/da collate differently).
|
||||
return rank(a) - rank(b) || a.path.localeCompare(b.path, 'en', { numeric: true })
|
||||
}
|
||||
|
||||
describe('compareGitStatusEntries', () => {
|
||||
|
||||
@@ -1,14 +1,10 @@
|
||||
import type { GitStatusEntry } from '../../../../shared/types'
|
||||
|
||||
// Why hoisted: localeCompare with an options object resolves a fresh ICU collator
|
||||
// on every comparison, so a changed-file sort paid for one per O(n log n) step.
|
||||
export const sourceControlPathCollator = new Intl.Collator(undefined, { numeric: true })
|
||||
import { compareFileNames } from '../../../../shared/file-name-sort'
|
||||
|
||||
export function compareGitStatusEntries(a: GitStatusEntry, b: GitStatusEntry): number {
|
||||
return (
|
||||
getConflictSortRank(a) - getConflictSortRank(b) ||
|
||||
sourceControlPathCollator.compare(a.path, b.path)
|
||||
)
|
||||
// Why: compareFileNames (not the raw collator) so numeric-collation ties
|
||||
// ("2" vs "02") stay a total order shared with the File Explorer.
|
||||
return getConflictSortRank(a) - getConflictSortRank(b) || compareFileNames(a.path, b.path)
|
||||
}
|
||||
|
||||
function getConflictSortRank(entry: GitStatusEntry): number {
|
||||
|
||||
@@ -39,6 +39,24 @@ describe('buildSourceControlTree', () => {
|
||||
])
|
||||
})
|
||||
|
||||
it('orders numbered sibling directories naturally, matching the file rows', () => {
|
||||
const tree = buildSourceControlTree('unstaged', [
|
||||
entry({ path: 'migrations/100 - b/a.sql' }),
|
||||
entry({ path: 'migrations/9 - a/a.sql' }),
|
||||
entry({ path: 'migrations/99 - c/a.sql' })
|
||||
])
|
||||
|
||||
expect(labels(flattenSourceControlTree(tree, new Set()))).toEqual([
|
||||
'directory:migrations',
|
||||
'directory:migrations/9 - a',
|
||||
'file:migrations/9 - a/a.sql',
|
||||
'directory:migrations/99 - c',
|
||||
'file:migrations/99 - c/a.sql',
|
||||
'directory:migrations/100 - b',
|
||||
'file:migrations/100 - b/a.sql'
|
||||
])
|
||||
})
|
||||
|
||||
it('keeps conflict rows ahead of normal rows within a directory', () => {
|
||||
const tree = buildGitStatusSourceControlTree('unstaged', [
|
||||
entry({ path: 'src/a.ts' }),
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
import { normalizeRelativePath } from '@/lib/path'
|
||||
import type { GitStatusEntry, GitStagingArea } from '../../../../shared/types'
|
||||
import { compareFileNames } from '../../../../shared/file-name-sort'
|
||||
import { splitPathSegments } from './path-tree'
|
||||
import { compareGitStatusEntries, sourceControlPathCollator } from './source-control-status-sort'
|
||||
import { compareGitStatusEntries } from './source-control-status-sort'
|
||||
|
||||
export type SourceControlTreeArea = Extract<GitStagingArea, 'unstaged' | 'staged' | 'untracked'>
|
||||
// Why: committed branch rows share the same path tree but do not carry
|
||||
@@ -51,7 +52,7 @@ type MutableDirectoryNode<Entry extends SourceControlTreeEntry, Area extends str
|
||||
}
|
||||
|
||||
function compareTreeEntriesByPath(a: SourceControlTreeEntry, b: SourceControlTreeEntry): number {
|
||||
return sourceControlPathCollator.compare(a.path, b.path)
|
||||
return compareFileNames(a.path, b.path)
|
||||
}
|
||||
|
||||
function makeDirectoryNode<Entry extends SourceControlTreeEntry, Area extends string>(
|
||||
@@ -90,7 +91,9 @@ function finalizeDirectoryNode<Entry extends SourceControlTreeEntry, Area extend
|
||||
}
|
||||
}
|
||||
|
||||
directories.sort((a, b) => a.name.localeCompare(b.name))
|
||||
// Why: keep directory ordering consistent with the numeric path collator used
|
||||
// for the file rows one line below.
|
||||
directories.sort((a, b) => compareFileNames(a.name, b.name))
|
||||
files.sort((a, b) => compareEntries(a.entry, b.entry))
|
||||
const fileCount =
|
||||
files.length + directories.reduce((count, directory) => count + directory.fileCount, 0)
|
||||
|
||||
@@ -29,7 +29,12 @@ beforeEach(() => {
|
||||
result: {
|
||||
resolvedPath: '/home/me',
|
||||
pathFlavor: 'posix',
|
||||
entries: [{ name: 'repo', isDirectory: true, isSymlink: false }]
|
||||
entries: [
|
||||
{ name: '100 - file.txt', isDirectory: false, isSymlink: false },
|
||||
{ name: '9 - file.txt', isDirectory: false, isSymlink: false },
|
||||
{ name: '10 - repo', isDirectory: true, isSymlink: false },
|
||||
{ name: '99 - file.txt', isDirectory: false, isSymlink: false }
|
||||
]
|
||||
},
|
||||
_meta: { runtimeId: 'remote-runtime' }
|
||||
})
|
||||
@@ -48,7 +53,12 @@ describe('runtime server directory browser', () => {
|
||||
await expect(browseRuntimeServerDirectory('env-1', '~')).resolves.toEqual({
|
||||
resolvedPath: '/home/me',
|
||||
pathFlavor: 'posix',
|
||||
entries: [{ name: 'repo', isDirectory: true, isSymlink: false }]
|
||||
entries: [
|
||||
{ name: '10 - repo', isDirectory: true, isSymlink: false },
|
||||
{ name: '9 - file.txt', isDirectory: false, isSymlink: false },
|
||||
{ name: '99 - file.txt', isDirectory: false, isSymlink: false },
|
||||
{ name: '100 - file.txt', isDirectory: false, isSymlink: false }
|
||||
]
|
||||
})
|
||||
|
||||
expect(runtimeEnvironmentCall).toHaveBeenLastCalledWith({
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { DirEntry, FilesystemPathFlavor } from '../../../shared/types'
|
||||
import { sortDirEntries } from '../../../shared/file-name-sort'
|
||||
import { callRuntimeRpc } from './runtime-rpc-client'
|
||||
|
||||
export type RuntimeServerDirectoryListing = {
|
||||
@@ -11,10 +12,11 @@ export async function browseRuntimeServerDirectory(
|
||||
environmentId: string,
|
||||
path: string
|
||||
): Promise<RuntimeServerDirectoryListing> {
|
||||
return callRuntimeRpc<RuntimeServerDirectoryListing>(
|
||||
const listing = await callRuntimeRpc<RuntimeServerDirectoryListing>(
|
||||
{ kind: 'environment', environmentId },
|
||||
'files.browseServerDir',
|
||||
{ path },
|
||||
{ timeoutMs: 15_000 }
|
||||
)
|
||||
return { ...listing, entries: sortDirEntries(listing.entries) }
|
||||
}
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { compareFileNames, sortDirEntries } from './file-name-sort'
|
||||
|
||||
describe('compareFileNames', () => {
|
||||
it('sorts numeric name segments naturally instead of lexicographically', () => {
|
||||
const names = [
|
||||
'1 - item.txt',
|
||||
'100 - item.txt',
|
||||
'2 - item.txt',
|
||||
'200 - item.txt',
|
||||
'409 - item.txt',
|
||||
'41 - item.txt',
|
||||
'410 - item.txt',
|
||||
'9 - item.txt',
|
||||
'99 - item.txt'
|
||||
]
|
||||
expect([...names].sort(compareFileNames)).toEqual([
|
||||
'1 - item.txt',
|
||||
'2 - item.txt',
|
||||
'9 - item.txt',
|
||||
'41 - item.txt',
|
||||
'99 - item.txt',
|
||||
'100 - item.txt',
|
||||
'200 - item.txt',
|
||||
'409 - item.txt',
|
||||
'410 - item.txt'
|
||||
])
|
||||
})
|
||||
|
||||
it('sorts embedded numbers naturally within alphabetical order', () => {
|
||||
expect(['b1.md', 'a10.md', 'a2.md', 'a1.md'].sort(compareFileNames)).toEqual([
|
||||
'a1.md',
|
||||
'a2.md',
|
||||
'a10.md',
|
||||
'b1.md'
|
||||
])
|
||||
})
|
||||
|
||||
it('breaks numeric-collation ties deterministically instead of readdir order', () => {
|
||||
// numeric collation ties "2" with "02"; the code-unit fallback keeps a total order.
|
||||
expect(['2.txt', '02.txt'].sort(compareFileNames)).toEqual(['02.txt', '2.txt'])
|
||||
expect(['02.txt', '2.txt'].sort(compareFileNames)).toEqual(['02.txt', '2.txt'])
|
||||
expect(compareFileNames('a.txt', 'a.txt')).toBe(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe('sortDirEntries', () => {
|
||||
it('keeps directories first, each group in natural order', () => {
|
||||
const entries = [
|
||||
{ name: '10 - notes', isDirectory: false },
|
||||
{ name: '2 - src', isDirectory: true },
|
||||
{ name: '9 - docs.txt', isDirectory: false },
|
||||
{ name: '10 - assets', isDirectory: true }
|
||||
]
|
||||
expect(sortDirEntries(entries).map((e) => e.name)).toEqual([
|
||||
'2 - src',
|
||||
'10 - assets',
|
||||
'9 - docs.txt',
|
||||
'10 - notes'
|
||||
])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,30 @@
|
||||
// Why hoisted: localeCompare with an options object resolves a fresh ICU collator
|
||||
// on every comparison (~14x this form). Numeric collation itself costs ~5x over
|
||||
// bare localeCompare — accepted for the feature; don't "optimize" the hoist away.
|
||||
// numeric: true orders "99 - a" before "100 - b", matching Finder/Explorer.
|
||||
// Locale pinned to 'en' so local main, renderer, relay, and remote runtime hosts
|
||||
// all produce one order regardless of each process's LANG (precedent:
|
||||
// skill-freshness pins 'en' for canonical ordering).
|
||||
export const fileNameCollator = new Intl.Collator('en', { numeric: true })
|
||||
|
||||
export function compareFileNames(a: string, b: string): number {
|
||||
const primary = fileNameCollator.compare(a, b)
|
||||
if (primary !== 0) {
|
||||
return primary
|
||||
}
|
||||
// Why: numeric collation ties distinct names ("2" vs "02"); fall back to code
|
||||
// units so sibling order stays a total order instead of readdir order.
|
||||
return a < b ? -1 : a > b ? 1 : 0
|
||||
}
|
||||
|
||||
/** Directories-first, then natural name order — the File Explorer listing contract. */
|
||||
export function sortDirEntries<T extends { name: string; isDirectory: boolean }>(
|
||||
entries: T[]
|
||||
): T[] {
|
||||
return entries.sort((a, b) => {
|
||||
if (a.isDirectory !== b.isDirectory) {
|
||||
return a.isDirectory ? -1 : 1
|
||||
}
|
||||
return compareFileNames(a.name, b.name)
|
||||
})
|
||||
}
|
||||
@@ -22,6 +22,29 @@ beforeEach(() => {
|
||||
})
|
||||
|
||||
describe('quick-open streaming directory reader', () => {
|
||||
it('orders numbered entries naturally before the recursive walk', async () => {
|
||||
opendirMock.mockResolvedValue({
|
||||
async *[Symbol.asyncIterator]() {
|
||||
for (const name of ['100 - notes', '9 - notes', '99 - notes']) {
|
||||
yield {
|
||||
name,
|
||||
isDirectory: () => true,
|
||||
isFile: () => false,
|
||||
isSymbolicLink: () => false
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
const entries = await readQuickOpenDirectoryEntries({
|
||||
absPath: '/numbered',
|
||||
allowSymlinkedRoot: false,
|
||||
budget: createQuickOpenReaddirBudget()
|
||||
})
|
||||
|
||||
expect(entries.map((entry) => entry.name)).toEqual(['9 - notes', '99 - notes', '100 - notes'])
|
||||
})
|
||||
|
||||
it('stops a huge directory one entry beyond the exact cap and closes its iterator', async () => {
|
||||
let produced = 0
|
||||
let closed = false
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { lstat, opendir } from 'node:fs/promises'
|
||||
import { compareFileNames } from './file-name-sort'
|
||||
import { isFileListingCancellation, throwIfFileListingCancelled } from './file-listing-cancellation'
|
||||
import { isQuickOpenReadableDirectory } from './quick-open-directory-validation'
|
||||
import {
|
||||
@@ -46,7 +47,7 @@ export async function readQuickOpenDirectoryEntries(opts: {
|
||||
: 'other'
|
||||
})
|
||||
}
|
||||
entries.sort((left, right) => (left.name < right.name ? -1 : left.name > right.name ? 1 : 0))
|
||||
entries.sort((left, right) => compareFileNames(left.name, right.name))
|
||||
|
||||
// Why: discard buffered names if the path became a symlink while its
|
||||
// directory handle was open; descendants must never escape the root.
|
||||
|
||||
Reference in New Issue
Block a user