mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
perf: reuse collators when scanning Warp themes (#19441)
* perf: reuse collators when scanning Warp themes * perf(warp-themes): filter before collating and skip trivial sorts Warp discovery collated every entry in the user's home or %APPDATA%\\warp before discarding the non-Warp ones; filter first so ICU only sees candidate names (order is unchanged: filtering commutes with a stable total-order sort). Also skip the collator entirely for 0/1-entry directories and single-file dialog picks, and drop the sort that ran only to be thrown away when the preview budget expired. --------- Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local> Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com>
This commit is contained in:
@@ -0,0 +1,11 @@
|
||||
// Warp theme discovery walks user home and app-data trees, so callers filter to
|
||||
// the entries they want *before* sorting: same resulting order (the comparator is
|
||||
// a total order over a stable sort), without collating the hundreds of unrelated
|
||||
// names a home directory holds.
|
||||
export function sortDirectoryEntriesByName<T extends { name: string }>(entries: T[]): T[] {
|
||||
if (entries.length < 2) {
|
||||
return entries
|
||||
}
|
||||
const compare = new Intl.Collator(undefined, { sensitivity: 'base' }).compare
|
||||
return entries.sort((left, right) => compare(left.name, right.name))
|
||||
}
|
||||
@@ -43,6 +43,78 @@ describe('getWarpThemeDirectories', () => {
|
||||
readdirSyncMock.mockReturnValue([])
|
||||
})
|
||||
|
||||
it('sorts dynamic directories with one collator and preserves locale ties', () => {
|
||||
platformMock.mockReturnValue('darwin')
|
||||
const names = ['éclair', 'Eclair', 'item2', 'item10', 'Ångström', 'zebra', 'İstanbul'].map(
|
||||
(name) => `.warp-${name}`
|
||||
)
|
||||
readdirSyncMock.mockReturnValue(names.map(directoryEntry))
|
||||
const expected = [...names].sort((a, b) =>
|
||||
// oxlint-disable-next-line sort-comparator-performance/no-repeated-collator -- Preserve the old comparator as the parity oracle.
|
||||
a.localeCompare(b, undefined, { sensitivity: 'base' })
|
||||
)
|
||||
const NativeCollator = Intl.Collator
|
||||
const construct = vi.spyOn(Intl, 'Collator').mockImplementation(function (locales, options) {
|
||||
return new NativeCollator(locales, options)
|
||||
})
|
||||
const localeCompare = vi.spyOn(String.prototype, 'localeCompare')
|
||||
try {
|
||||
expect(getWarpThemeDirectories().slice(6)).toEqual(
|
||||
expected.map((name) => `/Users/alice/${name}/themes`)
|
||||
)
|
||||
expect(construct).toHaveBeenCalledExactlyOnceWith(undefined, { sensitivity: 'base' })
|
||||
expect(localeCompare).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
construct.mockRestore()
|
||||
localeCompare.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('collates only the Warp entries in a crowded home directory', () => {
|
||||
platformMock.mockReturnValue('darwin')
|
||||
const noise = Array.from({ length: 500 }, (_, index) => directoryEntry(`project-${index}`))
|
||||
const warpNames = ['.warp-zebra', '.warp-Ångström', '.warp-éclair']
|
||||
readdirSyncMock.mockReturnValue([...noise, ...warpNames.map(directoryEntry)])
|
||||
const expected = [...warpNames].sort((a, b) =>
|
||||
// oxlint-disable-next-line sort-comparator-performance/no-repeated-collator -- Preserve the old comparator as the parity oracle.
|
||||
a.localeCompare(b, undefined, { sensitivity: 'base' })
|
||||
)
|
||||
const NativeCollator = Intl.Collator
|
||||
const compares: string[][] = []
|
||||
vi.spyOn(Intl, 'Collator').mockImplementation(function (locales, options) {
|
||||
const collator = new NativeCollator(locales, options)
|
||||
return {
|
||||
...collator,
|
||||
compare: (left: string, right: string) => {
|
||||
compares.push([left, right])
|
||||
return collator.compare(left, right)
|
||||
}
|
||||
}
|
||||
})
|
||||
try {
|
||||
expect(getWarpThemeDirectories().slice(6)).toEqual(
|
||||
expected.map((name) => `/Users/alice/${name}/themes`)
|
||||
)
|
||||
// Filtering first keeps the crowd out of ICU: only Warp names are collated.
|
||||
expect(compares.flat().every((name) => name.startsWith('.warp'))).toBe(true)
|
||||
expect(compares.length).toBeLessThan(10)
|
||||
} finally {
|
||||
vi.restoreAllMocks()
|
||||
}
|
||||
})
|
||||
|
||||
it('skips a directory scan that finds no dynamic Warp entries', () => {
|
||||
platformMock.mockReturnValue('darwin')
|
||||
readdirSyncMock.mockReturnValue([directoryEntry('Documents'), fileEntry('.warprc')])
|
||||
const construct = vi.spyOn(Intl, 'Collator')
|
||||
try {
|
||||
expect(getWarpThemeDirectories()).toHaveLength(6)
|
||||
expect(construct).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
vi.restoreAllMocks()
|
||||
}
|
||||
})
|
||||
|
||||
it('returns macOS Warp channel theme directories in stable-first order', () => {
|
||||
platformMock.mockReturnValue('darwin')
|
||||
expect(getWarpThemeDirectories()).toEqual([
|
||||
|
||||
@@ -2,6 +2,7 @@ import { readdirSync } from 'node:fs'
|
||||
import type { Dirent } from 'node:fs'
|
||||
import { homedir, platform } from 'node:os'
|
||||
import path from 'node:path'
|
||||
import { sortDirectoryEntriesByName } from './directory-entry-order'
|
||||
|
||||
const WARP_CHANNELS = [
|
||||
{ macName: '.warp', linuxName: 'warp-terminal', windowsName: 'Warp' },
|
||||
@@ -16,10 +17,13 @@ const WARP_CHANNELS = [
|
||||
}
|
||||
]
|
||||
|
||||
function readDirectoryEntries(directoryPath: string): Dirent[] {
|
||||
function readDirectoryEntries(
|
||||
directoryPath: string,
|
||||
include: (entry: Dirent) => boolean
|
||||
): Dirent[] {
|
||||
try {
|
||||
return readdirSync(directoryPath, { withFileTypes: true }).sort((left, right) =>
|
||||
left.name.localeCompare(right.name, undefined, { sensitivity: 'base' })
|
||||
return sortDirectoryEntriesByName(
|
||||
readdirSync(directoryPath, { withFileTypes: true }).filter(include)
|
||||
)
|
||||
} catch {
|
||||
return []
|
||||
@@ -57,9 +61,10 @@ function getMacWarpThemeDirectories(home: string): string[] {
|
||||
return warpThemeDirectoriesFromDataHomes(
|
||||
[
|
||||
...WARP_CHANNELS.map((channel) => pathImpl.join(home, channel.macName)),
|
||||
...readDirectoryEntries(home)
|
||||
.filter((entry) => entry.isDirectory() && entry.name.startsWith('.warp'))
|
||||
.map((entry) => pathImpl.join(home, entry.name))
|
||||
...readDirectoryEntries(
|
||||
home,
|
||||
(entry) => entry.isDirectory() && entry.name.startsWith('.warp')
|
||||
).map((entry) => pathImpl.join(home, entry.name))
|
||||
],
|
||||
pathImpl
|
||||
)
|
||||
@@ -77,13 +82,11 @@ function getLinuxWarpThemeDirectories(home: string): string[] {
|
||||
return warpThemeDirectoriesFromDataHomes(
|
||||
[
|
||||
...WARP_CHANNELS.map((channel) => pathImpl.join(dataHome, channel.linuxName)),
|
||||
...readDirectoryEntries(dataHome)
|
||||
.filter(
|
||||
(entry) =>
|
||||
entry.isDirectory() &&
|
||||
(entry.name === 'warp-terminal' || entry.name.startsWith('warp-'))
|
||||
)
|
||||
.map((entry) => pathImpl.join(dataHome, entry.name))
|
||||
...readDirectoryEntries(
|
||||
dataHome,
|
||||
(entry) =>
|
||||
entry.isDirectory() && (entry.name === 'warp-terminal' || entry.name.startsWith('warp-'))
|
||||
).map((entry) => pathImpl.join(dataHome, entry.name))
|
||||
],
|
||||
pathImpl
|
||||
)
|
||||
@@ -102,10 +105,7 @@ function getWindowsWarpThemeDirectories(home: string): string[] {
|
||||
path.win32
|
||||
)
|
||||
}
|
||||
for (const entry of readDirectoryEntries(warpAppData)) {
|
||||
if (!entry.isDirectory()) {
|
||||
continue
|
||||
}
|
||||
for (const entry of readDirectoryEntries(warpAppData, (entry) => entry.isDirectory())) {
|
||||
addDedupeDirectory(
|
||||
directories,
|
||||
seenDirectories,
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
import path from 'node:path'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
vi.mock('electron', () => ({ BrowserWindow: {}, dialog: {} }))
|
||||
|
||||
import { createManualWarpThemeFileCandidates } from './manual-warp-theme-files'
|
||||
|
||||
describe('manual theme ordering', () => {
|
||||
it('reuses one collator for labels and path ties without changing the selected order', () => {
|
||||
const names = ['éclair', 'Eclair', 'item2', 'item10', 'Ångström', 'zebra', 'İstanbul']
|
||||
const paths = Array.from({ length: 200 }, (_, index) =>
|
||||
path.join('themes', names[index % names.length]!, `${names[(index * 3) % names.length]}.yaml`)
|
||||
)
|
||||
const expected = [...paths].sort(
|
||||
(a, b) =>
|
||||
// oxlint-disable-next-line sort-comparator-performance/no-repeated-collator -- Preserve the old comparator as the parity oracle.
|
||||
path.basename(a).localeCompare(path.basename(b), undefined, { sensitivity: 'base' }) ||
|
||||
// oxlint-disable-next-line sort-comparator-performance/no-repeated-collator -- Preserve the old comparator as the parity oracle.
|
||||
a.localeCompare(b, undefined, { sensitivity: 'base' })
|
||||
)
|
||||
const NativeCollator = Intl.Collator
|
||||
const construct = vi.spyOn(Intl, 'Collator').mockImplementation(function (locales, options) {
|
||||
return new NativeCollator(locales, options)
|
||||
})
|
||||
const localeCompare = vi.spyOn(String.prototype, 'localeCompare')
|
||||
try {
|
||||
expect(createManualWarpThemeFileCandidates(paths).map((file) => file.path)).toEqual(expected)
|
||||
expect(construct).toHaveBeenCalledExactlyOnceWith(undefined, { sensitivity: 'base' })
|
||||
expect(localeCompare).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
vi.restoreAllMocks()
|
||||
}
|
||||
})
|
||||
|
||||
it('returns a single dialog selection without collating', () => {
|
||||
const construct = vi.spyOn(Intl, 'Collator')
|
||||
try {
|
||||
expect(createManualWarpThemeFileCandidates([]).map((file) => file.path)).toEqual([])
|
||||
expect(createManualWarpThemeFileCandidates(['a/one.yaml']).map((file) => file.path)).toEqual([
|
||||
'a/one.yaml'
|
||||
])
|
||||
expect(construct).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
vi.restoreAllMocks()
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -2,29 +2,28 @@ import { createHash } from 'node:crypto'
|
||||
import path from 'node:path'
|
||||
import { BrowserWindow, dialog, type OpenDialogOptions, type WebContents } from 'electron'
|
||||
import type { WarpThemeImportSkippedFile } from '../../shared/terminal-custom-themes'
|
||||
import {
|
||||
compareThemeFileLabels,
|
||||
isYamlFile,
|
||||
MAX_THEME_FILES,
|
||||
type ThemeFileCandidate
|
||||
} from './theme-file-scanner'
|
||||
import { isYamlFile, MAX_THEME_FILES, type ThemeFileCandidate } from './theme-file-scanner'
|
||||
|
||||
export function createManualWarpThemeFileCandidates(filePaths: string[]): ThemeFileCandidate[] {
|
||||
return filePaths
|
||||
.map((filePath) => ({
|
||||
path: filePath,
|
||||
label: path.basename(filePath),
|
||||
contentHashDiscriminator: true
|
||||
}))
|
||||
.sort((left, right) => {
|
||||
const labelComparison = compareThemeFileLabels(left, right)
|
||||
if (labelComparison !== 0) {
|
||||
return labelComparison
|
||||
}
|
||||
// Why: manual dialogs can return selections in click order. Sort only in
|
||||
// main so duplicate basenames get deterministic IDs without persisting paths.
|
||||
return left.path.localeCompare(right.path, undefined, { sensitivity: 'base' })
|
||||
})
|
||||
const candidates = filePaths.map((filePath) => ({
|
||||
path: filePath,
|
||||
label: path.basename(filePath),
|
||||
contentHashDiscriminator: true
|
||||
}))
|
||||
// Picking a single file is the common dialog outcome and needs no collation.
|
||||
if (candidates.length < 2) {
|
||||
return candidates
|
||||
}
|
||||
const compareLabels = new Intl.Collator(undefined, { sensitivity: 'base' }).compare
|
||||
return candidates.sort((left, right) => {
|
||||
const labelComparison = compareLabels(left.label, right.label)
|
||||
if (labelComparison !== 0) {
|
||||
return labelComparison
|
||||
}
|
||||
// Why: manual dialogs can return selections in click order. Sort only in
|
||||
// main so duplicate basenames get deterministic IDs without persisting paths.
|
||||
return compareLabels(left.path, right.path)
|
||||
})
|
||||
}
|
||||
|
||||
export function manualWarpThemeContentDiscriminator(label: string, content: string): string {
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
import { mkdir, mkdtemp, readdir, rm, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import path from 'node:path'
|
||||
import { expect, it, vi } from 'vitest'
|
||||
import { scanWarpThemeDirectory } from './theme-file-scanner'
|
||||
|
||||
it('reuses one collator per directory while preserving capped scan order', async () => {
|
||||
const directory = await mkdtemp(path.join(tmpdir(), 'orca-theme-order-'))
|
||||
const names = ['éclair', 'item2', 'item10', 'Ångström', 'zebra', 'İstanbul']
|
||||
try {
|
||||
await Promise.all(names.map((name) => writeFile(path.join(directory, `${name}.yaml`), '')))
|
||||
await mkdir(path.join(directory, 'nested'))
|
||||
await writeFile(path.join(directory, 'nested', 'theme.yaml'), '')
|
||||
const expected = (await readdir(directory))
|
||||
// oxlint-disable-next-line sort-comparator-performance/no-repeated-collator -- Preserve the old comparator as the parity oracle.
|
||||
.sort((a, b) => a.localeCompare(b, undefined, { sensitivity: 'base' }))
|
||||
.map((name) => (name === 'nested' ? path.join(name, 'theme.yaml') : name))
|
||||
const NativeCollator = Intl.Collator
|
||||
const construct = vi.spyOn(Intl, 'Collator').mockImplementation(function (locales, options) {
|
||||
return new NativeCollator(locales, options)
|
||||
})
|
||||
const localeCompare = vi.spyOn(String.prototype, 'localeCompare')
|
||||
try {
|
||||
const result = await scanWarpThemeDirectory(directory, undefined, { themeFileLimit: 6 })
|
||||
expect(result.files.map((file) => file.label)).toEqual(expected.slice(0, 6))
|
||||
expect(result.themeFileLimitHit).toBe(true)
|
||||
// One directory needs collation; the single-entry nested folder needs none.
|
||||
expect(construct).toHaveBeenCalledExactlyOnceWith(undefined, { sensitivity: 'base' })
|
||||
expect(localeCompare).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
vi.restoreAllMocks()
|
||||
}
|
||||
} finally {
|
||||
await rm(directory, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
@@ -2,6 +2,7 @@ import { opendir } from 'node:fs/promises'
|
||||
import type { Dirent } from 'node:fs'
|
||||
import path from 'node:path'
|
||||
import type { WarpThemeImportSkippedFile } from '../../shared/terminal-custom-themes'
|
||||
import { sortDirectoryEntriesByName } from './directory-entry-order'
|
||||
|
||||
export const MAX_THEME_FILES = 200
|
||||
const MAX_THEME_DIRECTORY_DEPTH = 3
|
||||
@@ -44,17 +45,6 @@ export function isYamlFile(filePath: string): boolean {
|
||||
return YAML_EXTENSIONS.has(path.extname(filePath).toLowerCase())
|
||||
}
|
||||
|
||||
export function compareThemeFileLabels(
|
||||
left: ThemeFileCandidate,
|
||||
right: ThemeFileCandidate
|
||||
): number {
|
||||
return left.label.localeCompare(right.label, undefined, { sensitivity: 'base' })
|
||||
}
|
||||
|
||||
function compareDirentNames(left: Dirent<string>, right: Dirent<string>): number {
|
||||
return left.name.localeCompare(right.name, undefined, { sensitivity: 'base' })
|
||||
}
|
||||
|
||||
function isYamlFileEntry(entry: Dirent<string>): boolean {
|
||||
return (entry.isFile() || entry.isSymbolicLink()) && isYamlFile(entry.name)
|
||||
}
|
||||
@@ -141,10 +131,10 @@ async function collectYamlFilesFromDirectory(
|
||||
return
|
||||
}
|
||||
|
||||
const sortedEntries = entries.sort(compareDirentNames)
|
||||
if (previewBudgetExpiredWhileReading) {
|
||||
return
|
||||
}
|
||||
const sortedEntries = sortDirectoryEntriesByName(entries)
|
||||
if (entryLimitHit && !budget.entryLimitReported) {
|
||||
skippedFiles.push({
|
||||
label: relativeDirectory || sourceLabel,
|
||||
|
||||
Reference in New Issue
Block a user