Skip to content

Commit 9b8f9dd

Browse files
committed
Search: David's fix-up rounds 2 part B, 3, and 4
Round 2 part B (the 17 items the round 2 first agent left): the Size and Modified filter popovers became list-style column grids ("any/>=/<=/between" plus a preset list plus a dynamic unit column); "Use current folder" gained a smart fallback that walks the active pane's history past any search-results entry; the result list got header alignment, ResizeObserver-driven path pill measurement, and per-row context menu fixes; the snapshot pane got the full keyboard set (PgUp/PgDn, Home/End, Space, Shift+Up/Down, F3/F4) plus a regression test for the Cmd+A off-by-one that round 2 had quietly fixed without one. Round 3 (David's next pass after running it): "Search ⏎" instead of "⏎Search⏎", AI search now overwrites the matching hand-typed buffer, filter chip respects kB vs KB, path pill widths re-measure after RAF to stop the late-collapse, Modified popover's Custom row is only selected when the user explicitly clicks it, the snapshot pane's breadcrumb shows "Search results / <label>" with the volume selector and the path the right way round, recent searches strip uses a greedy layout with always-on left label and trailing All searches button, Modified preset labels read as concrete dates ("today 0:00", "this Monday 0:00", "1st of May 0:00") with first-day-of-week from Intl.Locale, "Hide system folders" became "Hide boring folders" with the full exclude list in the tooltip, path column matches the filename font size, and the snapshot pane focuses itself when Show all in main window lands it. Round 4 (David's screenshot review): the "Search results no longer available" pane was a real bug. The fresh logs caught it: `read_directory_with_progress: ..., volume_id=search-results, path=/Library/...` and the matching "Dropping stale onPathChange" warning. When the user activated a real folder from a snapshot pane (or the search dialog's exit landed on a real path while the active pane was still on the snapshot volume), FilePane.handleNavigate and DualPaneExplorer.navigateToPath were setting currentPath to a real filesystem location without switching volumeId off `search-results`. The pane then tried to render SearchResultsView against a path that wasn't a snapshot URL, the snapshot-id extractor returned null, and the user saw the friendly error pane. The fix routes both call sites through the volume-change machinery. A new pure helper `isCrossVolumeNavigation(currentVolumeId, targetPath)` (in `pane/snapshot-pane-navigation.ts`) decides whether we're leaving the snapshot volume; when it returns true the caller resolves the real volume via `resolvePathVolume` and reconfigures the pane via `onVolumeChange` / `handleVolumeChange` BEFORE loading the target path. Tests pin the decision with the exact `/Library/Developer/CommandLineTools/...` path that reproduced the user bug. Also in round 4: the "Loading drive index..." status bar duplicate goes away (status bar stays empty whenever the content area is the source of truth, same rule as Searching and No-files-match). Cmd+Enter and Shift+Enter are explicit no-ops in the dialog; the earlier "Cmd+Enter runs AI" shortcut is gone, bare Enter is the only key that runs a search or opens the cursor row. The eject-volume worktree apparently churns the same files; left it alone.
1 parent 5c35d9e commit 9b8f9dd

43 files changed

Lines changed: 4119 additions & 436 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

