From 719287454b218928709dae10efd75cc803c53435 Mon Sep 17 00:00:00 2001 From: Evan Gress <106449014+evangress@users.noreply.github.com> Date: Mon, 17 Aug 2026 17:23:52 -0400 Subject: [PATCH 1/2] feat(sidebar): new, rename and delete, without leaving the app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Toril could open and save a note but not organize one: renaming meant going to the file manager and coming back. This adds new note, new folder, rename and delete to the files pane, and in doing so gives two already-built, already-tested pieces of Rust their first caller — trashbin shipped in v1.0.0 with no UI at all, and snapshots::rekey was written for exactly this rename. The rules go in crates/fileops rather than the command layer, because they are not obvious and they are mostly Windows: reserved device names that appear to work and then behave like hardware, names ending in a dot or space that Windows silently strips (so the file that appears is not the one we returned a path for), and a case-only rename that a case-insensitive filesystem reports as a collision with the file itself. Every operation refuses to clobber and requires its target inside the open folder, mirroring trashbin's boundary for the delete direction. Containment is checked with canonicalize and never used to build a result. On Windows that returns a \\?\C:\... path, which does I/O fine and matches nothing else in the app — not the sidebar tree, not an open tab, not a note's history key — so it would quietly split one note into two identities. Pinned by a test. Delete offers Undo instead of asking first: the file moves into .trash/, so the act is reversible and a confirmation would be friction in front of it. The one thing trash cannot bring back is an unsaved buffer, and that is the one case that stops and asks. The section 3 surface here is rename. The watcher reports it as delete then create, which is the shape removedOnDisk exists to catch — left alone, an open tab decides its file vanished and offers to recreate it, resurrecting the old note beside its new name. doRenameEntry re-points every affected tab (including tabs under a renamed folder), bumps the removal epoch for each old path so an in-flight reconcile cannot apply a stale "missing" verdict, and clears removedOnDisk — all in one synchronous block. base is deliberately untouched: a rename changes no bytes, so the merge base is still exactly right. The context menu is DOM, not native: a native popup cannot be driven by the headless gates, and native dialogs are the one thing documented to hang the app on the Linux dev box. vaultscan now keeps an empty directory. It pruned any folder without markdown in it, which meant New Folder created something that immediately disappeared and could not be put a note into. Asset-only folders stay pruned. Gates: cargo test -p fileops (28), tests/sidebar.test.ts, tests/contextmenu.test.ts. Two real defects were caught writing them — a window blur listener registered with capture, which sees every element's blur and so closed the menu the instant it focused its own first item; and the canonicalized-path leak above. Not verified on a device: the browser harness could not be driven in this session (the Chrome extension was not connected). Checklist in docs/ON-DEVICE-VERIFICATION.md section E. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 8 +- CHANGELOG.md | 22 + CLAUDE.md | 82 ++- ROADMAP.md | 36 +- dev-harness.html | 143 ++++- docs/ON-DEVICE-VERIFICATION.md | 43 +- src-tauri/Cargo.lock | 5 + src-tauri/Cargo.toml | 2 + src-tauri/crates/fileops/Cargo.toml | 7 + src-tauri/crates/fileops/src/lib.rs | 764 ++++++++++++++++++++++++++ src-tauri/crates/vaultscan/src/lib.rs | 44 +- src-tauri/src/commands/entries.rs | 114 ++++ src-tauri/src/commands/mod.rs | 1 + src-tauri/src/lib.rs | 4 + src/ipc.ts | 89 +++ src/main.ts | 207 ++++++- src/styles/chrome.css | 132 +++++ src/ui/contextmenu.ts | 198 +++++++ src/ui/sidebar.ts | 533 ++++++++++++++++-- tests/contextmenu.test.ts | 188 +++++++ tests/sidebar.test.ts | 315 +++++++++++ 21 files changed, 2868 insertions(+), 69 deletions(-) create mode 100644 src-tauri/crates/fileops/Cargo.toml create mode 100644 src-tauri/crates/fileops/src/lib.rs create mode 100644 src-tauri/src/commands/entries.rs create mode 100644 src/ui/contextmenu.ts create mode 100644 tests/contextmenu.test.ts create mode 100644 tests/sidebar.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 63240ef..c12d53b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,7 +85,7 @@ jobs: workspaces: ./src-tauri -> target # Named packages, NOT --workspace: that would pull in the Tauri app crate - # and with it WebKitGTK on Linux / WebView2 on Windows. These nine are + # and with it WebKitGTK on Linux / WebView2 on Windows. These ten are # split out (CLAUDE.md §4) precisely so their gates run without a webview. # # keystore's tests all run against its in-memory double. The real @@ -94,7 +94,10 @@ jobs: # # Windows matters here too: fsatomic is the §3.1 atomic-save gate and the # one crate doing real filesystem syscalls, where replace-over-existing - # differs between rename(2) and MoveFileEx. + # differs between rename(2) and MoveFileEx. fileops is the second such + # crate and needs Windows even more specifically: the names it rejects + # (reserved devices, trailing dots) and the case-only rename it must allow + # are *Windows* behaviours, and the Linux leg alone cannot see either. - name: Test logic crates run: > cargo test @@ -107,6 +110,7 @@ jobs: -p snapshots -p mergemd -p keystore + -p fileops # `--all` covers workspace *members*, and the vendored glib is excluded # from the workspace (§2) — so unlike clippy, this never touches upstream diff --git a/CHANGELOG.md b/CHANGELOG.md index 4406952..88a7ab6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,28 @@ GitHub Release notes plus the commits that shipped in it. ## [Unreleased] ### Added +- **Manage your notes from the files pane.** New note, new folder, rename and delete, + from a right-click on any file or folder — or the `＋` button beside the folder name. + Until now Toril could open and save notes but not *organize* them: renaming a note + meant leaving for the file manager and coming back. + + **Deleting moves the note to a `.trash` folder inside your workspace, and offers you + Undo.** Nothing is unlinked, so a delete is recoverable long after the message has + gone — and because it is recoverable, Toril does not interrupt you to confirm it. The + one thing trash cannot bring back is a buffer you never saved, so that is the one case + that stops and asks. + + **A rename takes the note's version history with it**, and moves any open tab — + including every tab inside a renamed folder — to the new path. Renaming does not + rewrite a single byte of the file. + + Names are checked before anything touches disk, against the rules Windows actually + enforces: no `< > : " / \ | ? *`, nothing ending in a space or a dot, and none of the + reserved device names (`CON`, `NUL`, `COM1`…) that appear to work and then behave like + hardware. Renaming `notes.md` to `Notes.md` — a change of case only — works. Nothing + can be created or renamed outside the folder you opened, and no operation will ever + overwrite a file that is already there. + - **Toril can update itself.** `v1.0.0` had no update path at all, so every copy was stranded on the version it was installed with — the only way forward was to notice a new release and download the installer by hand. Toril now checks for a newer build diff --git a/CLAUDE.md b/CLAUDE.md index ca6c3a0..8dcf893 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -136,6 +136,16 @@ The **QoL half of the same branch** has also landed: editor zoom, open-links-in- drag-and-drop open, a recent-files list in File → Open Recent, and a real first-run welcome note distinct from the empty state. See §5 (Quality-of-life batch). +**Sidebar file operations — `feat/sidebar-file-ops` (ROADMAP Movement II.12).** New note, +new folder, rename and delete, from a context menu in the files pane. This closes two +pieces of already-built, already-tested Rust that had **no caller at all**: `trashbin` +(shipped in v1.0.0 with no UI) and `snapshots::rekey` (written for exactly this rename). +New crate `crates/fileops` holds the rules; `commands/entries.rs` wraps it; the UX is +`src/ui/sidebar.ts` on a new `src/ui/contextmenu.ts`. Details and the rename-ordering +argument are in §5 (Sidebar file operations). Still open from the branch: drag-to-move in +the tree, multi-select, and a trash browser (`list_trash` has a command and a wrapper but +no UI, so Undo reaches only the most recent delete). + **Next:** finish Phase 4 — remaining is Azure Trusted Signing once an account exists, and on-device verification. A backlog of further QoL features is in §13. The **forward plan beyond Phase 4** — turning the editor into a notes *system* (search, links, version @@ -151,8 +161,8 @@ pnpm tauri build # production .exe + installer (Windows; see §9) pnpm test # vitest — round-trip + toolbar + theme + export + tabs + security (jsdom) pnpm typecheck # tsc --noEmit (TS strict) pnpm build # tsc + vite build (frontend only) -# logic crates — the same nine CI runs (plain `cargo test` also builds the app crate) -cd src-tauri && cargo test -p fsatomic -p vaultscan -p mdhtml -p mdrtf -p imgasset -p trashbin -p snapshots -p mergemd -p keystore +# logic crates — the same ten CI runs (plain `cargo test` also builds the app crate) +cd src-tauri && cargo test -p fsatomic -p vaultscan -p mdhtml -p mdrtf -p imgasset -p trashbin -p snapshots -p mergemd -p keystore -p fileops cd src-tauri && cargo fmt --all && cargo clippy # clean before commit (§10) ``` @@ -277,7 +287,8 @@ toril/ │ │ ├── html-serializer.ts # the ONE HTML <-> doc converter (§3.2/§3.3) │ │ └── html-constructs.ts # richer HTML-only schema: callout/details/dl/mark/sub/sup (§6) │ ├── ui/ -│ │ ├── sidebar.ts # file tree +│ │ ├── sidebar.ts # file tree + file operations UX (§5, ROADMAP II.12) +│ │ ├── contextmenu.ts # the one in-app context menu (DOM, not native — §8) │ │ ├── panes.ts # PURE pane state: visibility, widths, rail tab (§13) │ │ ├── rail.ts # the single tabbed right rail (outline | history) │ │ ├── resizer.ts # drag-to-resize: pure geometry + thin DOM binding @@ -310,13 +321,15 @@ toril/ │ ├── trashbin/ # soft-delete to workspace .trash/ + restore (§3) │ ├── snapshots/ # content-addressed local version history (§3, ROADMAP I.3) │ ├── mergemd/ # line-based 3-way merge + conflict filenames (§3, ROADMAP I.4) - │ └── keystore/ # OS-keychain API key storage (§3, ROADMAP IV.20) + │ ├── keystore/ # OS-keychain API key storage (§3, ROADMAP IV.20) + │ └── fileops/ # create/rename: name rules, containment, no clobber (ROADMAP II.12) └── src/ ├── main.rs # bin entry → lib::run() ├── lib.rs # Tauri builder + menu + command registration ├── menu.rs # native app menu → `menu` events (§8) ├── commands/ │ ├── files.rs # open / save (ATOMIC) / save_as + │ ├── entries.rs # create note/folder, rename (+ history rekey) — ROADMAP II.12 │ ├── workspace.rs # open folder, list tree, watch (notify crate) │ ├── export.rs # markdown_to_html + export_html; export_rtf (all-Rust) │ ├── images.rs # save_clipboard_image (imgasset) @@ -350,6 +363,10 @@ frontend never touches the filesystem directly; it asks via `invoke()`. | `save_recovery` | `entries` | `()` | **atomic** write of `recovery.json` in the app config dir — crash-recovery journal (§3) | | `load_recovery` | — | `RecoveryEntry[]` | empty on missing/corrupt (never bricks startup) | | `clear_recovery` | — | `()` | delete `recovery.json` — the clean-shutdown sentinel | +| `create_note` | `vault_root, dir, name` | `path` | Create an **empty** note in `dir` (`crates/fileops`). `name` may omit the extension (`.md` added). `create_new`, so the existence check and the creation are one operation — two racing creates cannot both win | +| `create_folder` | `vault_root, parent, name` | `path` | `create_dir`, not `create_dir_all`: an existing folder is an error, not a silent success | +| `suggest_note_name` | `vault_root, dir, stem` | `name` | `Untitled.md`, else `Untitled 2.md`… A **suggestion** for the inline field only; `create_note` still refuses a collision. Takes `vault_root` although it only reads: it answers "does this file exist?" for any path handed to it, and the webview is untrusted (§3.3) | +| `rename_entry` | `vault_root, path, new_name` | `path` | Rename within the same parent, refusing to clobber a *different* entry (a **case-only** rename is allowed — resolved via `canonicalize`, since on a case-insensitive filesystem the destination "exists" as the source itself). Carries version history through `snapshots::rekey` — for a folder, every note in the subtree — **best-effort and additive**: the rename already happened, and history failing to follow must not report it as a failure | | `move_to_trash` | `vault_root, path` | `TrashEntry` | soft-delete into workspace `.trash/` via `trashbin` — **atomic** move (§3) | | `list_trash` | `vault_root` | `TrashEntry[]` | newest first; empty when no `.trash/` | | `restore_from_trash` | `vault_root, id` | `path` | restore to original path; errors **without clobbering** an existing file | @@ -402,9 +419,34 @@ frontend never touches the filesystem directly; it asks via `invoke()`. > `manifest.json`) rather than `rm`; restore reads the manifest and atomically renames > it back, refusing to clobber a file that reappeared at the path. `.trash/` starts with > `.`, so `vaultscan` already hides it from the sidebar (§1) and Obsidian hides it too. -> Backed by `crates/trashbin`; commands are not yet called by any UI (the sidebar file-ops -> branch wires them). +> Backed by `crates/trashbin`, and wired to the sidebar as of Movement II.12. +> +> **Sidebar file operations (Movement II.12).** `crates/fileops` holds the rules — +> name validation, vault containment, refusing to clobber — and `commands/entries.rs` +> is a thin wrapper plus the history rekey. The UX is `src/ui/sidebar.ts` (context menu, +> inline name field) on `src/ui/contextmenu.ts`. **The menu is a DOM menu, not a native +> one**, and deliberately: a native popup cannot be driven by the headless gates, and +> native dialogs are the one thing documented to hang the app on the Linux dev box. The +> single native dialog left is the unsaved-changes confirm on delete (`confirmDiscard`), +> which is the close guard's prompt and the same judgement call. +> +> **Delete offers Undo instead of asking first.** The move is into `.trash/`, so it is +> reversible by construction; a confirmation would be friction in front of a reversible +> act. `restore_from_trash` — dead code until this branch — is what the Undo calls. What +> trash cannot restore is an **unsaved buffer**, so that case does stop and ask. > +> **A rename is the §3 surface of this feature, and the ordering in `doRenameEntry` is +> the fix.** The watcher reports a rename as delete-then-create, which is exactly the +> shape `removedOnDisk` exists to catch — left alone, an open tab would decide its file +> had vanished and offer to recreate it, resurrecting the old note beside its new name. +> So, in one synchronous block after the rename resolves: re-point every affected tab +> (including tabs *under* a renamed folder), bump `removalEpoch` for each old path (a +> `reconcile` may be in flight against it and would apply a `missing` verdict to a tab +> that has moved), and clear `removedOnDisk` in case the delete event already landed. +> `base` is deliberately **not** reset — a rename changes no bytes, so the merge base is +> still exactly right and re-reading would only invite a race. +> + > **Version history.** Every save records a content-addressed snapshot (`crates/snapshots`): > per-note dir under `/history//` — a `manifest.json` + gzip, > sha256-addressed blobs. It lives **outside the vault** (like recovery/session, §1) so it @@ -582,6 +624,18 @@ Phases 0–3 are complete and Phase 4 (polish) is in progress; the shipped detai never drops a line. The wire protocol's fifth outcome, `missing`, is produced one layer up in `src-tauri/src/commands/sync.rs` (an `io::ErrorKind::NotFound` on the read, before `mergemd` is even called) — that file has **no tests of its own**, so `missing` is exercised on-device only. +- **File operations:** `cargo test -p fileops` — the name rules (the Windows-forbidden character + set, control characters, trailing dot/space, reserved device names *with* an extension, and the + near-misses that must still be **accepted**: `console.md`, `com10.md`), containment against a + traversal dressed up as a subdirectory, every create/rename refusing to clobber while a + **case-only** rename still succeeds, and that a returned path keeps the *caller's* spelling — + the regression that pins it: containment is checked with `canonicalize`, which on Windows + returns `\\?\C:\…`, and building the result from that hands back a path matching neither the + sidebar tree, nor an open tab, nor the note's history key. Plus `tests/sidebar.test.ts` (the menu + offers only what is wired, a rejected name keeps the field open with the message, blur cancels + rather than commits, and a background refresh cannot delete what is being typed) and + `tests/contextmenu.test.ts` (one menu at a time; dismissal actually removes its document + listeners; the menu closes *before* its handler runs, since handlers take focus). - **External-change policy:** `tests/sync.test.ts` — `decideAction`'s outcome→action mapping is total and fails closed, HTML never auto-merges, and `selectSavable` excludes a diverged tab from every bulk write path (§5). @@ -638,9 +692,11 @@ Phases 0–3 are complete and Phase 4 (polish) is in progress; the shipped detai **CI runs these automatically** on every pull request and on pushes to `main` (`.github/workflows/ci.yml`): `pnpm typecheck` + `pnpm test` + `pnpm build`, and `cargo test` over the -nine logic crates — each on **Ubuntu and Windows**, plus `cargo fmt --all --check` on Ubuntu. The -Windows leg is not ceremony: `pnpm install --frozen-lockfile` is what applies the Milkdown patch, and -`fsatomic` is the §3.1 gate whose replace-over-existing semantics differ from POSIX there. +ten logic crates — each on **Ubuntu and Windows**, plus `cargo fmt --all --check` on Ubuntu. The +Windows leg is not ceremony: `pnpm install --frozen-lockfile` is what applies the Milkdown patch, +`fsatomic` is the §3.1 gate whose replace-over-existing semantics differ from POSIX there, and +`fileops` encodes *Windows* naming rules (reserved devices, trailing dots, case-only renames) that +the Linux leg alone cannot see. **What CI cannot cover — still yours to run:** interactive GUI flows (`pnpm tauri dev` — dialogs, menus, the reload prompt), macOS, the Tauri app crate, and `cargo clippy` (§10; excluded from CI @@ -826,6 +882,8 @@ Keep the project's rules: testable logic in `crates/*` or pure TS helpers, all d **Medium (more UI / a new command, but high value):** - **Global workspace search ("find in files")** — a Rust command scanning the vault (sibling to `vaultscan`, keeping scan logic unit-testable) + a results panel. Distinct from in-document Find. -- **Sidebar file operations** — new / rename / delete / new-folder via context menu, backed by new - **atomic** Rust commands; mind the watcher interplay. -- **Document outline / TOC panel** — list headings from the doc, click to scroll. +- ~~**Sidebar file operations**~~ — *shipped* (ROADMAP Movement II.12). Still open from that + branch: **drag-to-move** within the tree, **multi-select**, and a **trash browser** + (`list_trash` has a command and an `ipc.ts` wrapper but no UI — Undo is currently the only + way back, and it only reaches the most recent delete). +- ~~**Document outline / TOC panel**~~ — *shipped* (Movement II.11). diff --git a/ROADMAP.md b/ROADMAP.md index f5bddb3..d749eda 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -63,11 +63,18 @@ no AI. That gap is this roadmap. > after it, so the next feature could not have reached anyone. §7's *trust before reach* > is what settles the order. Read the pointer below as the ordering, not the ladder. > +> **Branch 12 (sidebar file operations) has also landed**, taken ahead of 6 for the same +> kind of reason 10 was: it wired two crates — `trashbin` and `snapshots::rekey` — that +> had shipped with **no caller at all**, and until it landed a note could be opened and +> saved but not renamed or deleted without leaving the app. +> > **▶ Pick up at Movement II, branch 6 — `feat/vault-search`.** (Branch 5 is done bar > Azure signing, which is blocked on an account, not on code.) > Search is the largest remaining functional gap, and branch 7 > (command palette) depends on it. Vet `tantivy` per §2 at -> adoption — it would be the project's largest new dependency. Branch 4's spec lived +> adoption — it would be the project's largest new dependency, and worth weighing +> against a hand-rolled inverted index the way the snapshot store was weighed +> against `gix` (§8). Branch 4's spec lived > **on its own branch**, not on `main`: > `docs/superpowers/specs/2026-07-24-sync-coexistence-design.md`; branch 10's is on > `main` at `docs/superpowers/specs/2026-08-17-frontmatter-properties-design.md`. @@ -303,11 +310,27 @@ editor something you keep using rather than something you tried. frontend (was §13 backlog). - *Touches:* `src/ui/outline.ts`. *Gate:* `tests/outline.test.ts`. -- [ ] **12. `feat/sidebar-file-ops`** — new / rename / delete / new-folder via context menu, - backed by **atomic** Rust commands; delete routes through `trashbin`. - - *Touches:* `commands/files.rs` (new atomic ops + contract rows), `src/ui/sidebar.ts`; - mind the watcher interplay. - - *Gate:* extend `fsatomic` / a new ops suite. +- [x] **12. `feat/sidebar-file-ops`** — *(shipped)* new / rename / delete / new-folder via a + context menu; delete routes through `trashbin` and offers **Undo** rather than a + confirmation, because a move into `.trash/` is reversible by construction. Wires two + crates that had no caller: `trashbin` and `snapshots::rekey`. + - *New crate:* `crates/fileops` — name validation (the Windows rules, applied on every + platform, since a vault is a folder that gets synced), vault containment, and + create/rename that refuse to clobber. Deliberately returns paths in the **caller's** + spelling: `canonicalize` is used to check containment and never to build a result, + because on Windows it yields `\\?\C:\…`, which matches neither the sidebar tree nor an + open tab nor a note's history key. + - *Built:* `commands/entries.rs` (thin wrapper + the history rekey, best-effort so it can + never fail a rename that already happened), `src/ui/contextmenu.ts`, file-ops UX in + `src/ui/sidebar.ts`. + - *§3:* the rename ordering in `main.ts`'s `doRenameEntry` — the watcher reports a rename + as delete-then-create, so tabs are re-pointed, `removalEpoch` bumped and `removedOnDisk` + cleared in one synchronous block, or an open tab would offer to recreate the old note + beside its new name. + - *Gate:* `cargo test -p fileops` + `tests/sidebar.test.ts` + `tests/contextmenu.test.ts`. + - *Open:* drag-to-move in the tree, multi-select, and a trash browser (`list_trash` is + wired to no UI, so Undo reaches only the most recent delete). On-device verification in + `docs/ON-DEVICE-VERIFICATION.md` §E. - [ ] **⬢ RELEASE `v0.5.0-beta.1`** — *"A real notes system."* First **beta**. --- @@ -499,6 +522,7 @@ All follow the `crates/*` pattern: webview-free, unit-tested, healthy pure-Rust | `trashbin` | Soft-delete + restore (atomic) | — | `feat/safe-delete-trash` | | `snapshots` | Content-addressed local version history | hand-rolled (`sha2` + `flate2`); see §8 | `feat/local-version-history` | | `mergemd` | 3-way markdown merge + conflict files | `similar` | `feat/sync-coexistence` | +| `fileops` | Create/rename: name rules, containment, no clobber | — | `feat/sidebar-file-ops` | | `vaultsearch` | Incremental full-text vault index | `tantivy` | `feat/vault-search` | | `linkgraph` | `[[link]]`/`#tag` parse + backlink index | hand-rolled / `pulldown-cmark` | `feat/wikilinks-backlinks` | | `highlight` | Code → highlighted spans | `syntect` (`fancy-regex`) | `feat/code-highlighting` | diff --git a/dev-harness.html b/dev-harness.html index 672ba81..2799bf1 100644 --- a/dev-harness.html +++ b/dev-harness.html @@ -96,6 +96,80 @@ { hash: "c9d0e1f2", saved_at: Date.now() - 86_400_000 * 3, bytes: 201 }, ]; + // ---- in-memory vault mutations (ROADMAP II.12) -------------------------- + // + // Enough of `fileops` / `trashbin` to drive the sidebar's file operations + // here: create, rename, delete-to-trash and restore against TREE + FILES. + // Deliberately *not* a reimplementation of the rules — the real ones are + // unit-tested in Rust. What this stub owes the harness is the small set of + // refusals the UI has to render (a taken name, a reserved device name), so + // the error path can be seen without a backend. + const TRASH = []; + let trashSeq = 0; + + /** The children array a path belongs in, or null if the folder is unknown. */ + function listFor(dir) { + if (dir === "/vault") return TREE; + let found = null; + const walk = (nodes) => { + for (const n of nodes) { + if (n.is_dir && n.path === dir) found = n.children; + else if (n.is_dir) walk(n.children); + } + }; + walk(TREE); + return found; + } + + function findNode(path) { + let hit = null; + const walk = (nodes, parent) => { + for (const n of nodes) { + if (n.path === path) hit = { node: n, siblings: nodes }; + else if (n.is_dir) walk(n.children, n); + } + }; + walk(TREE, null); + return hit; + } + + // vaultscan's order: directories first, then case-insensitive by name. + function insert(list, node) { + list.push(node); + list.sort((a, b) => + a.is_dir === b.is_dir + ? a.name.toLowerCase().localeCompare(b.name.toLowerCase()) + : a.is_dir ? -1 : 1, + ); + } + + const RESERVED = /^(con|prn|aux|nul|com[0-9]|lpt[0-9])(\.|$)/i; + function checkName(name) { + if (!name.trim()) throw new Error("Enter a name."); + if (/[<>:"/\\|?*]/.test(name)) throw new Error(`A name cannot contain ${name.match(/[<>:"/\\|?*]/)[0]}`); + if (/[. ]$/.test(name)) throw new Error("A name cannot end with a space or a dot."); + if (RESERVED.test(name)) throw new Error(`"${name.split(".")[0]}" is a reserved name on Windows.`); + } + + function withExtension(name) { + return /\.(md|markdown|html?)$/i.test(name) ? name : `${name}.md`; + } + + /** + * Move a subtree's paths (and any FILES entries) from one root to another. + * `from` stays the *original* root throughout, so a child's path — still + * un-rewritten when it is visited — slices correctly against it. + */ + function repath(node, from, to) { + const oldPath = node.path; + node.path = to + oldPath.slice(from.length); + if (!node.is_dir && oldPath in FILES) { + FILES[node.path] = FILES[oldPath]; + delete FILES[oldPath]; + } + for (const child of node.children) repath(child, from, to); + } + // ---- fake Tauri bridge -------------------------------------------------- const callbacks = new Map(); let nextCallbackId = 1; @@ -146,7 +220,74 @@ list_history: () => HISTORY, read_snapshot: ({ path }) => FILES[path] ?? "", restore_snapshot: () => null, - list_trash: () => [], + create_note: ({ dir, name }) => { + const fileName = withExtension(name); + checkName(fileName); + const list = listFor(dir); + if (!list) throw new Error("That location is outside the open folder."); + const path = `${dir}/${fileName}`; + if (findNode(path)) throw new Error(`"${fileName}" already exists`); + insert(list, { name: fileName, path, is_dir: false, children: [] }); + FILES[path] = ""; + return path; + }, + create_folder: ({ parent, name }) => { + checkName(name); + const list = listFor(parent); + if (!list) throw new Error("That location is outside the open folder."); + const path = `${parent}/${name}`; + if (findNode(path)) throw new Error(`"${name}" already exists`); + insert(list, { name, path, is_dir: true, children: [] }); + return path; + }, + suggest_note_name: ({ dir, stem }) => { // vaultRoot ignored: one vault here + + for (let n = 1; n < 100; n++) { + const candidate = n === 1 ? `${stem}.md` : `${stem} ${n}.md`; + if (!findNode(`${dir}/${candidate}`)) return candidate; + } + return `${stem}.md`; + }, + rename_entry: ({ path, newName }) => { + checkName(newName); + const hit = findNode(path); + if (!hit) throw new Error("no such entry"); + const parent = path.slice(0, path.lastIndexOf("/")); + const target = `${parent}/${newName}`; + if (target !== path && findNode(target)) throw new Error(`"${newName}" already exists`); + repath(hit.node, path, target); + hit.node.name = newName; + insert(hit.siblings, hit.siblings.splice(hit.siblings.indexOf(hit.node), 1)[0]); + return target; + }, + move_to_trash: ({ path }) => { + const hit = findNode(path); + if (!hit) throw new Error("no such entry"); + hit.siblings.splice(hit.siblings.indexOf(hit.node), 1); + const entry = { + id: `harness-${trashSeq++}`, + original_path: path, + name: hit.node.name, + deleted_at: Date.now(), + }; + TRASH.push({ entry, node: hit.node }); + return entry; + }, + restore_from_trash: ({ id }) => { + const idx = TRASH.findIndex((t) => t.entry.id === id); + if (idx === -1) throw new Error("no such trash entry"); + const { entry, node } = TRASH[idx]; + const parent = entry.original_path.slice(0, entry.original_path.lastIndexOf("/")); + const list = listFor(parent); + if (!list) throw new Error("the original folder is gone"); + // The real `restore` refuses rather than clobbers; so does this, since + // that refusal is a message the UI has to render. + if (findNode(entry.original_path)) throw new Error("something is already there"); + insert(list, node); + TRASH.splice(idx, 1); + return entry.original_path; + }, + list_trash: () => TRASH.map((t) => t.entry), list_api_keys: () => [ { provider: "anthropic", configured: false }, { provider: "openai", configured: false }, diff --git a/docs/ON-DEVICE-VERIFICATION.md b/docs/ON-DEVICE-VERIFICATION.md index 8bae349..faf452c 100644 --- a/docs/ON-DEVICE-VERIFICATION.md +++ b/docs/ON-DEVICE-VERIFICATION.md @@ -1,7 +1,7 @@ # On-Device Verification CI covers the headless gates: `pnpm typecheck` / `test` / `build` and `cargo test` over -the nine logic crates, on Ubuntu and Windows (CLAUDE.md §8). A green PR means those +the ten logic crates, on Ubuntu and Windows (CLAUDE.md §8). A green PR means those passed — **not** that the app was driven. This file is the standing list of what a green PR cannot tell you: interactive flows @@ -214,6 +214,47 @@ that a signed artifact downloads, verifies and replaces a running binary. appears. Ask via Help → Check for Updates: it says it could not check. That asymmetry is the whole design and is easy to regress. +## E. Sidebar file operations (`feat/sidebar-file-ops`, 2026-08-17) + +The rules are gated hard (`cargo test -p fileops`, `tests/sidebar.test.ts`, +`tests/contextmenu.test.ts`), but three things sit outside every headless gate: the +*Windows* filesystem the name rules describe, the watcher's reaction to an operation +Toril itself performed, and whether the version history actually followed the file. + +- [ ] **E1 — Rename does not resurrect the old note.** The §3 case, and the reason + `doRenameEntry` orders its work the way it does. Open a note, rename it from the + sidebar, then wait for the watcher: the tab must follow to the new name, must + **not** show "removed on disk", and autosave must not recreate the old path. Then + repeat with the note *dirty*, and with a note open inside a folder you rename. +- [ ] **E2 — Version history followed the rename.** Save a note two or three times, + rename it, then open the history panel: the earlier versions must still be listed. + `snapshots::rekey` had **no caller before this branch**, so this is its first + real exercise. Repeat for a note inside a renamed folder (the subtree path). +- [ ] **E3 — Delete and Undo.** Delete a note with a tab open: the tab closes, the file + appears under `.trash/`, and the status bar offers Undo. Click it — the note comes + back at its original path. Then delete a note, create a *different* file at that + path, and Undo: it must refuse rather than clobber, and say the note is still in + `.trash`. Finally delete a **dirty** tab's file and confirm the native prompt + appears (the one native dialog this feature uses — see B7 for the Linux hazard). +- [ ] **E4 — Windows name rules against the real filesystem.** The crate's tests assert + what we *reject*; only Windows proves the rejections are the right ones. Try + `CON.md`, `note.` , `note ` (trailing space), `a:b.md` and a 300-character name — + each refused with a readable message beside the field. Then rename `notes.md` to + `Notes.md`: this **must succeed** on NTFS, where the destination "already exists" + as the source itself, and the sidebar must show the new casing. +- [ ] **E5 — Paths keep their spelling.** After creating a note from the sidebar, confirm + the tab, the sidebar highlight and the window title all agree, and that saving it + produces history under the same key. A `\\?\`-prefixed path would work for I/O and + silently split the note into two identities; the crate pins this, the app proves it. +- [ ] **E6 — The context menu in both engines.** It is `position: fixed` at `--z-dialog`. + Confirm in WebKitGTK as well as WebView2 that it is not clipped by a pane's + `overflow: hidden`, that right-clicking near the right edge or the bottom keeps it + on screen, and that Shift+F10 / the menu key opens it anchored to the row rather + than the window corner. +- [ ] **E7 — The refresh race.** Start typing a new note's name, then touch an unrelated + file in the vault from outside (or let a sync client do it). The field must keep + what you typed; the new tree must appear as soon as you confirm or cancel. + ## B. Standing items (pre-existing, not from this branch) - [ ] **B1 — HTML as a first-class format.** Open a real AI-authored `.html` artifact, diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 083b48c..1d99d84 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -1216,6 +1216,10 @@ dependencies = [ "rustc_version", ] +[[package]] +name = "fileops" +version = "0.1.0" + [[package]] name = "filetime" version = "0.2.29" @@ -4839,6 +4843,7 @@ checksum = "756daf9b1013ebe47a8776667b466417e2d4c5679d441c26230efd9ef78692db" name = "toril-app" version = "1.0.0" dependencies = [ + "fileops", "fsatomic", "imgasset", "keystore", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 73e4263..62be048 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -12,6 +12,7 @@ members = [ "crates/snapshots", "crates/mergemd", "crates/keystore", + "crates/fileops", ] # Vendored, security-patched glib (see [patch.crates-io] below). Excluded so it # is never a workspace member — we don't fmt/clippy/test upstream third-party code. @@ -65,6 +66,7 @@ trashbin = { path = "crates/trashbin" } snapshots = { path = "crates/snapshots" } mergemd = { path = "crates/mergemd" } keystore = { path = "crates/keystore" } +fileops = { path = "crates/fileops" } # Self-update, desktop only (ROADMAP Movement I.5). `v1.0.0` shipped with no # update path, so every installed copy is stranded on the version it was diff --git a/src-tauri/crates/fileops/Cargo.toml b/src-tauri/crates/fileops/Cargo.toml new file mode 100644 index 0000000..3c8ff8d --- /dev/null +++ b/src-tauri/crates/fileops/Cargo.toml @@ -0,0 +1,7 @@ +[package] +name = "fileops" +version = "0.1.0" +edition = "2024" +description = "Create / rename primitives for a Toril workspace (CLAUDE.md §3, ROADMAP II.12). Name validation, vault containment and clobber-refusing moves; no Tauri dep so the rules are unit-testable anywhere." + +[dependencies] diff --git a/src-tauri/crates/fileops/src/lib.rs b/src-tauri/crates/fileops/src/lib.rs new file mode 100644 index 0000000..5ac8091 --- /dev/null +++ b/src-tauri/crates/fileops/src/lib.rs @@ -0,0 +1,764 @@ +//! Create / rename primitives for a Toril workspace (CLAUDE.md §3, ROADMAP +//! Movement II.12 `feat/sidebar-file-ops`). +//! +//! Deleting is not here — that is `trashbin`, which soft-deletes into the +//! workspace `.trash/` and can restore. What is here is everything that brings a +//! path into existence or changes which path a note lives at: +//! +//! - [`validate_name`] — the rules a single path component must satisfy. +//! - [`create_note`] / [`create_folder`] — refuse to clobber, never overwrite. +//! - [`rename`] — same parent, new name; refuses to clobber. +//! - [`available_note_name`] — pick an unused default (`Untitled 2.md`). +//! - [`descendant_files`] — the subtree a folder rename moved, so version +//! history can follow it. +//! +//! **Two rules carry the safety, and both are enforced here rather than in the +//! caller.** Every operation requires its target to resolve *inside* the vault +//! root — the webview is untrusted (§3.3) and a crafted `invoke` must not be able +//! to write outside the folder the user opened. And every operation that creates +//! a path refuses an existing one instead of replacing it: there is no flag to +//! force it, because "rename over the note that was already there" is exactly the +//! silent loss §3 forbids. Mirrors `trashbin::move_to_trash`, which draws the +//! same containment boundary for the delete direction. +//! +//! No Tauri dependency: pure `std`, fully unit-tested. + +use std::ffi::OsStr; +use std::fs; +use std::io; +use std::path::{Path, PathBuf}; + +/// Extensions Toril treats as a note, kept in step with `src/paths.ts`'s +/// `OPENABLE` and `tauri.conf.json`'s file associations. +const NOTE_EXTENSIONS: [&str; 4] = ["md", "markdown", "html", "htm"]; + +/// The extension a new note gets when the user types a bare name. +const DEFAULT_EXTENSION: &str = "md"; + +/// Longest single path component we will create. 255 is the limit on NTFS, ext4, +/// APFS and HFS+ alike; measured in bytes because that is what the strictest of +/// them counts. +const MAX_COMPONENT_BYTES: usize = 255; + +/// Characters no path component may contain. +/// +/// This is the **Windows** set, applied on every platform on purpose. Toril's +/// primary target is Windows (§1) but a vault is a plain folder that may be +/// synced from a Mac or a Linux box, so a name that is legal to create here and +/// impossible to open there is a portability bug we would be authoring into the +/// user's own files. `/` and `\` are in the set for a second reason as well: +/// they would turn one component into a path. +const FORBIDDEN_CHARS: [char; 9] = ['<', '>', ':', '"', '/', '\\', '|', '?', '*']; + +/// Device names Windows reserves, which cannot be used as a file name *even with +/// an extension* — `NUL.md` is still the null device. Creating one appears to +/// succeed and then behaves like a device, so it is rejected before we try. +const RESERVED_STEMS: [&str; 4] = ["con", "prn", "aux", "nul"]; + +/// Reserved device-name prefixes that take a single trailing digit (`COM1`, +/// `LPT9`). `0` is included: it is reserved on current Windows and costs nothing. +const RESERVED_NUMBERED: [&str; 2] = ["com", "lpt"]; + +fn invalid(msg: &str) -> io::Error { + io::Error::new(io::ErrorKind::InvalidInput, msg.to_string()) +} + +fn exists(path: &Path) -> io::Error { + io::Error::new( + io::ErrorKind::AlreadyExists, + format!( + "\"{}\" already exists", + path.file_name().unwrap_or_default().to_string_lossy() + ), + ) +} + +/// Whether `stem` is a Windows reserved device name (case-insensitive). +fn is_reserved_stem(stem: &str) -> bool { + let lower = stem.to_ascii_lowercase(); + if RESERVED_STEMS.contains(&lower.as_str()) { + return true; + } + RESERVED_NUMBERED.iter().any(|prefix| { + lower + .strip_prefix(prefix) + .is_some_and(|rest| rest.len() == 1 && rest.as_bytes()[0].is_ascii_digit()) + }) +} + +/// Check that `name` is a usable single path component. +/// +/// Rejects, with a message written for the user rather than the log: emptiness, +/// path separators and the other characters Windows forbids, control characters, +/// `.` / `..`, a trailing space or dot (Windows *silently strips* both, so the +/// file that appears is not the one that was asked for and the caller's returned +/// path would be a lie), Windows reserved device names, and anything over +/// [`MAX_COMPONENT_BYTES`]. +pub fn validate_name(name: &str) -> io::Result<()> { + if name.is_empty() { + return Err(invalid("Enter a name.")); + } + if name.trim().is_empty() { + return Err(invalid("A name cannot be only spaces.")); + } + if name.len() > MAX_COMPONENT_BYTES { + return Err(invalid("That name is too long.")); + } + if name == "." || name == ".." { + return Err(invalid("\".\" and \"..\" are not names.")); + } + if let Some(c) = name.chars().find(|c| FORBIDDEN_CHARS.contains(c)) { + return Err(invalid(&format!("A name cannot contain {c}"))); + } + if name.chars().any(|c| c.is_control()) { + return Err(invalid("A name cannot contain control characters.")); + } + if name.ends_with(' ') || name.ends_with('.') { + return Err(invalid("A name cannot end with a space or a dot.")); + } + // The stem before the *first* dot is what Windows matches a device against, + // so `nul.md` and `nul.tar.gz` are both the null device. + let stem = name.split('.').next().unwrap_or(name); + if is_reserved_stem(stem) { + return Err(invalid(&format!( + "\"{stem}\" is a reserved name on Windows." + ))); + } + Ok(()) +} + +/// Require `inside` to resolve to a location at or under `root`. Both must exist. +/// +/// `canonicalize` resolves `..` and symlinks, which is what makes this a +/// containment check rather than a string comparison, and on Windows it applies +/// the same verbatim prefix to both sides so they compare. +/// +/// **It returns nothing on purpose.** The obvious shape — hand back the resolved +/// path and build on that — would be wrong on Windows, where `canonicalize` +/// returns the extended-length `\\?\C:\…` form. Every path in this app is a +/// string that has to match: the sidebar tree's paths, the tab a path opens, +/// the key a note's version history is stored under. A `\\?\` path works for +/// I/O and matches none of them, so it would silently split one note into two +/// identities. Callers therefore validate here and keep building from the +/// caller's own path spelling. +fn require_inside(root: &Path, inside: &Path) -> io::Result<()> { + let canon_root = fs::canonicalize(root)?; + let canon = fs::canonicalize(inside)?; + if !canon.starts_with(&canon_root) { + return Err(invalid("That location is outside the open folder.")); + } + Ok(()) +} + +/// The name a new note gets on disk: `name` if it already carries a note +/// extension (case-insensitive), otherwise `name.md`. +/// +/// The condition is "*a note* extension", not "any extension", so a name that +/// merely contains a dot still becomes a note: `Meeting 2026.08.17` → +/// `Meeting 2026.08.17.md`, where an any-extension rule would read `.17` as +/// deliberate and leave a file the sidebar cannot show (`vaultscan` lists +/// markdown only). The cost is that `notes.txt` becomes `notes.txt.md` — the +/// right trade, because New Note means a note, and a wrong-looking name is +/// visible and one rename away, while an invisible file is neither. +pub fn with_note_extension(name: &str) -> String { + match Path::new(name).extension().and_then(OsStr::to_str) { + Some(ext) if NOTE_EXTENSIONS.contains(&ext.to_ascii_lowercase().as_str()) => { + name.to_string() + } + _ => format!("{name}.{DEFAULT_EXTENSION}"), + } +} + +/// Create an empty note called `name` in `dir`, and return its path. +/// +/// `create_new` makes the existence check and the creation one atomic operation, +/// so two racing creates cannot both believe they won and one silently truncate +/// the other's note. The file is created **empty**: a new note's first bytes are +/// whatever the user types, and writing a template here would be content they did +/// not ask for (§3). +pub fn create_note(vault_root: &Path, dir: &Path, name: &str) -> io::Result { + let file_name = with_note_extension(name); + validate_name(&file_name)?; + require_inside(vault_root, dir)?; + let path = dir.join(&file_name); + match fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&path) + { + Ok(_) => Ok(path), + Err(e) if e.kind() == io::ErrorKind::AlreadyExists => Err(exists(&path)), + Err(e) => Err(e), + } +} + +/// Create a folder called `name` inside `parent`, and return its path. +/// +/// `create_dir` (not `create_dir_all`) so an existing folder is an error rather +/// than a silent success — the caller asked to make something new, and reporting +/// success for a folder that was already there would let the UI claim it created +/// a note's home that in fact belongs to something else. +pub fn create_folder(vault_root: &Path, parent: &Path, name: &str) -> io::Result { + validate_name(name)?; + require_inside(vault_root, parent)?; + let path = parent.join(name); + match fs::create_dir(&path) { + Ok(()) => Ok(path), + Err(e) if e.kind() == io::ErrorKind::AlreadyExists => Err(exists(&path)), + Err(e) => Err(e), + } +} + +/// Rename `from` to `new_name` within the same parent, and return the new path. +/// +/// A no-op rename (the same name, byte for byte) succeeds and returns the +/// existing path — otherwise confirming an unchanged inline edit would report a +/// clobber error for a file against itself. +/// +/// **`fs::rename` overwrites an existing destination on both Unix and Windows**, +/// so the existence check below is the only thing standing between a rename and +/// a destroyed note. It is a check-then-act, and therefore racy against another +/// process creating that exact path in the window between — the race is accepted +/// (`std` exposes no atomic rename-if-absent that is portable), and it is the +/// same shape `trashbin::restore` already carries. What makes it survivable is +/// that the loser was on disk and is therefore in version history (§3): a +/// clobber is recoverable, undetected-but-recoverable being the standing edge of +/// the guarantee (CLAUDE.md §5). +/// +/// A file's extension is **not** forced or preserved: renaming `note.md` to +/// `note.html` is allowed, because the bytes are untouched and it is the user's +/// file. The caller must not reinterpret an open document's format on the +/// strength of a rename — re-serializing content that did not change is the +/// unrequested rewrite this project exists to avoid. +pub fn rename(vault_root: &Path, from: &Path, new_name: &str) -> io::Result { + validate_name(new_name)?; + require_inside(vault_root, from)?; + let parent = from + .parent() + .ok_or_else(|| invalid("That item has no parent folder."))?; + let to = parent.join(new_name); + + if to == from { + return Ok(to); + } + // Case-only renames (`notes.md` → `Notes.md`) are a real thing users want, and + // on a case-insensitive filesystem they arrive here as "destination exists" — + // against the source file itself. Comparing *names* would not tell that apart + // (they differ, by exactly the case being changed), so resolve the + // destination and ask whether it is the same file: `canonicalize` returns the + // real on-disk name, which equals the already-canonicalized `from` when the + // two are one file. A destination that cannot be resolved is treated as a + // genuine occupant — failing closed on an ambiguous read is the safe + // direction when the alternative is a clobbering rename. + if to.exists() { + let same_file = match (fs::canonicalize(&to), fs::canonicalize(from)) { + (Ok(a), Ok(b)) => a == b, + _ => false, + }; + if !same_file { + return Err(exists(&to)); + } + } + fs::rename(from, &to)?; + Ok(to) +} + +/// An unused `stem`-based note name in `dir` — `Untitled.md`, else `Untitled 2.md`, +/// `Untitled 3.md`, … +/// +/// Used for the default the inline rename field is pre-filled with, so pressing +/// New Note twice in a row proposes a name that will actually work rather than +/// one that fails on confirm. It is a *suggestion*: `create_note` still refuses a +/// collision, because the user can edit the field and something can appear on +/// disk between the suggestion and the create. +pub fn available_note_name(vault_root: &Path, dir: &Path, stem: &str) -> String { + let first = format!("{stem}.{DEFAULT_EXTENSION}"); + // Containment even though this only reads: it answers "does this file + // exist?" for any path handed to it, and the webview is untrusted (§3.3). + // On refusal it returns the plain first candidate — the caller is only + // pre-filling a field, and `create_note` enforces the same boundary for + // real. + if require_inside(vault_root, dir).is_err() { + return first; + } + if !dir.join(&first).exists() { + return first; + } + // Bounded so a pathological directory cannot spin forever; at that point the + // suggestion is wrong but harmless, and `create_note` reports the collision. + for n in 2..1000 { + let candidate = format!("{stem} {n}.{DEFAULT_EXTENSION}"); + if !dir.join(&candidate).exists() { + return candidate; + } + } + first +} + +/// Every regular file under `dir`, recursively, skipping hidden entries. +/// +/// Exists so a **folder** rename can carry version history: the snapshot store is +/// keyed by a note's absolute path, so moving a folder re-keys every note beneath +/// it, and the caller needs the before-list to pair with the after-list. Hidden +/// entries are skipped for the same reason `vaultscan` skips them — `.trash/` and +/// `.obsidian/` are not the user's notes. +/// +/// Unreadable subdirectories are skipped rather than failing the walk: this feeds +/// a best-effort, additive history migration (§3), and one permission-denied +/// folder must not turn a successful rename into a reported failure. +pub fn descendant_files(dir: &Path) -> Vec { + let mut out = Vec::new(); + collect_files(dir, &mut out); + out.sort(); + out +} + +fn collect_files(dir: &Path, out: &mut Vec) { + let Ok(entries) = fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let name = entry.file_name(); + if name.to_string_lossy().starts_with('.') { + continue; + } + let path = entry.path(); + match entry.file_type() { + Ok(t) if t.is_dir() => collect_files(&path, out), + Ok(t) if t.is_file() => out.push(path), + _ => {} + } + } +} + +/// Re-root `path` from under `old_root` to under `new_root`. +/// +/// The other half of the folder-rename history migration: given the file list +/// taken before the move, this says where each one landed. Returns `None` if +/// `path` was not under `old_root`, so a caller cannot accidentally map an +/// unrelated path into the new tree. +pub fn reroot(path: &Path, old_root: &Path, new_root: &Path) -> Option { + let rest = path.strip_prefix(old_root).ok()?; + Some(new_root.join(rest)) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::atomic::{AtomicU64, Ordering}; + use std::time::{SystemTime, UNIX_EPOCH}; + + struct TempDir(PathBuf); + impl TempDir { + fn new(tag: &str) -> TempDir { + static N: AtomicU64 = AtomicU64::new(0); + let n = N.fetch_add(1, Ordering::Relaxed); + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + let dir = std::env::temp_dir().join(format!("fileops-{tag}-{nanos}-{n}")); + fs::create_dir_all(&dir).unwrap(); + TempDir(dir) + } + fn path(&self) -> &Path { + &self.0 + } + } + impl Drop for TempDir { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } + } + + fn write_file(path: &Path, contents: &str) { + if let Some(p) = path.parent() { + fs::create_dir_all(p).unwrap(); + } + fs::write(path, contents).unwrap(); + } + + // ---- validate_name ---------------------------------------------------- + + #[test] + fn accepts_ordinary_names() { + for name in [ + "note.md", + "Meeting Notes.md", + "2026-08-17.md", + "café ☕.md", + "a.very.dotted.name.md", + "-leading-dash.md", + ".hidden.md", // legal; vaultscan just won't list it + ] { + assert!(validate_name(name).is_ok(), "should accept {name}"); + } + } + + #[test] + fn rejects_empty_and_whitespace() { + assert!(validate_name("").is_err()); + assert!(validate_name(" ").is_err()); + } + + #[test] + fn rejects_path_separators_so_a_name_cannot_become_a_path() { + assert!(validate_name("sub/note.md").is_err()); + assert!(validate_name("sub\\note.md").is_err()); + assert!(validate_name("../escape.md").is_err()); + } + + #[test] + fn rejects_the_windows_forbidden_set_on_every_platform() { + for name in [ + "ab.md", "a:b.md", "a\"b.md", "a|b.md", "a?b.md", "a*b.md", + ] { + assert!(validate_name(name).is_err(), "should reject {name}"); + } + } + + #[test] + fn rejects_control_characters() { + assert!(validate_name("note\u{0}.md").is_err()); + assert!(validate_name("note\n.md").is_err()); + assert!(validate_name("note\t.md").is_err()); + } + + #[test] + fn rejects_dot_names_and_trailing_dot_or_space() { + assert!(validate_name(".").is_err()); + assert!(validate_name("..").is_err()); + // Windows strips these silently, so the created file would not be the + // one we returned a path for. + assert!(validate_name("note.").is_err()); + assert!(validate_name("note ").is_err()); + assert!(validate_name("...").is_err()); + } + + #[test] + fn rejects_windows_reserved_device_names_with_or_without_extension() { + for name in [ + "CON", + "con", + "NUL.md", + "nul.tar.gz", + "AUX.md", + "prn.markdown", + "COM1.md", + "lpt9.md", + "COM0.md", + ] { + assert!(validate_name(name).is_err(), "should reject {name}"); + } + // Not reserved: a longer word that merely starts with one, or a + // multi-digit port that Windows does not reserve. + for name in ["console.md", "connection.md", "com10.md", "auxiliary.md"] { + assert!(validate_name(name).is_ok(), "should accept {name}"); + } + } + + #[test] + fn rejects_an_overlong_component() { + let long = format!("{}.md", "a".repeat(300)); + assert!(validate_name(&long).is_err()); + let ok = format!("{}.md", "a".repeat(200)); + assert!(validate_name(&ok).is_ok()); + } + + // ---- extension handling ------------------------------------------------ + + #[test] + fn appends_md_only_when_the_name_lacks_a_note_extension() { + assert_eq!(with_note_extension("note"), "note.md"); + assert_eq!(with_note_extension("note.md"), "note.md"); + assert_eq!(with_note_extension("note.MD"), "note.MD"); + assert_eq!(with_note_extension("note.markdown"), "note.markdown"); + assert_eq!(with_note_extension("page.html"), "page.html"); + // Not a *note* extension, so it still gets one — New Note makes a note. + assert_eq!(with_note_extension("notes.txt"), "notes.txt.md"); + // The case that rule exists for: a dotted name is not an extension. + assert_eq!( + with_note_extension("Meeting 2026.08.17"), + "Meeting 2026.08.17.md" + ); + assert_eq!(with_note_extension("v1.2.3"), "v1.2.3.md"); + } + + // ---- create_note ------------------------------------------------------- + + #[test] + fn creates_an_empty_note_and_returns_its_path() { + let t = TempDir::new("create"); + let path = create_note(t.path(), t.path(), "First note").unwrap(); + + assert_eq!(path.file_name().unwrap(), "First note.md"); + assert_eq!(fs::read_to_string(&path).unwrap(), ""); + } + + /// Every returned path must be spelled the way the caller spelled it. + /// + /// The regression this pins: containment is checked with `canonicalize`, + /// which on Windows returns the extended-length `\\?\C:\…` form. Building + /// the result from that would hand back a path that does real I/O fine and + /// matches nothing else in the app — not the sidebar tree, not an open + /// tab's path, not the note's version-history key — silently splitting one + /// note into two identities. + #[test] + fn returned_paths_keep_the_callers_spelling() { + let t = TempDir::new("spelling"); + let dir = t.path(); + + let created = create_note(dir, dir, "note").unwrap(); + assert_eq!(created, dir.join("note.md")); + + let folder = create_folder(dir, dir, "sub").unwrap(); + assert_eq!(folder, dir.join("sub")); + + let renamed = rename(dir, &created, "other.md").unwrap(); + assert_eq!(renamed, dir.join("other.md")); + + for path in [&created, &folder, &renamed] { + assert!( + !path.to_string_lossy().contains(r"\\?\"), + "{path:?} leaked a canonicalized prefix" + ); + } + } + + #[test] + fn create_note_refuses_to_clobber_an_existing_note() { + let t = TempDir::new("clobber"); + write_file(&t.path().join("note.md"), "precious"); + + let err = create_note(t.path(), t.path(), "note.md").unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::AlreadyExists); + assert_eq!( + fs::read_to_string(t.path().join("note.md")).unwrap(), + "precious", + "the existing note must be untouched" + ); + } + + #[test] + fn create_note_refuses_a_directory_outside_the_vault() { + let t = TempDir::new("outside"); + let vault = t.path().join("vault"); + let elsewhere = t.path().join("elsewhere"); + fs::create_dir_all(&vault).unwrap(); + fs::create_dir_all(&elsewhere).unwrap(); + + let err = create_note(&vault, &elsewhere, "escaped.md").unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + assert!( + !elsewhere.join("escaped.md").exists(), + "nothing may be created outside the vault" + ); + } + + #[test] + fn create_note_refuses_a_traversal_dressed_up_as_a_subdirectory() { + let t = TempDir::new("traverse"); + let vault = t.path().join("vault"); + fs::create_dir_all(&vault).unwrap(); + fs::create_dir_all(t.path().join("elsewhere")).unwrap(); + + let sneaky = vault.join("..").join("elsewhere"); + let err = create_note(&vault, &sneaky, "escaped.md").unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + } + + // ---- create_folder ----------------------------------------------------- + + #[test] + fn creates_a_folder_and_refuses_an_existing_one() { + let t = TempDir::new("folder"); + let path = create_folder(t.path(), t.path(), "Projects").unwrap(); + assert!(path.is_dir()); + + let err = create_folder(t.path(), t.path(), "Projects").unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::AlreadyExists); + } + + #[test] + fn create_folder_does_not_append_an_extension() { + let t = TempDir::new("folder-ext"); + let path = create_folder(t.path(), t.path(), "notes").unwrap(); + assert_eq!(path.file_name().unwrap(), "notes"); + } + + // ---- rename ------------------------------------------------------------ + + #[test] + fn renames_within_the_same_parent() { + let t = TempDir::new("rename"); + let sub = t.path().join("sub"); + fs::create_dir_all(&sub).unwrap(); + let from = sub.join("old.md"); + write_file(&from, "body"); + + let to = rename(t.path(), &from, "new.md").unwrap(); + + assert_eq!(to.parent().unwrap().file_name().unwrap(), "sub"); + assert_eq!(to.file_name().unwrap(), "new.md"); + assert_eq!(fs::read_to_string(&to).unwrap(), "body"); + assert!(!from.exists()); + } + + #[test] + fn rename_refuses_to_clobber_a_different_file() { + let t = TempDir::new("rename-clobber"); + let from = t.path().join("a.md"); + let occupied = t.path().join("b.md"); + write_file(&from, "mine"); + write_file(&occupied, "theirs"); + + let err = rename(t.path(), &from, "b.md").unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::AlreadyExists); + assert_eq!(fs::read_to_string(&occupied).unwrap(), "theirs"); + assert_eq!( + fs::read_to_string(&from).unwrap(), + "mine", + "a refused rename must leave the source in place" + ); + } + + #[test] + fn renaming_to_the_same_name_is_a_no_op_not_a_collision() { + let t = TempDir::new("rename-same"); + let from = t.path().join("note.md"); + write_file(&from, "body"); + + let to = rename(t.path(), &from, "note.md").unwrap(); + + assert_eq!(fs::read_to_string(&to).unwrap(), "body"); + } + + #[test] + fn case_only_rename_is_allowed() { + // On a case-insensitive filesystem the destination "exists" — it is the + // same file. Must not be reported as a collision. + let t = TempDir::new("rename-case"); + let from = t.path().join("notes.md"); + write_file(&from, "body"); + + let to = rename(t.path(), &from, "Notes.md").unwrap(); + + assert_eq!(to.file_name().unwrap(), "Notes.md"); + assert_eq!(fs::read_to_string(&to).unwrap(), "body"); + } + + #[test] + fn rename_refuses_a_name_that_is_a_path() { + let t = TempDir::new("rename-path"); + let from = t.path().join("note.md"); + write_file(&from, "body"); + + assert!(rename(t.path(), &from, "../escaped.md").is_err()); + assert!(rename(t.path(), &from, "sub/escaped.md").is_err()); + assert!(from.exists()); + } + + #[test] + fn rename_refuses_a_source_outside_the_vault() { + let t = TempDir::new("rename-outside"); + let vault = t.path().join("vault"); + fs::create_dir_all(&vault).unwrap(); + let outside = t.path().join("outside.md"); + write_file(&outside, "not yours"); + + let err = rename(&vault, &outside, "taken.md").unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + assert!(outside.exists()); + } + + #[test] + fn renames_a_folder_with_its_contents() { + let t = TempDir::new("rename-dir"); + let dir = t.path().join("old"); + write_file(&dir.join("a.md"), "a"); + write_file(&dir.join("deep").join("b.md"), "b"); + + let to = rename(t.path(), &dir, "new").unwrap(); + + assert_eq!(fs::read_to_string(to.join("a.md")).unwrap(), "a"); + assert_eq!(fs::read_to_string(to.join("deep/b.md")).unwrap(), "b"); + assert!(!dir.exists()); + } + + // ---- available_note_name ----------------------------------------------- + + #[test] + fn suggests_the_first_unused_name() { + let t = TempDir::new("suggest"); + let vault = t.path(); + assert_eq!(available_note_name(vault, vault, "Untitled"), "Untitled.md"); + + write_file(&vault.join("Untitled.md"), ""); + assert_eq!( + available_note_name(vault, vault, "Untitled"), + "Untitled 2.md" + ); + + write_file(&vault.join("Untitled 2.md"), ""); + assert_eq!( + available_note_name(vault, vault, "Untitled"), + "Untitled 3.md" + ); + } + + #[test] + fn suggestion_does_not_probe_outside_the_vault() { + let t = TempDir::new("suggest-outside"); + let vault = t.path().join("vault"); + let elsewhere = t.path().join("elsewhere"); + fs::create_dir_all(&vault).unwrap(); + fs::create_dir_all(&elsewhere).unwrap(); + write_file(&elsewhere.join("Untitled.md"), ""); + + // Would be `Untitled 2.md` if the existence check had run out there. + assert_eq!( + available_note_name(&vault, &elsewhere, "Untitled"), + "Untitled.md" + ); + } + + // ---- subtree walk / reroot --------------------------------------------- + + #[test] + fn lists_descendant_files_and_skips_hidden() { + let t = TempDir::new("walk"); + write_file(&t.path().join("a.md"), "a"); + write_file(&t.path().join("deep/b.md"), "b"); + write_file(&t.path().join(".trash/gone/c.md"), "c"); + write_file(&t.path().join(".hidden.md"), "h"); + + let files = descendant_files(t.path()); + + assert_eq!(files.len(), 2, "got {files:?}"); + assert!(files.iter().any(|p| p.ends_with("a.md"))); + assert!(files.iter().any(|p| p.ends_with("b.md"))); + } + + #[test] + fn descendant_files_of_a_missing_directory_is_empty_not_an_error() { + let t = TempDir::new("walk-missing"); + assert!(descendant_files(&t.path().join("nope")).is_empty()); + } + + #[test] + fn reroot_maps_a_subtree_path_and_refuses_an_unrelated_one() { + let old = Path::new("/vault/old"); + let new = Path::new("/vault/new"); + + assert_eq!( + reroot(Path::new("/vault/old/deep/b.md"), old, new), + Some(PathBuf::from("/vault/new/deep/b.md")) + ); + assert_eq!(reroot(Path::new("/vault/other/b.md"), old, new), None); + } +} diff --git a/src-tauri/crates/vaultscan/src/lib.rs b/src-tauri/crates/vaultscan/src/lib.rs index c6230e6..e5e386e 100644 --- a/src-tauri/crates/vaultscan/src/lib.rs +++ b/src-tauri/crates/vaultscan/src/lib.rs @@ -6,8 +6,11 @@ //! - Hidden entries (name starting with `.`) are skipped — this drops `.git`, //! `.obsidian`, etc. //! - Only `.md` / `.markdown` files are listed. -//! - A directory is included only if its subtree contains at least one markdown -//! file, so empty / asset-only folders don't clutter the tree. +//! - A directory is included if its subtree contains at least one markdown file +//! **or** it is empty, so asset-only folders don't clutter the tree but a +//! folder the user just made is still there. That second half is not a +//! refinement — without it, New Folder (ROADMAP II.12) creates a directory +//! that immediately disappears and cannot be put a note into. //! - Entries are sorted directories-first, then case-insensitive by name. //! //! No Tauri dependency: the walk is pure `std` + `serde` and fully unit-tested. @@ -46,7 +49,7 @@ fn scan_dir(dir: &Path) -> io::Result> { if file_type.is_dir() { let children = scan_dir(&path)?; - if !children.is_empty() { + if !children.is_empty() || is_visibly_empty(&path) { nodes.push(FileNode { name, path: path.to_string_lossy().into_owned(), @@ -72,6 +75,23 @@ fn scan_dir(dir: &Path) -> io::Result> { Ok(nodes) } +/// Whether `dir` holds nothing the user would see — no entries at all, or only +/// hidden ones. +/// +/// Distinguishes "you just made this" from "this is an assets folder". A folder +/// with a PNG in it stays pruned; a folder with nothing in it is shown, because +/// the alternative is New Folder appearing to do nothing. An unreadable +/// directory answers `false`: unknown is not empty, and guessing the other way +/// would put a folder in the tree that nothing can be done with. +fn is_visibly_empty(dir: &Path) -> bool { + match fs::read_dir(dir) { + Ok(entries) => !entries + .flatten() + .any(|e| !e.file_name().to_string_lossy().starts_with('.')), + Err(_) => false, + } +} + fn is_markdown(name: &str) -> bool { let lower = name.to_lowercase(); lower.ends_with(".md") || lower.ends_with(".markdown") @@ -139,6 +159,24 @@ mod tests { assert!(file.path.ends_with("b-note.md")); } + /// A folder the user just made has nothing in it yet. Pruning it would make + /// New Folder (ROADMAP II.12) look like it had failed, and leave nowhere to + /// put the first note. + #[test] + fn keeps_an_empty_directory_but_still_prunes_an_asset_only_one() { + let t = TempDir::new(); + let root = &t.0; + touch(&root.join("note.md")); + fs::create_dir_all(root.join("Brand new")).unwrap(); + touch(&root.join("assets/pic.png")); + // Only hidden entries: nothing the user can see, so it counts as empty. + touch(&root.join("quiet/.keep")); + + let names: Vec = scan(root).unwrap().into_iter().map(|n| n.name).collect(); + + assert_eq!(names, ["Brand new", "quiet", "note.md"]); + } + #[test] fn empty_folder_yields_empty_tree() { let t = TempDir::new(); diff --git a/src-tauri/src/commands/entries.rs b/src-tauri/src/commands/entries.rs new file mode 100644 index 0000000..1664e55 --- /dev/null +++ b/src-tauri/src/commands/entries.rs @@ -0,0 +1,114 @@ +//! Workspace entry commands (CLAUDE.md §5, ROADMAP Movement II.12): create a +//! note, create a folder, rename either. +//! +//! Thin wrappers over the `fileops` crate (aliased `ops`, the `commands/snapshots.rs` +//! precedent) — the rules that make these safe live there and are unit-tested +//! without Tauri: name validation, vault containment, and a refusal to clobber. +//! Deleting is not here; it is `commands/trash.rs`, which soft-deletes and can +//! restore. +//! +//! The one piece of real logic in this file is **carrying version history across +//! a rename**. The snapshot store is keyed by a note's absolute path, so a rename +//! would otherwise orphan every version of that note behind a path nothing points +//! at any more. `snapshots::rekey` exists for exactly this and has had no caller +//! until now. It is applied **best-effort and additively** (§3), matching how +//! snapshots are taken on save: the rename already succeeded on disk, and history +//! failing to follow must not turn it into a reported failure — the user would be +//! left with a completed rename and an error message, unable to tell what state +//! anything is in. + +use std::path::{Path, PathBuf}; + +use fileops as ops; +use tauri::AppHandle; + +fn as_string(path: PathBuf) -> String { + path.to_string_lossy().into_owned() +} + +/// Create an empty note called `name` inside `dir`, and return its path. +/// +/// `name` may omit the extension (`.md` is added). Refuses to overwrite an +/// existing file and refuses a `dir` outside `vault_root`. +#[tauri::command] +pub fn create_note(vault_root: String, dir: String, name: String) -> Result { + ops::create_note(Path::new(&vault_root), Path::new(&dir), &name) + .map(as_string) + .map_err(|e| e.to_string()) +} + +/// Create a folder called `name` inside `parent`, and return its path. +#[tauri::command] +pub fn create_folder(vault_root: String, parent: String, name: String) -> Result { + ops::create_folder(Path::new(&vault_root), Path::new(&parent), &name) + .map(as_string) + .map_err(|e| e.to_string()) +} + +/// A name the caller can safely propose for a new note in `dir` (`Untitled.md`, +/// `Untitled 2.md`, …). +/// +/// A suggestion only — `create_note` still refuses a collision, because the user +/// edits the field before confirming and a sync daemon can land a file in +/// between. Its job is to stop New Note twice in a row from proposing a name +/// that is already taken. +#[tauri::command] +pub fn suggest_note_name(vault_root: String, dir: String, stem: String) -> Result { + Ok(ops::available_note_name( + Path::new(&vault_root), + Path::new(&dir), + &stem, + )) +} + +/// Rename the file or folder at `path` to `new_name`, within the same parent. +/// +/// Returns the new path. Version history follows the note (or, for a folder, +/// every note beneath it) — see the module docs for why that is best-effort. +#[tauri::command] +pub fn rename_entry( + app: AppHandle, + vault_root: String, + path: String, + new_name: String, +) -> Result { + let from = Path::new(&path); + let was_dir = from.is_dir(); + // Taken *before* the move: after it, the old tree no longer exists to walk. + let old_files = if was_dir { + ops::descendant_files(from) + } else { + Vec::new() + }; + + let to = ops::rename(Path::new(&vault_root), from, &new_name).map_err(|e| e.to_string())?; + + // The rename is done. Everything below is additive (§3) and cannot fail it. + // + // `fileops` returns paths in the caller's own spelling, so the pre-move file + // list (built by walking `from`) and the new root (`to`) share a prefix and + // `reroot` pairs them directly — no canonicalization to reconcile. + if was_dir { + for old_file in old_files { + let Some(new_file) = ops::reroot(&old_file, from, &to) else { + continue; + }; + carry_history(&app, &as_string(old_file), &as_string(new_file)); + } + } else { + carry_history(&app, &path, &as_string(to.clone())); + } + + Ok(as_string(to)) +} + +/// Move one note's version history from `old_path` to `new_path`. Logged and +/// swallowed on failure — see the module docs. +fn carry_history(app: &AppHandle, old_path: &str, new_path: &str) { + let Ok(root) = super::snapshots::history_root(app) else { + return; + }; + if let Err(e) = snapshots::rekey(&root, old_path, new_path) { + eprintln!("version-history: rekey failed for {old_path} → {new_path}: {e}"); + } +} diff --git a/src-tauri/src/commands/mod.rs b/src-tauri/src/commands/mod.rs index 2d31541..a636061 100644 --- a/src-tauri/src/commands/mod.rs +++ b/src-tauri/src/commands/mod.rs @@ -1,3 +1,4 @@ +pub mod entries; pub mod export; pub mod files; pub mod images; diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 12851c1..d3addb9 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -84,6 +84,10 @@ pub fn run() { commands::recovery::save_recovery, commands::recovery::load_recovery, commands::recovery::clear_recovery, + commands::entries::create_note, + commands::entries::create_folder, + commands::entries::suggest_note_name, + commands::entries::rename_entry, commands::trash::move_to_trash, commands::trash::list_trash, commands::trash::restore_from_trash, diff --git a/src/ipc.ts b/src/ipc.ts index 76d5175..dcb0fc3 100644 --- a/src/ipc.ts +++ b/src/ipc.ts @@ -68,6 +68,82 @@ export function openFolder(path: string): Promise { return invoke("open_folder", { path }); } +/** + * Create an empty note called `name` inside `dir` (§5, ROADMAP II.12). + * + * `name` may omit the extension — Rust adds `.md`. Rejects, rather than + * overwrites, an existing file, and rejects a `dir` outside `vaultRoot`; both + * rules live in `crates/fileops` so there is one place they are enforced and + * one place they are tested. Resolves to the new note's path. + */ +export function createNote(vaultRoot: string, dir: string, name: string): Promise { + return invoke("create_note", { vaultRoot, dir, name }); +} + +/** Create a folder called `name` inside `parent`. Resolves to its path. */ +export function createFolder(vaultRoot: string, parent: string, name: string): Promise { + return invoke("create_folder", { vaultRoot, parent, name }); +} + +/** + * A note name not currently taken in `dir` — `Untitled.md`, `Untitled 2.md`, … + * + * Only a suggestion for the inline field's initial value; {@link createNote} + * still refuses a collision. + */ +export function suggestNoteName( + vaultRoot: string, + dir: string, + stem: string, +): Promise { + return invoke("suggest_note_name", { vaultRoot, dir, stem }); +} + +/** + * Rename the file or folder at `path` to `newName`, in the same parent. + * + * Resolves to the new path. Version history follows the note (or every note + * under a renamed folder) via `snapshots::rekey`, best-effort — see + * `commands/entries.rs`. Refuses to overwrite a different existing entry. + */ +export function renameEntry( + vaultRoot: string, + path: string, + newName: string, +): Promise { + return invoke("rename_entry", { vaultRoot, path, newName }); +} + +/** A soft-deleted item in the workspace trash (mirrors Rust `trashbin::TrashEntry`). */ +export interface TrashEntry { + id: string; + original_path: string; + name: string; + deleted_at: number; +} + +/** + * Soft-delete `path` into `/.trash/` (§3). Resolves to the entry, + * whose `id` is what {@link restoreFromTrash} takes — which is what makes the + * delete undoable rather than merely recoverable-if-you-know-where-to-look. + */ +export function moveToTrash(vaultRoot: string, path: string): Promise { + return invoke("move_to_trash", { vaultRoot, path }); +} + +/** The workspace trash, newest first. */ +export function listTrash(vaultRoot: string): Promise { + return invoke("list_trash", { vaultRoot }); +} + +/** + * Restore trash entry `id` to its original path. Resolves to that path; rejects + * (without clobbering) if a file has since reappeared there. + */ +export function restoreFromTrash(vaultRoot: string, id: string): Promise { + return invoke("restore_from_trash", { vaultRoot, id }); +} + /** Start watching `path` for external changes; events arrive via {@link onWorkspaceChange}. */ export function watchFolder(path: string): Promise { return invoke("watch_folder", { path }); @@ -138,6 +214,19 @@ export async function installCloseGuard( }); } +/** + * Ask the user to confirm an action that would discard unsaved work. + * + * Native, like the close guard's prompt and for the same reason: this is the + * question that must not be missable, and an in-page banner can be scrolled + * past or ignored. It is deliberately the *only* native dialog the file + * operations use — every other decision they need is answered inline in the + * sidebar, where it can be driven by the headless gates. + */ +export function confirmDiscard(question: string): Promise { + return ask(question, { title: "Toril", kind: "warning" }); +} + /** Show the native "About Toril" dialog (Help menu). */ export async function showAbout(): Promise { let version = ""; diff --git a/src/main.ts b/src/main.ts index 1bff961..d36a363 100644 --- a/src/main.ts +++ b/src/main.ts @@ -19,10 +19,14 @@ import { type WorkspaceChange, checkForUpdate, clearRecovery, + confirmDiscard, + createFolder, + createNote, exportHtml, exportRtf, installCloseGuard, listHistory, + moveToTrash, loadRecovery, loadSettings, markdownToHtml, @@ -39,6 +43,8 @@ import { pickFolder, readSnapshot, relaunchApp, + renameEntry, + restoreFromTrash, restoreSnapshot, saveClipboardImage, saveFile, @@ -47,7 +53,9 @@ import { saveSettings, setRecentFiles, showAbout, + suggestNoteName, takeLaunchPath, + type TrashEntry, watchFolder, writeConflictCopy, } from "./ipc"; @@ -178,6 +186,31 @@ function setStatus(msg: string): void { if (el) el.textContent = msg; } +/** + * A status message with one inline action — currently Undo, after a delete. + * + * It lives in the status bar rather than a toast because it must not steal + * focus or cover anything: the user just deleted a file and may be about to + * click another. The next `setStatus` clears it, which is the dismissal — and + * missing the button costs nothing, since the file is sitting in `.trash/` + * either way. This is an affordance for the common case, not the safety net; + * the safety net is that the delete was never destructive. + */ +function setStatusWithAction(msg: string, label: string, onAction: () => void): void { + const el = document.querySelector("#status"); + if (!el) return; + el.textContent = `${msg} `; + const btn = document.createElement("button"); + btn.type = "button"; + btn.className = "status-action"; + btn.textContent = label; + btn.addEventListener("click", () => { + setStatus(""); // one action, one use — a second click would restore twice + onAction(); + }); + el.append(btn); +} + function updateTitle(): void { const tab = tabs.active(); const shown = tab ? `${tab.name}${tab.dirty ? " *" : ""}` : "Toril"; @@ -357,7 +390,7 @@ async function doOpenFolder(): Promise { async function loadWorkspace(path: string): Promise { const tree = await openFolder(path); workspaceRoot = path; - sidebar.setRoot(basename(path), tree); + sidebar.setRoot(basename(path), tree, path); sidebar.setActivePath(tabs.active()?.path ?? null); setStatus(`Opened folder ${basename(path)}`); @@ -571,6 +604,167 @@ function doNew(): void { setStatus("New document"); } +// ---- Workspace file operations (ROADMAP Movement II.12) --------------------- +// +// The rules live in Rust (`crates/fileops` validates and refuses to clobber; +// `trashbin` soft-deletes) and the UX lives in `ui/sidebar.ts`. What is here is +// the part neither of them can own: keeping the *open tabs* consistent with +// what just happened on disk. +// +// That is the §3 surface of this feature. A rename moves the file out from under +// every tab pointing into it, and the watcher reports it as a delete followed by +// a create — the exact shape `removedOnDisk` exists to catch. Left alone, the +// tab would decide its file had vanished and offer to recreate it, resurrecting +// the old note beside its new name. + +/** + * Create a note and open it. + * + * Opening is the point: creating a file the user then has to find in the tree + * is a worse New Note than the Untitled buffer they already have. Rejections + * propagate — the sidebar keeps the name field open and shows the message. + */ +async function doCreateNote(dir: string, name: string): Promise { + if (!workspaceRoot) throw new Error("No folder is open."); + const path = await createNote(workspaceRoot, dir, name); + scheduleSidebarRefresh(); + // The file is empty and was just written by us, so there is nothing to read + // back: open the buffer directly rather than paying a round trip to learn it + // is empty. `base` is "" for the same reason, which is exactly right — that + // is what is on disk. + openDocument(path, basename(path), "", formatForPath(path)); + rememberRecent(path); + scheduleSessionSave(); + setStatus(`Created ${basename(path)}`); +} + +async function doCreateFolder(parent: string, name: string): Promise { + if (!workspaceRoot) throw new Error("No folder is open."); + await createFolder(workspaceRoot, parent, name); + scheduleSidebarRefresh(); + setStatus(`Created ${name}`); +} + +/** + * Rename a note or folder, and move every affected tab with it. + * + * Three things have to happen together, and the order matters: + * + * 1. **Re-point the tabs**, including every tab *underneath* a renamed folder. + * 2. **Bump the removal epoch** for each old path. A `reconcile` may already be + * in flight against the old path; it captured that path before its await and + * would apply its verdict — "missing", most likely — to a tab that has since + * moved. This is the same guard the watcher's remove branch uses. + * 3. **Clear `removedOnDisk`**, because the watcher's delete event for the old + * path may have landed while the rename was in flight. Steps 1–3 run in one + * synchronous block after the await, so no event can interleave between them. + * + * `base` is deliberately *not* reset: a rename does not change a single byte, so + * the merge base is still exactly right and re-reading would only invite a race. + */ +async function doRenameEntry(path: string, newName: string): Promise { + if (!workspaceRoot) throw new Error("No folder is open."); + const newPath = await renameEntry(workspaceRoot, path, newName); + + for (const tab of tabs.list()) { + const old = tab.path; + if (!old) continue; + // Exact match is the file case; the prefix branch is the folder case. The + // prefix is a raw slice because both strings come from the same scan of the + // same tree, so they share a spelling — `isAtOrUnder` is the predicate, + // `startsWith` is what makes the arithmetic safe. + let moved: string | null = null; + if (old === path) moved = newPath; + else if (isAtOrUnder(old, path) && old.startsWith(path)) moved = newPath + old.slice(path.length); + if (moved === null) continue; + + // A queued reconcile for the old path would read a file that no longer + // exists; cancel it as well as invalidating one already running. + const queued = reconcileTimers.get(old); + if (queued) { + clearTimeout(queued); + reconcileTimers.delete(old); + } + removalEpoch.set(old, (removalEpoch.get(old) ?? 0) + 1); + + tabs.setPath(tab.id, moved, basename(moved)); + tabs.setRemovedOnDisk(tab.id, false); + dropRecent(old); + rememberRecent(moved); + } + + scheduleSidebarRefresh(); + updateTitle(); + renderConflictBar(); + scheduleSessionSave(); // the session's open-file paths just changed + setStatus(`Renamed to ${basename(newPath)}`); +} + +/** + * Soft-delete a note or folder, and offer to undo it. + * + * The delete is reversible by construction — `trashbin` moves the file into the + * workspace `.trash/` rather than unlinking it — so this asks nothing in the + * ordinary case, and offers Undo instead. The one thing trash cannot bring back + * is an *unsaved buffer*, so that is the one case that stops and asks. + */ +async function doDeleteEntry(path: string, isDir: boolean): Promise { + if (!workspaceRoot) return; + const affected = tabs + .list() + .filter((t) => t.path !== null && (isDir ? isAtOrUnder(t.path, path) : t.path === path)); + + const unsaved = affected.filter((t) => t.dirty); + if (unsaved.length > 0) { + const names = unsaved.map((t) => t.name).join(", "); + const verb = unsaved.length === 1 ? "has" : "have"; + const ok = await confirmDiscard( + `${names} ${verb} unsaved changes that trash cannot bring back. Delete anyway?`, + ); + if (!ok) return; + } + + let entry: TrashEntry; + try { + entry = await moveToTrash(workspaceRoot, path); + } catch (e) { + setStatus(`Delete failed: ${String(e)}`); + return; + } + + // Closed only after the move succeeded — a failed delete must not also cost + // the user their open tab. + for (const tab of affected) { + if (tab.path) { + removalEpoch.set(tab.path, (removalEpoch.get(tab.path) ?? 0) + 1); + // Offering a deleted note in Open Recent is offering a failure; restoring + // it from trash re-opens it and puts it back at the front. + dropRecent(tab.path); + } + tabs.close(tab.id); + } + if (!isDir) dropRecent(path); // deleted without ever being opened + scheduleSidebarRefresh(); + updateTitle(); + renderConflictBar(); + scheduleSessionSave(); + setStatusWithAction(`Moved ${entry.name} to trash`, "Undo", () => void undoDelete(entry)); +} + +async function undoDelete(entry: TrashEntry): Promise { + if (!workspaceRoot) return; + try { + const path = await restoreFromTrash(workspaceRoot, entry.id); + scheduleSidebarRefresh(); + setStatus(`Restored ${basename(path)}`); + } catch (e) { + // `restore` refuses rather than clobbers when something has reappeared at + // the path, so the file is still safe in `.trash/` — say so, because the + // difference between "not restored" and "lost" is the whole point. + setStatus(`Could not restore ${entry.name} — it is still in .trash (${String(e)})`); + } +} + // ---- Pane layout (sidebar + tabbed rail) ----------------------------------- // // All pane decisions live in `panes.ts` as value transforms; this half only @@ -1051,7 +1245,7 @@ function scheduleSidebarRefresh(): void { if (!workspaceRoot) return; openFolder(workspaceRoot) .then((tree) => { - sidebar.setRoot(basename(workspaceRoot!), tree); + sidebar.setRoot(basename(workspaceRoot!), tree, workspaceRoot); sidebar.setActivePath(tabs.active()?.path ?? null); }) .catch(() => {}); @@ -1663,6 +1857,15 @@ window.addEventListener("DOMContentLoaded", async () => { // Routed through the same action as the menu item, so the empty state's // button cannot drift from File → Open Folder. onOpenFolder: () => runAction("menu_open_folder"), + // These three deliberately do **not** catch: the sidebar keeps its name + // field open on a rejection and shows the message beside it, which is where + // a bad name belongs. Swallowing the error here would leave the field + // looking like it had worked. + onCreateNote: (dir, name) => doCreateNote(dir, name), + onCreateFolder: (parent, name) => doCreateFolder(parent, name), + onRename: (path, newName) => doRenameEntry(path, newName), + onDelete: (path, isDir) => void doDeleteEntry(path, isDir), + suggestName: (dir) => suggestNoteName(workspaceRoot ?? dir, dir, "Untitled"), }); sidebar.setRoot(null, []); tabs = new TabManager(tabbar, { onDeactivate, onActivate, onCloseRequest }); diff --git a/src/styles/chrome.css b/src/styles/chrome.css index 4afe0b1..422b577 100644 --- a/src/styles/chrome.css +++ b/src/styles/chrome.css @@ -206,6 +206,9 @@ body { } .sidebar-root { + display: flex; + align-items: center; + gap: var(--sp-1); font-weight: 600; padding: var(--sp-1) var(--sp-1) var(--sp-2); text-transform: uppercase; @@ -214,6 +217,118 @@ body { color: var(--dim); } +/* The folder name yields to the buttons rather than pushing them out of the + pane — `min-width: 0` is what lets it ellipsize (§12b rule 3). */ +.sidebar-root-name { + flex: 1; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.sidebar-root-btn { + flex: none; + display: flex; + align-items: center; + justify-content: center; + min-width: var(--target-sm); + min-height: var(--target-sm); + padding: 0; + border: 0; + border-radius: var(--r-sm); + background: none; + color: var(--muted); + cursor: pointer; + font: inherit; + font-size: var(--text-md); + line-height: 1; +} + +.sidebar-root-btn:hover { + background: var(--hover-bg); + color: var(--fg); +} + +/* ---- Inline name field (new note / new folder / rename) ------------------- */ +.sidebar-edit { + display: flex; + flex-direction: column; + gap: var(--sp-1); + padding: var(--sp-1) 0; + min-width: 0; +} + +.sidebar-edit-input { + min-height: var(--target-sm); + width: 100%; + min-width: 0; + padding: 0 var(--sp-1); + border: 1px solid var(--accent); + border-radius: var(--r-sm); + background: var(--chrome-raised); + color: var(--fg); + font: inherit; +} + +/* The message the backend refused the name with. `anywhere` because it can + quote a name the user typed, which may be one long unbroken string (§12b + rule 4). */ +.sidebar-edit-error { + font-size: var(--text-xs); + color: var(--danger); + overflow-wrap: anywhere; +} + +/* ---- Context menu -------------------------------------------------------- */ +.context-menu { + position: fixed; + z-index: var(--z-dialog); + min-width: 10rem; + padding: var(--sp-1); + border: 1px solid var(--border-strong); + border-radius: var(--r-md); + background: var(--chrome-raised); + box-shadow: var(--shadow-panel); + font-size: var(--text-sm); +} + +.context-menu-item { + display: flex; + align-items: center; + width: 100%; + min-height: var(--target); + padding: 0 var(--sp-2); + border: 0; + border-radius: var(--r-sm); + background: none; + color: var(--fg); + cursor: pointer; + font: inherit; + text-align: left; + white-space: nowrap; +} + +.context-menu-item:hover:not(:disabled), +.context-menu-item:focus-visible { + background: var(--hover-bg); +} + +.context-menu-item[data-danger="true"] { + color: var(--danger); +} + +.context-menu-item:disabled { + color: var(--dim); + cursor: default; +} + +.context-menu-sep { + height: 1px; + margin: var(--sp-1) 0; + background: var(--border); +} + #sidebar .tree { list-style: none; margin: 0; @@ -1057,6 +1172,23 @@ body { border-top: 1px solid var(--border); } +/* The inline action a status message can carry (Undo, after a delete). Styled + as a link rather than a button: it sits inside a sentence, and the status bar + is 1.75rem tall — a bordered control would not fit without growing it. */ +.status-action { + padding: 0; + border: 0; + background: none; + color: var(--accent); + cursor: pointer; + font: inherit; + text-decoration: underline; +} + +.status-action:hover { + color: var(--fg); +} + .status-right { display: flex; align-items: center; diff --git a/src/ui/contextmenu.ts b/src/ui/contextmenu.ts new file mode 100644 index 0000000..f3c1cd9 --- /dev/null +++ b/src/ui/contextmenu.ts @@ -0,0 +1,198 @@ +// A small in-app context menu (ROADMAP Movement II.12, `feat/sidebar-file-ops`). +// +// **Why not a native menu.** `menu.rs` can build one, and the app menu already +// does — but a native popup cannot be driven by the headless gates, and native +// dialogs are the one thing documented to hang the app on the Linux dev box +// (`docs/ON-DEVICE-VERIFICATION.md`). A DOM menu is testable in jsdom, renders +// identically in both webview engines, and keeps the file operations reachable +// on the box where they are actually developed. +// +// Exactly one menu exists at a time: opening a second closes the first. That is +// enforced here rather than by callers, because a stray menu left behind would +// keep a document-level dismiss listener alive and swallow the next click. + +/** A selectable row. `danger` styles a destructive action (delete). */ +export interface MenuItem { + label: string; + onSelect(): void; + danger?: boolean; + /** Rendered but unselectable, with the reason as its tooltip. */ + disabled?: boolean; + title?: string; +} + +/** A rule between groups. Never focusable — it is decoration, not a row. */ +export type MenuEntry = MenuItem | "separator"; + +export interface ContextMenuOptions { + /** Viewport coordinates, normally from the triggering pointer event. */ + x: number; + y: number; + entries: MenuEntry[]; + /** Where to mount. Defaults to `document.body`. */ + host?: HTMLElement; +} + +let openMenu: HTMLElement | null = null; +let dismiss: (() => void) | null = null; +/** + * Where focus was before the menu opened. + * + * Restored on close, and it matters more here than in most menus: the sidebar + * row that opened this is the thing a keyboard user was navigating, and + * dropping focus to `` would send them back to the top of the document + * every time they pressed Escape. + */ +let returnFocusTo: HTMLElement | null = null; + +/** Close any open context menu. Safe to call when none is open. */ +export function closeContextMenu(): void { + dismiss?.(); +} + +function isItem(entry: MenuEntry): entry is MenuItem { + return entry !== "separator"; +} + +/** + * Open a context menu at `(x, y)`. + * + * Selecting an item closes the menu **before** running its handler: handlers + * open inline editors and move focus, and a menu still on screen would fight + * them for it. + */ +export function openContextMenu(opts: ContextMenuOptions): void { + closeContextMenu(); + + const host = opts.host ?? document.body; + const active = document.activeElement; + returnFocusTo = active instanceof HTMLElement ? active : null; + + const menu = document.createElement("div"); + menu.className = "context-menu"; + menu.setAttribute("role", "menu"); + menu.style.left = `${Math.max(0, opts.x)}px`; + menu.style.top = `${Math.max(0, opts.y)}px`; + + const buttons: HTMLButtonElement[] = []; + for (const entry of opts.entries) { + if (!isItem(entry)) { + const rule = document.createElement("div"); + rule.className = "context-menu-sep"; + // Decorative: exposing it as a menu row would make screen readers count + // separators among the choices. + rule.setAttribute("role", "none"); + menu.append(rule); + continue; + } + + const btn = document.createElement("button"); + btn.type = "button"; + btn.className = "context-menu-item"; + btn.setAttribute("role", "menuitem"); + btn.textContent = entry.label; + if (entry.danger) btn.dataset.danger = "true"; + if (entry.title) btn.title = entry.title; + if (entry.disabled) { + btn.disabled = true; + } else { + btn.addEventListener("click", () => { + close(); + entry.onSelect(); + }); + buttons.push(btn); + } + menu.append(btn); + } + + function close(restoreFocus = true): void { + document.removeEventListener("pointerdown", onPointerDown, true); + document.removeEventListener("keydown", onKeyDown, true); + window.removeEventListener("resize", onDismissEvent); + window.removeEventListener("blur", onDismissEvent); + menu.remove(); + if (openMenu === menu) { + openMenu = null; + dismiss = null; + } + // Only when the menu still owned focus. A pointer dismiss has already moved + // focus somewhere the user chose, and yanking it back would undo that. + if (restoreFocus && returnFocusTo?.isConnected) returnFocusTo.focus(); + returnFocusTo = null; + } + + function onPointerDown(e: PointerEvent): void { + if (e.target instanceof Node && menu.contains(e.target)) return; + close(false); + } + + function onDismissEvent(): void { + close(false); + } + + function onKeyDown(e: KeyboardEvent): void { + if (e.key === "Escape") { + e.preventDefault(); + e.stopPropagation(); + close(); + return; + } + // Tab out of a menu means "I'm done here" everywhere else; closing without + // swallowing the key lets focus move on naturally. + if (e.key === "Tab") { + close(false); + return; + } + if (buttons.length === 0) return; + + const current = buttons.indexOf(document.activeElement as HTMLButtonElement); + let next: number | null = null; + if (e.key === "ArrowDown") next = current < 0 ? 0 : (current + 1) % buttons.length; + else if (e.key === "ArrowUp") + next = current < 0 ? buttons.length - 1 : (current - 1 + buttons.length) % buttons.length; + else if (e.key === "Home") next = 0; + else if (e.key === "End") next = buttons.length - 1; + + if (next !== null) { + e.preventDefault(); + e.stopPropagation(); + buttons[next]?.focus(); + } + } + + host.append(menu); + openMenu = menu; + dismiss = () => close(); + + // Keep the menu on screen. Measured after mounting because the width is not + // known until the labels are laid out; `getBoundingClientRect` reports zeroes + // under jsdom, and the clamp is written to be a no-op in that case rather + // than to produce a negative offset. + const rect = menu.getBoundingClientRect(); + if (rect.width > 0 && opts.x + rect.width > window.innerWidth) { + menu.style.left = `${Math.max(0, window.innerWidth - rect.width)}px`; + } + if (rect.height > 0 && opts.y + rect.height > window.innerHeight) { + menu.style.top = `${Math.max(0, window.innerHeight - rect.height)}px`; + } + + // Capture phase on the document pair: the sidebar, the editor and the global + // keymap all listen at the document level, and an open menu must take the key + // or the click before they act on it. + document.addEventListener("pointerdown", onPointerDown, true); + document.addEventListener("keydown", onKeyDown, true); + // **Not** capture on the window pair, and `blur` is why. `blur` does not + // bubble, but a capturing window listener still sees every element's blur on + // the way down — so the menu would dismiss itself the instant it focused its + // own first item, taking whatever the user was about to click with it. Without + // capture this is what it reads as: the *window* losing focus. + window.addEventListener("resize", onDismissEvent); + window.addEventListener("blur", onDismissEvent); + + buttons[0]?.focus(); +} + +/** Whether a context menu is currently open (for tests and guards). */ +export function isContextMenuOpen(): boolean { + return openMenu !== null; +} diff --git a/src/ui/sidebar.ts b/src/ui/sidebar.ts index 32a98ba..79364c0 100644 --- a/src/ui/sidebar.ts +++ b/src/ui/sidebar.ts @@ -1,8 +1,25 @@ // Workspace file tree (CLAUDE.md §4). Renders the FileNode[] from `open_folder` // as a collapsible tree; clicking a markdown file asks the controller to open // it. Folders use native
for zero-JS collapse. +// +// File operations (ROADMAP Movement II.12) live here as *UX only*: the context +// menu, the inline name field, and where an error message lands. Every rule that +// decides whether an operation is allowed — name validation, vault containment, +// refusing to clobber — is in `crates/fileops`, and every disk write is a Rust +// command (§10). Nothing in this file may be the only thing standing between a +// bad name and the filesystem. +import { openContextMenu, type MenuEntry } from "./contextmenu"; import type { FileNode } from "../ipc"; +/** + * Callbacks the controller supplies. The file-operation ones are **optional**, + * and the menu offers only what is wired — the same rule the empty state already + * follows: a dead control that looks live is worse than an absent one. + * + * The operation callbacks may reject. The sidebar catches, shows the message + * beside the still-open name field, and lets the user correct it — so a rejected + * name costs a keystroke rather than the whole interaction. + */ export interface SidebarCallbacks { onOpenFile(path: string): void; /** @@ -13,68 +30,182 @@ export interface SidebarCallbacks { * is worse than a sentence. */ onOpenFolder?(): void; + /** Create an empty note called `name` in `dir`. */ + onCreateNote?(dir: string, name: string): Promise; + /** Create a folder called `name` in `parent`. */ + onCreateFolder?(parent: string, name: string): Promise; + /** Rename the entry at `path` to `newName`, in the same parent. */ + onRename?(path: string, newName: string): Promise; + /** Delete (soft, to trash) the entry at `path`. */ + onDelete?(path: string, isDir: boolean): void; + /** A note name not already taken in `dir`, to pre-fill the field with. */ + suggestName?(dir: string): Promise; +} + +/** What the open inline field is for. */ +type EditKind = "note" | "folder" | "rename"; + +interface EditState { + kind: EditKind; + /** Container the new entry goes in (create), or the entry's parent (rename). */ + dir: string; + /** The entry being renamed. Absent for creates. */ + target?: { path: string; name: string }; + /** Current field text, so a re-open after a rejected name keeps it. */ + value: string; + error: string | null; } export class Sidebar { + private rootName: string | null = null; + private rootPath: string | null = null; + private tree: FileNode[] = []; + private edit: EditState | null = null; + /** + * A tree that arrived while a name was being typed. + * + * The controller refreshes on every watcher event, and a sync daemon touching + * an unrelated file mid-rename would otherwise re-render the field out from + * under the user, losing what they had typed. Deferring is safe because the + * delay is bounded by the edit: confirming or cancelling flushes immediately, + * and the operation itself triggers a fresh refresh straight after. + */ + private pendingTree: { rootName: string | null; rootPath: string | null; tree: FileNode[] } | null = + null; + private activePath: string | null = null; + constructor( private readonly container: HTMLElement, private readonly cb: SidebarCallbacks, ) {} - /** Render a workspace, or an empty-state hint when no folder is open. */ - setRoot(rootName: string | null, tree: FileNode[]): void { - this.container.replaceChildren(); + /** + * Render a workspace, or an empty-state hint when no folder is open. + * + * `rootPath` is what the file operations need — a new note has to be created + * *somewhere*, and the root heading is a legitimate target. + */ + setRoot(rootName: string | null, tree: FileNode[], rootPath: string | null = null): void { + if (this.edit) { + this.pendingTree = { rootName, rootPath, tree }; + return; + } + this.rootName = rootName; + this.rootPath = rootPath; + this.tree = tree; + this.render(); + } - if (rootName === null) { - // An empty state that only *names* the emptiness leaves the user to find - // the menu; the fix for "no folder open" belongs next to the message. - const wrap = document.createElement("div"); - wrap.className = "sidebar-empty"; - - const hint = document.createElement("p"); - hint.className = "sidebar-empty-text"; - hint.textContent = "No folder open."; - wrap.append(hint); - - if (this.cb.onOpenFolder) { - const btn = document.createElement("button"); - btn.type = "button"; - btn.className = "sidebar-empty-btn"; - btn.textContent = "Open Folder…"; - btn.addEventListener("click", () => this.cb.onOpenFolder?.()); - wrap.append(btn); - - const note = document.createElement("p"); - note.className = "sidebar-empty-note"; - // Worth saying once, here: the folder is the user's, not Toril's. It is - // the single most load-bearing promise in §1 and the empty state is the - // moment someone is deciding whether to point it at a real vault. - note.textContent = - "Pick any folder of notes — including an Obsidian vault. Files stay plain Markdown where they are."; - wrap.append(note); - } + /** Highlight the currently active file by path (no-op if not in the tree). */ + setActivePath(path: string | null): void { + this.activePath = path; + this.applyActive(); + } + + /** Whether an inline name field is currently open (renders are deferred while it is). */ + isEditing(): boolean { + return this.edit !== null; + } + + private applyActive(): void { + for (const el of this.container.querySelectorAll(".file-entry")) { + el.dataset.active = String(el.dataset.path === this.activePath); + } + } - this.container.append(wrap); + private render(): void { + this.container.replaceChildren(); + + if (this.rootName === null) { + this.container.append(this.renderEmptyState()); return; } + this.container.append(this.renderHeading(this.rootName)); + this.container.append(this.renderNodes(this.tree, this.rootPath)); + this.mountEditor(); + this.applyActive(); + } + + private renderEmptyState(): HTMLElement { + // An empty state that only *names* the emptiness leaves the user to find + // the menu; the fix for "no folder open" belongs next to the message. + const wrap = document.createElement("div"); + wrap.className = "sidebar-empty"; + + const hint = document.createElement("p"); + hint.className = "sidebar-empty-text"; + hint.textContent = "No folder open."; + wrap.append(hint); + + if (this.cb.onOpenFolder) { + const btn = document.createElement("button"); + btn.type = "button"; + btn.className = "sidebar-empty-btn"; + btn.textContent = "Open Folder…"; + btn.addEventListener("click", () => this.cb.onOpenFolder?.()); + wrap.append(btn); + + const note = document.createElement("p"); + note.className = "sidebar-empty-note"; + // Worth saying once, here: the folder is the user's, not Toril's. It is + // the single most load-bearing promise in §1 and the empty state is the + // moment someone is deciding whether to point it at a real vault. + note.textContent = + "Pick any folder of notes — including an Obsidian vault. Files stay plain Markdown where they are."; + wrap.append(note); + } + return wrap; + } + + private renderHeading(rootName: string): HTMLElement { const heading = document.createElement("div"); heading.className = "sidebar-root"; - heading.textContent = rootName; - this.container.append(heading); - this.container.append(this.renderNodes(tree)); - } - /** Highlight the currently active file by path (no-op if not in the tree). */ - setActivePath(path: string | null): void { - for (const el of this.container.querySelectorAll(".file-entry")) { - el.dataset.active = String(el.dataset.path === path); + const label = document.createElement("span"); + label.className = "sidebar-root-name"; + label.textContent = rootName; + heading.append(label); + + const root = this.rootPath; + if (root !== null) { + // Buttons as well as the context menu: right-click is invisible, and a + // brand-new note is the one operation someone needs on their first minute + // in a folder. + if (this.cb.onCreateNote) { + heading.append( + this.iconButton("New note", "＋", () => this.beginEdit({ kind: "note", dir: root })), + ); + } + if (this.cb.onCreateFolder) { + heading.append( + this.iconButton("New folder", "🗀", () => this.beginEdit({ kind: "folder", dir: root })), + ); + } + heading.addEventListener("contextmenu", (e) => this.openMenuForFolder(e, root, null)); } + return heading; + } + + private iconButton(label: string, glyph: string, onClick: () => void): HTMLButtonElement { + const btn = document.createElement("button"); + btn.type = "button"; + btn.className = "sidebar-root-btn"; + btn.title = label; + btn.setAttribute("aria-label", label); + // The glyph is decorative — the accessible name comes from aria-label, and + // announcing "plus sign" after it would be noise. + btn.textContent = glyph; + btn.addEventListener("click", onClick); + return btn; } - private renderNodes(nodes: FileNode[]): HTMLElement { + private renderNodes(nodes: FileNode[], dir: string | null): HTMLElement { const ul = document.createElement("ul"); ul.className = "tree"; + // Marks which folder this list belongs to, so an inline "new note" field can + // be mounted into the right one after a render. + if (dir !== null) ul.dataset.dir = dir; for (const node of nodes) { ul.append(node.is_dir ? this.renderDir(node) : this.renderFile(node)); } @@ -87,7 +218,9 @@ export class Sidebar { details.open = true; const summary = document.createElement("summary"); summary.textContent = node.name; - details.append(summary, this.renderNodes(node.children)); + summary.dataset.path = node.path; + summary.addEventListener("contextmenu", (e) => this.openMenuForFolder(e, node.path, node)); + details.append(summary, this.renderNodes(node.children, node.path)); li.append(details); return li; } @@ -99,7 +232,323 @@ export class Sidebar { entry.dataset.path = node.path; entry.textContent = node.name; entry.addEventListener("click", () => this.cb.onOpenFile(node.path)); + entry.addEventListener("contextmenu", (e) => this.openMenuForFile(e, node)); li.append(entry); return li; } + + // ---- context menus ------------------------------------------------------- + + /** + * Menu coordinates. A `contextmenu` event raised from the keyboard (Shift+F10, + * the menu key) carries no pointer position and reports 0,0 — anchoring to the + * row's own box puts the menu where the user is looking instead of the corner + * of the window. + */ + private menuPosition(e: MouseEvent): { x: number; y: number } { + if (e.clientX !== 0 || e.clientY !== 0) return { x: e.clientX, y: e.clientY }; + const target = e.currentTarget; + if (target instanceof HTMLElement) { + const rect = target.getBoundingClientRect(); + return { x: rect.left, y: rect.bottom }; + } + return { x: 0, y: 0 }; + } + + private openMenuForFile(e: MouseEvent, node: FileNode): void { + e.preventDefault(); + e.stopPropagation(); + const dir = parentDir(node.path); + const entries: MenuEntry[] = [ + { label: "Open", onSelect: () => this.cb.onOpenFile(node.path) }, + ]; + if (this.cb.onRename) { + entries.push({ + label: "Rename…", + onSelect: () => + this.beginEdit({ kind: "rename", dir, target: { path: node.path, name: node.name } }), + }); + } + if (this.cb.onDelete) { + entries.push("separator", { + label: "Delete", + danger: true, + title: "Move to the workspace .trash folder — this can be undone", + onSelect: () => this.cb.onDelete?.(node.path, false), + }); + } + openContextMenu({ ...this.menuPosition(e), entries }); + } + + /** `node` is null for the workspace root, which can be added to but not renamed. */ + private openMenuForFolder(e: MouseEvent, path: string, node: FileNode | null): void { + e.preventDefault(); + e.stopPropagation(); + const entries: MenuEntry[] = []; + if (this.cb.onCreateNote) { + entries.push({ + label: "New Note…", + onSelect: () => this.beginEdit({ kind: "note", dir: path }), + }); + } + if (this.cb.onCreateFolder) { + entries.push({ + label: "New Folder…", + onSelect: () => this.beginEdit({ kind: "folder", dir: path }), + }); + } + if (node) { + if (this.cb.onRename) { + entries.push("separator", { + label: "Rename…", + onSelect: () => + this.beginEdit({ + kind: "rename", + dir: parentDir(node.path), + target: { path: node.path, name: node.name }, + }), + }); + } + if (this.cb.onDelete) { + if (!this.cb.onRename) entries.push("separator"); + entries.push({ + label: "Delete", + danger: true, + title: "Move the folder and everything in it to .trash — this can be undone", + onSelect: () => this.cb.onDelete?.(node.path, true), + }); + } + } + if (entries.length === 0) return; + openContextMenu({ ...this.menuPosition(e), entries }); + } + + // ---- the inline name field ---------------------------------------------- + + private beginEdit(edit: Omit & { value?: string }): void { + this.edit = { + ...edit, + value: edit.value ?? edit.target?.name ?? "", + error: null, + }; + this.render(); + + // A create field starts empty and is filled in asynchronously, so the user + // can begin typing immediately and a slow suggestion never overwrites what + // they typed — checked against the field's *current* value, not the state's. + if (edit.kind === "note" && this.cb.suggestName && !edit.value) { + const dir = edit.dir; + void this.cb.suggestName(dir).then((name) => { + const input = this.fieldInput(); + if (!this.edit || this.edit.kind !== "note" || this.edit.dir !== dir) return; + if (!input || input.value !== "") return; + input.value = name; + this.edit.value = name; + selectStem(input); + }); + } + } + + private endEdit(): void { + this.edit = null; + const pending = this.pendingTree; + this.pendingTree = null; + if (pending) { + this.rootName = pending.rootName; + this.rootPath = pending.rootPath; + this.tree = pending.tree; + } + this.render(); + } + + private fieldInput(): HTMLInputElement | null { + return this.container.querySelector(".sidebar-edit-input"); + } + + /** + * Put the field into the rendered tree. + * + * Called after every render so the field survives one — a create is mounted + * into its folder's list, a rename replaces the row it renames. If the anchor + * is missing (the folder disappeared from under us) the edit is abandoned + * rather than mounted somewhere arbitrary. + */ + private mountEditor(): void { + const edit = this.edit; + if (!edit) return; + + const field = this.buildField(edit); + + if (edit.kind === "rename" && edit.target) { + const row = + this.container.querySelector(`.file-entry[data-path="${cssEscape(edit.target.path)}"]`) ?? + this.container.querySelector(`summary[data-path="${cssEscape(edit.target.path)}"]`); + if (!row) { + this.edit = null; + return; + } + row.replaceWith(field); + } else { + const list = this.container.querySelector( + `ul[data-dir="${cssEscape(edit.dir)}"]`, + ); + if (!list) { + this.edit = null; + return; + } + const li = document.createElement("li"); + li.append(field); + list.prepend(li); + // A collapsed folder would hide the field the user is typing into. + const details = list.closest("details"); + if (details) details.open = true; + } + + const input = field.querySelector("input"); + input?.focus(); + if (input) selectStem(input); + } + + private buildField(edit: EditState): HTMLElement { + const wrap = document.createElement("div"); + wrap.className = "sidebar-edit"; + wrap.dataset.kind = edit.kind; + + const input = document.createElement("input"); + input.type = "text"; + input.className = "sidebar-edit-input"; + input.value = edit.value; + input.spellcheck = false; + input.setAttribute( + "aria-label", + edit.kind === "rename" ? `Rename ${edit.target?.name ?? ""}` : `Name of the new ${edit.kind}`, + ); + input.addEventListener("input", () => { + if (this.edit) this.edit.value = input.value; + }); + input.addEventListener("keydown", (e) => { + if (e.key === "Enter") { + e.preventDefault(); + void this.commit(input.value); + } else if (e.key === "Escape") { + e.preventDefault(); + // Escape must not also reach the global keymap (which closes panes). + e.stopPropagation(); + this.endEdit(); + } + }); + // Blur **cancels** rather than commits. Committing on blur is how a + // half-typed name becomes a real rename because someone clicked away; the + // cost of cancelling is retyping, which is recoverable, and this direction + // never touches the filesystem on an action the user did not confirm. + // + // `isConnected` distinguishes the user leaving the field from the field + // being replaced by a re-render — showing an error rebuilds it, and treating + // that as a cancel would wipe the message the moment it appeared. + input.addEventListener("blur", () => { + if (!input.isConnected) return; + if (this.edit && !this.busy) this.endEdit(); + }); + wrap.append(input); + + if (edit.error) { + const err = document.createElement("div"); + err.className = "sidebar-edit-error"; + err.setAttribute("role", "alert"); + err.textContent = edit.error; + wrap.append(err); + } + return wrap; + } + + /** + * True while an operation is in flight. + * + * Guards two things: the blur handler (the field loses focus while the awaited + * command runs, and that must not read as a cancel), and double submission + * from a second Enter before the first resolves — which for a create would be + * a second create attempt against a path the first one is making. + */ + private busy = false; + + private async commit(value: string): Promise { + const edit = this.edit; + if (!edit || this.busy) return; + + const name = value.trim(); + if (name === "") { + this.showError("Enter a name."); + return; + } + // A rename to the same name is a no-op the backend also accepts; short- + // circuiting here keeps it from showing up as a save in version history. + if (edit.kind === "rename" && name === edit.target?.name) { + this.endEdit(); + return; + } + + this.busy = true; + try { + if (edit.kind === "note") await this.cb.onCreateNote?.(edit.dir, name); + else if (edit.kind === "folder") await this.cb.onCreateFolder?.(edit.dir, name); + else if (edit.target) await this.cb.onRename?.(edit.target.path, name); + this.busy = false; + this.endEdit(); + } catch (e) { + this.busy = false; + this.showError(messageOf(e)); + } + } + + /** Re-render the field with an error beside it, keeping what was typed. */ + private showError(error: string): void { + if (!this.edit) return; + this.edit.error = error; + this.render(); + } +} + +/** The containing directory of `path`, using whichever separator it carries. */ +function parentDir(path: string): string { + const cut = Math.max(path.lastIndexOf("/"), path.lastIndexOf("\\")); + return cut > 0 ? path.slice(0, cut) : path; +} + +/** + * Select the name but not the extension, so typing replaces `note` in `note.md`. + * + * The convention every file manager uses, and it matters more here: the field is + * pre-filled with a full file name, and a plain select-all means renaming + * usually starts by retyping `.md`. + */ +function selectStem(input: HTMLInputElement): void { + const dot = input.value.lastIndexOf("."); + // `setSelectionRange` is not implemented for every input type in every engine; + // a failure here costs a nicety, never the rename. + try { + if (dot > 0) input.setSelectionRange(0, dot); + else input.select(); + } catch { + /* selection is cosmetic */ + } +} + +/** + * Quote a path for use inside an attribute selector. + * + * Paths are arbitrary user data and Windows paths are full of backslashes, which + * a selector reads as escapes. `CSS.escape` is the correct tool and exists in + * both webview engines; the fallback keeps this working under a bare jsdom that + * has not implemented it. + */ +function cssEscape(value: string): string { + if (typeof CSS !== "undefined" && typeof CSS.escape === "function") return CSS.escape(value); + return value.replace(/["\\]/g, "\\$&"); +} + +/** The user-facing text of a rejection, which from Tauri is a plain string. */ +function messageOf(e: unknown): string { + if (typeof e === "string") return e; + if (e instanceof Error) return e.message; + return String(e); } diff --git a/tests/contextmenu.test.ts b/tests/contextmenu.test.ts new file mode 100644 index 0000000..8e9f376 --- /dev/null +++ b/tests/contextmenu.test.ts @@ -0,0 +1,188 @@ +// Context-menu gate (CLAUDE.md §8, ROADMAP Movement II.12). +// +// The menu is the entry point to every destructive file operation, so what is +// pinned here is the behaviour that decides whether the *right* action runs: +// only one menu at a time, dismissal actually removing its document listeners, +// and the menu closing before its handler runs (the handlers open an inline +// field and take focus). +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + closeContextMenu, + isContextMenuOpen, + openContextMenu, + type MenuEntry, +} from "../src/ui/contextmenu"; + +afterEach(() => { + closeContextMenu(); + document.body.replaceChildren(); +}); + +function items(): HTMLButtonElement[] { + return Array.from(document.querySelectorAll(".context-menu-item")); +} + +function press(key: string): void { + document.dispatchEvent(new KeyboardEvent("keydown", { key, bubbles: true })); +} + +function open(entries: MenuEntry[]): void { + openContextMenu({ x: 10, y: 10, entries }); +} + +describe("openContextMenu", () => { + it("renders one row per item and skips separators as choices", () => { + open([ + { label: "Open", onSelect: () => {} }, + "separator", + { label: "Delete", onSelect: () => {}, danger: true }, + ]); + + expect(items().map((b) => b.textContent)).toEqual(["Open", "Delete"]); + expect(document.querySelectorAll(".context-menu-sep")).toHaveLength(1); + expect(items()[1].dataset.danger).toBe("true"); + }); + + it("opening a second menu replaces the first", () => { + open([{ label: "A", onSelect: () => {} }]); + open([{ label: "B", onSelect: () => {} }]); + + expect(document.querySelectorAll(".context-menu")).toHaveLength(1); + expect(items().map((b) => b.textContent)).toEqual(["B"]); + }); + + it("closes before running the handler, so the handler can take focus", () => { + let openWhenHandlerRan: boolean | null = null; + open([{ label: "Rename…", onSelect: () => (openWhenHandlerRan = isContextMenuOpen()) }]); + + items()[0].click(); + + expect(openWhenHandlerRan).toBe(false); + expect(isContextMenuOpen()).toBe(false); + }); + + it("Escape closes without selecting anything", () => { + const onSelect = vi.fn(); + open([{ label: "Delete", onSelect }]); + + press("Escape"); + + expect(isContextMenuOpen()).toBe(false); + expect(onSelect).not.toHaveBeenCalled(); + }); + + it("a pointer press outside closes it; one inside does not", () => { + open([{ label: "Open", onSelect: () => {} }]); + + items()[0].dispatchEvent(new MouseEvent("pointerdown", { bubbles: true })); + expect(isContextMenuOpen()).toBe(true); + + document.body.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true })); + expect(isContextMenuOpen()).toBe(false); + }); + + /** + * The failure this pins is silent: a menu that closes but leaves its + * document-level listeners attached goes on swallowing the next Escape or + * click, and nothing about the UI shows why. + */ + it("stops handling keys once closed", () => { + const onSelect = vi.fn(); + open([{ label: "Delete", onSelect }]); + closeContextMenu(); + + press("ArrowDown"); + press("Enter"); + + expect(onSelect).not.toHaveBeenCalled(); + expect(document.querySelector(".context-menu")).toBeNull(); + }); + + it("arrow keys move focus and wrap in both directions", () => { + open([ + { label: "One", onSelect: () => {} }, + { label: "Two", onSelect: () => {} }, + { label: "Three", onSelect: () => {} }, + ]); + const [one, two, three] = items(); + + expect(document.activeElement).toBe(one); + press("ArrowDown"); + expect(document.activeElement).toBe(two); + press("ArrowUp"); + expect(document.activeElement).toBe(one); + press("ArrowUp"); // wraps to the end + expect(document.activeElement).toBe(three); + press("ArrowDown"); // wraps to the start + expect(document.activeElement).toBe(one); + press("End"); + expect(document.activeElement).toBe(three); + press("Home"); + expect(document.activeElement).toBe(one); + }); + + it("skips a disabled item in keyboard traversal and cannot select it", () => { + const onSelect = vi.fn(); + open([ + { label: "Open", onSelect: () => {} }, + { label: "Rename…", onSelect, disabled: true }, + { label: "Delete", onSelect: () => {} }, + ]); + + press("ArrowDown"); + + expect((document.activeElement as HTMLElement).textContent).toBe("Delete"); + items()[1].click(); + expect(onSelect).not.toHaveBeenCalled(); + }); + + it("restores focus to whatever opened it", () => { + const trigger = document.createElement("button"); + document.body.append(trigger); + trigger.focus(); + + open([{ label: "Open", onSelect: () => {} }]); + expect(document.activeElement).not.toBe(trigger); + + press("Escape"); + expect(document.activeElement).toBe(trigger); + }); + + it("does not yank focus back when dismissed by a click elsewhere", () => { + const trigger = document.createElement("button"); + const other = document.createElement("input"); + document.body.append(trigger, other); + trigger.focus(); + + open([{ label: "Open", onSelect: () => {} }]); + other.focus(); + other.dispatchEvent(new MouseEvent("pointerdown", { bubbles: true })); + + expect(document.activeElement).toBe(other); + }); + + /** + * Regression: the dismiss-on-window-blur listener was registered with + * `capture: true`, and a capturing window listener sees every *element's* + * blur too — so opening a menu while anything was focused closed it again + * the instant it focused its own first item. + */ + it("survives opening while another element has focus", () => { + const trigger = document.createElement("button"); + document.body.append(trigger); + trigger.focus(); + + open([{ label: "Open", onSelect: () => {} }]); + + expect(isContextMenuOpen()).toBe(true); + expect(items()).toHaveLength(1); + }); + + it("keeps a negative anchor on screen", () => { + openContextMenu({ x: -50, y: -20, entries: [{ label: "Open", onSelect: () => {} }] }); + const menu = document.querySelector(".context-menu"); + + expect(menu?.style.left).toBe("0px"); + expect(menu?.style.top).toBe("0px"); + }); +}); diff --git a/tests/sidebar.test.ts b/tests/sidebar.test.ts new file mode 100644 index 0000000..0f2a897 --- /dev/null +++ b/tests/sidebar.test.ts @@ -0,0 +1,315 @@ +// Sidebar file-operations gate (CLAUDE.md §8, ROADMAP Movement II.12). +// +// What the sidebar owns is the *interaction*, so that is what is pinned here: +// which operations the menu offers, that a rejected name keeps the field open +// with the message rather than silently doing nothing, that blur cancels rather +// than commits, and that a background refresh cannot delete what the user is +// typing. The rules the operations are judged against live in `crates/fileops` +// and are tested there; the disk itself is never touched from this file. +import { afterEach, describe, expect, it, vi } from "vitest"; +import { closeContextMenu } from "../src/ui/contextmenu"; +import { Sidebar, type SidebarCallbacks } from "../src/ui/sidebar"; +import type { FileNode } from "../src/ipc"; + +const ROOT = "/vault"; + +function file(name: string, dir = ROOT): FileNode { + return { name, path: `${dir}/${name}`, is_dir: false, children: [] }; +} + +function folder(name: string, children: FileNode[], dir = ROOT): FileNode { + return { name, path: `${dir}/${name}`, is_dir: true, children }; +} + +function mount(cb: Partial = {}, tree: FileNode[] = [file("note.md")]) { + const container = document.createElement("div"); + document.body.append(container); + const sidebar = new Sidebar(container, { onOpenFile: () => {}, ...cb }); + sidebar.setRoot("vault", tree, ROOT); + return { sidebar, container }; +} + +function rightClick(el: Element): void { + el.dispatchEvent(new MouseEvent("contextmenu", { bubbles: true, clientX: 5, clientY: 5 })); +} + +function menuLabels(): string[] { + return Array.from(document.querySelectorAll(".context-menu-item")).map( + (b) => b.textContent ?? "", + ); +} + +function clickMenu(label: string): void { + const item = Array.from( + document.querySelectorAll(".context-menu-item"), + ).find((b) => b.textContent === label); + if (!item) throw new Error(`no menu item "${label}" (have: ${menuLabels().join(", ")})`); + item.click(); +} + +function field(container: HTMLElement): HTMLInputElement { + const input = container.querySelector(".sidebar-edit-input"); + if (!input) throw new Error("no name field is open"); + return input; +} + +function typeAndConfirm(input: HTMLInputElement, value: string): void { + input.value = value; + input.dispatchEvent(new Event("input", { bubbles: true })); + input.dispatchEvent(new KeyboardEvent("keydown", { key: "Enter", bubbles: true })); +} + +/** Let the awaited callback inside `commit` settle. */ +const settle = (): Promise => new Promise((r) => setTimeout(r, 0)); + +afterEach(() => { + closeContextMenu(); + document.body.replaceChildren(); +}); + +describe("the tree", () => { + it("renders files and folders, and opens a file on click", () => { + const onOpenFile = vi.fn(); + const { container } = mount({ onOpenFile }, [folder("sub", [file("deep.md", "/vault/sub")])]); + + const entry = container.querySelector(".file-entry"); + expect(entry?.textContent).toBe("deep.md"); + entry?.click(); + expect(onOpenFile).toHaveBeenCalledWith("/vault/sub/deep.md"); + }); +}); + +describe("the context menu", () => { + /** + * The empty state already follows this rule for Open Folder: a control that + * looks live and does nothing is worse than an absent one. It matters more + * here, because the absent handler would be Delete. + */ + it("offers only the operations that are wired", () => { + const { container } = mount({}); + rightClick(container.querySelector(".file-entry")!); + expect(menuLabels()).toEqual(["Open"]); + + closeContextMenu(); + document.body.replaceChildren(); + + const wired = mount({ onRename: vi.fn(), onDelete: vi.fn() }); + rightClick(wired.container.querySelector(".file-entry")!); + expect(menuLabels()).toEqual(["Open", "Rename…", "Delete"]); + }); + + it("offers creation on a folder, and on the workspace root", () => { + const { container } = mount( + { onCreateNote: vi.fn(), onCreateFolder: vi.fn(), onRename: vi.fn(), onDelete: vi.fn() }, + [folder("sub", [file("deep.md", "/vault/sub")])], + ); + + rightClick(container.querySelector("summary")!); + expect(menuLabels()).toEqual(["New Note…", "New Folder…", "Rename…", "Delete"]); + + closeContextMenu(); + rightClick(container.querySelector(".sidebar-root")!); + // The root is the folder the user opened: it can be added to, but Toril has + // no business renaming or trashing it from inside the tree. + expect(menuLabels()).toEqual(["New Note…", "New Folder…"]); + }); + + it("marks Delete as destructive", () => { + const { container } = mount({ onDelete: vi.fn() }); + rightClick(container.querySelector(".file-entry")!); + + const del = Array.from( + document.querySelectorAll(".context-menu-item"), + ).find((b) => b.textContent === "Delete"); + expect(del?.dataset.danger).toBe("true"); + }); + + it("passes is_dir so the caller can find every tab under a folder", () => { + const onDelete = vi.fn(); + const { container } = mount({ onDelete }, [folder("sub", [file("deep.md", "/vault/sub")])]); + + rightClick(container.querySelector("summary")!); + clickMenu("Delete"); + + expect(onDelete).toHaveBeenCalledWith("/vault/sub", true); + }); +}); + +describe("creating", () => { + it("creates a note in the folder that was right-clicked", async () => { + const onCreateNote = vi.fn().mockResolvedValue(undefined); + const { container } = mount({ onCreateNote }, [ + folder("sub", [file("deep.md", "/vault/sub")]), + ]); + + rightClick(container.querySelector("summary")!); + clickMenu("New Note…"); + typeAndConfirm(field(container), "Ideas"); + await settle(); + + expect(onCreateNote).toHaveBeenCalledWith("/vault/sub", "Ideas"); + expect(container.querySelector(".sidebar-edit-input")).toBeNull(); + }); + + it("creates at the root from the header button", async () => { + const onCreateNote = vi.fn().mockResolvedValue(undefined); + const { container } = mount({ onCreateNote }); + + container.querySelector(".sidebar-root-btn")?.click(); + typeAndConfirm(field(container), "Ideas"); + await settle(); + + expect(onCreateNote).toHaveBeenCalledWith(ROOT, "Ideas"); + }); + + it("pre-fills a suggested name without overwriting what is already typed", async () => { + let release!: (name: string) => void; + const suggestName = vi.fn(() => new Promise((r) => (release = r))); + const { container } = mount({ onCreateNote: vi.fn(), suggestName }); + + container.querySelector(".sidebar-root-btn")?.click(); + const input = field(container); + // The user does not wait for the round trip. + input.value = "Ideas"; + input.dispatchEvent(new Event("input", { bubbles: true })); + release("Untitled 2.md"); + await settle(); + + expect(input.value).toBe("Ideas"); + }); + + it("fills in the suggestion when the field is still untouched", async () => { + const suggestName = vi.fn().mockResolvedValue("Untitled 2.md"); + const { container } = mount({ onCreateNote: vi.fn(), suggestName }); + + container.querySelector(".sidebar-root-btn")?.click(); + await settle(); + + expect(field(container).value).toBe("Untitled 2.md"); + }); + + it("refuses an empty name without calling the backend", async () => { + const onCreateNote = vi.fn(); + const { container } = mount({ onCreateNote }); + + container.querySelector(".sidebar-root-btn")?.click(); + typeAndConfirm(field(container), " "); + await settle(); + + expect(onCreateNote).not.toHaveBeenCalled(); + expect(container.querySelector(".sidebar-edit-error")?.textContent).toBe("Enter a name."); + }); +}); + +describe("renaming", () => { + it("sends the new name and closes the field", async () => { + const onRename = vi.fn().mockResolvedValue(undefined); + const { container } = mount({ onRename }); + + rightClick(container.querySelector(".file-entry")!); + clickMenu("Rename…"); + expect(field(container).value).toBe("note.md"); + + typeAndConfirm(field(container), "renamed.md"); + await settle(); + + expect(onRename).toHaveBeenCalledWith("/vault/note.md", "renamed.md"); + }); + + it("treats an unchanged name as a cancel, not a rename", async () => { + const onRename = vi.fn(); + const { container } = mount({ onRename }); + + rightClick(container.querySelector(".file-entry")!); + clickMenu("Rename…"); + typeAndConfirm(field(container), "note.md"); + await settle(); + + // A no-op rename that reached Rust would still be a write in the eyes of the + // watcher, and would show up as a change to a file nothing changed. + expect(onRename).not.toHaveBeenCalled(); + expect(container.querySelector(".sidebar-edit-input")).toBeNull(); + }); + + it("keeps the field open with the message when the backend refuses", async () => { + const onRename = vi.fn().mockRejectedValue('"CON" is a reserved name on Windows.'); + const { container } = mount({ onRename }); + + rightClick(container.querySelector(".file-entry")!); + clickMenu("Rename…"); + typeAndConfirm(field(container), "CON.md"); + await settle(); + + expect(container.querySelector(".sidebar-edit-error")?.textContent).toBe( + '"CON" is a reserved name on Windows.', + ); + // Still editable, and still holding what was typed — a rejected name should + // cost a keystroke, not the whole interaction. + expect(field(container).value).toBe("CON.md"); + }); + + it("Escape cancels without renaming", async () => { + const onRename = vi.fn(); + const { container } = mount({ onRename }); + + rightClick(container.querySelector(".file-entry")!); + clickMenu("Rename…"); + const input = field(container); + input.value = "half-typed"; + input.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true })); + await settle(); + + expect(onRename).not.toHaveBeenCalled(); + expect(container.querySelector(".file-entry")?.textContent).toBe("note.md"); + }); + + /** + * Blur must **cancel**. Committing on blur is how a half-typed name becomes a + * real rename because the user clicked somewhere else — and unlike a cancel, + * that one touches the filesystem on an action nobody confirmed. + */ + it("blur cancels rather than committing", async () => { + const onRename = vi.fn(); + const { container } = mount({ onRename }); + + rightClick(container.querySelector(".file-entry")!); + clickMenu("Rename…"); + const input = field(container); + input.value = "half-typed"; + input.dispatchEvent(new Event("blur", { bubbles: false })); + await settle(); + + expect(onRename).not.toHaveBeenCalled(); + expect(container.querySelector(".sidebar-edit-input")).toBeNull(); + }); +}); + +describe("refreshes while a name is being typed", () => { + /** + * The controller refreshes the tree on every watcher event, so a sync daemon + * touching an unrelated file mid-rename would otherwise re-render the field + * out from under the user and lose what they had typed. + */ + it("defers the new tree until the edit ends, then applies it", async () => { + const onRename = vi.fn().mockResolvedValue(undefined); + const { sidebar, container } = mount({ onRename }); + + rightClick(container.querySelector(".file-entry")!); + clickMenu("Rename…"); + const input = field(container); + input.value = "half-typed"; + input.dispatchEvent(new Event("input", { bubbles: true })); + + sidebar.setRoot("vault", [file("note.md"), file("arrived.md")], ROOT); + + expect(sidebar.isEditing()).toBe(true); + expect(field(container).value).toBe("half-typed"); + expect(container.textContent).not.toContain("arrived.md"); + + input.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true })); + await settle(); + + expect(sidebar.isEditing()).toBe(false); + expect(container.textContent).toContain("arrived.md"); + }); +}); From 073dbbd493b827f604d30448d984d30e385b5426 Mon Sep 17 00:00:00 2001 From: Evan Gress <106449014+evangress@users.noreply.github.com> Date: Tue, 18 Aug 2026 06:49:42 -0400 Subject: [PATCH 2/2] docs(roadmap): correct three stale entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Branch 12b (the chrome rework) shipped on main on 2026-08-12 as PR #36 — panes.ts, rail.ts, resizer.ts and the tokens/chrome/editor stylesheet split are all there — but its checkbox was never ticked, and the status block listed it under "landed outside the movement ladder" when it is a numbered branch in the ladder. Two records of the same thing, both wrong in different directions. The v0.2.0-alpha release point under branch 5 was overtaken by the v1.0.0 tag. The prose above it already says so; the unticked checkbox did not, which left the document implying a version still to be cut. Marked as overtaken and kept for what it meant, rather than deleted — the release point is the record of when the data-safety floor became handable to a stranger, and that is worth keeping even though the number is gone. Branch 12 now names its PR. It was ticked as shipped while nothing was on main and no branch was even pushed, which is the failure mode this document exists to prevent. Co-Authored-By: Claude Opus 5 (1M context) --- ROADMAP.md | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index d749eda..9ef1c6e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -46,7 +46,7 @@ It is a fine **editor**. It is not yet a **notes system** — no global search, quick switcher, no links, no tags, no version history, no sync conflict handling, no AI. That gap is this roadmap. -> **Status (2026-08-17).** Shipped through **`v1.0.0`** (see `CHANGELOG.md`). +> **Status (2026-08-18).** Shipped through **`v1.0.0`** (see `CHANGELOG.md`). > **Movement I, branches 1–4 are complete** (autosave + crash-recovery journal; > safe-delete-to-trash; local version history; sync coexistence — 3-way merge, conflict > banner, parked conflict copies). **Branch 5 has landed** on `feat/release-readiness` — @@ -63,12 +63,14 @@ no AI. That gap is this roadmap. > after it, so the next feature could not have reached anyone. §7's *trust before reach* > is what settles the order. Read the pointer below as the ordering, not the ladder. > -> **Branch 12 (sidebar file operations) has also landed**, taken ahead of 6 for the same -> kind of reason 10 was: it wired two crates — `trashbin` and `snapshots::rekey` — that -> had shipped with **no caller at all**, and until it landed a note could be opened and -> saved but not renamed or deleted without leaving the app. +> **Branch 12 (sidebar file operations) has also landed** — PR #41, stacked on branch 5's +> PR #40 — taken ahead of 6 for the same kind of reason 10 was: it wired two crates — +> `trashbin` and `snapshots::rekey` — that had shipped with **no caller at all**, and until +> it landed a note could be opened and saved but not renamed or deleted without leaving the +> app. **Branch 12b (chrome rework) is ticked too**; it shipped on `main` 2026-08-12 and this +> document had simply never recorded it. > -> **▶ Pick up at Movement II, branch 6 — `feat/vault-search`.** (Branch 5 is done bar +> **▶ In progress: Movement II, branch 6 — `feat/vault-search`.** (Branch 5 is done bar > Azure signing, which is blocked on an account, not on code.) > Search is the largest remaining functional gap, and branch 7 > (command palette) depends on it. Vet `tantivy` per §2 at @@ -82,9 +84,10 @@ no AI. That gap is this roadmap. > *Landed since, outside the movement ladder:* a serializer-normalization precursor > (canonical markdown now matches Obsidian — `-` bullets, `---` rules, tight lists > preserved), which exists to keep branch 4's 3-way merge from drowning in -> reformatting noise; the chrome/layout rework (CLAUDE.md §12b); a fix for programmatic -> loads marking every document dirty; CI running the headless gates on Ubuntu and -> Windows for every pull request; and the GitHub community-standards docs. +> reformatting noise; a fix for programmatic loads marking every document dirty; CI running +> the headless gates on Ubuntu and Windows for every pull request; and the GitHub +> community-standards docs. (The chrome/layout rework — CLAUDE.md §12b — used to be listed +> here as outside the ladder. It is not: it is **branch 12b**, and it is ticked below.) > > *Version note, and read it before leaning on the number:* `v1.0.0` says the editor and > its data-safety floor are ready to depend on — **not** that this roadmap is finished. @@ -246,8 +249,12 @@ nice in a synced folder. This movement is also the prerequisite for the AI wedge `menu.rs`, `main.ts`. - *Gate:* `tests/update.test.ts` for the policy; §D of `docs/ON-DEVICE-VERIFICATION.md` for everything downstream of the network, which no headless gate can reach. - - [ ] **⬢ RELEASE `v0.2.0-alpha`** — *"Safe to live in."* First build you can hand to - someone without an asterisk on their data. + - [~] **⬢ RELEASE `v0.2.0-alpha`** — *"Safe to live in."* **Overtaken by the `v1.0.0` + tag**, which shipped the data-safety floor under a number this ladder never planned + for. Kept as the record of what the release point *meant* — the first build you can + hand to someone without an asterisk on their data — not as a version still to be cut. + The floor itself is complete; only Azure Trusted Signing is outstanding, and it is + blocked on provisioning, not on code. --- @@ -343,7 +350,7 @@ between an editor that works and one that is pleasant to spend hours in. **Branches** -- [ ] **12b. `feat/chrome-ux`** — the UI shell around the editor: a **single tabbed right +- [x] **12b. `feat/chrome-ux`** — *(shipped 2026-08-12)* the UI shell around the editor: a **single tabbed right rail** (outline and history become alternatives, not two rails), pane collapse/expand that actually animates, drag-to-resize with persisted widths, pointer-adaptive touch targets, a real design-token layer, and **real menu accelerators**. Taken out of