From 750aef0ef5b70bcf8f02dfd10b4c872fca81b90e Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 6 Sep 2026 10:23:15 +0000 Subject: [PATCH] feat(desktop): open a whole selection, together or one at a time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Open with" now acts on the selection rather than the one row you happened to right-click, and the handler is chosen once for all of it. Being asked which application to use twelve times in a row is not a feature. Two modes, because a set of files is not one kind of thing: - **All at once** hands every file to the application in a single invocation, so an entry taking %F or %U gets them together, which is what "open all of them" means to the application as well as to the person asking. - **One at a time** opens a file, waits until the application is finished with it, and then opens the next. A series of episodes is watched in order and opening twelve at once is never what anyone meant. The default is picked from what the files are: video or audio defaults to one at a time, everything else to all at once, and a single file is not offered a mode at all. A mixed selection has no shared type, so it gets all at once and each file opens in its own default application. **Waiting for an application to finish, without reimplementing Exec.** `gio launch` returns almost immediately (measured at 17ms): it hands the file to the desktop's launcher and exits. But the application it starts inherits our stdio, so the PIPES close when the application does, measured at 3018ms for a three second app. Waiting on the pipes rather than the process is therefore a reliable "done with this file" signal, and it costs nothing: the alternative is parsing the desktop entry's Exec field codes (%f, %F, %u, %U) ourselves, which is a small parser and a large number of ways to hand an application the wrong thing. **The case that breaks it, handled rather than ignored.** A single-instance application hands the file to the copy already running and exits at once, so there is no "it closed" to wait for. A close under one second is read as that rather than as somebody watching an episode in under a second: the run parks and offers Open next instead of advancing. Auto-advancing there would dump the whole list into the running player, which is the exact thing this mode exists to prevent. The dialog says so in as many words rather than looking stalled. Both fixture bugs found while testing this are worth knowing, and are recorded in the test that hit them: - `Exec=sleep 1` does not work as a fake player, because gio appends the file to the Exec line and `sleep 1 /tmp/a.mkv` dies instantly with "invalid time interval". The code then correctly reads that as a hand-off, so the naive fixture tests the opposite of what it appears to. - A one second app is *under* the hand-off threshold. The fixture has to be clearly above it or the test asserts the wrong branch. That also moved the threshold from 1500ms to 1000ms, which sits well clear of a real hand-off (tens of ms) and nowhere near somebody actually using a file. Verified in headless Chromium under the app's real CSP: selecting three .mkv files names the count in the menu ("Open 3 files with…"), the dialog defaults to one at a time because they are video, the run carries all three paths and a null handler, the hand-off parks with "1 of 3" and an explanation, Open next advances exactly once, and a lone markdown file is offered no mode at all. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VScug5VRbcTuhiAoieeQ52 --- apps/desktop/electron/main/ipc.ts | 22 +- .../main/services/open-series.test.ts | 131 +++++++++++ .../electron/main/services/open-with.ts | 206 +++++++++++++++++- apps/desktop/electron/preload/index.ts | 14 +- apps/desktop/electron/shared/contract.ts | 37 +++- .../src/components/open-with-dialog.tsx | Bin 6782 -> 12736 bytes apps/desktop/src/components/pane.tsx | 39 +++- apps/desktop/src/lib/api.ts | 32 ++- apps/desktop/src/lib/open-mode.test.ts | 41 ++++ apps/desktop/src/lib/open-mode.ts | 23 ++ 10 files changed, 508 insertions(+), 37 deletions(-) create mode 100644 apps/desktop/electron/main/services/open-series.test.ts create mode 100644 apps/desktop/src/lib/open-mode.test.ts create mode 100644 apps/desktop/src/lib/open-mode.ts diff --git a/apps/desktop/electron/main/ipc.ts b/apps/desktop/electron/main/ipc.ts index 7295bc5..22ff55e 100644 --- a/apps/desktop/electron/main/ipc.ts +++ b/apps/desktop/electron/main/ipc.ts @@ -17,6 +17,7 @@ import { FleetRunIdSchema, IPC, JobIdSchema, + OpenSeriesRequestSchema, OpenWithRequestSchema, PreviewRequestSchema, PathSchema, @@ -44,7 +45,7 @@ import { } from './services/fleet.js' import { browserFor, dropSession, sessionFor } from './services/sessions.js' import { store } from './services/store.js' -import { handlersFor, openWith } from './services/open-with.js' +import { advanceSeries, handlersFor, openSeries, openWith, stopSeries } from './services/open-with.js' import { cancelPreview, cancelTransfer, previewTransfer, saveProfile, startTransfer } from './services/transfers.js' /** @@ -304,13 +305,26 @@ export function registerIpc(): void { // Local only. A pane pointed at a server has no path this machine can open, // and the renderer disables the item there rather than sending a remote one. - handle(IPC.fsHandlers, z.object({ path: PathSchema }), async ({ path }) => handlersFor(resolveLocalPath(path))) + handle(IPC.fsHandlers, z.object({ paths: z.array(PathSchema).min(1).max(500) }), async ({ paths }) => + handlersFor(paths.map(resolveLocalPath)), + ) + + handle(IPC.fsOpenWith, OpenWithRequestSchema, async ({ paths, handlerId }) => { + await openWith(paths.map(resolveLocalPath), handlerId) + return true + }) - handle(IPC.fsOpenWith, OpenWithRequestSchema, async ({ path, handlerId }) => { - await openWith(resolveLocalPath(path), handlerId) + handle(IPC.fsOpenSeries, OpenSeriesRequestSchema, async ({ seriesId, paths, handlerId }, event) => { + // Not awaited: the run outlives the call by design, and reports over + // eventOpenSeries as it goes. + void openSeries(seriesId, paths.map(resolveLocalPath), handlerId, event.sender) return true }) + handle(IPC.fsOpenSeriesAdvance, z.object({ seriesId: JobIdSchema }), async ({ seriesId }) => advanceSeries(seriesId)) + + handle(IPC.fsOpenSeriesStop, z.object({ seriesId: JobIdSchema }), async ({ seriesId }) => stopSeries(seriesId)) + // --- transfers ----------------------------------------------------------- handle(IPC.transfersPreview, PreviewRequestSchema, async (request, event) => previewTransfer(request, event.sender)) diff --git a/apps/desktop/electron/main/services/open-series.test.ts b/apps/desktop/electron/main/services/open-series.test.ts new file mode 100644 index 0000000..8fa620b --- /dev/null +++ b/apps/desktop/electron/main/services/open-series.test.ts @@ -0,0 +1,131 @@ +import { mkdirSync, mkdtempSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it, vi } from 'vitest' + +vi.mock('electron', () => ({ shell: { openPath: async () => '' } })) + +const applications = mkdtempSync(join(tmpdir(), 'diskpush-apps-')) +process.env.XDG_DATA_HOME = join(applications, 'home') +process.env.XDG_DATA_DIRS = applications +const appsDir = join(applications, 'applications') +mkdirSync(appsDir, { recursive: true }) + +/** + * A fake "player": a script that ignores its argument and takes `seconds`. + * + * It has to be a script rather than `Exec=sleep 1`, because gio appends the + * file to the Exec line and `sleep 1 /tmp/a.mkv` dies instantly with "invalid + * time interval" -- which the code then correctly reads as a hand-off, so the + * naive fixture tests the opposite of what it looks like it tests. + */ +function fakeApp(name: string, seconds: number) { + const script = join(applications, `${name}.sh`) + writeFileSync(script, `#!/bin/sh\nsleep ${seconds}\n`, { mode: 0o755 }) + writeFileSync( + join(appsDir, name), + `[Desktop Entry]\nType=Application\nName=${name}\nExec=${script}\nTerminal=false\n`, + ) +} + +// Comfortably above HANDOFF_MS, so it reads as somebody using the file. +fakeApp('slow.desktop', 2) +fakeApp('instant.desktop', 0) + +const { openSeries, advanceSeries, stopSeries } = await import('./open-with.js') + +type Event = { type: string; index?: number; total?: number; handedOff?: boolean; opened?: number; stopped?: boolean } + +function collector() { + const events: Event[] = [] + return { + events, + sender: { + isDestroyed: () => false, + send: (_channel: string, payload: { event: Event }) => events.push(payload.event), + } as never, + } +} + +const until = async (condition: () => boolean, ms = 8000) => { + const end = Date.now() + ms + while (Date.now() < end) { + if (condition()) return + await new Promise((resolve) => setTimeout(resolve, 10)) + } + throw new Error('condition never held') +} + +describe('openSeries', () => { + /* + * The mechanism this rests on, measured rather than assumed: `gio launch` + * returns in ~17ms, but the application it starts inherits the stdio pipes, + * so the PIPES close when the application ends (~3018ms for a 3s app). + * Waiting on the pipes is what makes "opens the next when you finish" work + * without reimplementing the desktop entry's Exec syntax. + */ + it('waits for one application to finish before opening the next', async () => { + const { events, sender } = collector() + const files = ['/tmp/a.mkv', '/tmp/b.mkv'] + + const started = Date.now() + await openSeries('s-1', files, 'slow.desktop', sender) + const elapsed = Date.now() - started + + const opening = events.filter((e) => e.type === 'opening') + expect(opening).toHaveLength(2) + expect(events.at(-1)).toMatchObject({ type: 'done', opened: 2, stopped: false }) + // Two two-second applications in sequence cannot finish in under three. + expect(elapsed).toBeGreaterThan(3000) + // And it did not auto-advance: each one was genuinely waited for. + expect(events.filter((e) => e.type === 'finished-one').every((e) => e.handedOff === false)).toBe(true) + }, 20000) + + /* + * A single-instance application hands the file to the copy already running + * and exits at once. Advancing on that would dump the whole list into it, + * which is the exact thing this mode exists to prevent, so it stops and + * waits to be told instead. + */ + it('stops and waits when the application hands off instead of finishing', async () => { + const { events, sender } = collector() + const run = openSeries('s-2', ['/tmp/a.mkv', '/tmp/b.mkv'], 'instant.desktop', sender) + + await until(() => events.some((e) => e.type === 'finished-one')) + const first = events.find((e) => e.type === 'finished-one')! + expect(first.handedOff).toBe(true) + + // It is parked: the second file has not been opened on its own. + await new Promise((resolve) => setTimeout(resolve, 300)) + expect(events.filter((e) => e.type === 'opening')).toHaveLength(1) + + // ...until it is advanced by hand. + expect(advanceSeries('s-2')).toBe(true) + await run + expect(events.filter((e) => e.type === 'opening')).toHaveLength(2) + expect(events.at(-1)).toMatchObject({ type: 'done', opened: 2 }) + }, 20000) + + it('stops the rest of the list on request', async () => { + const { events, sender } = collector() + const run = openSeries('s-3', ['/tmp/a.mkv', '/tmp/b.mkv', '/tmp/c.mkv'], 'instant.desktop', sender) + + await until(() => events.some((e) => e.type === 'finished-one')) + expect(stopSeries('s-3')).toBe(true) + await run + + expect(events.at(-1)).toMatchObject({ type: 'done', stopped: true }) + expect(events.filter((e) => e.type === 'opening')).toHaveLength(1) + }, 20000) + + it('reports an application that is no longer installed', async () => { + const { events, sender } = collector() + await openSeries('s-4', ['/tmp/a.mkv'], 'gone.desktop', sender) + expect(events).toEqual([{ type: 'error', message: 'That application is no longer installed.' }]) + }) + + it('advancing an unknown or unparked series does nothing', () => { + expect(advanceSeries('nope')).toBe(false) + expect(stopSeries('nope')).toBe(false) + }) +}) diff --git a/apps/desktop/electron/main/services/open-with.ts b/apps/desktop/electron/main/services/open-with.ts index f51e112..f6ba829 100644 --- a/apps/desktop/electron/main/services/open-with.ts +++ b/apps/desktop/electron/main/services/open-with.ts @@ -1,9 +1,11 @@ -import { execFile } from 'node:child_process' +import { execFile, spawn } from 'node:child_process' import { readFile, readdir } from 'node:fs/promises' import { homedir } from 'node:os' import { basename, join } from 'node:path' import { promisify } from 'node:util' +import type { WebContents } from 'electron' import { shell } from 'electron' +import { IPC } from '../../shared/contract.js' const execFileAsync = promisify(execFile) @@ -18,7 +20,11 @@ export type FileHandler = { } export type HandlerList = { - /** The file's content type, for the dialog to show. Null when it could not be read. */ + /** + * The content type the whole selection shares, or null when it is mixed (or + * unreadable). Mixed is not a failure: it is the case where "one handler for + * all of them" is the wrong question, and each file's own default is right. + */ contentType: string | null handlers: FileHandler[] /** @@ -167,7 +173,9 @@ async function contentTypeOf(path: string): Promise { * back empty and the dialog offers the system default alone, which is still * the thing most people want. */ -export async function handlersFor(path: string): Promise { +export async function handlersFor(paths: readonly string[]): Promise { + if (paths.length === 0) return { contentType: null, handlers: [], note: null } + if (process.platform !== 'linux') { return { contentType: null, @@ -176,9 +184,25 @@ export async function handlersFor(path: string): Promise { } } - const contentType = await contentTypeOf(path) + const types = new Set() + for (const path of paths) { + const type = await contentTypeOf(path) + if (type) types.add(type) + // Two distinct types is already enough to know the answer. + if (types.size > 1) break + } + + if (types.size > 1) { + return { + contentType: null, + handlers: [], + note: `These ${paths.length} files are not all the same type, so each one opens with its own default application.`, + } + } + + const contentType = [...types][0] ?? null if (!contentType) { - return { contentType: null, handlers: [], note: 'Could not read this file’s type. The system default is used.' } + return { contentType: null, handlers: [], note: 'Could not read the file type. The system default is used.' } } let ids: string[] = [] @@ -218,6 +242,163 @@ export async function handlersFor(path: string): Promise { } } +/** + * Opens one file and resolves when the application is finished with it. + * + * `gio launch` returns almost immediately: it hands the file to the desktop's + * launcher and exits (measured at 17ms). The application it started inherits + * this process's stdio, so the PIPES close when the application does, not when + * gio does (measured at 3018ms for a 3 second app). Waiting on the pipes is + * therefore the one reliable "the user is done with this file" signal that + * does not involve reimplementing the desktop entry's Exec syntax. + * + * The caveat is real and handled by the caller: a single-instance application + * hands the file to an already-running copy and exits at once, so a fast close + * means "we cannot tell", not "they finished in half a second". + */ +function openAndWait(desktopPath: string, file: string): Promise<{ elapsedMs: number }> { + return new Promise((resolve, reject) => { + const started = Date.now() + const child = spawn('gio', ['launch', desktopPath, file], { stdio: ['ignore', 'pipe', 'pipe'] }) + let pipes = 2 + let failed: Error | null = null + + child.on('error', (error) => reject(error)) + child.on('exit', (code) => { + if (code !== 0 && code !== null) failed = new Error(`Could not open ${file}.`) + }) + + const closed = () => { + pipes -= 1 + if (pipes > 0) return + if (failed) reject(failed) + else resolve({ elapsedMs: Date.now() - started }) + } + child.stdout.on('end', closed).resume() + child.stderr.on('end', closed).resume() + }) +} + +/** + * Below this, the application almost certainly handed the file to a copy that + * was already running rather than finishing with it. + * + * A real hand-off is tens of milliseconds (gio itself returns in about 17). + * Somebody actually watching or reading something takes seconds at the very + * least. One second sits well clear of the first and nowhere near the second. + */ +const HANDOFF_MS = 1000 + +export type SeriesEvent = + | { type: 'opening'; index: number; total: number; path: string } + | { type: 'finished-one'; index: number; total: number; path: string; handedOff: boolean } + | { type: 'done'; opened: number; total: number; stopped: boolean } + | { type: 'error'; message: string } + +type RunningSeries = { + advance: () => void + stop: () => void + waiting: boolean +} + +const series = new Map() + +/** + * Opens a list of files one after another. + * + * Each file waits for the previous application to finish, which is the point: + * a series of episodes is something you watch in order, and opening all twelve + * at once is not a thing anyone wants. When a hand-off is detected the run + * pauses and waits to be advanced by hand instead, because auto-advancing + * would then dump the whole list into the running player at once, which is the + * exact failure this mode exists to avoid. + */ +export async function openSeries( + seriesId: string, + paths: readonly string[], + handlerId: string | null, + sender: WebContents, +): Promise { + const send = (event: SeriesEvent) => { + if (!sender.isDestroyed()) sender.send(IPC.eventOpenSeries, { seriesId, event }) + } + + let stopped = false + let resumeManual: (() => void) | null = null + const entry: RunningSeries = { + advance: () => { + const resume = resumeManual + resumeManual = null + entry.waiting = false + resume?.() + }, + stop: () => { + stopped = true + entry.advance() + }, + waiting: false, + } + series.set(seriesId, entry) + + const found = handlerId ? await findDesktopFile(handlerId) : null + if (handlerId && !found) { + series.delete(seriesId) + send({ type: 'error', message: 'That application is no longer installed.' }) + return + } + + let opened = 0 + try { + for (const [index, path] of paths.entries()) { + if (stopped) break + send({ type: 'opening', index, total: paths.length, path }) + + let handedOff = false + if (found) { + const { elapsedMs } = await openAndWait(found.path, path) + handedOff = elapsedMs < HANDOFF_MS + } else { + const error = await shell.openPath(path) + if (error) throw new Error(error) + // openPath never blocks, so there is nothing to wait on and every step + // is a hand-off as far as we can tell. + handedOff = true + } + opened += 1 + send({ type: 'finished-one', index, total: paths.length, path, handedOff }) + + const isLast = index === paths.length - 1 + if (handedOff && !isLast && !stopped) { + // Wait to be told, rather than guessing that they are done. + entry.waiting = true + await new Promise((resolve) => { + resumeManual = resolve + }) + } + } + send({ type: 'done', opened, total: paths.length, stopped }) + } catch (error) { + send({ type: 'error', message: error instanceof Error ? error.message : String(error) }) + } finally { + series.delete(seriesId) + } +} + +/** Opens the next file in a series that is waiting to be advanced. */ +export function advanceSeries(seriesId: string): boolean { + const entry = series.get(seriesId) + if (!entry?.waiting) return false + entry.advance() + return true +} + +export function stopSeries(seriesId: string): boolean { + const entry = series.get(seriesId) + if (!entry) return false + entry.stop() + return true +} + /** * Opens a file, optionally with a chosen application. * @@ -226,17 +407,22 @@ export async function handlersFor(path: string): Promise { * applies the desktop entry's own Exec line rather than us trying to * reconstruct its argument syntax. */ -export async function openWith(path: string, handlerId?: string | null): Promise { +export async function openWith(paths: readonly string[], handlerId?: string | null): Promise { if (!handlerId) { - const error = await shell.openPath(path) - // openPath resolves with a message rather than rejecting. - if (error) throw new Error(error) + for (const path of paths) { + const error = await shell.openPath(path) + // openPath resolves with a message rather than rejecting. + if (error) throw new Error(error) + } return } const found = await findDesktopFile(handlerId) if (!found) throw new Error('That application is no longer installed.') - await execFileAsync('gio', ['launch', found.path, path]) + // One invocation with every file: a desktop entry that takes %F or %U gets + // them together, which is what "open all at once" means to the application + // as well as to the person asking for it. + await execFileAsync('gio', ['launch', found.path, ...paths]) } /** Exported for the tests: the search path is environment-dependent. */ diff --git a/apps/desktop/electron/preload/index.ts b/apps/desktop/electron/preload/index.ts index fd944d8..7817622 100644 --- a/apps/desktop/electron/preload/index.ts +++ b/apps/desktop/electron/preload/index.ts @@ -31,8 +31,13 @@ const api = { call(connectionId ? IPC.fsCreateFileRemote : IPC.fsCreateFileLocal, { connectionId, directory, name }), rename: (directory: string, from: string, to: string, connectionId?: string) => call(connectionId ? IPC.fsRenameRemote : IPC.fsRenameLocal, { connectionId, directory, from, to }), - handlers: (path: string) => call(IPC.fsHandlers, { path }), - openWith: (path: string, handlerId: string | null = null) => call(IPC.fsOpenWith, { path, handlerId }), + handlers: (paths: string[]) => call(IPC.fsHandlers, { paths }), + openWith: (paths: string[], handlerId: string | null = null) => + call(IPC.fsOpenWith, { paths, handlerId }), + openSeries: (seriesId: string, paths: string[], handlerId: string | null = null) => + call(IPC.fsOpenSeries, { seriesId, paths, handlerId }), + advanceSeries: (seriesId: string) => call(IPC.fsOpenSeriesAdvance, { seriesId }), + stopSeries: (seriesId: string) => call(IPC.fsOpenSeriesStop, { seriesId }), remove: (directory: string, name: string, isDirectory: boolean, connectionId?: string) => call(connectionId ? IPC.fsDeleteRemote : IPC.fsDeleteLocal, { connectionId, @@ -90,6 +95,11 @@ const api = { ipcRenderer.on(IPC.eventFleet, wrapped) return () => ipcRenderer.off(IPC.eventFleet, wrapped) }, + onOpenSeries(listener: (payload: { seriesId: string; event: unknown }) => void): () => void { + const wrapped = (_event: unknown, payload: { seriesId: string; event: unknown }) => listener(payload) + ipcRenderer.on(IPC.eventOpenSeries, wrapped) + return () => ipcRenderer.off(IPC.eventOpenSeries, wrapped) + }, onPreview(listener: (payload: { previewId: string; progress: unknown }) => void): () => void { const wrapped = (_event: unknown, payload: { previewId: string; progress: unknown }) => listener(payload) ipcRenderer.on(IPC.eventPreview, wrapped) diff --git a/apps/desktop/electron/shared/contract.ts b/apps/desktop/electron/shared/contract.ts index 287be18..3deaf70 100644 --- a/apps/desktop/electron/shared/contract.ts +++ b/apps/desktop/electron/shared/contract.ts @@ -30,6 +30,9 @@ export const IPC = { fsCreateFileLocal: 'fs:create-file-local', fsHandlers: 'fs:handlers', fsOpenWith: 'fs:open-with', + fsOpenSeries: 'fs:open-series', + fsOpenSeriesAdvance: 'fs:open-series-advance', + fsOpenSeriesStop: 'fs:open-series-stop', transfersPreview: 'transfers:preview', transfersPreviewCancel: 'transfers:preview-cancel', @@ -60,6 +63,8 @@ export const IPC = { /** Main -> renderer, one channel carrying every job event. */ eventTransfer: 'event:transfer', + /** Main -> renderer, progress through a one-at-a-time open. */ + eventOpenSeries: 'event:open-series', /** Main -> renderer, one channel carrying every fleet event. */ eventFleet: 'event:fleet', /** @@ -207,15 +212,31 @@ export const ProfileSaveSchema = z.object({ * main process before anything is launched: a renderer that could pass a path * here would be choosing which program runs. */ +const HandlerIdSchema = z + .string() + .min(1) + .max(255) + .regex(/^[^/\\\0]+\.desktop$/, 'That is not an application id.') + .nullable() + .default(null) + +/** + * The files to open, and what to open them with. + * + * A list rather than a single path: the handler is chosen once for the whole + * selection, because being asked which application to use twelve times in a + * row is not a feature. + */ export const OpenWithRequestSchema = z.object({ - path: PathSchema, - handlerId: z - .string() - .min(1) - .max(255) - .regex(/^[^/\\\0]+\.desktop$/, 'That is not an application id.') - .nullable() - .default(null), + paths: z.array(PathSchema).min(1).max(500), + handlerId: HandlerIdSchema, +}) + +/** Opening a list one at a time, carrying the id it is advanced and stopped by. */ +export const OpenSeriesRequestSchema = z.object({ + seriesId: JobIdSchema, + paths: z.array(PathSchema).min(1).max(500), + handlerId: HandlerIdSchema, }) export const RemotePathRequestSchema = z.object({ diff --git a/apps/desktop/src/components/open-with-dialog.tsx b/apps/desktop/src/components/open-with-dialog.tsx index d8f9f798e6f6135b0a2eaf6f2e82f2b817a859ab..83366b41ce01c17bc58ac7cc2a8b8745f728c322 100644 GIT binary patch literal 12736 zcmd5@-EP~+74Eg4Vj?UmDK(`yX?vlSoMxRYc7fewvG!t7xIsdRBa1b~VM)resx{E3 z=_~ZUkJ2aUcg~p^4rMBfqCcoEERmc!bLRYh=M3H7R9Y>vROfa72jUsapp;9n?;)F=x~L&NWn+GE#u-; z)w@Egui`wxDn#1exzuHqEY#ip)J9t>F+NFv9yYP?CubAB9* zf1Spexj9wxePQw%L*3TVEPe>(X6)rDJT|U0tPh*2wX9{*f?W$U@m0j_Lfl0CUHPoZFX}wap zQ5lpsQcuSx3zJv1y8Qj+)thfWyngfLs~^6(QZv>6@ImN*_<$u@?V@Bc#~ExFW~c8b zD&MTLO> zH*z@4%A^Z?nL8M>s7oRyjd^C2}uHS$>TLd0bYAA z_`2$`jo6hPL8m&;$3zW=YId&fOq!fL44Eadt820|2v^q>E#AYhpbj>nX%Iy$!SexG z#1P!t$xwf#;oO*h!9QCB#J0`{ZQBi|lSA=j^0?Mix^X_}JMHv?NxCe_Yq8`T`qi2y zD<0!^zNvOR@eBM2=M9!|R_S1kio+$36{iYEPXj+Yl!#3HIzes_QR2t+lU$J%wnl$- z_3bwjhTmj*jd1GL@M2#hdZh)NWG0q3f?eDoYMiUH6AbcMY9LA{`%@}is=*UB`JaAj zPn*qV>e*108h)1ZSZfVl+saG3eM@xSpk!#SyB?3sk+?2Rq-LCRD-zM znKU9c%JDf*Y|aVJCG=Wt7v7+Z8g^St9XORey5{E7Zv6{ zoT_WGa3_dAnlvBuf6*VlA0GD$iDdIKO|T8t*zs|0Sv>TzG!2R3>0>Q{DBydQ@IddoU-_Vi+rYoF2$k)Fo zIFXS^gA#PO803PW;&AXl)`s>QP#J8m0)ioPV+KbAOd6A0!~ofS(h)F0f5LJmQ-tja zF^JBh7Mac$fM^0;EJ?Z$9Zy8OBl7jr2E(vPLFS(R9ENjiuY`wXny1xDClSDHzzAgc zE+_X@6h|UJGq`Jf2hT>Vu^8!d0T$a7dx3*1GEew3W0p9>O3BVI40`bNv^#bT6eBo=x$NGVrdha@Nrh{_qecs14kem|xfIdr zi$MG8m%mgFQ8b?FICN5{{gu0-Hlq?YWY$0Y@b&9JeQsFWMBH8P4o7k}1%yjBG9MOg z3_oQ*L@n#n8IbZAd4~%pOq;L^N^wXv0Xbw*nj54?WMe4^)LZgU%9JPvQC4i5GTv%`Hv4iPQa1RahsFW*kNd?MF59`#+mkRX~$U9iU zD@~;;+M1swE7&-Mcf)10RG7kqMqSco)$3KHd!>=}1y8<5uo)47Bj z42Q#qb6!L#yUc}vE~pMgbDVu?7Mn^|g~*_Ipt$NSDz#Z}9p6XWXq}*}rpl60OoV#C zAO*~CqQVqZIrkiWzk+l|n%n+?G)aOLY63^sXTL7)-}laG!(5|J$086O-a#g4&4lbq zy-A>p;CT+<2w;oNM%T-Doo2g;>d(ntTn?g$@2^d6hVPYS;%xLAX2VKLV*Q?rV&_}# z9xIhx(t=k=ep(ek$A?>6#(;vobM^PXsXg~Zf=Lh4G0i!Lw#dN3HJku~jW%L%51o`f zzk6>61Ah;q7qkA(g%#l98WtsEAJn)ZOm@?nnWT5&j?mAGR?)LS-z1N{3%?LTIGdhL z3HyJ?*g#R}p6%%`Y|L!(dbSTU1X;YZc6b?0Y%xdVh(Qrmus?9{ghWf3;0dSQE=CTv zvdr{7fQ?>P(E>U|8FLdC(eu%7RI!VG?JNSqI`K%DQw%<8(5gknLC>aZ0(UpJ}LF?odyJSY7ku(FXXOtx7A1XKGB{jD2RY?7z_ zCV{q@{Mj=yHCi&ll8OTBGBev~l_rVKyR0oxL=bkYlM1D? zj(ISIE;rEc|Z zwr5hKH0frA))zI|fs`GO4|PhiM61_w4uM$PC!cX05!>e>6}ja{r7p5~p$FqXj1pbl z0-}62K2`nx@c3#_C7&L-O}OjhGdgwZSU5iIxIW$7qg)=*B>-24;yQvaDPTHoyto~j zyW=$jOqwVk#uP6{h`yR2ND*f1qULNsPt4gfbCSlsE3z%>m{Aj6HWks$At&$ZPGXF|7tDP7r8HgTvR+H*^14bX6s zexwd)*-x>+T8;-p)~fa7kJRN`?jn5v8K(BI5^yw)0fu^JQ&aNqF?0uHau5xQFN1>aKp%WyA+4Li z@If_Fp#!FF+u~x$Z~AbbdzwgjfLI3<1@a{03bOe}-qdGpy#JB1I6Y{YANkp10}Mi{ ztA51U953#!t!?GDyGd8W6Zk8gQoaJ{29BfiT0V;O$EMOcWk^x2fHD|SuYvc0Fi_c1 zikr0+Ez1)&QST$gpcMwmjTU5wi}4riwMXl1^}!nd*0dZ z7aRb%^*nk3v<#1;KZGBv=s8DQn5+*2T#Siqy=|qaM`!z~yw1^Kq+AihNE;Wa={<*8 zFA!tn1r8&KzUdhFvptu34<8!>O`@jlN;A)&ls3Z-6SEUjc&zG1`%L_)EuutxCvU3! zEBky0wZmLuKGu4)(;oNcj3Cq%x)(dMft65mz8TxKVF^Hi&Y!AO&`k~O=kj>MNucCZ%VMm=Q*`}7V0RXZq#!7`7s3^R*-qiKl%(7W{k7s-U2{Bg+^=&o(&%L=Bub#{G* zD)bZWm%W&>>!LBOseQ-GM||%W0ux*ljLtaay*CQZ{Bxq-n*1%=64XCzy(B@$pLF)w zHr0a=jJDB>Cej`}>=rz_OMV6f#9TN?0&zEH`ZG|18cYuKiYB}IMTcnNkJC7 z%11Hm>A*yL&IkjoH?|c>szX+N#HeJ)8Jmt!2tHB5W+P?nfn$>(*tX5DgB84Vh6A02 zTI2P*4LU|`@m`-vtN2(o<^A1r$_K&fSLzI1&ZSc|9}7E=Al$>CFO%W6rFT6=u<#jQ{ro=Js3^w798o057*o-;YPPB!a;*+Ax? z&LRKiZZi7z1`Q|I)O$L1n)-yJwhM67rPNS%$_UK*lfC3CYBPp%yVZC>gz@JVc4W3} z=7dvsXno~)eH9{komRKsZK@Ssa>=O|TWf$Xye}w-ggT>iDn3%*@yVjNM7zfDgSV(g zSqw|OeQK~IawFkhFS(#vZr902AW73KWJtn2r#2LE-JPF>B|JAk8YUGlc&&8ew!(z@A_u{~{Xk_HzV`DJ1}f^U(G4-%{}MaknJ|*HW<{ z#>rxxcr-+C%W0j#77i;Ny0RV_;+POoiNA8d49NMs6#yJ(Ygk}<<=%F0HrJDl$k`@J zD3rU?SMGKCjuM2=K!Z~2ARioNOX~A)TZeIv66!oNi`z$8Aw1Ly^Q%4SY%kl98@s7C zx#p9>EzC5eq*oBP{Xc;)}g}>k6OaiR~)PQ@$6DJ`8bG8d6cQvt%a2M$93ol`yW{L6>lw2< zu49oXr=Hpa;i(7y0R%l2A#sZUwc<)oy|zNF5K<3SdZ>id!pu6q+P&<|x9@%LecyZg z%=vfnj$Nf+x%Hna4tfKve`$ z?3!dXst_FpT*#>i-A#phKtt+NM^ZPcqZ`$HDqL_1OH>sL#1aM4BrO|45fM0|Rudc? z5>jwfXA!rjbl7mSA)^qVDGBVvZgUz9C^S8#swEoGBwQ-lE~{1*uFv(la)U@FI3DOc zp0Eu7PHGe{b{5i8p!(28tpIQM{`RLIJHN+=T@EhCO8BZfjlafvx<=|mdiXAO6`gp1 zKgTbu{nxNSG!+?Xg|MxBYRjOeN7~KTh@;e~X7w~wt@cx?fGgVL*s-lWJKd)8Z&yG& z6K*vamHuoC9FOn?sx8>dMtInyK{PDFTiWlYC)&f;1wu8s^IXzEEe+r&$sycKe4AXg zVP3C>4O-`oaa`&h-_m@)cN%G5&bY1?t)Rc!c(%8Qr;^JDSJi%T;;R+9ZQ#`&6JI3z z@a`c$A^40hLRxri^?%fHPz!-GXCi9+V&Fbr8$62t431zao5JI%%lK6)#J8ye9!(#@ zlj(l^SiL__U&Qaym+@`-63%7j_7^J#j$}u=Yh=0PmF6cV@rO(q|H_;|NTu*xwtzf4 zjCZo*xRH%_jN)eY$i7hn|Fp(%#vDaveuxju0eos6h%4J%p9c8SG*uE{(&{p{urhP; zQ!}l2Vm-IFv1ng3x{*!nBn#`<&iS780sdqeJNI9#!&o*`|0iXpwHI19Q&CG_=lYfM z0eq6nM-IHs_2S>TMRf8RT*+7QxBNLQ3=MXluGFWaLE|lZFgl#Bxa`YmltTo6ADWKO l5bjXlz~?ZGF_5?dnZ%g>_r_K=n8!Z`61WMIkI^o5y$AUeb?^WH diff --git a/apps/desktop/src/components/pane.tsx b/apps/desktop/src/components/pane.tsx index f0c7216..72c66b9 100644 --- a/apps/desktop/src/components/pane.tsx +++ b/apps/desktop/src/components/pane.tsx @@ -236,14 +236,27 @@ export function Pane({ // space, which is what distinguishes "new file here" from "rename this". const [target, setTarget] = useState(null) const [dialog, setDialog] = useState<'mkdir' | 'create-file' | 'rename' | 'delete' | null>(null) - /** The file the Open with dialog is about, or null while it is closed. */ - const [openWithTarget, setOpenWithTarget] = useState<{ path: string; name: string } | null>(null) + /** The files the Open with dialog is about, or null while it is closed. */ + const [openWithTarget, setOpenWithTarget] = useState<{ paths: string[]; label: string } | null>(null) const [busy, setBusy] = useState(false) /* - * "Open with" needs a file this machine can actually open. A pane pointed at - * a server names a path on that server, and a directory is not a document. + * "Open with" needs files this machine can actually open. A pane pointed at + * a server names paths on that server, and a directory is not a document. + * + * It acts on the SELECTION when the right-clicked row is part of it, and on + * that row alone otherwise -- which is how right-click behaves everywhere + * else: clicking outside a selection is a new, single target. */ - const canOpenWith = state.endpoint.kind === 'local' && target !== null && target.type !== 'directory' + const openWithFiles = useMemo(() => { + if (state.endpoint.kind !== 'local' || !target) return [] + const chosen = + state.selected.has(target.name) && state.selected.size > 1 + ? state.entries.filter((entry) => state.selected.has(entry.name)) + : [target] + return chosen.filter((entry) => entry.type !== 'directory') + }, [state.endpoint.kind, state.selected, state.entries, target]) + + const canOpenWith = openWithFiles.length > 0 const [opError, setOpError] = useState(null) useEffect(() => { @@ -611,11 +624,19 @@ export function Pane({ { - if (target) setOpenWithTarget({ path: target.path, name: target.name }) + if (openWithFiles.length === 0) return + setOpenWithTarget({ + paths: openWithFiles.map((entry) => entry.path), + label: + openWithFiles.length === 1 + ? (openWithFiles[0]?.name ?? '') + : `${openWithFiles.length} files`, + }) }} > - Open with… + {/* Says how many, so a menu opened over a selection is not a guess. */} + {openWithFiles.length > 1 ? `Open ${openWithFiles.length} files with…` : 'Open with…'} onNavigate(state.path)}> @@ -679,8 +700,8 @@ export function Pane({ setOpenWithTarget(null)} /> diff --git a/apps/desktop/src/lib/api.ts b/apps/desktop/src/lib/api.ts index 08de5dc..5c19e99 100644 --- a/apps/desktop/src/lib/api.ts +++ b/apps/desktop/src/lib/api.ts @@ -57,12 +57,30 @@ export type FileHandler = { } export type HandlerList = { + /** The type the whole selection shares, or null when it is mixed. */ contentType: string | null handlers: FileHandler[] /** Why the list is short or empty, when that is worth saying. */ note: string | null } +/** Progress through a one-at-a-time open. */ +export type SeriesEvent = + | { type: 'opening'; index: number; total: number; path: string } + | { + type: 'finished-one' + index: number + total: number + path: string + /** + * The application handed the file to a copy that was already running, so + * we cannot tell when it is finished and will not advance on its own. + */ + handedOff: boolean + } + | { type: 'done'; opened: number; total: number; stopped: boolean } + | { type: 'error'; message: string } + export type Change = { action: 'add' | 'update' | 'metadata' | 'delete' | 'unchanged' | 'error' path: string @@ -271,10 +289,15 @@ type Api = { createFile(directory: string, name: string, connectionId?: string): Promise> rename(directory: string, from: string, to: string, connectionId?: string): Promise> remove(directory: string, name: string, isDirectory: boolean, connectionId?: string): Promise> - /** The applications registered for a local file, system default marked. */ - handlers(path: string): Promise> - /** Opens a local file; a null handler means the system default. */ - openWith(path: string, handlerId?: string | null): Promise> + /** The applications registered for a selection, system default marked. */ + handlers(paths: string[]): Promise> + /** Opens local files at once; a null handler means each file's own default. */ + openWith(paths: string[], handlerId?: string | null): Promise> + /** Opens them one after another, reporting over `events.onOpenSeries`. */ + openSeries(seriesId: string, paths: string[], handlerId?: string | null): Promise> + /** Opens the next file in a series that is waiting to be advanced. */ + advanceSeries(seriesId: string): Promise> + stopSeries(seriesId: string): Promise> } transfers: { preview(request: unknown): Promise> @@ -315,6 +338,7 @@ type Api = { onTransfer(listener: (payload: { jobId: string; event: TransferEvent }) => void): () => void onFleet(listener: (payload: { runId: string; event: FleetEvent }) => void): () => void onPreview(listener: (payload: { previewId: string; progress: PreviewProgress }) => void): () => void + onOpenSeries(listener: (payload: { seriesId: string; event: SeriesEvent }) => void): () => void } } diff --git a/apps/desktop/src/lib/open-mode.test.ts b/apps/desktop/src/lib/open-mode.test.ts new file mode 100644 index 0000000..65e2dcf --- /dev/null +++ b/apps/desktop/src/lib/open-mode.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from 'vitest' +import { defaultModeFor } from './open-mode.js' + +describe('defaultModeFor', () => { + /* + * The case that prompted the feature: a series of episodes is watched in + * order, and opening all twelve at once is never what was meant. + */ + it('queues a set of videos', () => { + expect(defaultModeFor('video/x-matroska', 12)).toBe('series') + expect(defaultModeFor('video/mp4', 2)).toBe('series') + }) + + it('queues a set of audio too', () => { + expect(defaultModeFor('audio/flac', 9)).toBe('series') + }) + + it('opens documents and images together', () => { + expect(defaultModeFor('application/pdf', 4)).toBe('together') + expect(defaultModeFor('image/jpeg', 30)).toBe('together') + expect(defaultModeFor('text/markdown', 3)).toBe('together') + }) + + /* + * One file has no meaningful mode. Answering `series` would park a queue of + * one waiting to be advanced past its own only entry. + */ + it('never queues a single file, whatever it is', () => { + expect(defaultModeFor('video/x-matroska', 1)).toBe('together') + expect(defaultModeFor('video/mp4', 0)).toBe('together') + }) + + /* + * A mixed selection has no shared type, and each file opens in its own + * default application. Queueing unrelated files behind each other would be a + * worse guess than opening them. + */ + it('opens a mixed selection together', () => { + expect(defaultModeFor(null, 5)).toBe('together') + }) +}) diff --git a/apps/desktop/src/lib/open-mode.ts b/apps/desktop/src/lib/open-mode.ts new file mode 100644 index 0000000..7e7676a --- /dev/null +++ b/apps/desktop/src/lib/open-mode.ts @@ -0,0 +1,23 @@ +/** All the files at once, or one after another. */ +export type OpenMode = 'together' | 'series' + +/** + * How a selection opens unless told otherwise. + * + * Video and audio are the things you go through in order, so a set of them + * defaults to one at a time: opening twelve episodes at once is not something + * anyone means. Everything else opens together, because a handful of photos or + * documents is a set you want in front of you at the same time. + * + * A mixed selection has no shared type and gets `together`, which pairs with + * each file opening in its own default application: twelve unrelated files + * queued behind each other would be a worse guess than just opening them. + * + * One file has no meaningful mode. The dialog does not offer the choice there, + * and this answers `together` so nothing waits on a queue of one. + */ +export function defaultModeFor(contentType: string | null, count: number): OpenMode { + if (count < 2) return 'together' + if (!contentType) return 'together' + return contentType.startsWith('video/') || contentType.startsWith('audio/') ? 'series' : 'together' +}