apps/desktop/src/lib/file-explorer/CLAUDE.md

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,17 @@ Navigation:
182182
the underlying path. ⌘[ returns to the snapshot view; the snapshot's still pinned by the history entry, so the view
183183
re-renders from memory with no re-query.
184184

185+
R4 cross-volume bug fix (`pane/snapshot-pane-navigation.ts::isCrossVolumeNavigation`): when the user activates a real
186+
folder from a snapshot pane (or the search dialog's "Go to file" exit lands on a real path while the active pane is
187+
still on the snapshot volume), the navigation MUST route through the volume-change machinery. Two call sites:
188+
`FilePane.handleNavigate` (Enter / double-click on a row) calls `resolvePathVolume` and then `onVolumeChange`;
189+
`DualPaneExplorer.navigateToPath` (the dialog's exit + MCP `nav_to_path`) does the same via the internal
190+
`handleVolumeChange`. Without this, the pane ends up with `volumeId === 'search-results'` while `path` points at a real
191+
filesystem location, and `SearchResultsView` shows "Search results no longer available" (the snapshot-id extractor
192+
returns null because the path doesn't start with `search-results://`). The fresh-logs symptom was
193+
`read_directory_with_progress: …, volume_id=search-results, path=/Library/…` followed by
194+
`Dropping stale onPathChange on search-results pane: /Library/…`.
195+
185196
DualPaneExplorer extensions:
186197

187198
- `applyPathChange` extends its virtual-volume branch from `currentVolumeId === 'network'` to also accept
@@ -198,12 +209,48 @@ FilePane suppresses the trailing path segments entirely for search-results panes
198209

199210
Source-side operations (M8d): selection works in the snapshot pane (Space, Insert, Shift+click range, Cmd+click toggle,
200211
Cmd+A / Cmd+Shift+A). `effectiveTotalCount` returns the snapshot's entry count so range selection spans the result set.
201-
Cmd+C / Cmd+X call the paths-by-value clipboard IPCs (`copy_paths_to_clipboard` / `cut_paths_to_clipboard`) instead of
202-
the listing-id-keyed family. F5 / F6 (the unified transfer dialog) detect `volumeId === 'search-results'` and call
203-
`transfer-operations::buildTransferPropsFromSnapshot` with paths resolved from `snapshot-store::resolveSnapshotPaths`;
204-
the existing `copy_files` / `move_files` IPCs already accept paths-by-value, so no IPC change was needed for the
205-
transfer path. Drag-out uses the new `'paths'` drag context (see `drag/CLAUDE.md`) which routes through
206-
`start_drag_paths`. Post-move snapshot cleanup is the M8c hook in `dialog-state::handleTransferComplete`.
212+
213+
Round 2 P-series keyboard contract: `FilePane.handleSearchResultsKeyDown` routes through the pure
214+
`pane/search-results-keys.ts::computeSearchPaneKeyAction` helper, which translates each keypress into an action enum
215+
(`move-cursor`, `open-cursor`, `toggle-selection-at-cursor`, `toggle-selection-and-advance`, `view-file`, `edit-file`,
216+
`noop`). Splitting dispatch from side effects keeps the keyboard contract unit-testable without spinning up the whole
217+
pane. Covered: PgUp / PgDn (visible-page step), Home / End, Shift+Up / Shift+Down (extends selection via the same
218+
toggle-and-fill helper the regular pane uses), Space (toggle), Insert (toggle + advance), F3 (view), F4 (edit). Left /
219+
Right return `noop` (no parent-folder semantics in a flat snapshot; the caller still calls `preventDefault` so the
220+
regular full-pane handler can't jump to first / last row underneath). Cmd+A flows through the unified command dispatch
221+
in `command-dispatch.ts` as before.
222+
223+
Round 2 P6 fix: `hasParent` now `false` for `search-results` panes. Previously the path comparison
224+
`currentPath !== effectiveVolumeRoot` was true (a `search-results://sr-N` URL never matches a real volume root), so
225+
`selection.selectAll(hasParent=true, ...)` skipped index 0. Setting `hasParent = false` for snapshot panes restores the
226+
off-by-one and keeps `..` row handling untouched for real panes.
227+
228+
R3 T1: the round-2 fix was correct but had no test. The derivation now lives in `pane/has-parent.ts` as
229+
`computeHasParent({ isSearchResultsView, currentPath, effectiveVolumeRoot })` and is pinned by
230+
`pane/has-parent.test.ts`, which also covers the integration with `selection.selectAll` (snapshot pane → all 5 indices
231+
selected; non-snapshot pane with hasParent → indices 1..4, skipping the `..` row at 0).
232+
233+
R3 B6: the search-results breadcrumb shape changed. The volume selector reads the static "Search results" label (set by
234+
`VolumeBreadcrumb.svelte::currentVolume`) and `FilePane.svelte::breadcrumbDisplayPath` renders the snapshot's friendly
235+
label (`*.svelte`, the AI title, ...) as the path. The `breadcrumbSegments` derived produces a single-segment list for
236+
search-results panes so a label containing `/` (regex mode like `/foo\/bar/`) doesn't get broken into path-style
237+
segments. Round 2 had these inverted (label in the volume slot, empty path).
238+
239+
Round 2 P8 / P9 / P10 (context-menu wiring):
240+
241+
- `DualPaneExplorer.getFileAndPathUnderCursor()` now prefers the pane-reported `getPathUnderCursor()` over the round-1
242+
`${currentPath}/${filename}` concatenation. Without this fix, `file.showInFinder` / `file.copyPath` / `file.edit` on a
243+
snapshot pane built a `search-results://sr-N/<name>` path that downstream IPCs couldn't act on.
244+
- `SearchResultsView.svelte::onContextMenu` now hands the Rust menu builder the path's basename, not the adapted entry's
245+
`name` (which is the friendly full path like `~/Library/.../test.md`). Without this, the menu label read
246+
`Copy ~/Library/.../test.md` instead of `Copy test.md`. The action itself was already correct because
247+
`entryUnderCursor.name` on a snapshot pane mirrors the raw `SearchResultEntry.name` (a basename). Cmd+C / Cmd+X call
248+
the paths-by-value clipboard IPCs (`copy_paths_to_clipboard` / `cut_paths_to_clipboard`) instead of the
249+
listing-id-keyed family. F5 / F6 (the unified transfer dialog) detect `volumeId === 'search-results'` and call
250+
`transfer-operations::buildTransferPropsFromSnapshot` with paths resolved from `snapshot-store::resolveSnapshotPaths`;
251+
the existing `copy_files` / `move_files` IPCs already accept paths-by-value, so no IPC change was needed for the
252+
transfer path. Drag-out uses the new `'paths'` drag context (see `drag/CLAUDE.md`) which routes through
253+
`start_drag_paths`. Post-move snapshot cleanup is the M8c hook in `dialog-state::handleTransferComplete`.
207254

208255
For the dialog-side wiring see [`apps/desktop/src/lib/search/CLAUDE.md`](../../search/CLAUDE.md).
209256

apps/desktop/src/lib/file-explorer/navigation/VolumeBreadcrumb.svelte

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
requestVolumeRefresh,
3636
} from '$lib/stores/volume-store.svelte'
3737
import { groupByCategory, getIconForVolume } from './volume-grouping'
38-
import { getSnapshot } from '$lib/search/snapshot-store.svelte'
3938
import { createVolumeSpaceManager } from './volume-space-manager.svelte'
4039
import {
4140
createBreadcrumbPopupController,
@@ -99,8 +98,12 @@
9998
? { id: 'network', name: 'Network', path: 'smb://', category: 'network' as const, isEjectable: false }
10099
: volumeId === 'search-results'
101100
? {
101+
// R3 B6: the volume selector reads "Search results", a
102+
// generic noun matching every other volume's slot. The
103+
// search-specific label (the AI title / pattern) moved to
104+
// the path slot in `FilePane.svelte::breadcrumbDisplayPath`.
102105
id: 'search-results',
103-
name: 'Search',
106+
name: 'Search results',
104107
path: 'search-results://',
105108
category: 'network' as const,
106109
isEjectable: false,
@@ -110,20 +113,16 @@
110113
)
111114
112115
/**
113-
* Search-results breadcrumb label: extract the snapshot id from `currentPath`
114-
* (`search-results://<id>`) and look up the snapshot's friendly label. Falls
115-
* back to "Search" when the snapshot is missing — same defensive posture as
116-
* the SearchResultsView itself. Re-derives reactively on path / volume changes.
116+
* R3 B6: the snapshot's friendly label is now rendered as the trailing
117+
* path text (in `FilePane.svelte`'s `breadcrumbDisplayPath`). The volume
118+
* selector here reads the static "Search results" label so the
119+
* volume-selector slot describes the KIND of volume (matching every
120+
* other volume: "Network", "Macintosh HD", an MTP device name) and the
121+
* path slot carries the QUERY-specific label. Round 2 had these
122+
* inverted, which David flagged in the round 3 brief (the snapshot
123+
* label was the volume name and the path was empty).
117124
*/
118-
const searchResultsLabel = $derived.by(() => {
119-
if (volumeId !== 'search-results') return null
120-
const prefix = 'search-results://'
121-
if (!currentPath.startsWith(prefix)) return null
122-
const id = currentPath.slice(prefix.length)
123-
return getSnapshot(id)?.label ?? 'Search'
124-
})
125-
126-
const currentVolumeName = $derived(searchResultsLabel ?? currentVolume?.name ?? 'Volume')
125+
const currentVolumeName = $derived(currentVolume?.name ?? 'Volume')
127126
const currentVolumeIcon = $derived(getIconForVolume(currentVolume))
128127
129128
// Generic macOS folder icon used as fallback when a volume has no icon (for example,

apps/desktop/src/lib/file-explorer/pane/DualPaneExplorer.svelte

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { pluralize } from '$lib/utils/pluralize'
44
import FilePane from './FilePane.svelte'
55
import type { FilePaneAPI } from './types'
6+
import { isCrossVolumeNavigation } from './snapshot-pane-navigation'
67
import PaneResizer from './PaneResizer.svelte'
78
import LoadingIcon from '$lib/ui/LoadingIcon.svelte'
89
import DialogManager from './DialogManager.svelte'
@@ -51,6 +52,7 @@
5152
import { resolveValidPath } from '../navigation/path-resolution'
5253
import { getSnapshot, resolveSnapshotPaths } from '$lib/search/snapshot-store.svelte'
5354
import { SEARCH_RESULTS_NOT_A_FOLDER_TOAST } from '$lib/search/capabilities'
55+
import { resolveSearchableFolder } from '$lib/search/searchable-folder'
5456
5557
import {
5658
createHistory,
@@ -2293,14 +2295,24 @@
22932295
22942296
/**
22952297
* Get the path and filename of the file under the cursor in the focused pane.
2298+
*
2299+
* For the search-results virtual pane the underlying entries carry their own absolute
2300+
* `path`, and `currentPath` is the opaque `search-results://<id>` URL — concatenating
2301+
* the two would produce `search-results://sr-1/test.md`, which downstream commands
2302+
* (`showInFinder`, `openInEditor`, `copyToClipboard`) can't act on. Round-2 P8 / P9.
2303+
* We prefer the pane-reported path under the cursor when present and fall back to the
2304+
* legacy `${currentPath}/${filename}` form for regular panes.
22962305
*/
22972306
export function getFileAndPathUnderCursor(): { path: string; filename: string } | null {
22982307
const paneRef = getPaneRef(focusedPane)
2299-
const currentPath = getPanePath(focusedPane)
23002308
const filename = paneRef?.getFilenameUnderCursor()
23012309
if (!filename || filename === '..') return null
2302-
const path = `${currentPath}/${filename}`
2303-
return { path, filename }
2310+
const cursorPath = paneRef?.getPathUnderCursor()
2311+
if (cursorPath) {
2312+
return { path: cursorPath, filename }
2313+
}
2314+
const currentPath = getPanePath(focusedPane)
2315+
return { path: `${currentPath}/${filename}`, filename }
23042316
}
23052317
23062318
/**
@@ -2398,6 +2410,26 @@
23982410
return getPanePath(focusedPane)
23992411
}
24002412
2413+
/**
2414+
* Returns the "current folder" the Search dialog's `Search in → Use current folder`
2415+
* action should act on. Round-2 D12: when the focused pane is a `search-results://`
2416+
* snapshot, walks back through history for the most recent real folder; when none is
2417+
* available, surfaces a disabled state with a tooltip. See
2418+
* `lib/search/searchable-folder.ts` for the pure helper this delegates to.
2419+
*/
2420+
// noinspection JSUnusedGlobalSymbols -- consumed by +page.svelte via explorerRef
2421+
export function getFocusedPaneSearchableFolder(): {
2422+
path: string | null
2423+
disabled: boolean
2424+
disabledReason: string
2425+
} {
2426+
const history = getPaneHistory(focusedPane)
2427+
return resolveSearchableFolder({
2428+
currentPath: getPanePath(focusedPane),
2429+
history: history.stack.map((e) => e.path),
2430+
})
2431+
}
2432+
24012433
/** Volume id of the focused pane's active tab. Used by Quick Look's IPC gate. */
24022434
export function getFocusedPaneVolumeId(): string {
24032435
return getPaneVolumeId(focusedPane)
@@ -2571,6 +2603,30 @@
25712603
return `Pane is on the ${volumeName ?? volumeId} volume. Use select_volume to switch to a local volume first.`
25722604
}
25732605
2606+
// R4: when the pane is on the snapshot volume and the caller asks us to navigate
2607+
// to a real path (the search dialog's "Go to file" exit path is the common
2608+
// case: `+page.svelte::handleSearchNavigate` calls us with the file's parent
2609+
// directory). Resolve the real volume and route through `handleVolumeChange`
2610+
// so the pane reconfigures BEFORE it tries to load a real path. Without this,
2611+
// the FilePane would loadDirectory the real path while volumeId stays as
2612+
// `search-results`, leaving `SearchResultsView` rendering "Search results no
2613+
// longer available" against a path that isn't a snapshot URL.
2614+
if (isCrossVolumeNavigation(volumeId, path)) {
2615+
return (async () => {
2616+
try {
2617+
const result = await resolvePathVolume(path)
2618+
const volume = result.volume
2619+
if (!volume) {
2620+
log.warn(`navigateToPath: no volume resolved for ${path}; cannot leave snapshot pane`)
2621+
return
2622+
}
2623+
handleVolumeChange(pane, volume.id, volume.path, path)
2624+
} catch (err) {
2625+
log.error(`navigateToPath: resolvePathVolume failed for ${path}: ${String(err)}`)
2626+
}
2627+
})()
2628+
}
2629+
25742630
const mtpError = validateMtpNavigation(path, volumeId, volumeName)
25752631
if (mtpError) return mtpError
25762632

0 commit comments

Comments
 (0)