From 54d055bc3e0dc2fa0c7dbfcfed9e8e8c32e24706 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 22:05:22 +0000 Subject: [PATCH 1/4] ui: a keyboard registry, a collision gate, and CodeMirror's keymap folded in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A chord used to be spelled twice — a modifier test in main.ts's keydown handler and a row of display text in shortcuts.ts — with nothing but discipline keeping them equal. K1 promises every action has a keyboard path *and* that the path is findable, so a sheet free to fall behind the wiring is a promise waiting to break. And because main.ts can't be imported by node, none of it was testable: the app's whole keyboard contract sat in the one file the suite can't see. The chord is now declared once, in `ui/src/bindings.ts` — id, chord, scope — and the rest derives. The handler matches with `isBound(e, id)`, the editor's keymap takes `chordFor(id)` (the chord syntax is CodeMirror's, so one spelling serves both), and the `?` sheet names ids rather than spelling chords. `shortcuts.test.ts` fails on a binding no row documents, which turns "add the row in the same change" from a convention into something the suite keeps. What deliberately stays as code: the order the surfaces get their turn, and the guard beside each branch. A table rich enough to express "⌘G, but only while the find bar is open" is a `when`-clause language, which is a worse trade at this size than a guard you can read. `conflicts()` is the gate — two commands answering to one keystroke in one scope, run by `npm test` inside `just check` and `just ci`. It knows that a `Mod-` chord answers to ⌃X as well as ⌘X, so `Ctrl-Tab` and `Mod-Tab` are seen to collide. Scopes nest (`overlay:link` under `overlay`), which is what keeps two ⏎ bindings from reading as a clash while still reporting the shadows that are real: the overlay's Tab trap takes ⌃Tab from the Settings rail, and only branch order saves it. `editorkeys.ts` folds in the other keyboard. CodeMirror ships ~100 stock bindings B2 never wrote, and three of B2's chords work while editing only because it happens to leave them alone — an assumption that lived in a parenthesis ("CodeMirror leaves Mod-e unbound, so the event bubbles here"). It's a pinned list now. main.ts installs the stock keymaps *from* that module, so the set checked is the set that runs. Two findings from the fold-in, both pinned rather than changed: - CodeMirror binds Mod-i (selectParentSyntax). ⌘I stays italic only because the format chords are installed ahead of the defaults — load- bearing ordering that nothing noticed before. - The ⌃-as-⌘ alias puts ten B2 chords on macOS's emacs text bindings inside the editor: ⌃F moves the caret *and* opens the find bar, ⌃E goes to end-of-line *and* leaves edit mode. Pre-existing, and it all retires at once if `Mod` stops accepting ⌃ — worth its own decision. Behavior deltas, all narrow: chord matching is now strict about ⌥/⇧ where the hand-rolled tests weren't, so ⇧⌘S no longer force-saves and ⌘⏎ no longer commits the link dialog. Escape and Tab keep their any-modifier leniency explicitly, via an `Any-` chord — the escape hatch must not depend on a modifier the user hasn't let go of. In the sheet, ⇧⌃Tab now renders ⌃⇧Tab (Apple's HIG modifier order, applied in one place rather than by hand), and a new row documents ⏎ committing an open dialog — a chord that was bound and undocumented, which is exactly what the coverage check is for. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01A5LV3uA69fYYJBPes6ubF2 --- CLAUDE.md | 6 +- crates/b2-desktop/CLAUDE.md | 19 +- ui/src/anomalies.ts | 8 +- ui/src/bindings.test.ts | 323 +++++++++++++++++++++++ ui/src/bindings.ts | 493 ++++++++++++++++++++++++++++++++++++ ui/src/editorkeys.test.ts | 185 ++++++++++++++ ui/src/editorkeys.ts | 134 ++++++++++ ui/src/format.test.ts | 13 +- ui/src/format.ts | 20 +- ui/src/main.ts | 176 +++++++------ ui/src/shortcuts.test.ts | 67 ++++- ui/src/shortcuts.ts | 137 +++++++--- 12 files changed, 1431 insertions(+), 150 deletions(-) create mode 100644 ui/src/bindings.test.ts create mode 100644 ui/src/bindings.ts create mode 100644 ui/src/editorkeys.test.ts create mode 100644 ui/src/editorkeys.ts diff --git a/CLAUDE.md b/CLAUDE.md index 2c3e1a8..36a4f8c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -266,7 +266,11 @@ if/when one lands — `index-engine.md` §5.)* for the overlay layer), every overlay traps and restores it, `⇧F10` is the keyboard's right-click, and the Settings dialog (⌘,) is a tabbed surface whose rail follows the ARIA `tabs` pattern over `ui/src/settingstabs.ts` — General / Embedding / **Keyboard**, the last being the whole chord table - (`ui/src/shortcuts.ts`) that `?` jumps straight to. The four obligations a new surface owes are in + (`ui/src/shortcuts.ts`) that `?` jumps straight to. Chords themselves are declared once in + `ui/src/bindings.ts` — the keyboard registry — and the dispatcher, the editor's keymap and that + sheet all derive from it, so none of the three can drift; `conflicts()` fails the suite on two + commands sharing a keystroke in one scope, and `ui/src/editorkeys.ts` checks B2's chords against + CodeMirror's own ~100 stock bindings so an upgrade can't quietly take one. The four obligations a new surface owes are in [`crates/b2-desktop/CLAUDE.md`](crates/b2-desktop/CLAUDE.md). ### The `Vault` façade (`b2-core/src/vault.rs`) diff --git a/crates/b2-desktop/CLAUDE.md b/crates/b2-desktop/CLAUDE.md index 019c81c..55a28b1 100644 --- a/crates/b2-desktop/CLAUDE.md +++ b/crates/b2-desktop/CLAUDE.md @@ -126,10 +126,21 @@ Every new surface owes all four. They are cheap while you're building it and exp whatever opened it on close. `Escape` dismisses innermost-first; `Enter` confirms. All of this is one hook — `syncOverlayFocus()` in `main.ts`, called at the end of every `render()` and acting only on the open/close *edge*, so a toast timer's repaint never steals focus mid-⇥. -4. **Discoverable.** A chord nobody can find is not kept. Add the row to `ui/src/shortcuts.ts` (the - `?` sheet) **in the same change** that wires it, and put the chord in the control's `title` — and, - where the action lives in a menu, beside the menu item, which is where a keyboard user learns the - shortcut that lets them skip the menu next time. +4. **Discoverable.** A chord nobody can find is not kept. A chord is **declared once**, in + `ui/src/bindings.ts` — id, chord, scope — and everything else derives: the handler matches it with + `isBound(e, id)`, the editor's keymap takes it from `chordFor(id)`, and the `?` sheet + (`ui/src/shortcuts.ts`) names the id rather than spelling the chord. So add the binding, then add + its row to the sheet: `shortcuts.test.ts` fails on a binding no row documents, which is what makes + "in the same change" a rule the suite keeps rather than one you have to remember. Put the chord in + the control's `title` too — projected with `displayKeys([id])`, never typed out — and, where the + action lives in a menu, beside the menu item, which is where a keyboard user learns the shortcut + that lets them skip the menu next time. + + Two things the registry will tell you before a user does. `conflicts()` fails the suite if your + chord already means something else in the same scope, so pick the scope honestly — it's what + separates "⏎ commits *this* dialog" from a clash. And `editorkeys.test.ts` compares B2's chords + against CodeMirror's ~100 stock bindings, so if your chord needs to work while the note is being + edited, that check is what proves the editor isn't already using it. ### Where the pieces live diff --git a/ui/src/anomalies.ts b/ui/src/anomalies.ts index 66ceeca..12792ea 100644 --- a/ui/src/anomalies.ts +++ b/ui/src/anomalies.ts @@ -17,6 +17,7 @@ // index still say so, and it stops existing the moment a pass comes back clean. A panel // reopened after a restart shows nothing until the next pass re-derives the same anomaly. +import { displayKeys } from "./bindings.ts"; import type { B2idCollision, RestampedNote } from "./types.ts"; /** The slice of a `ProjectReport` the anomaly surfaces read (GH #81). */ @@ -26,9 +27,10 @@ export interface IndexAnomalies { } /** The chord that opens the review panel — quoted in the ping, so the toast tells you - * how to get the detail back after it clears. Its row in the `?` sheet - * (shortcuts.ts) and its wiring (main.ts) spell the same chord; keep the three in step. */ -export const REVIEW_CHORD = "⇧⌘A"; + * how to get the detail back after it clears. Projected from the keyboard registry + * (bindings.ts), which is what keeps it equal to the wiring and to the `?` sheet + * rather than being a third place the same chord is spelled by hand. */ +export const REVIEW_CHORD = displayKeys(["anomalies.toggle"]); /** * One path an anomaly row names, and what B2 can do with it. diff --git a/ui/src/bindings.test.ts b/ui/src/bindings.test.ts new file mode 100644 index 0000000..e00d2a0 --- /dev/null +++ b/ui/src/bindings.test.ts @@ -0,0 +1,323 @@ +// The keyboard registry (bindings.ts), pinned — and the collision gate itself. Pure — +// no DOM — so node runs it straight off the source: `npm test`. Dependency-free like the +// others. +// +// Two jobs. The first is that B2's own chords don't collide: `conflicts(BINDINGS)` must +// be empty, and because `npm test` runs inside both `just check` and `just ci`, that +// assertion *is* the gate. The second is proving the gate can fail — a checker that has +// only ever seen a clean table is indistinguishable from one that returns `[]` +// unconditionally, so the interesting cases below are synthetic tables built to collide. +// +// The matcher gets the same treatment. What a keyboard layer actually gets wrong is +// dull and invisible: a chord that never fires because the table spells a key the way +// the docs write it rather than the way `KeyboardEvent.key` reports it, a `?` that +// demands ⇧ on top of the ⇧ the browser already applied, a ⌘-chord that also answers to +// ⌥⌘ because nobody checked `altKey`. None of that shows up until a user reports that a +// shortcut "sometimes" doesn't work. +import { FORMATS } from "./format.ts"; +import { + type Binding, + BINDINGS, + type KeyEventLike, + allKeys, + canonicalKey, + chordFor, + chordMatches, + conflicts, + displayChord, + displayKeys, + isBound, + parseChord, + scopeContains, + shadows, +} from "./bindings.ts"; + +let passed = 0; + +function assert(cond: boolean, msg: string): void { + if (!cond) throw new Error(`assertion failed: ${msg}`); +} +function assertEq(actual: unknown, expected: unknown, msg: string): void { + const [a, b] = [JSON.stringify(actual), JSON.stringify(expected)]; + if (a !== b) throw new Error(`assertion failed: ${msg}\n actual: ${a}\n expected: ${b}`); +} +function check(name: string, fn: () => void): void { + fn(); + passed++; + console.log(` ok ${name}`); +} + +/** A keydown, as the matcher sees it. Modifiers default to "not held". */ +function press(key: string, mods: Partial = {}): KeyEventLike { + return { key, metaKey: false, ctrlKey: false, shiftKey: false, altKey: false, ...mods }; +} + +// --- the table itself --------------------------------------------------------------- + +check("every chord in the table parses", () => { + // parseChord throws on an unknown key or modifier, so a typo — "Mod-Delete" for + // "Mod-Backspace", "Cmd+f" for "Mod-f" — fails here rather than shipping a dead chord + // that nothing reports because nothing was ever bound to notice. + for (const b of BINDINGS) { + for (const spec of allKeys(b)) parseChord(spec); + } +}); + +check("command ids are unique", () => { + // The lookup is a Map, so a duplicated id wouldn't error — the later row would quietly + // win and the earlier command would stop responding. + const seen = new Set(); + for (const b of BINDINGS) { + assert(!seen.has(b.id), `duplicate command id: ${b.id}`); + seen.add(b.id); + } +}); + +check("every format in FORMATS has a chord in the registry", () => { + // format.ts carries the marker; the chord lives here, and main.ts builds the editor's + // keymap by asking for `format.`. That lookup throws on a miss — at editor + // construction, in the app — so assert it now instead. + for (const f of FORMATS) chordFor(`format.${f.id}`); +}); + +// --- the gate ----------------------------------------------------------------------- + +check("B2's own chords do not collide", () => { + const found = conflicts(); + assertEq(found, [], `${found.length} colliding chord(s)`); +}); + +check("two commands on one chord in one scope is a conflict", () => { + // The gate above proves nothing on its own — this is what proves it can fail. + const table: Binding[] = [ + { id: "a", keys: ["Mod-k"], scope: "global" }, + { id: "b", keys: ["Mod-k"], scope: "global" }, + ]; + assertEq(conflicts(table), [{ a: "a", b: "b", form: "⌘k", scope: "global" }], "the clash"); +}); + +check("an alias collides as loudly as a listed chord", () => { + // ⌘← fires nav.back without appearing in the sheet. A chord nobody can *see* in the + // reference is exactly the one a new binding would land on unnoticed. + const table: Binding[] = [ + { id: "a", keys: ["Mod-["], aliases: ["Mod-ArrowLeft"], scope: "global" }, + { id: "b", keys: ["Mod-ArrowLeft"], scope: "global" }, + ]; + assertEq(conflicts(table).map((c) => c.form), ["⌘ArrowLeft"], "the alias clashes"); +}); + +check("⌃X collides with a Mod chord, because Mod answers to ⌃ too", () => { + // The subtle one. `Mod-x` and `Ctrl-x` look like different rows and are one keystroke + // apart in the hardware: the matcher takes ⌃ as ⌘'s alias, so both fire on ⌃X. + const table: Binding[] = [ + { id: "a", keys: ["Mod-Tab"], scope: "global" }, + { id: "b", keys: ["Ctrl-Tab"], scope: "global" }, + ]; + assertEq(conflicts(table).map((c) => c.form), ["⌃Tab"], "⌃ is where they meet"); +}); + +check("the same chord in sibling scopes is not a conflict", () => { + // ⏎ commits the link dialog and the delete confirm. Only one can be open, so they are + // not competing — modelling that as two scopes is what keeps the gate from crying wolf. + const table: Binding[] = [ + { id: "a", keys: ["Enter"], scope: "overlay:link" }, + { id: "b", keys: ["Enter"], scope: "overlay:delete" }, + ]; + assertEq(conflicts(table), [], "siblings don't collide"); +}); + +check("an inner scope shadows the outer one, and that is reported, not failed", () => { + const table: Binding[] = [ + { id: "outer", keys: ["Escape"], scope: "global" }, + { id: "inner", keys: ["Escape"], scope: "textentry:rename" }, + ]; + assertEq(conflicts(table), [], "shadowing is legal"); + assertEq(shadows(table), [{ outer: "outer", inner: "inner", form: "Escape" }], "and reported"); +}); + +check("B2 shadows exactly these five, and each is an ordering the handler relies on", () => { + // A shadow is a scoped binding taking a keystroke the surface around it would + // otherwise get, so each one is a claim about branch order in main.ts's handler: + // + // - The three Escapes: an inline input's Esc backs *that* input out instead of running + // the overlay cascade, so each input's branch has to come before `dismiss`. + // - ⌃Tab and ⌃⇧Tab: the overlay's Tab trap is `Any-Tab` and swallows unconditionally, + // so the Settings rail's section chords only ever run because their branch is above + // it. That ordering is load-bearing and easy to undo by tidying; this is what + // notices. + // + // Pinned, so a sixth has to be argued for rather than accumulated. + assertEq( + shadows().map((s) => `${s.outer} > ${s.inner} (${s.form})`), + [ + "dismiss > create.cancel (Escape)", + "dismiss > rename.cancel (Escape)", + "dismiss > find.input.close (Escape)", + "overlay.focus.step > settings.section.next (⌃Tab)", + "overlay.focus.step > settings.section.prev (⌃⇧Tab)", + ], + "the shadow set", + ); +}); + +check("scope containment is global-over-all, then the : namespace", () => { + assert(scopeContains("global", "textentry:find"), "global contains everything"); + assert(scopeContains("overlay:link", "overlay:link"), "a scope contains itself"); + assert(scopeContains("overlay", "overlay:link"), "and the layer contains its members"); + assert(!scopeContains("overlay:link", "overlay:delete"), "siblings contain nothing"); + assert(!scopeContains("editor", "global"), "and containment doesn't run backwards"); +}); + +// --- matching ----------------------------------------------------------------------- + +check("a Mod chord answers to ⌘ and to ⌃, and to nothing else", () => { + assert(isBound(press("f", { metaKey: true }), "find.open"), "⌘F opens find"); + assert(isBound(press("f", { ctrlKey: true }), "find.open"), "⌃F does too — the dev alias"); + assert(!isBound(press("f"), "find.open"), "bare F types an f"); + assert(!isBound(press("f", { metaKey: true, altKey: true }), "find.open"), "⌥⌘F is not ⌘F"); +}); + +check("⇧ separates two commands on one letter", () => { + const shiftF = press("F", { metaKey: true, shiftKey: true }); + assert(isBound(shiftF, "search.focus"), "⇧⌘F searches the vault"); + assert(!isBound(shiftF, "find.open"), "and is not ⌘F"); + assert(!isBound(press("f", { metaKey: true }), "search.focus"), "nor the reverse"); +}); + +check("a shifted letter arrives uppercase and still matches", () => { + // `KeyboardEvent.key` reports "A" when ⇧ is down. The table writes chords lowercase, + // so the canonical form has to fold the case or ⇧⌘A would never fire. + assertEq(canonicalKey("A"), "a", "the key folds"); + assert(isBound(press("A", { metaKey: true, shiftKey: true }), "anomalies.toggle"), "⇧⌘A"); +}); + +check("? asks for no ⇧ of its own, because the browser already applied it", () => { + // ⇧/ reports key "?" — the shift is *in* the character. A chord that also demanded + // shiftKey would be fine here but unfireable on a layout where ? is unshifted, and one + // that demanded !shiftKey would never fire at all. + assert(isBound(press("?", { shiftKey: true }), "help.keyboard"), "⇧/ opens the reference"); + assert(isBound(press("?"), "help.keyboard"), "and so does a ? that needed no shift"); + assert(!isBound(press("?", { metaKey: true }), "help.keyboard"), "but ⌘? is a different chord"); +}); + +check("⇧ does separate two commands on a named key", () => { + // The counterpart to the rule above: F10's identity doesn't change under ⇧, so there + // the modifier is real and has to be matched. + assert(isBound(press("F10", { shiftKey: true }), "menu.open"), "⇧F10 is the keyboard's right-click"); + assert(!isBound(press("F10"), "menu.open"), "bare F10 is not"); +}); + +check("the space bar is spelled, not written as a literal space", () => { + assertEq(canonicalKey(" "), "Space", "canonical"); + assert(isBound(press(" "), "graph.activate"), "Space opens a focused graph node"); + assert(isBound(press("Enter"), "graph.activate"), "so does ⏎"); +}); + +check("⌃Tab is literal Control, not Mod", () => { + // The one chord in the app that wants ⌃ itself. If it went through Mod it would also + // fire on ⌘Tab — which macOS owns — and would collide with anything bound to Mod-Tab. + assert(isBound(press("Tab", { ctrlKey: true }), "settings.section.next"), "⌃Tab"); + assert(!isBound(press("Tab", { metaKey: true }), "settings.section.next"), "⌘Tab is not ⌃Tab"); + assert( + isBound(press("Tab", { ctrlKey: true, shiftKey: true }), "settings.section.prev"), + "⌃⇧Tab steps back", + ); +}); + +check("an alias fires the command the sheet doesn't show it under", () => { + assert(isBound(press("[", { metaKey: true }), "nav.back"), "⌘["); + assert(isBound(press("ArrowLeft", { metaKey: true }), "nav.back"), "⌘← too"); + assert(isBound(press("s", { metaKey: true }), "fm.save"), "⌘S also saves the drawer"); +}); + +check("Esc gets you out with anything held down", () => { + // `Any-Escape`. The escape hatch must not be conditional on a modifier the user hasn't + // let go of yet — you press ⌘F, change your mind, and hit Escape with ⌘ still down. + for (const mods of [{}, { metaKey: true }, { shiftKey: true }, { altKey: true, ctrlKey: true }]) { + assert(isBound(press("Escape", mods), "dismiss"), `Esc with ${JSON.stringify(mods)}`); + } + assert(isBound(press("Tab", { metaKey: true }), "overlay.focus.step"), "and no Tab escapes"); + assert(!isBound(press("Enter"), "dismiss"), "but Any- is about modifiers, not keys"); +}); + +check("an Any- chord claims every keystroke over its key", () => { + // Which is what makes it shadow — and collide with — the bindings it would really take + // the key from. A strict ⌃Tab under a scope the trap covers is not a free chord. + const table: Binding[] = [ + { id: "trap", keys: ["Any-Tab"], scope: "overlay" }, + { id: "rail", keys: ["Ctrl-Tab"], scope: "overlay:settings" }, + { id: "elsewhere", keys: ["Ctrl-Tab"], scope: "editor" }, + ]; + assertEq( + shadows(table).map((s) => `${s.outer} > ${s.inner}`), + ["trap > rail"], + "the trap takes the rail's chord, and nothing outside the overlay layer", + ); +}); + +check("Any- and a named modifier is a contradiction", () => { + let threw = false; + try { + parseChord("Any-Shift-Escape"); + } catch { + threw = true; + } + assert(threw, "Any-Shift- asks for both 'any modifier' and 'this one'"); +}); + +check("an Any- chord prints as the bare key", () => { + assertEq(displayChord("Any-Escape"), "Esc", "not a list of twelve ways to hold it"); + assertEq(displayKeys(["overlay.focus.step"]), "Tab", "likewise"); +}); + +check("chordMatches reads the modifiers it is given, not the ones it isn't", () => { + const chord = parseChord("Mod-Backspace"); + assert(chordMatches(chord, press("Backspace", { metaKey: true })), "⌘⌫"); + assert(!chordMatches(chord, press("Backspace", { metaKey: true, shiftKey: true })), "⇧⌘⌫ is not"); + assert(!chordMatches(chord, press("Backspace")), "and a bare ⌫ deletes a character"); +}); + +check("parseChord refuses what it can't honour", () => { + const rejects = ["Cmd+f", "Mod-Ctrl-x", "Mod-Meh", "Mod-Retrun", ""]; + for (const spec of rejects) { + let threw = false; + try { + parseChord(spec); + } catch { + threw = true; + } + assert(threw, `parseChord should refuse ${JSON.stringify(spec)}`); + } +}); + +// --- display ------------------------------------------------------------------------ + +check("modifiers print in Apple's order — ⌃⌥⇧⌘ — then the key", () => { + assertEq(displayChord("Mod-Shift-f"), "⇧⌘F", "shift before command"); + assertEq(displayChord("Ctrl-Shift-Tab"), "⌃⇧Tab", "control before shift"); + assertEq(displayChord("Mod-Shift-v"), "⇧⌘V", "paste as plain text"); +}); + +check("keys print as macOS writes them — glyphs, but words where macOS uses words", () => { + assertEq(displayChord("Mod-Backspace"), "⌘⌫", "delete is a glyph"); + assertEq(displayChord("Mod-Enter"), "⌘⏎", "so is return"); + assertEq(displayChord("Escape"), "Esc", "escape is a word — ⎋ exists, nobody reads it"); + assertEq(displayChord("Space"), "Space", "and so is space"); + assertEq(displayChord("ArrowUp"), "↑", "arrows are arrows"); + assertEq(displayChord("F2"), "F2", "function keys are themselves"); +}); + +check("a row prints its commands' chords, distinct ones only", () => { + assertEq(displayKeys(["find.next", "find.prev"]), "⌘G / ⇧⌘G", "two chords, two cells"); + assertEq(displayKeys(["link.commit", "delete.confirm"]), "⏎", "one chord, said once"); + assertEq(displayKeys(["graph.activate"]), "⏎ / Space", "one command, two chords"); +}); + +check("a row does not print the aliases the prose covers", () => { + // nav.back also answers to ⌘←; the row says so in words rather than listing it, which + // is the whole reason `aliases` is a separate field from `keys`. + assertEq(displayKeys(["nav.back", "nav.forward"]), "⌘[ / ⌘]", "brackets only"); + assertEq(displayKeys(["menu.open"]), "⇧F10", "not the Menu key"); +}); + +console.log(`bindings: ${passed} checks passed`); diff --git a/ui/src/bindings.ts b/ui/src/bindings.ts new file mode 100644 index 0000000..cf7a856 --- /dev/null +++ b/ui/src/bindings.ts @@ -0,0 +1,493 @@ +// The keyboard registry — the one machine-readable table of the chords B2 answers to, +// and the source both the dispatcher (main.ts) and the reference sheet (shortcuts.ts) +// read. Pure data + pure functions, no DOM, so node runs its test straight off the +// source (`npm test`), like format.ts / treenav.ts / settingstabs.ts. +// +// Why it exists. A chord used to be spelled twice — once as a modifier test in main.ts's +// keydown handler (`(e.metaKey || e.ctrlKey) && !e.altKey && e.key.toLowerCase() === "f"`) +// and once as a row of display text in shortcuts.ts — with nothing but discipline keeping +// them equal. K1 (docs/design/invariants.md) promises every mouse action has a keyboard +// path *and* that the path is findable, so a sheet free to fall behind the wiring is a +// promise waiting to break. The repo's habit everywhere else is to close that kind of gap +// by construction rather than by convention (sanitize.ts as marked's `postprocess` hook, +// sidenav.ts's row order reused by the paint, the ui suite's globbed test files). This is +// the keyboard's version: the chord is written once, here, and the other two derive. +// +// The second thing it buys is a suite. main.ts pulls in CodeMirror and the stylesheet, so +// node can't import it, and until now *nothing* about which chord is bound to what was +// testable at all — the app's whole keyboard contract sat in the one file the suite +// can't see. Everything in this module is data or a pure function over data. +// +// What lives here and what doesn't. The rule is **who owns the key → action mapping**: +// +// - Here: every chord the global keydown handler matches by hand. +// - Not here: the arrow-navigation families, whose key → move mapping is already owned +// and tested by a pure module of its own — treenav.ts (`arrowMove`), sidenav.ts +// (`sideArrowMove`), settingstabs.ts (`tabMove`). Copying their keys in would recreate +// exactly the two-sources-of-truth problem this module exists to end, so the sheet +// carries them as literal rows instead and shortcuts.ts says why. +// +// The chord syntax is CodeMirror's (`Mod-Shift-v`) on purpose: the editor's own bindings +// come out of this same table and are handed to `keymap.of` verbatim, so there is one +// spelling of a chord for both halves of the app. + +/** Where a chord applies. `global` is the whole window; the rest are surfaces inside it, + * each named after the state that turns it on. `:` nests (`modal:link` is inside + * `modal`), which is what lets two ⏎ bindings coexist without ambiguity. */ +export type Scope = + | "global" + | "editor" // CodeMirror has the keyboard (state.editing) + | "fm" // the frontmatter drawer's mini-editor (state.fmEditing) + | "find" // the find bar is open (findOpen) + | "graph" // a graph node holds focus + // The overlay layer. `currentOverlay()` returns exactly one of these or null — the + // openers all run `dismissOverlays` first, so they can never stack — which is what + // makes them siblings, and what makes two ⏎ bindings unambiguous rather than a + // collision. They nest under `overlay` because the Tab trap is bound to the *layer*: + // it takes Tab from whichever one is up, so it shadows all of them. + | "overlay" + | "overlay:settings" + | "overlay:menu" + | "overlay:link" + | "overlay:delete" + | "textentry:create" + | "textentry:rename" + | "textentry:find"; + +/** One command and the chords that fire it. Deliberately *only* chord, scope and id — + * the conditions under which a chord applies stay ordinary code in main.ts's handler. + * A table rich enough to express "⌘G, but only while the find bar is open and not in a + * text field" is a `when`-clause expression language, which is a worse trade at this + * size than a guard you can read. */ +export interface Binding { + /** Stable command id. Referenced by main.ts's dispatcher and shortcuts.ts's rows. */ + readonly id: string; + /** The chords that fire it, in the order the sheet shows them. */ + readonly keys: readonly string[]; + /** Chords that also fire it but the sheet doesn't list, because the row's prose + * already mentions them ("Back / forward (⌘← / ⌘→ too)"). */ + readonly aliases?: readonly string[]; + readonly scope: Scope; +} + +// --- the table --------------------------------------------------------------------- + +export const BINDINGS = [ + // Global — the app's own chords, matched by the document-level keydown handler. + { id: "find.open", keys: ["Mod-f"], scope: "global" }, + { id: "search.focus", keys: ["Mod-Shift-f"], scope: "global" }, + { id: "tree.new-note", keys: ["Mod-n"], scope: "global" }, + { id: "tree.new-folder", keys: ["Mod-Shift-n"], scope: "global" }, + // The Menu key is the same gesture on a keyboard that has one; only ⇧F10 is listed. + { id: "menu.open", keys: ["Shift-F10"], aliases: ["ContextMenu"], scope: "global" }, + { id: "tree.rename", keys: ["F2"], scope: "global" }, + { id: "help.keyboard", keys: ["?"], scope: "global" }, + { id: "pane.tree", keys: ["Mod-1"], scope: "global" }, + { id: "pane.note", keys: ["Mod-2"], scope: "global" }, + { id: "pane.discovery", keys: ["Mod-3"], scope: "global" }, + { id: "delete.focused", keys: ["Mod-Backspace"], scope: "global" }, + { id: "settings.toggle", keys: ["Mod-,"], scope: "global" }, + { id: "anomalies.toggle", keys: ["Mod-Shift-a"], scope: "global" }, + { id: "edit.toggle", keys: ["Mod-e"], scope: "global" }, + // `Any-` because Escape is the way out and must not be conditional on what else is + // held down: a user who just pressed ⌘F and hasn't let go of ⌘ yet still gets out. + { id: "dismiss", keys: ["Any-Escape"], scope: "global" }, + { id: "nav.back", keys: ["Mod-["], aliases: ["Mod-ArrowLeft"], scope: "global" }, + { id: "nav.forward", keys: ["Mod-]"], aliases: ["Mod-ArrowRight"], scope: "global" }, + + // The find bar, once it's open. + { id: "find.next", keys: ["Mod-g"], scope: "find" }, + { id: "find.prev", keys: ["Mod-Shift-g"], scope: "find" }, + + // The editor. The first four are handed to CodeMirror's own keymap (ahead of its + // defaults, so they win); ⌘S is the document handler's, and reaches it only because + // CodeMirror leaves Mod-s unbound — editorkeys.test.ts is what keeps that true. + { id: "format.bold", keys: ["Mod-b"], scope: "editor" }, + { id: "format.italic", keys: ["Mod-i"], scope: "editor" }, + { id: "editor.table", keys: ["Mod-t"], scope: "editor" }, + { id: "editor.paste-plain", keys: ["Mod-Shift-v"], scope: "editor" }, + { id: "editor.save", keys: ["Mod-s"], scope: "editor" }, + + // The frontmatter drawer — a separate surface from the body editor, hence its own + // scope: ⌘S means "save this drawer" here and "flush the note" there. + { id: "fm.save", keys: ["Mod-Enter"], aliases: ["Mod-s"], scope: "fm" }, + + // The overlay layer. `Any-Tab` for the same reason as `Any-Escape`: the trap's contract + // is that *no* Tab walks the page behind the backdrop, whatever rode along with it. + { id: "overlay.focus.step", keys: ["Any-Tab"], scope: "overlay" }, + { id: "link.commit", keys: ["Enter"], scope: "overlay:link" }, + { id: "delete.confirm", keys: ["Enter"], scope: "overlay:delete" }, + { id: "menu.item.next", keys: ["ArrowDown"], scope: "overlay:menu" }, + { id: "menu.item.prev", keys: ["ArrowUp"], scope: "overlay:menu" }, + { id: "settings.section.next", keys: ["Ctrl-Tab"], scope: "overlay:settings" }, + { id: "settings.section.prev", keys: ["Ctrl-Shift-Tab"], scope: "overlay:settings" }, + + // SVG has no native button activation, so the graph binds what the platform would + // otherwise give a - @@ -3615,18 +3629,21 @@ function wireEvents(): void { } }); - // ⌘, toggles Settings (the macOS Preferences reflex); Escape closes whichever modal is - // up; Cmd/Ctrl+S forces an immediate flush while editing (autosave means it's never - // *required* — this is for the reflex); ⌘N / ⇧⌘N create a note / folder in the - // selection's folder (the tree-head icons' shortcuts). + // The app's chords. Every `isBound(e, …)` below asks the keyboard registry + // (bindings.ts) whether this keystroke is that command — the registry owns *what* the + // chord is, and the sheet in Settings → Keyboard is projected from the same table, so + // the two can't drift. What stays here is everything the table deliberately doesn't + // model: the order the surfaces get their turn (innermost first), and the guard beside + // each branch saying when its command applies at all — an overlay owns the keyboard, + // the tree has no focused row, ⌘⌫ must not hijack delete-to-line-start while editing. document.addEventListener("keydown", (e) => { // The tree's inline create input owns its keys first: Enter commits, Escape // cancels, and nothing else typed there leaks into the global chords below. if (state.treeCreate && (e.target as HTMLElement).id === "tree-create-input") { - if (e.key === "Enter") { + if (isBound(e, "create.commit")) { e.preventDefault(); void commitTreeCreate((e.target as HTMLInputElement).value, true); - } else if (e.key === "Escape") { + } else if (isBound(e, "create.cancel")) { e.preventDefault(); cancelTreeCreate(); } @@ -3634,10 +3651,10 @@ function wireEvents(): void { } // The rename input owns its keys the same way. if (state.treeRename && (e.target as HTMLElement).id === "tree-rename-input") { - if (e.key === "Enter") { + if (isBound(e, "rename.commit")) { e.preventDefault(); void commitTreeRename((e.target as HTMLInputElement).value); - } else if (e.key === "Escape") { + } else if (isBound(e, "rename.cancel")) { e.preventDefault(); cancelTreeRename(); } @@ -3653,9 +3670,10 @@ function wireEvents(): void { // those keys belong to the panel's own controls (and to the panel itself, which // scrolls) the moment focus leaves the rail. if (state.settingsOpen) { - if (e.key === "Tab" && e.ctrlKey && !e.metaKey && !e.altKey) { + const forward = isBound(e, "settings.section.next"); + if (forward || isBound(e, "settings.section.prev")) { e.preventDefault(); - selectSettingsTab(tabStep(state.settingsTab, e.shiftKey ? -1 : 1), true); + selectSettingsTab(tabStep(state.settingsTab, forward ? 1 : -1), true); return; } const onRail = @@ -3673,10 +3691,12 @@ function wireEvents(): void { // An open overlay owns Tab (K1): without a trap, Tab walks the page *behind* the // modal — focus vanishes under the backdrop and the overlay becomes dismissible // only with the mouse. Wrapping keeps every control in it reachable, forever. - if (e.key === "Tab" && currentOverlay() !== null) { - // Swallowed unconditionally — an overlay that somehow renders with no focusable - // control must still not let Tab walk the page behind it, which is the exact - // failure this block exists to prevent. + if (isBound(e, "overlay.focus.step") && currentOverlay() !== null) { + // The binding is `Any-Tab`: this branch's contract is that *no* Tab reaches the + // page, so a modifier the user is still holding must not defeat it. Swallowed + // unconditionally for the same reason — an overlay that somehow renders with no + // focusable control must still not let Tab walk the page behind it, which is the + // exact failure this block exists to prevent. e.preventDefault(); const items = overlayFocusables(); if (items.length > 0) { @@ -3690,57 +3710,65 @@ function wireEvents(): void { } // ↑/↓ walk an open context menu — the menu-pattern sibling of the tree's arrows. // (⏎ needs no binding: the items are buttons.) - if (state.contextMenu && (e.key === "ArrowDown" || e.key === "ArrowUp")) { - const items = overlayFocusables(); - if (items.length > 0) { - e.preventDefault(); - const i = items.indexOf(document.activeElement as HTMLElement); - const n = items.length; - const next = - i < 0 ? (e.key === "ArrowDown" ? 0 : n - 1) : (i + (e.key === "ArrowDown" ? 1 : -1) + n) % n; - items[next].focus(); + if (state.contextMenu) { + const down = isBound(e, "menu.item.next"); + if (down || isBound(e, "menu.item.prev")) { + const items = overlayFocusables(); + if (items.length > 0) { + e.preventDefault(); + const i = items.indexOf(document.activeElement as HTMLElement); + const n = items.length; + const next = i < 0 ? (down ? 0 : n - 1) : (i + (down ? 1 : -1) + n) % n; + items[next].focus(); + } + return; } - return; } // The find bar's input: Enter steps (⇧Enter back), Escape closes. Everything else // falls through so the global chords (⌘F itself, ⇧⌘F) still work from the bar. if (findOpen && (e.target as HTMLElement).id === "find-input") { - if (e.key === "Enter") { + const forward = isBound(e, "find.input.next"); + if (forward || isBound(e, "find.input.prev")) { e.preventDefault(); - findStep(e.shiftKey ? -1 : 1); + findStep(forward ? 1 : -1); return; } - if (e.key === "Escape") { + if (isBound(e, "find.input.close")) { e.preventDefault(); closeFind(); return; } } // ⌘F — find in the open note; ⇧⌘F — jump to the global vault-search box. - if ((e.metaKey || e.ctrlKey) && !e.altKey && e.key.toLowerCase() === "f") { + const vaultSearch = isBound(e, "search.focus"); + if (vaultSearch || isBound(e, "find.open")) { if (currentOverlay() !== null) return; e.preventDefault(); - if (e.shiftKey) focusGlobalSearch(); + if (vaultSearch) focusGlobalSearch(); else openFind(); return; } // ⌘G / ⇧⌘G — the classic find-next/previous chords, live while the bar is open. - if (findOpen && (e.metaKey || e.ctrlKey) && !e.altKey && e.key.toLowerCase() === "g") { - e.preventDefault(); - findStep(e.shiftKey ? -1 : 1); - return; + if (findOpen) { + const forward = isBound(e, "find.next"); + if (forward || isBound(e, "find.prev")) { + e.preventDefault(); + findStep(forward ? 1 : -1); + return; + } } - if ((e.metaKey || e.ctrlKey) && !e.altKey && e.key.toLowerCase() === "n") { + const newFolder = isBound(e, "tree.new-folder"); + if (newFolder || isBound(e, "tree.new-note")) { if (currentOverlay() !== null) return; // an overlay owns the keyboard e.preventDefault(); - startTreeCreate(e.shiftKey ? "folder" : "note", state.selectedDir); + startTreeCreate(newFolder ? "folder" : "note", state.selectedDir); return; } // ⇧F10 / the Menu key — the keyboard's right-click, and the entry point that makes // Rename / Move… / Link… reachable without a mouse at all (they live only in the // context menu). Opens the *same* menu the mouse does, anchored under whatever the // keyboard is on: a tree row, or a discovery card / graph ghost. - if (e.key === "ContextMenu" || (e.shiftKey && e.key === "F10")) { + if (isBound(e, "menu.open")) { if (currentOverlay() !== null || state.vaultRoot === null) return; const row = focusedTreeRow(); if (row) { @@ -3771,7 +3799,7 @@ function wireEvents(): void { } // F2 — rename the focused tree row. The platform rename chord, and the direct path // the context menu advertises next to the item. - if (e.key === "F2" && !e.shiftKey && !e.metaKey && !e.ctrlKey) { + if (isBound(e, "tree.rename")) { const row = focusedTreeRow(); if (!row) return; e.preventDefault(); @@ -3784,7 +3812,7 @@ function wireEvents(): void { // then ask again) — the sheet used to render over them, and folding it into Settings // is what makes this chord an ordinary one. A toggle, like ⌘,: pressing it while // already reading the section closes the dialog. - if (e.key === "?" && !e.metaKey && !e.ctrlKey && !e.altKey) { + if (isBound(e, "help.keyboard")) { if (state.editing || inTextEntry()) return; const overlay = currentOverlay(); if (overlay !== null && overlay !== "settings") return; @@ -3796,28 +3824,28 @@ function wireEvents(): void { // ⌘1 / ⌘2 / ⌘3 — put the keyboard in the files, the note, or discovery. Without // them, reaching the tree means Tab-ing through the whole top bar first, which is // "operable" only in the letter of K1, not its spirit. - if ( - (e.metaKey || e.ctrlKey) && - !e.altKey && - !e.shiftKey && - (e.key === "1" || e.key === "2" || e.key === "3") - ) { + const focusPane = isBound(e, "pane.tree") + ? focusTreePane + : isBound(e, "pane.note") + ? focusNotePane + : isBound(e, "pane.discovery") + ? focusSidePane + : null; + if (focusPane) { if (currentOverlay() !== null) return; e.preventDefault(); - if (e.key === "1") focusTreePane(); - else if (e.key === "2") focusNotePane(); - else focusSidePane(); + focusPane(); return; } // Enter commits the link modal from anywhere inside it — the keyboard sibling of // "Commit link" (the explanation field is a plain input, so ⏎ would do nothing). - if (state.linkTarget && e.key === "Enter" && !e.shiftKey) { + if (state.linkTarget && isBound(e, "link.commit")) { e.preventDefault(); void commitLink(); return; } // Enter commits the folder-delete confirm (its keyboard sibling of the button). - if (state.deleteTarget && e.key === "Enter") { + if (state.deleteTarget && isBound(e, "delete.confirm")) { e.preventDefault(); const node = state.deleteTarget; state.deleteTarget = null; @@ -3827,7 +3855,7 @@ function wireEvents(): void { // ⏎ / Space on a focused graph node. SVG has no native button activation, so the // key becomes the click the delegation above already answers — one activation path // for both hands, not two implementations to keep in step. - if (e.key === "Enter" || e.key === " ") { + if (isBound(e, "graph.activate")) { const active = document.activeElement; const node = active instanceof SVGElement || active instanceof HTMLElement @@ -3844,7 +3872,7 @@ function wireEvents(): void { // it falls back to the open document, the reader's expectation. Reading view only: // while editing — or in any text field — ⌘⌫ is the platform delete-to-line-start // and must not be hijacked. - if ((e.metaKey || e.ctrlKey) && !e.altKey && !e.shiftKey && e.key === "Backspace") { + if (isBound(e, "delete.focused")) { if (currentOverlay() !== null) return; if (state.editing || inTextEntry()) return; const row = focusedTreeRow(); @@ -3864,7 +3892,7 @@ function wireEvents(): void { requestDelete(node); return; } - if ((e.metaKey || e.ctrlKey) && e.key === ",") { + if (isBound(e, "settings.toggle")) { e.preventDefault(); if (state.settingsOpen) closeSettings(); else void openSettings(); @@ -3874,7 +3902,7 @@ function wireEvents(): void { // toggle like ⌘,, and reachable while editing for the same reason: it opens a modal // over the pane rather than touching the live buffer. Nothing to review is not a // reason to refuse — the panel then says so, which beats a chord that looks broken. - if ((e.metaKey || e.ctrlKey) && e.shiftKey && !e.altKey && e.key.toLowerCase() === "a") { + if (isBound(e, "anomalies.toggle")) { e.preventDefault(); if (state.anomaliesOpen) closeAnomalies(); else openAnomalies(); @@ -3883,7 +3911,7 @@ function wireEvents(): void { // ⌘E toggles edit mode — the keyboard sibling of the Edit / Done buttons. A modal // owns the keyboard first; a resource or empty pane has nothing to edit. Works while // editing (CodeMirror leaves Mod-e unbound, so the event bubbles here) to flip back. - if ((e.metaKey || e.ctrlKey) && !e.altKey && !e.shiftKey && e.key.toLowerCase() === "e") { + if (isBound(e, "edit.toggle")) { if (currentOverlay() !== null) return; if (state.editing) { e.preventDefault(); @@ -3894,7 +3922,7 @@ function wireEvents(): void { } return; } - if (e.key === "Escape") { + if (isBound(e, "dismiss")) { // Innermost first, always — and with the `?` sheet folded into Settings, the // overlay layer is flat, so this is simply the one overlay that is up. if (state.contextMenu) { @@ -3927,18 +3955,14 @@ function wireEvents(): void { if (state.graphOpen && state.current && !state.editing) toggleGraph(); return; } - if (state.editing && (e.metaKey || e.ctrlKey) && e.key.toLowerCase() === "s") { + if (state.editing && isBound(e, "editor.save")) { e.preventDefault(); void saveNow(); return; } // ⌘⏎ / ⌘S save the frontmatter mini-editor — its explicit-save chords (the // buttons' keyboard siblings, K1). Plain Enter stays a newline in the textarea. - if ( - state.fmEditing && - (e.metaKey || e.ctrlKey) && - (e.key === "Enter" || e.key.toLowerCase() === "s") - ) { + if (state.fmEditing && isBound(e, "fm.save")) { e.preventDefault(); void saveFmEdit(); return; @@ -3949,12 +3973,10 @@ function wireEvents(): void { // mean caret-to-edge, so the brackets still navigate (e.g. straight from the // search field). The buttons and mouse back/forward stay live everywhere — they // flush through navGo's edit-mode guard. - if ((e.metaKey || e.ctrlKey) && !e.shiftKey && !e.altKey && !state.editing) { + const back = isBound(e, "nav.back"); + if ((back || isBound(e, "nav.forward")) && !state.editing) { if (currentOverlay() !== null) return; - const back = e.key === "[" || e.key === "ArrowLeft"; - const forward = e.key === "]" || e.key === "ArrowRight"; - if (!back && !forward) return; - if ((e.key === "ArrowLeft" || e.key === "ArrowRight") && inTextEntry()) return; + if (canonicalKey(e.key).startsWith("Arrow") && inTextEntry()) return; e.preventDefault(); void navGo(back ? -1 : 1); } diff --git a/ui/src/shortcuts.test.ts b/ui/src/shortcuts.test.ts index 70f7b3c..c91ca03 100644 --- a/ui/src/shortcuts.test.ts +++ b/ui/src/shortcuts.test.ts @@ -1,14 +1,24 @@ // The keyboard reference's own shape (shortcuts.ts), pinned. Pure data — no DOM — so // node runs it straight off the source: `npm test`. Dependency-free like the others. // -// These check the *sheet as an artifact*, not the wiring: whether a given chord is -// actually bound lives in main.ts's keydown handler, which pulls in CodeMirror and the -// stylesheet and so can't be imported here. What a table like this really gets wrong is -// smaller and duller — a half-filled row that renders as a blank line, one chord listed -// twice in the same group meaning two different things, or an entry spelled "Cmd+N" -// among a page of ⌘N. Those are exactly the drifts that make a reference stop reading -// as authoritative, and they're what's asserted below (K1, GH #78). -import { SHORTCUTS } from "./shortcuts.ts"; +// These check the *sheet as an artifact*. Since the sheet became a projection of the +// keyboard registry, one old worry is gone and a better one has replaced it. Gone: a row +// spelling a chord the wiring doesn't answer to, because rows no longer spell chords — +// they name commands, and `displayKeys` renders whatever bindings.ts says. Merely +// importing this module proves every id in it resolves, since SHORTCUTS is built at load +// and the lookup throws on a miss. +// +// The new worry is the other direction: a chord that exists and is documented *nowhere*. +// That's the K1 failure that matters (docs/design/invariants.md, GH #78) — an action +// reachable from the keyboard but discoverable only by reading main.ts. The coverage +// check below is what makes adding a binding without a row impossible. +// +// What's left is the dull editorial stuff a table like this really gets wrong: a +// half-filled row that renders as a blank line, one chord listed twice in the same group +// meaning two different things, an entry spelled "Cmd+N" among a page of ⌘N. Those are +// the drifts that make a reference stop reading as authoritative. +import { BINDINGS, allKeys } from "./bindings.ts"; +import { SHEET, SHORTCUTS } from "./shortcuts.ts"; let passed = 0; @@ -40,8 +50,8 @@ check("every row is a full pair — a chord and what it does", () => { check("no chord is listed twice within one group", () => { // Across groups is fine and deliberate — ⇧F10 opens a menu on a tree row *and* on a - // discovery card, ⏎ activates whatever is focused. Two meanings in one group is the - // contradiction: the reader has no way to tell which one applies. + // discovery card, Esc closes an overlay and closes Settings. Two meanings in one group + // is the contradiction: the reader has no way to tell which one applies. for (const g of SHORTCUTS) { const seen = new Set(); for (const s of g.items) { @@ -53,7 +63,9 @@ check("no chord is listed twice within one group", () => { check("modifiers are written as macOS glyphs, never spelled out", () => { // The sheet sits beside button tooltips that render ⌘/⇧, so a stray "Cmd+N" reads as - // a different app's documentation. Only the *modifiers* are held to this: macOS spells + // a different app's documentation. Projected rows get this from `displayChord`; the + // literal rows — the platform's own keys, the arrow families — are hand-written, and + // are what this still guards. Only the *modifiers* are held to it: macOS spells // Esc / Tab / Space / Home / End out in its own menus, and so does B2's existing // chrome ("Close (Esc)"), so those stay words — shortcuts.ts says why. const spelled = /\b(cmd|command|ctrl|control|shift|alt|option|enter|return|backspace)\b/i; @@ -65,4 +77,37 @@ check("modifiers are written as macOS glyphs, never spelled out", () => { } }); +check("every chord B2 binds is documented somewhere in the sheet", () => { + // The K1 guarantee, held by construction: you cannot add a binding without a row here. + // + // The one exemption is text entry, and it's a category rather than a list — ⏎ commits + // and Esc backs out of an inline input (rename, new note, the find field). Nobody + // needs telling, and spelling out six rows of it would bury the chords that genuinely + // have to be found. Anything else — including a chord that only applies inside a + // modal, or only while the find bar is open — has to earn a row. + const documented = new Set(); + for (const group of SHEET) { + for (const row of group.rows) { + if ("ids" in row) for (const id of row.ids) documented.add(id); + } + } + for (const b of BINDINGS) { + if (b.scope.startsWith("textentry")) continue; + assert(documented.has(b.id), `${b.id} (${allKeys(b).join(", ")}) is bound but undocumented`); + } +}); + +check("the sheet documents no command that isn't bound", () => { + // The mirror image, and the cheap half: SHORTCUTS is built at import, and resolving a + // row's ids throws on an unknown one, so a stale row can't survive to be rendered. + // Asserted anyway so the failure names the sheet rather than a module-load stack. + const ids = new Set(BINDINGS.map((b) => b.id)); + for (const group of SHEET) { + for (const row of group.rows) { + if (!("ids" in row)) continue; + for (const id of row.ids) assert(ids.has(id), `${id} is documented but not bound`); + } + } +}); + console.log(`shortcuts: ${passed} checks passed`); diff --git a/ui/src/shortcuts.ts b/ui/src/shortcuts.ts index a2de3f4..1e47911 100644 --- a/ui/src/shortcuts.ts +++ b/ui/src/shortcuts.ts @@ -1,21 +1,37 @@ -// The keyboard reference — the single source of truth for what the `?` cheat sheet -// shows. Pure data, no DOM, so node runs its test straight off the source +// The keyboard reference — the single source of truth for what Settings' Keyboard +// section shows. Pure data, no DOM, so node runs its test straight off the source // (`npm test`), like newentry.ts / move.ts / treenav.ts. // // Why a table rather than prose in a docs page: invariant K1 (docs/design/invariants.md, // GH #78) promises every mouse action has a keyboard path, and a promise nobody can // *find* is not kept. A shortcut that exists only in a button's `title` is discoverable // exactly once — by hovering the button you already knew about. This list is the app's -// answer to "what can I do without the mouse", and adding a chord means adding a row -// here in the same change, so the sheet can't quietly fall behind the wiring. +// answer to "what can I do without the mouse". +// +// What changed when bindings.ts arrived: a row no longer *spells* its chord, it **names +// the command** and the chord is projected from the registry. So the sheet can't drift +// from the wiring by a typo, and — because bindings.test.ts asserts every binding lands +// in some row — a new chord can't be added without a row here either. That's the K1 +// promise held by construction instead of by remembering. +// +// The prose stays hand-written, deliberately. Grouping ⌘1/⌘2/⌘3 into one row, or saying +// "Back / forward (⌘← / ⌘→ too)" rather than listing four chords, is editorial judgement +// a generator would flatten — and the sheet's job is to be *read*. +// +// Literal rows (`keys:` instead of `ids:`) are the things that aren't B2 chords at all: +// the platform's own behavior (Tab, ⏎ on a focused button, first-letter typeahead), and +// the arrow families whose key → move mapping belongs to a pure module of its own — +// treenav.ts, sidenav.ts, settingstabs.ts. Copying those keys into the registry would +// give them two owners, which is the drift the registry exists to end; bindings.ts says +// the same thing from the other side. // // B2 ships on macOS only (crates/b2-desktop), so modifiers are the platform's glyphs — // ⌘ command, ⇧ shift, ⌫ delete, ⏎ return — while keys macOS itself spells out in menus // stay spelled out (Esc, Tab, Space, Home/End). That's the split the app's existing -// tooltips already use ("Close (Esc)"), and the one the sheet must not drift from: -// ⎋ and ⇥ exist, but nobody reads them. +// tooltips already use ("Close (Esc)"); `displayChord` in bindings.ts now applies it. +import { type BindingId, displayKeys } from "./bindings.ts"; -/** One chord and what it does. `keys` is display text — the wiring lives in main.ts. */ +/** One chord and what it does. `keys` is display text, projected from the registry. */ export interface Shortcut { keys: string; action: string; @@ -26,43 +42,62 @@ export interface ShortcutGroup { items: Shortcut[]; } -export const SHORTCUTS: ShortcutGroup[] = [ +/** A row of the sheet: either the commands it documents, or — for the platform's own + * keys and the arrow families — the literal text to print. */ +export type SheetRow = + | { readonly ids: readonly BindingId[]; readonly action: string } + | { readonly keys: string; readonly action: string }; + +export interface SheetGroup { + readonly title: string; + readonly rows: readonly SheetRow[]; +} + +export const SHEET: readonly SheetGroup[] = [ { title: "Getting around", - items: [ - { keys: "⌘1 / ⌘2 / ⌘3", action: "Focus the files, the note, or discovery" }, - { keys: "⇧⌘F", action: "Search the vault" }, - { keys: "⌘F", action: "Find in this note" }, - { keys: "⌘G / ⇧⌘G", action: "Next / previous match" }, - { keys: "⌘[ / ⌘]", action: "Back / forward (⌘← / ⌘→ too)" }, + rows: [ + { + ids: ["pane.tree", "pane.note", "pane.discovery"], + action: "Focus the files, the note, or discovery", + }, + { ids: ["search.focus"], action: "Search the vault" }, + { ids: ["find.open"], action: "Find in this note" }, + { ids: ["find.next", "find.prev"], action: "Next / previous match" }, + { ids: ["nav.back", "nav.forward"], action: "Back / forward (⌘← / ⌘→ too)" }, + // The platform's own activation of a focused button or link — not a B2 binding. + // The graph's ⏎ *is* one (SVG has no native activation); it has its own row below. { keys: "⏎", action: "Follow the focused link, card, or graph node" }, ], }, { title: "The file tree", - items: [ + rows: [ { keys: "↑ / ↓", action: "Move between rows" }, { keys: "→", action: "Expand a folder, or step into it" }, { keys: "←", action: "Collapse a folder, or step out to its parent" }, { keys: "Home / End", action: "First / last row" }, { keys: "A–Z", action: "Jump to the next row starting with that letter" }, { keys: "⏎ / Space", action: "Open the note or file; fold the folder" }, - { keys: "⌘N / ⇧⌘N", action: "New note / new folder in the selected folder" }, - { keys: "F2", action: "Rename the focused row" }, - { keys: "⌘⌫", action: "Delete the focused row (a folder confirms first)" }, - { keys: "⇧F10", action: "Open the row's menu — Rename, Move…, Delete" }, + { + ids: ["tree.new-note", "tree.new-folder"], + action: "New note / new folder in the selected folder", + }, + { ids: ["tree.rename"], action: "Rename the focused row" }, + { ids: ["delete.focused"], action: "Delete the focused row (a folder confirms first)" }, + { ids: ["menu.open"], action: "Open the row's menu — Rename, Move…, Delete" }, ], }, { title: "Reading and editing", - items: [ - { keys: "⌘E", action: "Enter or leave edit mode" }, - { keys: "⌘S", action: "Save now (editing autosaves anyway)" }, - { keys: "⌘B / ⌘I", action: "Bold / italic" }, - { keys: "⌘T", action: "Insert a table" }, - { keys: "⇧⌘V", action: "Paste as plain text" }, + rows: [ + { ids: ["edit.toggle"], action: "Enter or leave edit mode" }, + { ids: ["editor.save"], action: "Save now (editing autosaves anyway)" }, + { ids: ["format.bold", "format.italic"], action: "Bold / italic" }, + { ids: ["editor.table"], action: "Insert a table" }, + { ids: ["editor.paste-plain"], action: "Paste as plain text" }, { keys: "[[", action: "Wikilink completion — ↑↓ then ⏎" }, - { keys: "⌘⏎", action: "Save the frontmatter drawer (Esc discards)" }, + { ids: ["fm.save"], action: "Save the frontmatter drawer (Esc discards)" }, ], }, // Discovery gets its own group now that the right column navigates like the tree @@ -70,29 +105,39 @@ export const SHORTCUTS: ShortcutGroup[] = [ // chord with two meanings in a single group is a group the reader can't trust. { title: "Discovery (the right column)", - items: [ + rows: [ { keys: "↑ / ↓", action: "Move between section heads and cards" }, { keys: "→ / ←", action: "Unfold / fold a section or a card's details" }, { keys: "Home / End", action: "First / last row" }, { keys: "⏎ / Space", action: "Open the card's note; fold a section head" }, - { keys: "⇧F10", action: "Open a card's menu — Open note, Link…" }, + { ids: ["menu.open"], action: "Open a card's menu — Open note, Link…" }, ], }, { title: "The graph and menus", - items: [ - { keys: "⏎ / Space", action: "Open a graph node; a ghost opens the link palette" }, - { keys: "↑ / ↓", action: "Move through an open menu" }, - { keys: "Esc", action: "Close a menu, a modal, the find bar, or the graph" }, + rows: [ + { + ids: ["graph.activate"], + action: "Open a graph node; a ghost opens the link palette", + }, + { ids: ["menu.item.prev", "menu.item.next"], action: "Move through an open menu" }, + { ids: ["dismiss"], action: "Close a menu, a modal, the find bar, or the graph" }, ], }, { title: "The app", - items: [ - { keys: "⌘,", action: "Settings" }, - { keys: "⇧⌘A", action: "Review the last index pass's anomalies" }, - { keys: "?", action: "This table (Settings → Keyboard)" }, - { keys: "Tab", action: "Step through the controls on screen" }, + rows: [ + { ids: ["settings.toggle"], action: "Settings" }, + { ids: ["anomalies.toggle"], action: "Review the last index pass's anomalies" }, + { ids: ["help.keyboard"], action: "This table (Settings → Keyboard)" }, + { ids: ["overlay.focus.step"], action: "Step through the controls on screen" }, + // The dialogs' commit chord lives here rather than beside the graph's ⏎: two ⏎ rows + // in one group is the ambiguity the per-group duplicate check exists to catch, and + // this one is a sibling of Tab above it — both are how a dialog is driven. + { + ids: ["link.commit", "delete.confirm"], + action: "Commit the open dialog — Link…, or a delete confirm", + }, ], }, // The settings dialog is a tabbed surface (settingstabs.ts), and a tab rail is exactly @@ -100,11 +145,23 @@ export const SHORTCUTS: ShortcutGroup[] = [ // sections are visibly *there*, so nobody thinks to look for a chord. { title: "Settings (⌘,)", - items: [ + rows: [ { keys: "↑ / ↓", action: "Move between the sections, with the rail focused" }, { keys: "Home / End", action: "First / last section" }, - { keys: "⌃Tab / ⇧⌃Tab", action: "Next / previous section, from anywhere in the dialog" }, - { keys: "Esc", action: "Close Settings" }, + { + ids: ["settings.section.next", "settings.section.prev"], + action: "Next / previous section, from anywhere in the dialog", + }, + { ids: ["dismiss"], action: "Close Settings" }, ], }, ]; + +/** The sheet as render.ts paints it — every row's chords resolved to display text. */ +export const SHORTCUTS: ShortcutGroup[] = SHEET.map((group) => ({ + title: group.title, + items: group.rows.map((row) => ({ + keys: "ids" in row ? displayKeys(row.ids) : row.keys, + action: row.action, + })), +})); From 99172355bc2ae60165dd4098f5388dfa91616b7d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 23:04:55 +0000 Subject: [PATCH 2/4] =?UTF-8?q?ui:=20=E2=8C=83=20is=20not=20=E2=8C=98=20?= =?UTF-8?q?=E2=80=94=20drop=20the=20alias=20that=20put=20six=20chords=20on?= =?UTF-8?q?=20macOS's=20own=20bindings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every handler read `(e.metaKey || e.ctrlKey)`, so each ⌘ chord answered to ⌃ as well. That's the right reflex on Windows and Linux, where ⌃ is the platform modifier, and the wrong one on the only platform B2 ships on. macOS gives ⌃F/⌃B/⌃N/⌃P/⌃A/⌃E emacs meanings in every text field, and CodeMirror implements the same set so its editor feels native. A CodeMirror binding calls `preventDefault` but never `stopPropagation` (0 of the 73 stock bindings opt in), so the keystroke still reached the document handler and ran a second command. ⌃E moved the caret to end-of-line *and* left edit mode. ⌃F moved it forward a character *and* opened the find bar. Also ⌃⇧F, ⌃N, ⌃⇧N and ⌃⇧A; ⌃←/⌃→ were spared only by nav's unrelated `!state.editing` guard. `Mod` now means ⌘ and nothing else, and every modifier is compared literally — which also converges B2's reading of `Mod` with CodeMirror's (`normalizeKeyName` resolves it to meta on mac). That matters more than it sounds: the two share this chord syntax because `chordFor` hands specs straight to `keymap.of`, so the same word meaning two things was a latent trap, and it was already making editorkeys.ts over-expand CodeMirror's own chords when normalizing them. Falling out of the strictness: a chord now has exactly one keystroke form (`Any-` excepted, which is why forms are still a list), so the collision checker no longer reasons about aliasing; ⌘⌃X becomes representable, so parseChord stops rejecting it; nothing binds it. Two guards, both verified to fail when the alias is put back. In bindings.test.ts, a property over the whole table — a binding may claim a ⌃ keystroke only by asking for ⌃ — so it holds for chords not yet written. In editorkeys.test.ts, the other end: whatever B2 binds, it never meets CodeMirror on a ⌃ keystroke. The eight real overlaps are unchanged; the alias set that check used to pin is now empty, and `isAliasOnly` went with it. Unaffected and left alone: `livepreview.ts`'s ⌘-click-to-follow, which takes ⌃-click too. A mouse gesture rather than a chord, and ⌃-click is macOS's secondary click — the same shape of problem, worth its own look. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01A5LV3uA69fYYJBPes6ubF2 --- ui/src/bindings.test.ts | 62 +++++++++++++++++++++------ ui/src/bindings.ts | 90 ++++++++++++++++++++++++--------------- ui/src/editorkeys.test.ts | 53 ++++++++--------------- ui/src/editorkeys.ts | 34 ++++++--------- 4 files changed, 136 insertions(+), 103 deletions(-) diff --git a/ui/src/bindings.test.ts b/ui/src/bindings.test.ts index e00d2a0..da841b3 100644 --- a/ui/src/bindings.test.ts +++ b/ui/src/bindings.test.ts @@ -27,6 +27,7 @@ import { displayChord, displayKeys, isBound, + keystrokes, parseChord, scopeContains, shadows, @@ -106,14 +107,14 @@ check("an alias collides as loudly as a listed chord", () => { assertEq(conflicts(table).map((c) => c.form), ["⌘ArrowLeft"], "the alias clashes"); }); -check("⌃X collides with a Mod chord, because Mod answers to ⌃ too", () => { - // The subtle one. `Mod-x` and `Ctrl-x` look like different rows and are one keystroke - // apart in the hardware: the matcher takes ⌃ as ⌘'s alias, so both fire on ⌃X. +check("an Any- chord collides with every strict chord over its key", () => { + // The subtle one, now that modifiers are compared literally. `Any-Escape` and + // `Mod-Escape` look like unrelated rows and are the same key press. const table: Binding[] = [ - { id: "a", keys: ["Mod-Tab"], scope: "global" }, - { id: "b", keys: ["Ctrl-Tab"], scope: "global" }, + { id: "a", keys: ["Any-Escape"], scope: "global" }, + { id: "b", keys: ["Mod-Escape"], scope: "global" }, ]; - assertEq(conflicts(table).map((c) => c.form), ["⌃Tab"], "⌃ is where they meet"); + assertEq(conflicts(table).map((c) => c.form), ["⌘Escape"], "⌘Esc is where they meet"); }); check("the same chord in sibling scopes is not a conflict", () => { @@ -170,11 +171,45 @@ check("scope containment is global-over-all, then the : namespace", () => { // --- matching ----------------------------------------------------------------------- -check("a Mod chord answers to ⌘ and to ⌃, and to nothing else", () => { +check("a Mod chord answers to ⌘, and to nothing else", () => { assert(isBound(press("f", { metaKey: true }), "find.open"), "⌘F opens find"); - assert(isBound(press("f", { ctrlKey: true }), "find.open"), "⌃F does too — the dev alias"); assert(!isBound(press("f"), "find.open"), "bare F types an f"); assert(!isBound(press("f", { metaKey: true, altKey: true }), "find.open"), "⌥⌘F is not ⌘F"); + assert(!isBound(press("f", { ctrlKey: true }), "find.open"), "and ⌃F is not ⌘F"); +}); + +check("⌃ is not a synonym for ⌘, anywhere in the table", () => { + // The regression guard for the alias B2 used to have. `(e.metaKey || e.ctrlKey)` is the + // right reflex on Windows and Linux and the wrong one on the only platform B2 ships on: + // macOS gives ⌃F/⌃B/⌃N/⌃P/⌃A/⌃E emacs meanings in every text field, CodeMirror + // implements them, and a CodeMirror binding calls `preventDefault` without + // `stopPropagation` — so the keystroke ran the caret move *and* B2's command. ⌃E went to + // end-of-line and left edit mode at once. + // + // Stated as a property rather than a list of six, so it holds for chords not yet + // written: a binding may claim a ⌃ keystroke only by asking for ⌃. + for (const b of BINDINGS) { + for (const spec of allKeys(b)) { + const declares = spec.includes("Ctrl-") || spec.includes("Any-"); + for (const form of keystrokes(spec)) { + assert( + !form.startsWith("⌃") || declares, + `${b.id} answers to ${form} but its chord (${spec}) never asks for ⌃`, + ); + } + } + } +}); + +check("⌃X and ⌘X are two different chords", () => { + // Which is what makes the Settings rail's ⌃Tab a chord of its own rather than a second + // spelling of something else. + assert(isBound(press("Tab", { ctrlKey: true }), "settings.section.next"), "⌃Tab"); + const table: Binding[] = [ + { id: "meta", keys: ["Mod-k"], scope: "global" }, + { id: "control", keys: ["Ctrl-k"], scope: "global" }, + ]; + assertEq(conflicts(table), [], "they no longer meet"); }); check("⇧ separates two commands on one letter", () => { @@ -213,10 +248,9 @@ check("the space bar is spelled, not written as a literal space", () => { assert(isBound(press("Enter"), "graph.activate"), "so does ⏎"); }); -check("⌃Tab is literal Control, not Mod", () => { - // The one chord in the app that wants ⌃ itself. If it went through Mod it would also - // fire on ⌘Tab — which macOS owns — and would collide with anything bound to Mod-Tab. - assert(isBound(press("Tab", { ctrlKey: true }), "settings.section.next"), "⌃Tab"); +check("⌃Tab is literal Control, and ⌘Tab is not it", () => { + // The one chord in the app that asks for ⌃. ⌘Tab is macOS's app switcher and never + // reaches the webview at all, which is exactly why the rail wants the other one. assert(!isBound(press("Tab", { metaKey: true }), "settings.section.next"), "⌘Tab is not ⌃Tab"); assert( isBound(press("Tab", { ctrlKey: true, shiftKey: true }), "settings.section.prev"), @@ -278,7 +312,9 @@ check("chordMatches reads the modifiers it is given, not the ones it isn't", () }); check("parseChord refuses what it can't honour", () => { - const rejects = ["Cmd+f", "Mod-Ctrl-x", "Mod-Meh", "Mod-Retrun", ""]; + // `Mod-Ctrl-x` is absent on purpose: ⌘⌃X is a real chord now that the two modifiers + // are compared separately, so the parser has nothing to object to. Nothing binds it. + const rejects = ["Cmd+f", "Mod-Meh", "Mod-Retrun", ""]; for (const spec of rejects) { let threw = false; try { diff --git a/ui/src/bindings.ts b/ui/src/bindings.ts index cf7a856..cb5eaba 100644 --- a/ui/src/bindings.ts +++ b/ui/src/bindings.ts @@ -29,11 +29,15 @@ // // The chord syntax is CodeMirror's (`Mod-Shift-v`) on purpose: the editor's own bindings // come out of this same table and are handed to `keymap.of` verbatim, so there is one -// spelling of a chord for both halves of the app. +// spelling of a chord for both halves of the app. Sharing the syntax means sharing the +// *semantics* too — `Mod` is ⌘ here because `Mod` is ⌘ there (CodeMirror's +// `normalizeKeyName` resolves it to meta on mac). That agreement is load-bearing, and +// see `Chord.mod` for what it cost to notice. `Any-` is B2's one addition to the syntax, +// and the one thing that must not be handed to CodeMirror. /** Where a chord applies. `global` is the whole window; the rest are surfaces inside it, - * each named after the state that turns it on. `:` nests (`modal:link` is inside - * `modal`), which is what lets two ⏎ bindings coexist without ambiguity. */ + * each named after the state that turns it on. `:` nests (`overlay:link` is inside + * `overlay`), which is what lets two ⏎ bindings coexist without ambiguity. */ export type Scope = | "global" | "editor" // CodeMirror has the keyboard (state.editing) @@ -148,9 +152,23 @@ export type BindingId = (typeof BINDINGS)[number]["id"]; export interface Chord { /** Canonical key name — a lowercase character, or a named key ("Enter", "ArrowUp"). */ key: string; - /** ⌘. Matched against `metaKey || ctrlKey`, the alias B2's handlers have always taken. */ + /** ⌘ — and only ⌘. + * + * The handlers used to read `metaKey || ctrlKey`, taking ⌃ as a synonym. That's the + * right reflex on Windows and Linux, where ⌃ *is* the platform modifier, and the wrong + * one here: B2 ships on macOS (crates/b2-desktop), where ⌃ is not spare. Cocoa gives + * ⌃F/⌃B/⌃N/⌃P/⌃A/⌃E their emacs meanings in every text field on the machine, and + * CodeMirror implements the same set so its editor feels native — so the alias put six + * of B2's chords on top of standard system bindings. ⌃E ran `cursorLineEnd` *and* left + * edit mode, from two listeners, because CodeMirror's keymap handler calls + * `preventDefault` but never `stopPropagation`, so the event still reached the + * document. See editorkeys.ts. + * + * Dropping it also makes `Mod` mean the same thing here as it does in CodeMirror + * (`normalizeKeyName` resolves it to meta on mac), which matters because the two share + * this syntax — `chordFor` hands specs straight to `keymap.of`. */ mod: boolean; - /** Literal ⌃, held *without* ⌘ — the Settings rail's ⌃Tab is the only chord wanting it. */ + /** ⌃ — the Settings rail's ⌃Tab is the only chord that asks for it. */ ctrl: boolean; shift: boolean; alt: boolean; @@ -262,11 +280,6 @@ export function parseChord(spec: string): Chord { if (chord.key.length > 1 && !isNamed(chord.key)) { throw new Error(`unknown key "${chord.key}" in chord: ${spec}`); } - if (chord.mod && chord.ctrl) { - // ⌃ is accepted as ⌘'s alias when matching, so "Mod-Ctrl-x" would describe a chord - // whose two halves contradict each other. Nothing needs it; refuse it. - throw new Error(`chord combines Mod and Ctrl, which alias each other: ${spec}`); - } if (chord.any && (chord.mod || chord.ctrl || chord.shift || chord.alt)) { // "Any modifier" and "this exact modifier" are the same contradiction the other way. throw new Error(`chord combines Any with a named modifier: ${spec}`); @@ -280,10 +293,10 @@ export function chordMatches(chord: Chord, e: KeyEventLike): boolean { if (chord.any) return true; if (chord.alt !== e.altKey) return false; if (shiftDistinguishes(chord.key) && chord.shift !== e.shiftKey) return false; - if (chord.ctrl) return e.ctrlKey && !e.metaKey; - // ⌘ or ⌃ both count as Mod: B2 ships on macOS, but the handlers have always taken ⌃ as - // the alias so the app is drivable from a browser during development. - return chord.mod === (e.metaKey || e.ctrlKey); + // Every modifier compared, none aliased: ⌃X is ⌃X and ⌘X is ⌘X. A chord that doesn't + // ask for ⌃ must not answer to it — that's what keeps B2 off the system's own ⌃ + // bindings, and it's the whole of the fix described on `Chord.mod`. + return chord.mod === e.metaKey && chord.ctrl === e.ctrlKey; } // --- lookup ------------------------------------------------------------------------ @@ -321,12 +334,11 @@ export function isBound(e: KeyEventLike, id: BindingId): boolean { /** The physical key presses a chord answers to. * - * A `Mod-` chord has two: ⌘X and ⌃X, because the matcher takes either. That is exactly - * the overlap a collision check has to see — a `Ctrl-` chord and a `Mod-` chord over the - * same key are one keystroke apart from the hardware's point of view, however different - * they look in the table. Comparing *these* rather than the chords themselves is what + * One, for every chord except `Any-` — which claims the lot, and is the reason this + * returns a list at all. Comparing *these* rather than the chords themselves is what * makes "do these two answer to the same keystroke?" a set intersection, and it's why - * editorkeys.ts can ask the same question of a keymap B2 didn't write. */ + * editorkeys.ts can ask the same question of a keymap B2 didn't write, whose chords are + * spelled by someone else's conventions. */ export function keystrokes(spec: string): string[] { return physicalForms(parseChord(spec)); } @@ -337,26 +349,33 @@ export function chordsOverlap(a: string, b: string): boolean { return keystrokes(b).some((f) => forms.has(f)); } +/** One chord, as one keystroke string — modifiers in Apple's order, then the key. */ +function formOf(chord: Chord): string { + return ( + (chord.ctrl ? "⌃" : "") + + (chord.alt ? "⌥" : "") + + (shiftDistinguishes(chord.key) && chord.shift ? "⇧" : "") + + (chord.mod ? "⌘" : "") + + chord.key + ); +} + function physicalForms(chord: Chord): string[] { + if (!chord.any) return [formOf(chord)]; // An `Any-` chord answers to every keystroke over its key, so it claims every form a - // strict chord over that key could produce — which is what makes it collide with, and - // shadow, exactly the bindings it would really take the key from. - if (chord.any) { - const out: string[] = []; - for (const primary of ["", "⌘", "⌃"]) { - for (const alt of ["", "⌥"]) { - for (const shift of shiftDistinguishes(chord.key) ? ["", "⇧"] : [""]) { - out.push(`${primary}${alt}${shift}${chord.key}`); + // strict chord over that key could produce — enumerated through the same `formOf`, so + // the two can't drift apart into a near-miss that never intersects. + const out: string[] = []; + for (const ctrl of [false, true]) { + for (const alt of [false, true]) { + for (const shift of shiftDistinguishes(chord.key) ? [false, true] : [false]) { + for (const mod of [false, true]) { + out.push(formOf({ ...chord, any: false, ctrl, alt, shift, mod })); } } } - return out; } - const mods = (chord.alt ? "⌥" : "") + (shiftDistinguishes(chord.key) && chord.shift ? "⇧" : ""); - const tail = `${mods}${chord.key}`; - if (chord.ctrl) return [`⌃${tail}`]; - if (chord.mod) return [`⌘${tail}`, `⌃${tail}`]; - return [tail]; + return out; } /** `global` contains every scope; otherwise containment is the `:` namespace prefix, so @@ -368,7 +387,10 @@ export function scopeContains(outer: Scope | string, inner: Scope | string): boo /** Two commands in the same scope that answer to the same keystroke. Which one runs * depends on the order of the handler's branches, which is not a contract anyone can * read — so this is the error tier, and bindings.test.ts fails the suite on a non-empty - * result. That check is the gate: `npm test` runs in both `just check` and `just ci`. */ + * result. That check is the gate: `npm test` runs in both `just check` and `just ci`. + * + * Keystrokes rather than chords, because the two aren't the same question: `Any-Escape` + * and `Escape` are different chords that the same key press satisfies. */ export interface Conflict { a: string; b: string; diff --git a/ui/src/editorkeys.test.ts b/ui/src/editorkeys.test.ts index 4f7e5ca..e2c5a6c 100644 --- a/ui/src/editorkeys.test.ts +++ b/ui/src/editorkeys.test.ts @@ -18,7 +18,6 @@ import { STOCK_KEYMAPS, editorChords, editorOverlaps, - isAliasOnly, } from "./editorkeys.ts"; let passed = 0; @@ -73,10 +72,7 @@ check("the chords B2 needs to reach it while editing are unbound by CodeMirror", ["tree.new-note", "⌘N creates a note without leaving the one you're writing"], ["pane.tree", "⌘1 is how the keyboard gets *out* of the editor"], ]; - // Alias-only rows are excluded on purpose: the question here is whether CodeMirror has - // taken the chord the sheet documents (⌘E), not whether ⌃E means something else — it - // does, and the check below is where that lives. - const taken = editorOverlaps().filter((o) => !isAliasOnly(o)); + const taken = editorOverlaps(); for (const [id, why] of mustBubble) { const hit = taken.find((o) => o.id === id); assert(!hit, `CodeMirror now binds ${hit?.chord} to ${hit?.command} — ${why}`); @@ -105,9 +101,7 @@ check("B2 and CodeMirror overlap on exactly these chords", () => { // stop guarding against something. Either way it should be looked at, not re-pinned // reflexively. assertEq( - editorOverlaps() - .filter((o) => !isAliasOnly(o)) - .map((o) => `${o.id} ${o.chord} — ${o.source}: ${o.command}`), + editorOverlaps().map((o) => `${o.id} ${o.chord} — ${o.source}: ${o.command}`), [ "delete.focused Mod-Backspace — defaultKeymap: deleteLineBoundaryBackward", "dismiss Any-Escape — defaultKeymap: simplifySelection", @@ -122,36 +116,23 @@ check("B2 and CodeMirror overlap on exactly these chords", () => { ); }); -check("the ⌃ alias lands on macOS's emacs bindings, on exactly these ten chords", () => { - // Not a CodeMirror problem — a consequence of B2's matcher taking ⌃X wherever it takes - // ⌘X (`(e.metaKey || e.ctrlKey)`, as the handler has always been written). macOS gives - // ⌃F/⌃B/⌃N/⌃P/⌃A/⌃E their emacs meanings system-wide and CodeMirror implements them, - // so inside the editor each row below is one keystroke running two commands: the caret - // moves *and* B2's chord fires. ⌃F is the clearest — forward-a-character, plus the - // find bar opening over the note. +check("B2 and the editor never meet on a ⌃ keystroke — the emacs bindings are CodeMirror's", () => { + // The check that found the bug this file exists for, kept as the guard that it stays + // fixed. B2's matcher used to read `(e.metaKey || e.ctrlKey)`, so every ⌘ chord answered + // to ⌃ as well. On macOS that lands on the system's emacs text bindings — which + // CodeMirror implements — and since a CodeMirror binding calls `preventDefault` without + // `stopPropagation`, the event still reached the document handler. One keystroke ran + // both commands: ⌃E moved the caret to end-of-line *and* left edit mode; ⌃F moved it + // forward a character *and* opened the find bar. Six chords did this (⌃E ⌃F ⌃⇧F ⌃N ⌃⇧N + // ⌃⇧A), and two more would have but for an unrelated `!state.editing` guard. // - // Pinned rather than fixed, because the fix is a behavior change with its own argument - // to have: B2 ships on macOS, where ⌃ is not the platform modifier, so `Mod` could - // simply stop accepting it. That would retire all ten at once and cost only the - // convenience of driving the app from a desktop browser. Left as it was found; this is - // the record of what it costs. + // bindings.test.ts holds the matcher's end of this — no chord claims ⌃ without asking + // for it. This is the other end: whatever B2 binds, it doesn't land on the editor's ⌃. + const onControl = editorOverlaps().filter((o) => o.shared.some((f) => f.startsWith("⌃"))); assertEq( - editorOverlaps() - .filter(isAliasOnly) - .map((o) => `${o.id} ${o.chord} [${o.shared.join(" ")}] — ${o.command}`), - [ - "find.open Mod-f [⌃f] — cursorCharRight", - "search.focus Mod-Shift-f [⌃⇧f] — selectCharRight", - "tree.new-note Mod-n [⌃n] — cursorLineDown", - "tree.new-folder Mod-Shift-n [⌃⇧n] — selectLineDown", - "anomalies.toggle Mod-Shift-a [⌃⇧a] — selectLineStart", - "edit.toggle Mod-e [⌃e] — cursorLineEnd", - "nav.back Mod-ArrowLeft [⌃ArrowLeft] — cursorSyntaxLeft", - "nav.forward Mod-ArrowRight [⌃ArrowRight] — cursorSyntaxRight", - "format.bold Mod-b [⌃b] — cursorCharLeft", - "editor.table Mod-t [⌃t] — transposeChars", - ], - "the alias set", + onControl.map((o) => `${o.id} ${o.chord} [${o.shared.join(" ")}] — ${o.command}`), + [], + "B2 chords meeting CodeMirror on ⌃", ); }); diff --git a/ui/src/editorkeys.ts b/ui/src/editorkeys.ts index 9fcca8f..0d48887 100644 --- a/ui/src/editorkeys.ts +++ b/ui/src/editorkeys.ts @@ -15,6 +15,13 @@ // comment beside each handler. A CodeMirror release that binds Mod-e would break ⌘E // silently, in the one code path a user hits constantly and a test suite never does. // `editorOverlaps` turns that assumption into a list, and editorkeys.test.ts pins it. +// +// The first thing it found was a bug rather than a risk. B2's matcher used to take ⌃X +// wherever it took ⌘X, and macOS gives ⌃F/⌃B/⌃N/⌃P/⌃A/⌃E emacs meanings in every text +// field — which CodeMirror implements. So six of B2's chords sat on standard system +// bindings, and because a CodeMirror binding calls `preventDefault` but not +// `stopPropagation`, the keystroke ran *both* commands: ⌃E moved the caret to end-of-line +// and left edit mode. The alias is gone (see `Chord.mod`); this is the check that noticed. import { completionKeymap } from "@codemirror/autocomplete"; import { defaultKeymap, historyKeymap } from "@codemirror/commands"; import type { KeyBinding } from "@codemirror/view"; @@ -77,28 +84,13 @@ export interface EditorOverlap { id: string; /** B2's chord, as the registry spells it. */ chord: string; - /** The keystrokes the two actually share — the useful detail, because a `Mod-` chord - * answers to two of them and the two collide for completely different reasons. */ + /** The keystrokes the two actually share — normally one, but `Any-` chords claim a + * spread of them, and knowing *which* is what tells you whether a row matters. */ shared: string[]; source: string; command: string; } -/** Does this overlap exist only on the ⌃ alias — never on the chord as documented? - * - * B2's matcher takes ⌃X wherever it takes ⌘X, a convenience for driving the app in a - * browser during development. On macOS that alias lands on the system's emacs-style - * text bindings, which CodeMirror implements: ⌃F is forward-a-character, ⌃E is - * end-of-line, ⌃N is next-line. So ⌃F inside the editor moves the caret *and* opens - * B2's find bar — one keystroke, two commands. - * - * Kept separate from the real clashes because the fix is different in kind: these all - * go away at once if `Mod` stops accepting ⌃, and none of them is about the chord the - * sheet documents. */ -export function isAliasOnly(o: EditorOverlap): boolean { - return o.shared.every((form) => form.startsWith("⌃")); -} - /** The scopes that can be live while CodeMirror holds the keyboard, and so the only ones * an overlap can actually bite in. A modal's ⏎, the tree rename field's Esc and the * graph's Space all collide with a stock binding on paper, but none of those surfaces @@ -120,9 +112,11 @@ export function editorOverlaps(bindings: readonly Binding[] = BINDINGS): EditorO const mine = keystrokes(spec); for (const c of stock) { // Keystrokes, not chord equality: `Any-Escape` and CodeMirror's plain `Escape` - // are different chords that answer to the same key, and `Mod-f` meets `Ctrl-f` - // on one of its two forms and not the other. Both pairs are worth knowing about, - // and only this comparison sees either. + // are different chords that the same key press satisfies, and that pair is one + // of the ones worth knowing about. Reading CodeMirror's specs through B2's + // parser is sound because the two agree on what every modifier means — `Mod` in + // both is ⌘ on macOS. That agreement is load-bearing here and is the reason the + // ⌃ alias had to go rather than be worked around. const shared = mine.filter((f) => c.forms.has(f)); if (shared.length > 0) { out.push({ id: b.id, chord: spec, shared, source: c.source, command: c.command }); From dee34d90985d522ef62ff4839391800f7248275a Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 23:10:23 +0000 Subject: [PATCH 3/4] =?UTF-8?q?ui:=20=E2=8C=98-click=20follows=20a=20wikil?= =?UTF-8?q?ink=20=E2=80=94=20=E2=8C=83-click=20is=20the=20secondary=20clic?= =?UTF-8?q?k,=20not=20a=20synonym?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The live-preview editor's follow-a-link gesture read `(e.metaKey || e.ctrlKey)`, the mouse's version of the keyboard alias dropped in the previous commit. It's worse on this side: on macOS ⌃-click *is* the secondary click, so the OS synthesizes a context-menu gesture from it — ⌃-clicking a wikilink navigated away and right-clicked at once. ⌘ only now. The rule is a named exported function rather than an inline test, because the handler it lives in is buried in a ViewPlugin's `eventHandlers` where the suite can't reach it, and "restore the Ctrl branch for cross-platform symmetry" is a plausible-looking edit somebody will eventually make. bindings.test.ts and editorkeys.test.ts hold this line for the keyboard; neither can see a mouse handler. Two changes were needed to get livepreview.ts into the suite at all, and both are worth having on their own: - The two widget classes used constructor parameter properties, the one TypeScript construct node's `--experimental-strip-types` refuses. That made the entire module unimportable by the test runner. Now plain field declarations. - `./render` is now `./render.ts`. Node resolves specifiers literally; the extensionless form is what the app-only modules use, and this file isn't app-only any more. Verified by putting the Ctrl branch back and watching the new check fail. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01A5LV3uA69fYYJBPes6ubF2 --- ui/src/livepreview.test.ts | 41 +++++++++++++++++++++++++++++++ ui/src/livepreview.ts | 49 ++++++++++++++++++++++++++++---------- 2 files changed, 78 insertions(+), 12 deletions(-) create mode 100644 ui/src/livepreview.test.ts diff --git a/ui/src/livepreview.test.ts b/ui/src/livepreview.test.ts new file mode 100644 index 0000000..6ee4d11 --- /dev/null +++ b/ui/src/livepreview.test.ts @@ -0,0 +1,41 @@ +// The live-preview rules that are decidable without a running editor (livepreview.ts). +// +// Only one so far, and it's here because of a bug rather than for completeness. ⌘-click +// follows a wikilink in the editor; a plain click places the cursor, as an editor must +// (spec §3). That handler used to accept ⌃-click too — the same `metaKey || ctrlKey` +// reflex the keyboard had — and on macOS ⌃-click *is* the secondary click, so the one +// gesture navigated away and opened a context menu. +// +// The keyboard's own version of this rule is enforced twice over in bindings.test.ts and +// editorkeys.test.ts. Neither reaches a mouse handler, and "restore the Ctrl branch for +// cross-platform symmetry" is a plausible-looking edit somebody will eventually make, so +// the decision gets its own named function and this check rather than a comment. +import { isFollowClick } from "./livepreview.ts"; + +let passed = 0; + +function assert(cond: boolean, msg: string): void { + if (!cond) throw new Error(`assertion failed: ${msg}`); +} +function check(name: string, fn: () => void): void { + fn(); + passed++; + console.log(` ok ${name}`); +} + +check("⌘-click follows a wikilink; a plain click does not", () => { + assert(isFollowClick({ metaKey: true }), "⌘-click follows"); + assert(!isFollowClick({ metaKey: false }), "a bare click places the cursor"); +}); + +check("⌃-click does not follow — on macOS it is the secondary click", () => { + // The regression this file exists for. ⌃-click arrives as a primary-button mousedown + // with `ctrlKey` set *and* generates the OS context-menu gesture, so treating it as a + // synonym for ⌘ makes one gesture do two unrelated things. + assert(!isFollowClick({ metaKey: false, ctrlKey: true } as MouseEvent), "⌃-click is a right-click"); + // ⌃ riding along with ⌘ is still a follow: the rule is about what ⌘ means, and the + // predicate must not start rejecting extra modifiers it was never asked to police. + assert(isFollowClick({ metaKey: true, ctrlKey: true } as MouseEvent), "⌃⌘-click still follows"); +}); + +console.log(`livepreview: ${passed} checks passed`); diff --git a/ui/src/livepreview.ts b/ui/src/livepreview.ts index 017ca5d..2d3ebc8 100644 --- a/ui/src/livepreview.ts +++ b/ui/src/livepreview.ts @@ -28,7 +28,9 @@ import { } from "@codemirror/view"; import type { SyntaxNodeRef } from "@lezer/common"; import type { InlineContext, MarkdownConfig } from "@lezer/markdown"; -import { renderMarkdown } from "./render"; +// Extension-qualified, unlike the app-only modules: node's test runner resolves +// specifiers literally, and this file is in the suite now (livepreview.test.ts). +import { renderMarkdown } from "./render.ts"; // --- the wikilink tree extension (spec §4, insight §2.3) -------------------------- // @@ -126,11 +128,16 @@ const ruleDeco = Decoration.replace({ widget: new RuleWidget() }); // `state.doc` remains the source of truth. `from` is the marker's `[`; the state char // sits at `from + 1`. class TaskWidget extends WidgetType { - constructor( - readonly checked: boolean, - readonly from: number, - ) { + // Fields declared and assigned rather than written as constructor parameter + // properties: those are the one TypeScript construct node's `--experimental-strip-types` + // can't erase, and using them here made the *whole module* unimportable by the test + // runner — including the pure rules at the bottom of the file. Same below. + readonly checked: boolean; + readonly from: number; + constructor(checked: boolean, from: number) { super(); + this.checked = checked; + this.from = from; } eq(o: TaskWidget): boolean { return o.checked === this.checked && o.from === this.from; @@ -166,11 +173,12 @@ class TaskWidget extends WidgetType { // cursor at its start, revealing the raw source for editing. `from` is the table's // first-line start. class TableWidget extends WidgetType { - constructor( - readonly md: string, - readonly from: number, - ) { + readonly md: string; + readonly from: number; + constructor(md: string, from: number) { super(); + this.md = md; + this.from = from; } eq(o: TableWidget): boolean { return o.md === this.md && o.from === this.from; @@ -431,12 +439,29 @@ const blockField = StateField.define({ provide: (f) => EditorView.decorations.from(f), }); +/** + * Does this click mean "follow the wikilink" rather than "put the cursor here"? + * + * ⌘ only. It used to take ⌃ as well — the same `metaKey || ctrlKey` reflex the keyboard + * handlers had (bindings.ts `Chord.mod`), and wrong here for a sharper reason than there: + * on macOS **⌃-click *is* the secondary click**. The OS synthesizes a context-menu gesture + * from it, so ⌃-clicking a wikilink navigated away *and* right-clicked, which is not a + * thing any one gesture should do. + * + * Its own function, exported, because a mouse handler buried in a `ViewPlugin`'s + * `eventHandlers` is unreachable from the suite — and this rule is exactly the sort that + * gets casually re-broken by someone restoring "cross-platform" symmetry. + */ +export function isFollowClick(e: Pick): boolean { + return e.metaKey; +} + /** * The live-preview extension: the ViewPlugin folding tree+selection into inline/line * decorations, the `blockField` feeding block widgets (tables — spec §8), plus the * `lp-body` class that swaps the editor to the reading view's proportional voice - * (spec §3, §5). Cmd/Ctrl+click a wikilink follows it via `onFollow`; a plain click - * falls through to place the cursor, as an editor must (spec §3). + * (spec §3, §5). ⌘-click a wikilink follows it via `onFollow`; a plain click falls + * through to place the cursor, as an editor must (spec §3). */ export function livePreview(onFollow: (target: string) => void): Extension { const plugin = ViewPlugin.fromClass( @@ -455,7 +480,7 @@ export function livePreview(onFollow: (target: string) => void): Extension { decorations: (v) => v.decorations, eventHandlers: { mousedown(e: MouseEvent): boolean { - if (!(e.metaKey || e.ctrlKey)) return false; + if (!isFollowClick(e)) return false; const span = (e.target as HTMLElement | null)?.closest?.("[data-target]"); const target = (span as HTMLElement | null)?.dataset.target; if (!target) return false; From 1ca4b88ce8a6c0ce1aaa285283c91efb2e883ec2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 02:52:40 +0000 Subject: [PATCH 4/4] ui: escape the projected chord in the find bar's button title MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review catch (PR #118). `displayKeys(["dismiss"])` was interpolated straight into the shell's `innerHTML`. The value is registry-derived and reads "Esc" today, so nothing is wrong on the screen — but `displayChord` renders whatever key the chord names, and `Mod-<` parses perfectly well, so a future binding over an HTML metacharacter would land in markup raw. The repo's rule is that every value B2 itself interpolates goes through `escapeHtml` (invariant E5) precisely so nobody has to re-derive a value's provenance at each call site. This was the one site in the change that skipped it: the anomaly badge sets `.title` as a DOM property, the toast writes `textContent`, and the `?` sheet already escapes both columns in `shortcutsGridHtml`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01A5LV3uA69fYYJBPes6ubF2 --- ui/src/main.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/src/main.ts b/ui/src/main.ts index c89396d..293b8c3 100644 --- a/ui/src/main.ts +++ b/ui/src/main.ts @@ -3089,9 +3089,9 @@ function buildShell(): void { -