diff --git a/README.md b/README.md index 1009037..4fe3852 100644 --- a/README.md +++ b/README.md @@ -173,14 +173,32 @@ scope it had. Both controls are a **directory listing**, not a text box: the panel's vocabulary is paths relative to the session folder, and "which folder is `../shared`" is the one question free text cannot answer. Pressing it opens a second dialog over the -panel, shaped like the host's own workspace browser — one level of child folders, -a breadcrumb trail, a hidden-entries toggle — driven through -`uiWorkspace.listDirectory`, the same Host primitive that browser uses. A pick is -stored in the form that keeps its meaning: `/work/root/src/shared` becomes -`src/shared` when the session started in `/work/root`. The picker's own path field -stays editable, because the Host lists folders and an extra may legitimately name -a file — which is why that action reads **Add path or file** and its field **Path -or file**, rather than promising a folder the list would then fail to show. +panel, shaped like the host's own workspace browser — one level of children, a +breadcrumb trail, a hidden-entries toggle, and a file-type glyph per row. A pick +is stored in the form that keeps its meaning: `/work/root/src/shared` becomes +`src/shared` when the session started in `/work/root`. + +The listing is driven by **two Host listers**, because they answer different +questions and the extras need both: + +| lister | returns | reach | +|---|---|---| +| `uiWorkspace.listDirectory` | folders | anywhere on the machine | +| `remote.workspaceFiles.list` | files and folders | the session's workspace only | + +The extras picker takes the file listing while the level is inside the session +folder, which is how a file is chosen by clicking its name instead of typing it — +the same listing behind the sidebar's own file tree, applied one level at a time. +Above the session folder the Host confines it, and the level falls back to folders +and says so in the dialog: a file outside the workspace is still added by typing +its path, which is why the action reads **Add path or file** and its field **Path +or file**. The focus picker never offers files at all — a focus is a folder, so a +file row there would offer something that cannot be chosen. + +A folder row is a way down; a file row is a thing to take. Clicking a file fills +the path field and marks the row, and the footer's action then adds it in declared +form; pressing Enter on a name the level already lists is not an error, because +the path is exactly what this dialog is for. Because the panel owns widening, `facet action=add` is the *other* path and the narrower one: it is gated on an approval channel, and a deployment with none — the @@ -246,7 +264,7 @@ Honest boundaries, because each one is a decision rather than an oversight: Verified: -- 88 tests (`npm test`) over the scope algebra, the policy state, the guard, and +- 93 tests (`npm test`) over the scope algebra, the policy state, the guard, and the browser half — including that containment is decided on canonical targets (a symlink inside the facet pointing outside it is refused), that a prefix is not a parent (`/a/room` does not contain `/a/roommate`), that a malformed tool @@ -256,8 +274,9 @@ Verified: declared scope rather than the session directory, that a search naming no path is refused when the directory it defaults to is outside the facet, that a delegated session inherits its parent's root at any depth and is not a way back - out of a self-narrowing, and that every `facet` result satisfies the tool's own - declared output schema. + out of a self-narrowing, that both listings normalise into one picker level and + that a file listing the Host confines still leaves a browsable one, and that + every `facet` result satisfies the tool's own declared output schema. - The bundle's patch layers parse, merge and land correctly against the real profile: `dsh --profile web --patch ./cordis.patch.yml --patch ./presets/facet.patch.yml --dump-config` shows `facet-policy`, `preset-facet`, @@ -335,7 +354,7 @@ lib/scope.js the pure half — argument extraction, text, reason lib/guard.js preset row: rule, binding, tool, pre-execute guard presets/facet.patch.yml the `facet` agent preset presets/standard-compaction.patch.yml optional: the same budget for `standard` -test/ 88 tests +test/ 93 tests ``` `client/client.js` is a browser half rather than a module the host imports, and @@ -360,14 +379,16 @@ refuses a path that is not under a volatile node, and refusing is exactly right the panel is the only writer, and a plugin that could not serve the namespace would be a control that silently stored nothing. -The listing reaches the panel through `ctx.get('uiWorkspace')` rather than through -an injection, and the injection's `available()` is asked on every render. Both are -deliberate: `ctx.inject` would make the whole context button conditional on a -service the plugin does not own, and the panel is still the only way to declare -scope in a deployment with no directory browser — it would simply have no listing -to offer, which is what it says instead. The service is resolved at the call -because the renderer memoizes a registration's inject result, so a boolean read -there would freeze at whatever the first render saw. +The listings reach the panel through `ctx.get` rather than through an injection — +`uiWorkspace` for folders and `remote.workspaceFiles` for files — and each +`available()` is asked on every render. Both are deliberate: `ctx.inject` would +make the whole context button conditional on services the plugin does not own, and +the panel is still the only way to declare scope in a deployment with no directory +browser — it would simply have no listing to offer, which is what it says instead. +The services are resolved at the call because the renderer memoizes a +registration's inject result, so a boolean read there would freeze at whatever the +first render saw. Both packages are named in this bundle's `dsh.client.inject`, so +a profile that installs this one gets the services its dialog asks for. The two dialogs are wider than the primitive's 380px card, which is fixed in the primitives' own stylesheet. `client/client.js` installs one small stylesheet of diff --git a/client/client.js b/client/client.js index 149d312..46b7cee 100644 --- a/client/client.js +++ b/client/client.js @@ -46,6 +46,7 @@ window.__ModuleLoader__.load({ SettingsFormModel, SettingsValueField, IconFolderCloseRegular, + FileTypeIcon, } = primitives const h = react.createElement @@ -448,7 +449,10 @@ window.__ModuleLoader__.load({ * * The Host's own business message is preferred to the transport error, * because a browse refusal is an answer — the folder is unreadable, the path - * is not absolute — and that sentence is the one worth reading. + * is not absolute — and that sentence is the one worth reading. Both listers + * reach here: the folder primitive rejects with its own error and the remote + * one folds its failure into a `RemoteError`, which is an `Error` carrying + * the same message field. */ function browseFailure(error) { const rpc = error !== null && typeof error === 'object' ? error.rpcError : undefined @@ -456,6 +460,121 @@ window.__ModuleLoader__.load({ return error instanceof Error ? error.message : String(error) } + // ── one level, in the picker's own vocabulary ──────────────────────────── + // + // The Host has two listers and they answer different questions, so the + // picker reads them through one shape and does not care which answered: + // + // * `uiWorkspace.listDirectory` browses anywhere and returns folders only; + // * `remote.workspaceFiles.list` returns files as well, and confines itself + // to the session's workspace — the same listing the sidebar's file tree + // drives, and the only way this deployment can show a file at all. + // + // `{base, crumbs, entries, truncated, files}` is that shape. `files` says + // whether this level could carry file rows, which is what the copy under the + // path field tells the user. + + /** Join one name onto a directory, with the separator the Host resolves. */ + function joinUnder(base, name) { + return `${String(base).replace(/[\\/]+$/, '')}/${name}` + } + + /** Whether a path is a directory or sits inside one, either separator. */ + function withinPath(inner, outer) { + const path = String(inner).replace(/\\/g, '/').replace(/\/+$/, '') + const base = String(outer).replace(/\\/g, '/').replace(/\/+$/, '') + return base !== '' && (path === base || path.startsWith(`${base}/`)) + } + + /** The last segment of a path, for a trail's first crumb. */ + function basenameOf(path) { + const segments = pathSegments(path) + return segments.length === 0 ? String(path) : segments[segments.length - 1] + } + + /** + * The ancestry of one path inside the workspace, workspace root first. + * + * The folder primitive reports its own trail — from the filesystem root, so + * the user can go above the session — and the file listing reports none, + * because the workspace root is as far up as it goes. This builds the one it + * does not send. + */ + function trailTo(workspace, path) { + const trail = [{ name: basenameOf(workspace), path: workspace }] + const rest = relativeTo(workspace, path) + // A path outside the workspace cannot be walked to from it, and `relativeTo` + // says so by answering in `..` segments or absolutely. + if (rest === '' || rest.startsWith('..') || isAbsolutePath(rest)) return trail + let walked = workspace + for (const segment of rest.split('/')) { + if (segment === '') continue + walked = joinUnder(walked, segment) + trail.push({ name: segment, path: walked }) + } + return trail + } + + /** + * One folder listing, as the picker reads it. + * + * @param {object} listing - a `DirectoryListing` from the browse primitive. + * @param {string} [fallback] - the path asked for, when the answer carries none. + * @returns {object} the picker's level. + */ + function folderListing(listing, fallback) { + const base = typeof listing?.path === 'string' && listing.path !== '' ? listing.path : String(fallback ?? '') + const home = typeof listing?.home === 'string' ? listing.home : '' + return { + base, + crumbs: (Array.isArray(listing?.crumbs) ? listing.crumbs : []).map((crumb) => ({ + // A root crumb carries its whole path as its name; "Home" is the label + // the host's own browser gives it. + name: crumb.path === home ? 'Home' : crumb.name, + path: crumb.path, + })), + entries: (Array.isArray(listing?.entries) ? listing.entries : []).map((entry) => ({ + name: entry.name, + path: entry.path, + directory: true, + hidden: entry.hidden === true, + })), + truncated: listing?.truncated === true, + files: false, + } + } + + /** + * One workspace listing, as the picker reads it. + * + * Paths arrive relative to the workspace root and the picker works in + * absolute paths, so they are joined here — with `/`, whatever the level's + * own separators, because the Host resolves mixed separators and this is only + * a key and something to display. + * + * @param {object} listing - a `WorkspaceDirectoryListing`. + * @param {string} workspace - the session folder, absolute. + * @returns {object} the picker's level. + */ + function workspaceListing(listing, workspace) { + const relative = typeof listing?.path === 'string' ? listing.path : '' + const base = relative === '' ? workspace : joinUnder(workspace, relative) + return { + base, + crumbs: trailTo(workspace, base), + entries: (Array.isArray(listing?.entries) ? listing.entries : []).map((entry) => ({ + name: entry.name, + path: joinUnder(base, entry.name), + directory: entry.type === 'directory', + // The workspace listing has no hidden flag; the platform convention is + // the dot, which is what the folder primitive reports for the same name. + hidden: String(entry.name).startsWith('.'), + })), + truncated: listing?.truncated === true, + files: true, + } + } + /** * The state behind the picker: the level on screen, the path being edited, * and the last failure. @@ -466,17 +585,25 @@ window.__ModuleLoader__.load({ * a sequence number and only the newest may commit. It also keeps the whole * interaction testable without a DOM, which is the same reason the settings * card has a controller. + * + * Two listers in, one level out: see the note above {@link folderListing} + * for why there are two and what the picker does with either. */ var DirectoryPickerModel = class { /** * @param {object} options - * @param {(path?: string, signal?: AbortSignal) => Promise} options.list - the host listing call. + * @param {(path?: string, signal?: AbortSignal) => Promise} options.folders - the folder listing; browses anywhere. + * @param {(sessionId: string, path: string, signal?: AbortSignal) => Promise} [options.files] - the file listing; files and folders, confined to the workspace. + * @param {string} [options.sessionId] - the session whose workspace the file listing is confined to. * @param {string} options.root - the session folder, absolute, or ''. * @param {string} [options.start] - the directory to open first; the root by default. */ constructor(options) { const root = typeof options.root === 'string' ? options.root : '' - this.list = options.list + this.folders = options.folders + this.files = options.files + this.sessionId = options.sessionId + this.root = root this.listeners = new Set() this.request = 0 this.state = { @@ -524,18 +651,56 @@ window.__ModuleLoader__.load({ const target = String(path ?? '').trim() const request = (this.request += 1) this.update({ loading: true, error: '', draft: target }) - Promise.resolve() - .then(() => this.list(target === '' ? undefined : target)) - .then( - (listing) => { - if (request !== this.request) return - this.update({ listing, loading: false, error: '', draft: String(listing?.path ?? target) }) - }, - (reason) => { - if (request !== this.request) return - this.update({ loading: false, error: browseFailure(reason) }) - }, - ) + this.read(target, request) + } + + /** + * One level, from whichever lister can answer for it. + * + * The file listing is tried first while the level is inside the session's + * workspace, because that is the listing the extras need: it carries files + * as well as folders. The Host confines it to the workspace, so a level + * above the workspace root — or a deployment with no file service — falls + * back to the folder listing, which browses anywhere and carries folders + * only. The fallback is also the answer to a file listing that failed for + * any other reason, so a level the file service cannot read is still + * browsable as folders rather than reported as unreadable. + * + * @param {string} target - the absolute directory, or '' for the Host's home. + * @param {number} request - the sequence number this read belongs to. + */ + read(target, request) { + const files = this.files + const inside = target === '' ? this.root !== '' : withinPath(target, this.root) + const asFolders = () => this.folders(target === '' ? undefined : target).then((listing) => folderListing(listing, target)) + const listing = + files === undefined || !inside + ? asFolders() + : files(this.sessionId, target === '' ? this.root : target) + .then((answer) => workspaceListing(answer, this.root)) + .catch(asFolders) + Promise.resolve(listing).then( + (level) => { + if (request !== this.request) return + this.update({ listing: level, loading: false, error: '', draft: level.base }) + }, + (reason) => { + if (request !== this.request) return + // A path that names a file in the level already on screen is not a + // failure, it is the thing this dialog is for: the list asked for is + // not a list, and the footer can still add what it names. That + // matters now that a file row is one keystroke away from the field. + const wanted = absoluteFrom(this.root, target) + const named = (this.state.listing?.entries ?? []).some( + (entry) => entry.directory !== true && entry.path === wanted, + ) + if (named) { + this.update({ loading: false, error: '' }) + return + } + this.update({ loading: false, error: browseFailure(reason) }) + }, + ) } /** @param {string} text - the path field's own value. */ @@ -571,6 +736,13 @@ window.__ModuleLoader__.load({ } const pickerNameStyle = { flex: 1, minWidth: 0, overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap' } const pickerGlyphStyle = { display: 'inline-flex', flex: 'none', color: 'var(--dsw-alias-label-secondary)' } + /** A file row the path field already names, so a click is visibly a choice. */ + const pickerChosenRowStyle = { + ...pickerRowStyle, + // The token the Host's own directory browser uses for a chosen row, fallback + // and all, so a pick looks the same here as it does one panel over. + background: 'var(--dsw-alias-interactive-bg-active,var(--dsw-alias-interactive-bg-hover))', + } const crumbStyle = { flex: 'none', border: 0, @@ -599,7 +771,13 @@ window.__ModuleLoader__.load({ /** * The directory picker: one editable path, the ancestry above it, and one - * level of child folders. + * level of children — folders always, and files wherever the Host will list + * them. + * + * A folder row is a way down; a file row is a thing to take, so it fills the + * path field rather than navigating, and the primary action adds what the + * field holds. That is the one interaction the two row kinds do not share, + * and it is why a file row is marked selected once the field names it. * * It drives no host call of its own — {@link DirectoryPickerModel} holds the * listing and the path, and the panel builds one per opening so each visit @@ -622,6 +800,9 @@ window.__ModuleLoader__.load({ // folder: the focus's own default, and for an extra — already the root — // nothing worth adding. const declared = relativeTo(root, typed) + // The field's own value as an absolute path, so a file row can tell whether + // it is the one the field names. + const chosen = absoluteFrom(root, typed) const rows = entries.map((entry) => h( @@ -630,14 +811,22 @@ window.__ModuleLoader__.load({ key: entry.path, type: 'button', className: ROW_CLASS, - style: pickerRowStyle, + style: entry.directory || entry.path !== chosen ? pickerRowStyle : pickerChosenRowStyle, title: entry.path, disabled: blocked || state.loading, - // A click walks down rather than picking: the primary action is the - // path field's own value, so a folder is chosen by standing in it. - onClick: () => model.open(entry.path), + // A folder is chosen by standing in it; a file is chosen by naming + // it, so the click puts it in the field for the footer to add. The + // field holds an absolute path everywhere else in this dialog, and + // the declared form is what the panel's write turns it into. + onClick: () => (entry.directory || target === 'focus' ? model.open(entry.path) : model.setDraft(entry.path)), }, - h('span', { style: pickerGlyphStyle }, h(IconFolderCloseRegular, { size: 14 })), + h( + 'span', + { style: pickerGlyphStyle }, + entry.directory + ? h(IconFolderCloseRegular, { size: 14 }) + : h(FileTypeIcon, { path: entry.name, size: 16 }), + ), h('span', { style: pickerNameStyle }, entry.name), ), ) @@ -656,10 +845,7 @@ window.__ModuleLoader__.load({ disabled: blocked || state.loading, onClick: () => model.open(crumb.path), }, - // A root crumb carries its whole path as its name; "Home" is the - // shorter label the host's own browser gives it, and the title - // attribute still names where it is. - crumb.path === listing.home ? 'Home' : crumb.name, + crumb.name, ), ) } @@ -672,10 +858,19 @@ window.__ModuleLoader__.load({ : listing === null ? h('p', { key: 'status', style: pickerStatusStyle }, 'Type a folder to start from.') : entries.length === 0 - ? h('p', { key: 'status', style: pickerStatusStyle }, 'No folders in here.') + ? h('p', { key: 'status', style: pickerStatusStyle }, 'Nothing in here.') : listing.truncated === true - ? h('p', { key: 'status', style: pickerStatusStyle }, 'Too many folders to list; only the beginning is shown.') - : null + ? h('p', { key: 'status', style: pickerStatusStyle }, 'Too many entries to list; only the beginning is shown.') + : // Above the session folder the Host's file listing stops — it is + // confined to the workspace — so the rows become folders only. + // Saying so is better than letting file rows silently vanish. + target === 'extra' && listing.files !== true + ? h( + 'p', + { key: 'status', style: pickerStatusStyle }, + 'Files are listed inside the session folder only; above it the list is folders.', + ) + : null return h(Modal, { open: true, @@ -702,8 +897,9 @@ window.__ModuleLoader__.load({ value: typed, disabled: blocked, spellCheck: false, - // The Host lists folders, so this box is also how a file — or a - // folder the session will create — is named at all. + // The listing carries the files it can reach; the box is still how a + // path outside the session's workspace — or a folder the session + // will create — is named at all. 'data-modal-autofocus': true, placeholder: root === '' ? absoluteFrom(root, 'src/shared') : 'relative/to/the/session/folder', onChange: (event) => model.setDraft(event.target.value), @@ -718,10 +914,10 @@ window.__ModuleLoader__.load({ { style: hintStyle }, target === 'focus' ? 'Relative to the session folder, or absolute. Enter lists whatever is there.' - : // The list holds folders only — the Host's browse primitive has no - // file rows — so the box is where a file is named, and saying so - // is the difference between a dead end and an instruction. - 'Relative to the session folder, or absolute. The list holds folders; type a file\u2019s name to add the file itself.', + : // Files where the Host lists them, folders everywhere. The box is + // still where a path the listing cannot reach gets named, so both + // halves of that are said rather than left to be discovered. + 'Relative to the session folder, or absolute. Folders open; a file is taken as the path to add.', ), ), h( @@ -843,6 +1039,24 @@ window.__ModuleLoader__.load({ ) } + /** + * The file lister an opening should offer, if any. + * + * Files are what an extra may name, so that is the only control which offers + * them: a focus is a folder, and a listing that could hand the picker a file + * would only offer something that cannot be chosen. A deployment without the + * file service gets the folder listing as before, which is why this asks + * rather than assumes. + * + * @param {object} browse - the browsing face from {@link directoryBrowse}. + * @param {string} target - 'focus' or 'extra'. + * @returns {Function|undefined} the lister, or nothing to offer. + */ + function fileLister(browse, target) { + if (target !== 'extra') return undefined + return browse?.files?.available?.() === true ? browse.files.list : undefined + } + /** * The facet context panel: what this conversation may see, and who decides. * @@ -856,8 +1070,8 @@ window.__ModuleLoader__.load({ * "which folder is `../shared`" is exactly the question a free-text box * cannot answer; the listing answers it from the Host's own directory rather * than from a guess about what the path was meant to name. The path field is - * still editable inside the picker, because the Host lists folders and the - * extras may name a file. + * still editable inside the picker, for a path no listing reaches: a folder + * the session will create, or a file above the session's workspace. * * @param {object} props - the bound form, the session, its folder, the browse face, and the close command. */ @@ -911,7 +1125,9 @@ window.__ModuleLoader__.load({ setPicker({ target, model: new DirectoryPickerModel({ - list: browse.list, + folders: browse.list, + files: fileLister(browse, target), + sessionId, root, start: isAbsolutePath(stored) ? stored : '', }), @@ -1016,7 +1232,7 @@ window.__ModuleLoader__.load({ h('h3', { style: { ...sectionTitleStyle, marginTop: '18px' } }, 'Also allowed'), declared.extras.length === 0 - ? h('p', { style: bodyStyle }, 'Nothing yet. Shared types, project docs, or a sibling folder go here.') + ? h('p', { style: bodyStyle }, 'Nothing yet. Shared types, project docs, or a single file go here.') : h('ul', { style: { listStyle: 'none', margin: 0, padding: 0 } }, rows), h( 'div', @@ -1155,7 +1371,7 @@ window.__ModuleLoader__.load({ } /** - * The directory-browsing face the picker drives. + * The browsing face the picker drives: two listers, because the Host has two. * * `uiWorkspace` is read lazily rather than injected. Injecting it would make * the whole context button conditional on it, and the panel is still the @@ -1168,8 +1384,8 @@ window.__ModuleLoader__.load({ * would freeze at whatever the first render saw, and the service may well * activate after this bundle does. * - * @param {object} ctx - the client context to resolve the service from. - * @returns {object} `available()` and `list()`, the two calls the panel makes. + * @param {object} ctx - the client context to resolve the services from. + * @returns {object} `available()`/`list()` for folders, and the same pair again under `files`. */ function directoryBrowse(ctx) { const workspace = () => ctx.get('uiWorkspace') @@ -1183,6 +1399,24 @@ window.__ModuleLoader__.load({ } return service.listDirectory(path, signal) }, + // Folders only, anywhere on the machine. The file lister below is what + // makes an extra selectable by name; this one is what makes it + // reachable at all, and the two are needed together. + files: { + available: () => typeof ctx.get('remote.workspaceFiles')?.list === 'function', + list: (sessionId, path, signal) => { + const server = ctx.get('remote.workspaceFiles') + if (server === undefined || typeof server.list !== 'function') { + return Promise.reject(new Error('This deployment cannot list files.')) + } + // The Host confines this to the Session's workspace, which is why the + // model falls back to the folder listing above the session folder. + return server.list(sessionId, path, signal).then((result) => { + if (result?.ok !== true) throw result?.error ?? new Error('The Host refused that listing.') + return result.value + }) + }, + }, } } @@ -1266,7 +1500,13 @@ window.__ModuleLoader__.load({ absoluteFrom, relativeTo, isAbsolutePath, + withinPath, + joinUnder, + trailTo, + folderListing, + workspaceListing, directoryBrowse, + fileLister, } return module.exports }, diff --git a/package.json b/package.json index 4797353..11b3c45 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,8 @@ "@deepseek-ai/dsh-client-ui-settings", "@deepseek-ai/dsh-client-ui-plugin-manager", "@deepseek-ai/dsh-client-ui-conversation", - "@deepseek-ai/dsh-client-ui-workspace" + "@deepseek-ai/dsh-client-ui-workspace", + "@deepseek-ai/dsh-api-workspace-files" ] } }, diff --git a/test/client.test.js b/test/client.test.js index 183c0db..c8488df 100644 --- a/test/client.test.js +++ b/test/client.test.js @@ -35,9 +35,10 @@ async function loadClient() { SettingsForm: 'SettingsForm', SettingsValueField: 'SettingsValueField', SegmentedControl: 'SegmentedControl', - // The listing's row glyph; a client half may only use what the primitives - // actually export, so the stand-in is named the same way. + // The listing's row glyphs; a client half may only use what the primitives + // actually export, so the stand-ins are named the same way. IconFolderCloseRegular: 'IconFolderCloseRegular', + FileTypeIcon: 'FileTypeIcon', /** Stands in for the real model: records the specs and answers with the value. */ SettingsFormModel: class { constructor(scope, specs) { @@ -558,19 +559,46 @@ test('the browse face is read from uiWorkspace at the call, not at registration' const missing = mod.directoryBrowse({ get: () => undefined }) assert.equal(missing.available(), false) await assert.rejects(missing.list('/tmp'), /no directory browser/) + // The file lister is a second service and probes separately: a deployment can + // have the folder browser without the workspace file service. + assert.equal(missing.files.available(), false) + await assert.rejects(missing.files.list('session-1', '/tmp'), /cannot list files/) const calls = [] const present = mod.directoryBrowse({ - get: () => ({ - listDirectory: (path, signal) => { - calls.push([path, signal]) - return Promise.resolve('listing') - }, - }), + get: (name) => + ({ + uiWorkspace: { + listDirectory: (path, signal) => { + calls.push(['folders', path, signal]) + return Promise.resolve('listing') + }, + }, + 'remote.workspaceFiles': { + list: (sessionId, path, signal) => { + calls.push(['files', sessionId, path, signal]) + return Promise.resolve({ ok: true, value: 'workspace listing' }) + }, + }, + })[name], }) assert.equal(present.available(), true) assert.equal(await present.list('/tmp', 'signal'), 'listing') - assert.deepEqual(calls, [['/tmp', 'signal']]) + assert.deepEqual(calls, [['folders', '/tmp', 'signal']]) + + // The remote answers with a result envelope rather than rejecting, so its + // failure branch is turned into one here — the model's catch expects a throw. + assert.equal(present.files.available(), true) + assert.equal(await present.files.list('session-1', '/work/root', 'signal'), 'workspace listing') + assert.deepEqual(calls[1], ['files', 'session-1', '/work/root', 'signal']) + + const refusing = mod.directoryBrowse({ + get: (name) => + name === 'remote.workspaceFiles' + ? { list: () => Promise.resolve({ ok: false, error: new Error('workspace files are unavailable') }) } + : undefined, + }) + await assert.rejects(refusing.files.list('session-1', '/work/root'), /workspace files are unavailable/) }) test('the picker model lists a level, supersedes a stale one, and keeps a typed path on failure', async () => { @@ -578,7 +606,7 @@ test('the picker model lists a level, supersedes a stale one, and keeps a typed const asked = [] const model = new mod.DirectoryPickerModel({ - list: (path) => { + folders: (path) => { asked.push(path) return Promise.resolve(listingFixture()) }, @@ -588,7 +616,7 @@ test('the picker model lists a level, supersedes a stale one, and keeps a typed await settle() assert.deepEqual(asked, ['/work/root/src'], 'the opening level is listed, not guessed at') - assert.equal(model.getSnapshot().listing.path, '/work/root/src') + assert.equal(model.getSnapshot().listing.base, '/work/root/src') assert.equal(model.getSnapshot().draft, '/work/root/src', 'the field follows a successful listing') assert.equal(model.getSnapshot().loading, false) @@ -597,10 +625,11 @@ test('the picker model lists a level, supersedes a stale one, and keeps a typed model.open('/work/root/other') model.setDraft('notes/CONTROLLER.md') - // A failure keeps the level on screen and the path as typed: the Host lists - // folders, and the extras legitimately name a file. + // A failure keeps the level on screen and the path as typed: a listing can + // fail on a path this panel may still legitimately declare. const failing = new mod.DirectoryPickerModel({ - list: () => Promise.reject(Object.assign(new Error('transport'), { rpcError: { message: 'directory-unreadable' } })), + folders: () => + Promise.reject(Object.assign(new Error('transport'), { rpcError: { message: 'directory-unreadable' } })), root: '/work/root', start: 'notes/CONTROLLER.md', }) @@ -613,7 +642,7 @@ test('the picker lists folders, hides hidden ones until asked, and answers in de const { module: mod } = await loadClient() const model = new mod.DirectoryPickerModel({ - list: () => Promise.resolve(listingFixture()), + folders: () => Promise.resolve(listingFixture()), root: '/work/root', start: '/work/root/src', }) @@ -668,7 +697,7 @@ test('the extras picker names what it accepts and says what the list holds', asy const { module: mod } = await loadClient() const model = new mod.DirectoryPickerModel({ - list: () => Promise.resolve(listingFixture()), + folders: () => Promise.resolve(listingFixture()), root: '/work/root', start: '/work/root/src', }) @@ -683,14 +712,14 @@ test('the extras picker names what it accepts and says what the list holds', asy onPick: () => {}, }) - // "folder or path" promised one thing and offered another: the rows are - // folders, and a file is added by naming it. The copy says both. + // "folder or path" promised one thing and offered another. The rows now carry + // files, and the copy says what a row of each kind does. assert.equal(tree.props.title, 'Add a path or a file') const texts = textsOf(tree) assert.ok(texts.includes('Path or file'), 'the field is named for what it accepts') assert.ok( - texts.some((text) => text.includes('The list holds folders')), - 'the copy explains why no file rows are shown', + texts.some((text) => text.includes('Folders open; a file is taken')), + 'the copy explains what a file row does', ) assert.ok( buttonsOf(tree).some((button) => (button.children ?? [])[0] === 'Add'), @@ -698,3 +727,220 @@ test('the extras picker names what it accepts and says what the list holds', asy ) }) +// ── files in the extras picker ─────────────────────────────────────────────── +// +// Two Host listers, one of which carries files. The folder primitive browses +// anywhere and returns folders; the workspace file listing returns files and +// folders but confines itself to the session's workspace. The extras picker +// takes the file listing wherever the Host allows it, and the folder listing +// everywhere else, so a path above the session folder is still reachable. + +/** A level shaped the way `remote.workspaceFiles.list` answers. */ +function workspaceFixture() { + return { + path: 'src', + entries: [ + { name: 'shared', type: 'directory' }, + { name: '.git', type: 'directory' }, + { name: 'index.ts', type: 'file', size: 120 }, + { name: 'CONTROLLER.md', type: 'file', size: 900 }, + { name: 'socket', type: 'other' }, + ], + truncated: false, + } +} + +test('the model takes the file listing inside the workspace and the folder listing above it', async () => { + const { module: mod } = await loadClient() + + const asked = [] + const model = new mod.DirectoryPickerModel({ + folders: (path) => { + asked.push(['folders', path]) + return Promise.resolve(listingFixture()) + }, + files: (sessionId, path) => { + asked.push(['files', sessionId, path]) + return Promise.resolve(workspaceFixture()) + }, + sessionId: 'session-1', + root: '/work/root', + start: '/work/root/src', + }) + await settle() + + // Inside the workspace: the file listing, normalised into the picker's own + // shape, with the workspace-relative paths joined onto the session folder. + assert.deepEqual(asked, [['files', 'session-1', '/work/root/src']]) + const level = model.getSnapshot().listing + assert.equal(level.base, '/work/root/src') + assert.equal(level.files, true, 'this level could carry file rows') + assert.deepEqual( + level.entries.map((entry) => [entry.name, entry.path, entry.directory, entry.hidden]), + [ + ['shared', '/work/root/src/shared', true, false], + ['.git', '/work/root/src/.git', true, true], + ['index.ts', '/work/root/src/index.ts', false, false], + ['CONTROLLER.md', '/work/root/src/CONTROLLER.md', false, false], + ['socket', '/work/root/src/socket', false, false], + ], + ) + // The trail is the workspace's own, root first: the file listing stops at the + // session folder, so there is nothing above it to offer. + assert.deepEqual( + level.crumbs.map((crumb) => crumb.path), + ['/work/root', '/work/root/src'], + ) + + // Above the workspace root: the Host confines the file listing to the + // workspace, so the level falls back to folders — which browse anywhere. + model.open('/work') + await settle() + assert.deepEqual(asked[1], ['folders', '/work']) + assert.equal(model.getSnapshot().listing.files, false) +}) + +test('a file listing that fails still leaves a browsable level', async () => { + const { module: mod } = await loadClient() + + // The comparison the model makes is a prefix test on a path the Host may + // resolve differently — a symlinked session folder, say — so a refusal from + // the file listing is answered rather than shown: the folder listing runs. + const model = new mod.DirectoryPickerModel({ + folders: () => Promise.resolve(listingFixture()), + files: () => Promise.reject(new Error('outside the workspace')), + sessionId: 'session-1', + root: '/work/root', + start: '/work/root/src', + }) + await settle() + + assert.equal(model.getSnapshot().error, '') + assert.equal(model.getSnapshot().listing.files, false) + assert.equal(model.getSnapshot().listing.base, '/work/root/src') +}) + +test('a file row is taken as the path to add rather than walked into', async () => { + const { module: mod } = await loadClient() + + // Both listers refuse a path that names a file, which is what the Host does: + // the file listing because a file has no children, the folder listing because + // the same. Only a level can be listed. + const isFile = (path) => /\.(md|ts)$/.test(String(path)) + const refuseFile = (path, answer) => + isFile(path) ? Promise.reject(new Error('not a directory')) : Promise.resolve(answer) + + const model = new mod.DirectoryPickerModel({ + folders: (path) => refuseFile(path, listingFixture()), + files: (sessionId, path) => refuseFile(path, workspaceFixture()), + sessionId: 'session-1', + root: '/work/root', + start: '/work/root/src', + }) + await settle() + + const picked = [] + const render = () => + mod.DirectoryPicker({ + model, + root: '/work/root', + target: 'extra', + disabled: false, + onClose: () => {}, + onPick: (path) => picked.push(path), + }) + + const rowAt = (tree, path) => + buttonsOf(tree) + .filter((button) => button.props.className === mod.ROW_CLASS) + .find((button) => button.props.title === path) + + const fileRow = rowAt(render(), '/work/root/src/CONTROLLER.md') + assert.notEqual(fileRow, undefined, 'the file is a row in the listing') + assert.equal(fileRow.props.style.background, 'transparent', 'an unpicked file row is not marked') + // A folder row carries the folder glyph and a file row the file-type one, which + // is the only thing telling the two apart before the click. + const glyphOf = (row) => (row.children ?? [])[0].children?.[0]?.type + assert.equal(glyphOf(fileRow), 'FileTypeIcon') + assert.equal(glyphOf(rowAt(render(), '/work/root/src/shared')), 'IconFolderCloseRegular') + + // Clicking it fills the field instead of listing it, and marks the row: a file + // is taken, not entered. + fileRow.props.onClick() + assert.equal(model.getSnapshot().draft, '/work/root/src/CONTROLLER.md') + assert.notEqual(rowAt(render(), '/work/root/src/CONTROLLER.md').props.style.background, 'transparent') + + // Enter on a file the level lists asks for a listing of a file, which is not a + // failure: the level stays and nothing is reported. + model.open(model.getSnapshot().draft) + await settle() + assert.equal(model.getSnapshot().error, '') + assert.equal(model.getSnapshot().listing.base, '/work/root/src', 'the level survives it') + + // The footer then adds exactly that, in the declared form. + const addButton = render().props.footer.find((entry) => (entry.children ?? [])[0] === 'Add') + addButton.props.onClick() + assert.deepEqual(picked, ['src/CONTROLLER.md']) +}) + +test('files are offered to the extras control only, and only where the Host has them', async () => { + const { module: mod } = await loadClient() + + const browse = mod.directoryBrowse({ + get: (name) => (name === 'remote.workspaceFiles' ? { list: () => {} } : undefined), + }) + assert.equal(mod.fileLister(browse, 'extra'), browse.files.list, 'an extra can name a file') + // A focus is a folder, and file rows there would only offer something that + // cannot be chosen. + assert.equal(mod.fileLister(browse, 'focus'), undefined) + // No file service, or no browse face at all: the folder listing stands. + assert.equal(mod.fileLister(mod.directoryBrowse({ get: () => undefined }), 'extra'), undefined) + assert.equal(mod.fileLister({}, 'extra'), undefined) + assert.equal(mod.fileLister(undefined, 'extra'), undefined) +}) + +test('the picker says so when a level is folders only, and not when it is not', async () => { const { module: mod } = await loadClient() + + const model = new mod.DirectoryPickerModel({ + folders: () => Promise.resolve(listingFixture()), + root: '/work/root', + start: '/work/root/src', + }) + await settle() + + const tree = mod.DirectoryPicker({ + model, + root: '/work/root', + target: 'extra', + disabled: false, + onClose: () => {}, + onPick: () => {}, + }) + assert.ok( + textsOf(tree).some((text) => text.includes('Files are listed inside the session folder only')), + 'a folders-only level explains itself', + ) + + // A level that does carry files needs no note about it. + const withFiles = new mod.DirectoryPickerModel({ + folders: () => Promise.resolve(listingFixture()), + files: () => Promise.resolve(workspaceFixture()), + sessionId: 'session-1', + root: '/work/root', + start: '/work/root/src', + }) + await settle() + assert.equal( + textsOf( + mod.DirectoryPicker({ + model: withFiles, + root: '/work/root', + target: 'extra', + disabled: false, + onClose: () => {}, + onPick: () => {}, + }), + ).some((text) => text.includes('Files are listed inside the session folder only')), + false, + ) +})