diff --git a/.agent-skills/jsdoc/SKILL.md b/.agent-skills/jsdoc/SKILL.md new file mode 100644 index 00000000..5521445d --- /dev/null +++ b/.agent-skills/jsdoc/SKILL.md @@ -0,0 +1,343 @@ +--- +name: jsdoc +description: Add or revise source-level JSDoc for Shift APIs. Use this skill before writing or editing documentation comments for exported classes, methods, constructors, domain data structures, render frames, reactive state, or any API where caller intent, side effects, lifetime, ownership, or nullability are easy to misunderstand. +--- + +# /jsdoc — Source API Contracts + +Write JSDoc as the stable public contract a caller needs without reading the implementation. + +JSDoc comments must sit immediately before the documented symbol and use `/** ... */` so tooling can parse them. Follow the standard tag vocabulary from , adapted for TypeScript source. + +## Why this matters + +- **VS Code hover** renders the first line in bold and the rest as body. A one-sentence contract on line 1 is the single highest-leverage thing you can write. +- **TypeScript already encodes shape.** JSDoc is for what types can't say: ownership, lifetime, mutation, side effects, side-channel reactivity, nullability semantics, performance class, call ordering. +- **Code review** is the second reader. Reviewers should be able to judge a call site against the doc without opening the implementation. + +## Runbook + +1. Identify the audience: caller, implementer, renderer, tool author, or maintainer. +2. State the stable contract in one short opening sentence. +3. Add details only for ownership, lifetime, reactivity, mutation, side effects, nullability, performance, or call ordering. Put long detail under `@remarks`. +4. Use tags for callable APIs: + - Add `@param` for every public constructor, method, or function parameter, and make the text describe the parameter's role, constraint, ownership, or valid range. + - Add `@returns` when a method returns a value, nullable result, created object, snapshot, or read-only view. + - Add `@throws {ErrorType} when …` for every observable failure mode (custom error class, semantic Error). + - Do not add `@returns void`; describe the side effect in prose instead. +5. Cross-reference siblings with `@see {@link OtherSymbol}` (one tag per related symbol). +6. Add `@example` only when the intended call flow, ordering, or output is not obvious from the signature. +7. Re-read the comment and delete implementation trivia, current call-site anecdotes, and unstable examples. + +## Hard Style Rules + +- **One-line contract.** First sentence is a verb phrase (`Returns…`, `Applies…`, `Triggers…`), ending with a period. No "This function…". +- **`@remarks` for the long explanation.** If you need more than one sentence of context, demote it under `@remarks` so the hover summary stays clean. +- **`@param name - description` documents meaning, not type.** For public callable APIs, include the tag and make it earn its place by describing role, constraint, ownership, valid range, or call-order semantics. +- **`@returns` documents _meaning_, never "void".** Drop the tag entirely for void returns; describe the side effect in the summary instead. Use `@returns` to clarify ownership ("a fresh array; caller owns it"), nullability semantics ("null when the glyph has no contours, not when it's missing"), or that the result is a snapshot vs a live reactive view. +- **Document side effects, lifetime, and reactivity.** TypeScript can't encode "runs after render", "mutates the Glyph signal", "JS-only — does not call NAPI", "transfers ownership of the buffer". That is exactly what JSDoc is for. +- **Stable terms over current implementation names.** Document the concept, not today's wiring. +- **No warnings, no scolding.** State the contract directly. +- **Do not document private helpers** unless they encode a non-obvious invariant. +- **Do not name current callers** ("used by FooManager") — rots fast. +- **Never use JSDoc as a TODO list.** That belongs in commits, issues, or `// TODO` comments. + +## What To Document + +Document where the type signature is silent. If the type fully encodes the contract, write nothing. Otherwise, prioritize these dimensions: + +- **Effects** — purity, mutation of arguments, mutation of shared state, I/O. +- **Ownership** — who owns the return value, who may mutate it, aliasing with internal state. +- **Identity vs value** — handles/refs/IDs that look like the loaded object but aren't; snapshots vs live views. +- **Nullability semantics** — what `null` / `undefined` / empty actually _means_ (absent, error, not-yet-loaded, end-of-stream). +- **Resolution semantics** — strict vs fallback, find vs find-or-create, exact vs nearest. +- **Lifetime and ordering** — preconditions, disposal, idempotence, what makes the result go stale. +- **Failure modes** — which errors, under which conditions; whether failure is observable or swallowed. +- **Concurrency and context** — thread, render phase, re-entrancy, async cancellation behavior. +- **Performance class** — Big-O, hot-path safety, sync-vs-async cost, when a convenient method is wrong. + +Pick only the dimensions that apply. Do not force every doc to address all of them. + +## Tag Reference Card (TS-first) + +| Tag | Use it when | Example | +| ------------------------- | --------------------------------------------------------------------------- | --------------------------------------------------------------------- | +| `@param name - desc` | Every public param. Document the constraint, not the type. | `@param glyph - must be loaded (not a GlyphRef)` | +| `@returns desc` | Nullable / created / snapshot / read-only / non-obvious return. | `@returns null when no source is active; never throws.` | +| `@throws {Err} when …` | Every observable failure mode. Always type + condition. | `@throws {GlyphNotLoadedError} when called on a ref-only glyph.` | +| `@example` | Call order, setup, or output carries the lesson. | See below — fenced ts, imports, `// Output:` line. | +| `@remarks` | Long explanation that would bloat the summary. | One short paragraph; not multi-paragraph essays. | +| `@see {@link Foo}` | One tag per related sibling API. | `@see {@link createDraft}` | +| `@deprecated ` | Always with replacement or removal reason. | `@deprecated Use draft.setPositions instead — avoids NAPI per frame.` | +| `@template T - desc` | Generic with a _semantic_ constraint that isn't obvious from the signature. | `@template T - coordinate-space tag; controls bound interpretation.` | + +### Avoid in TypeScript + +These re-encode information TS already owns. Including them is noise and risks drift. + +- `@type`, `@typedef`, `@property` — TS variable annotations, `type`, and `interface` are the source of truth. +- `@class`, `@constructor`, `@extends`, `@implements`, `@function`, `@method` — the declaration shape says this. +- `{Type}` annotations inside `@param` / `@returns` — never write `@param {string} name`. Document meaning; TS owns the type. + +### Skip in app code + +These are doc-generator ceremony for published packages. Shift is not a published package; do not write these in app code. + +- `@since`, `@public`, `@beta`, `@alpha`, `@experimental`, `@category` +- `@author`, `@version`, `@copyright` +- date-fns-style `@name`/`@summary`/`@description` triples + +## Tag Format + +In TypeScript files, omit JSDoc type annotations. Let TypeScript own the type; let JSDoc own meaning. + +```ts +/** + * Snapshot of state required to redraw the scene layer. + * + * Building this frame establishes the reactive dependencies for the scene + * output. Drawing code consumes the frame as plain data. + * + * @param dependencies - Values that invalidate or describe one scene redraw. + */ +constructor(dependencies: SceneFrameDependencies) {} +``` + +For functions with multiple parameters, document each parameter by role: + +```ts +/** + * Converts a screen-space pointer into editor coordinate spaces. + * + * @param screen - Pointer position in canvas pixels. + * @param drawOffset - Glyph-local offset applied by the current editor view. + * @returns Coordinates in screen, scene, and glyph-local space. + */ +function resolveCoordinates( + screen: Point2D, + drawOffset: Point2D, +): Coordinates {} +``` + +When failure paths are observable, document them with `@throws`: + +```ts +/** + * Loads a glyph by handle. Resolves once the source is hydrated. + * + * @param handle - identity returned by {@link glyphHandleForUnicode}. + * @returns the loaded glyph; never a {@link GlyphRef}. + * @throws {GlyphNotFoundError} when the handle does not resolve in the active font. + * @see {@link glyphHandleForUnicode} + */ +async function loadGlyph(handle: GlyphHandle): Promise {} +``` + +When deprecating, name the replacement: + +```ts +/** + * @deprecated Use {@link draft.setPositions} — `bridge.setNodePositions` sends + * one NAPI call per point and causes ~450ms frames on dense glyphs. + */ +function setNodePositions(updates: NodePositionUpdate[]): void {} +``` + +## Examples — the rules + +Examples must be runnable assertions, not decoration. + +- **Always fenced and language-tagged** with ` ```ts `. VS Code highlights inside fences. +- **Self-contained.** Include imports. The reader should be able to paste the snippet and have it compile. +- **Show expected output** with a `// Output:` or `// =>` comment when the value carries the lesson. +- **Short.** 8–12 lines is the typical good length; 25 is the ceiling. If it doesn't fit, the example is the wrong shape. +- **One concept per `@example`.** Multiple `@example` blocks are fine and better than one mega-block. + +A good example for a Shift API: + +````ts +/** + * Begins a JS-only edit of the active glyph. Pair with {@link GlyphDraft.finish} + * to persist, or {@link GlyphDraft.discard} to revert. + * + * @returns a draft scoped to the active glyph; `null` when no glyph is loaded. + * + * @example + * ```ts + * const draft = editor.createDraft(); + * if (!draft) return; + * + * for (const update of dragFrame) { + * draft.setPositions(update); // JS-only; no NAPI + * } + * + * draft.finish("translate"); // syncs once, records undo + * ``` + */ +createDraft(): GlyphDraft | null {} +```` + +When TS inference is non-obvious, annotate the type position inline (Effect pattern): + +````ts +/** + * @example + * ```ts + * // ┌─── Option + * // ▼ + * const result = font.glyphForUnicode(0x41); + * ``` + */ +```` + +Avoid examples that depend on hidden setup, test fixtures, or implicit globals. + +## Overloads + +- **Per-overload JSDoc** when parameter _meanings_ differ. (This is the TS standard-library convention.) Copy the contract on each signature; do not put one block on the implementation signature. +- **Top-overload JSDoc only** when the contract is identical and only the type shape differs. The implementation signature stays bare. + +```ts +/** + * Resolves a glyph from its Unicode codepoint. + * @param codepoint - the Unicode scalar value. + */ +function glyphFor(codepoint: number): Glyph | null; +/** + * Resolves a glyph from its handle. + * @param handle - identity from a prior lookup; cheaper than codepoint resolution. + */ +function glyphFor(handle: GlyphHandle): Glyph | null; +function glyphFor(arg: number | GlyphHandle): Glyph | null { + /* impl */ +} +``` + +## Anti-Patterns (bad → good) + +### `@returns void` + +```ts +// ❌ +/** @returns void */ +clear(): void {} + +// ✅ describe the side effect; drop @returns entirely +/** Clears all queued render frames. Idempotent. */ +clear(): void {} +``` + +### Re-stating the signature in prose + +```ts +// ❌ +/** + * @param glyph - the glyph + * @param index - the index + */ + +// ✅ document the constraint +/** + * @param glyph - must be loaded (not a {@link GlyphRef}). + * @param index - zero-based contour index; -1 selects the outer hull. + */ +``` + +### Multi-paragraph summary + +```ts +// ❌ VS Code hover becomes a wall of text +/** + * This function is used to set positions. It is part of the GlyphDraft API and + * is used during drag operations. It does not call NAPI. It must be paired + * with either finish() or discard(). + */ + +// ✅ one-line contract + @remarks +/** + * Updates JS-side glyph positions; pair with {@link finish} or {@link discard}. + * + * @remarks + * JS-only — does not call NAPI. Use during drag hot path; call `finish()` once + * at gesture end to sync to Rust, or `discard()` to revert. + */ +``` + +### Naming current callers + +```ts +// ❌ rots fast +/** Called by GlyphSidebar and TransformPanel. */ + +// ✅ describe what it produces +/** Returns the active glyph's tight bounds, accounting for sidebearings. */ +``` + +### Bare `@deprecated` + +```ts +// ❌ +/** @deprecated */ +function oldThing() {} + +// ✅ name the replacement or the reason +/** @deprecated Use {@link newThing} — removes the legacy 2D-only path. */ +function oldThing() {} +``` + +### `@example` for trivial calls + +````ts +// ❌ noise +/** + * @example + * ```ts + * const id = glyph.id; + * ``` + */ +get id(): string {} + +// ✅ no @example; the name is the whole story +get id(): string {} +```` + +### Examples that depend on hidden setup + +````ts +// ❌ what is `editor`? +/** @example editor.commit(); */ + +// ✅ self-contained +/** + * @example + * ```ts + * const editor = createTestEditor(); + * await editor.loadGlyph("A"); + * editor.commit(); + * ``` + */ +```` + +### General "do not" list + +- API dumps covering every accessor. +- Long examples that obscure the method being documented. +- Compat wrappers or aliases that hide which API should be used. +- Rewriting behavior while documenting unless the user requested the API fix too. +- Describing bugs, migrations, or "currently used by X" in API docs. +- Listing concrete state variants as examples when the actual contract is broader. +- Documenting private helpers with full `@param`/`@returns`/`@example` blocks — a one-line summary is enough. +- Mixing `{Type}`-style JSDoc with TSDoc tags in the same module; pick one (in TS, drop `{Type}`). + +## Quick Checklist + +Before saving, scan your doc against this list: + +- [ ] First line is one sentence, verb-phrase, ends with a period. +- [ ] No `@type`, `@typedef`, or `{Type}` annotations. +- [ ] No `@returns void`. Side effect described in summary. +- [ ] Every observable failure has `@throws {Type} when …`. +- [ ] `@example` (if present) has imports, is fenced ` ```ts `, and shows output when it carries the lesson. +- [ ] No current-caller name-drops, no migration notes, no TODOs. +- [ ] No ceremony tags (`@since`, `@public`, `@category`, `@author`). +- [ ] If deprecated, the replacement or reason is named. diff --git a/.claude/settings.json b/.claude/settings.json index 562606d3..856f69d2 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "git diff --name-only --diff-filter=ACMR HEAD | grep -E '\\.(ts|tsx|json|md|css)$' | xargs pnpm prettier --write --ignore-unknown 2>/dev/null; true" + "command": "git diff --name-only --diff-filter=ACMR HEAD | grep -E '\\.(ts|tsx|json|md|css)$' | xargs pnpm oxfmt 2>/dev/null; true" } ] } diff --git a/.claude/skills/commit/SKILL.md b/.claude/skills/commit/SKILL.md new file mode 100644 index 00000000..0392951d --- /dev/null +++ b/.claude/skills/commit/SKILL.md @@ -0,0 +1,144 @@ +--- +name: commit +description: Canonical rules for writing git commits in the Shift codebase. Use whenever the user asks to commit, stage and commit, or "make a commit" — and whenever you're about to draft a commit message. Enforces conventional prefixes, concise subjects, and splitting unrelated changes into separate commits. +--- + +# /commit — How commits are written in this codebase + +The goal: a `git log --oneline` that reads like a changelog. Each line tells you what changed and why in under ~70 characters, with a prefix that lets you grep for features vs fixes vs refactors. + +## The rule + +**Every commit subject is `: `. Every commit is one logical change.** + +If you can't describe the change in one short clause, it's probably two commits. + +## Step 0 — ask the user before drafting + +Before writing any commit message, **ask the user to describe the changes in their own words**. They know the why; you only see the diff. A one-line description from the user beats five minutes of you guessing from code. + +Skip this step only if: + +- The user already told you the goal in this conversation +- The change is trivially self-describing (e.g. a single typo fix, a single test rename) + +When asking, keep it tight: + +> "Before I commit — give me a one-line description of what these changes do and why. If multiple things are in flight, list them separately so I can split commits." + +## Allowed types + +| Type | Use for | +| ---------- | ----------------------------------------------------------------------------------------------------------- | +| `feat` | New user-visible functionality | +| `fix` | Bug fix | +| `refactor` | Code change with no behavior change | +| `test` | Adding or rewriting tests, no production code changes | +| `docs` | Documentation only (DOCS.md, README, comments) | +| `perf` | Performance improvement, no behavior change | +| `style` | Formatting only (prettier, oxlint --fix). **Not** "look-and-feel UI tweaks\*\* — those are `feat` or `fix`. | +| `build` | Build system, native compile, package scripts | +| `chore` | Tooling, config, dependencies, repo housekeeping | +| `ci` | CI workflows | + +If a change touches multiple types, split it. A `feat` that also fixes an unrelated bug is two commits. + +## Subject line rules + +- **≤ 72 characters total**, including the `: ` prefix. Aim for 50. +- **Lowercase after the colon.** `feat: add ...`, not `feat: Add ...`. +- **Imperative mood.** "add reactive glyph signal", not "added" / "adds". +- **No trailing period.** +- **No file paths or symbol names** unless they're load-bearing for understanding (`fix: GlyphView interpolation on composite scrub` is fine; `fix: update GlyphView.ts line 142` is not). +- **No emoji.** No "🤖 Generated with Claude Code" footer unless the user explicitly asks. + +## Body rules + +Most commits don't need a body. Add one when: + +- The "why" isn't obvious from the subject (constraint, prior incident, non-local consequence). +- You need to call out a follow-up, a known limitation, or a behavioral subtlety. + +When you write a body: + +- Blank line after the subject. +- Wrap at ~72 chars. +- Bullets are fine. Walls of prose are not. +- Don't restate the diff. Explain what the diff doesn't say. + +## Splitting commits + +Before staging, look at what's modified and ask: **does this collapse to one logical change, or several?** + +Strong signals to split: + +- Unrelated subsystems touched (e.g. text layout AND clipboard). +- A file rename / directory reorg mixed with logic edits in those files. → reorg first, edits second. +- Test-only changes that are independent of the feature (e.g. migrating an old test to TestEditor while also adding a new feature). → split. +- Generated code regenerated alongside hand-edits. → split (`chore: regenerate types` separate from the `feat`). +- A formatting sweep landed on top of real changes. → `style:` separate. +- Docs refresh alongside code. → fine to bundle if the docs describe the code in the same commit; split if the docs are a broader refresh. + +Signals to **keep together**: + +- Production code + the tests that cover it. +- A type change + the call sites it forces. +- A rename + the imports that follow from it. + +When in doubt: propose the split to the user and let them confirm before you stage anything. A wrong split is annoying to unwind. + +### How to propose a split + +After the user describes the changes, send a short plan like: + +> Proposed commits: +> +> 1. `refactor: move text/* under lib/text` — pure file moves, no logic changes +> 2. `feat: ` — Editor.ts, NativeBridge.ts, App.tsx +> 3. `test: cover ` — new Editor.test.ts and updated suites +> 4. `docs: refresh text/ DOCS.md` +> +> Want me to stage and commit in that order? + +Wait for confirmation before running `git add` / `git commit`. + +## Examples from this repo + +**Good** (keep doing these): + +- `feat: text-system rewrite — bottom-up rebuild around Cell + TextRun` +- `fix: canvas interpolates composite components on slider scrub` +- `test: GlyphView interpolation against MutatorSans` +- `refactor: sweep leaky naming suffixes` + +**Bad** (don't do these): + +- `add variation cache to the editor, fix bug where edit sessions did not have current variation when opening a new glyph for editing` + → no prefix, two unrelated changes, runs to ~120 chars. Should have been: + - `feat: cache variation state on Editor` + - `fix: hydrate edit session with current variation on glyph open` +- `style fixes` + → vague. What style? Where? Use `style: prettier sweep across renderer/` or just merge into the parent commit. +- `component restructure` + → no prefix, says nothing. `refactor: split into + `. +- `more home sidebar design` + → "more" is a smell that says "I didn't bother to describe this". Squash into the parent or rename to `feat: home sidebar — `. + +## Process + +1. **Ask the user for a one-line description** (Step 0) unless already covered. +2. Run `git status` and `git diff` (and `git diff --cached` if anything is already staged) in parallel. Read enough of the diff to know what changed — don't trust the file list alone. +3. Look at recent `git log --oneline -10` to match style and prefix conventions. +4. **Decide: one commit or several?** Apply the splitting rules above. If splitting, propose the plan to the user and wait for confirmation. +5. Stage explicitly with `git add ` — never `git add -A` or `git add .`. Per-commit staging keeps splits clean. +6. Commit each unit with `git commit -m "$(cat <<'EOF' ... EOF)"` (heredoc, so multi-line bodies survive shell quoting). +7. Run `git status` after each commit to confirm what's left. + +## Hard rules + +- **Never** `git commit` until the user has explicitly asked for a commit, or has confirmed the proposed split. +- **Never** skip hooks (`--no-verify`, `--no-gpg-sign`) unless the user explicitly asks. If a pre-commit hook fails, fix the issue and create a NEW commit — do not `--amend`. +- **Never** commit files that look like secrets (`.env`, `credentials.json`, anything with API keys in the diff). Flag them and ask. +- **Never** stage with `-A` / `.` — pick paths. +- **Never** push as part of a commit task unless the user asks. +- **Never** add Claude Code attribution / co-author trailers unless the user asks. diff --git a/.claude/skills/docs/SKILL.md b/.claude/skills/docs/SKILL.md index 89bcdb4a..5a613103 100644 --- a/.claude/skills/docs/SKILL.md +++ b/.claude/skills/docs/SKILL.md @@ -48,9 +48,10 @@ This is the most valuable section because it captures knowledge that is invisibl Each invariant states a rule and explains why it exists: -> **Architecture Invariant:** Rust is never touched during the draft hot path. `GlyphDraft.setPositions` calls only `glyph.apply()` — a JS-only signal update. Rust sees the final result once when `finish()` calls `bridge.sync()`. This exists because NAPI struct marshaling at thousands of points per frame causes ~450ms frames + GC pressure. +> **Architecture Invariant:** Rust is never touched during the draft preview hot path. `SourceEditDraft.previewPositionPatch()` applies a sparse patch to local reactive geometry only. Rust sees the final sparse patch once when `commit()` calls `GlyphSource.commitPositionPatch()`. This exists because round-tripping full glyph values for thousands of points per frame causes long frames and GC pressure. Good invariants describe: + - What never happens and why ("X never imports Y because Z") - Performance-motivated design choices ("uses flat arrays, never JSON, because Y") - Semantic distinctions invisible in types ("`$glyph` fires on identity changes, not data changes") diff --git a/.claude/skills/jsdoc/SKILL.md b/.claude/skills/jsdoc/SKILL.md new file mode 100644 index 00000000..5521445d --- /dev/null +++ b/.claude/skills/jsdoc/SKILL.md @@ -0,0 +1,343 @@ +--- +name: jsdoc +description: Add or revise source-level JSDoc for Shift APIs. Use this skill before writing or editing documentation comments for exported classes, methods, constructors, domain data structures, render frames, reactive state, or any API where caller intent, side effects, lifetime, ownership, or nullability are easy to misunderstand. +--- + +# /jsdoc — Source API Contracts + +Write JSDoc as the stable public contract a caller needs without reading the implementation. + +JSDoc comments must sit immediately before the documented symbol and use `/** ... */` so tooling can parse them. Follow the standard tag vocabulary from , adapted for TypeScript source. + +## Why this matters + +- **VS Code hover** renders the first line in bold and the rest as body. A one-sentence contract on line 1 is the single highest-leverage thing you can write. +- **TypeScript already encodes shape.** JSDoc is for what types can't say: ownership, lifetime, mutation, side effects, side-channel reactivity, nullability semantics, performance class, call ordering. +- **Code review** is the second reader. Reviewers should be able to judge a call site against the doc without opening the implementation. + +## Runbook + +1. Identify the audience: caller, implementer, renderer, tool author, or maintainer. +2. State the stable contract in one short opening sentence. +3. Add details only for ownership, lifetime, reactivity, mutation, side effects, nullability, performance, or call ordering. Put long detail under `@remarks`. +4. Use tags for callable APIs: + - Add `@param` for every public constructor, method, or function parameter, and make the text describe the parameter's role, constraint, ownership, or valid range. + - Add `@returns` when a method returns a value, nullable result, created object, snapshot, or read-only view. + - Add `@throws {ErrorType} when …` for every observable failure mode (custom error class, semantic Error). + - Do not add `@returns void`; describe the side effect in prose instead. +5. Cross-reference siblings with `@see {@link OtherSymbol}` (one tag per related symbol). +6. Add `@example` only when the intended call flow, ordering, or output is not obvious from the signature. +7. Re-read the comment and delete implementation trivia, current call-site anecdotes, and unstable examples. + +## Hard Style Rules + +- **One-line contract.** First sentence is a verb phrase (`Returns…`, `Applies…`, `Triggers…`), ending with a period. No "This function…". +- **`@remarks` for the long explanation.** If you need more than one sentence of context, demote it under `@remarks` so the hover summary stays clean. +- **`@param name - description` documents meaning, not type.** For public callable APIs, include the tag and make it earn its place by describing role, constraint, ownership, valid range, or call-order semantics. +- **`@returns` documents _meaning_, never "void".** Drop the tag entirely for void returns; describe the side effect in the summary instead. Use `@returns` to clarify ownership ("a fresh array; caller owns it"), nullability semantics ("null when the glyph has no contours, not when it's missing"), or that the result is a snapshot vs a live reactive view. +- **Document side effects, lifetime, and reactivity.** TypeScript can't encode "runs after render", "mutates the Glyph signal", "JS-only — does not call NAPI", "transfers ownership of the buffer". That is exactly what JSDoc is for. +- **Stable terms over current implementation names.** Document the concept, not today's wiring. +- **No warnings, no scolding.** State the contract directly. +- **Do not document private helpers** unless they encode a non-obvious invariant. +- **Do not name current callers** ("used by FooManager") — rots fast. +- **Never use JSDoc as a TODO list.** That belongs in commits, issues, or `// TODO` comments. + +## What To Document + +Document where the type signature is silent. If the type fully encodes the contract, write nothing. Otherwise, prioritize these dimensions: + +- **Effects** — purity, mutation of arguments, mutation of shared state, I/O. +- **Ownership** — who owns the return value, who may mutate it, aliasing with internal state. +- **Identity vs value** — handles/refs/IDs that look like the loaded object but aren't; snapshots vs live views. +- **Nullability semantics** — what `null` / `undefined` / empty actually _means_ (absent, error, not-yet-loaded, end-of-stream). +- **Resolution semantics** — strict vs fallback, find vs find-or-create, exact vs nearest. +- **Lifetime and ordering** — preconditions, disposal, idempotence, what makes the result go stale. +- **Failure modes** — which errors, under which conditions; whether failure is observable or swallowed. +- **Concurrency and context** — thread, render phase, re-entrancy, async cancellation behavior. +- **Performance class** — Big-O, hot-path safety, sync-vs-async cost, when a convenient method is wrong. + +Pick only the dimensions that apply. Do not force every doc to address all of them. + +## Tag Reference Card (TS-first) + +| Tag | Use it when | Example | +| ------------------------- | --------------------------------------------------------------------------- | --------------------------------------------------------------------- | +| `@param name - desc` | Every public param. Document the constraint, not the type. | `@param glyph - must be loaded (not a GlyphRef)` | +| `@returns desc` | Nullable / created / snapshot / read-only / non-obvious return. | `@returns null when no source is active; never throws.` | +| `@throws {Err} when …` | Every observable failure mode. Always type + condition. | `@throws {GlyphNotLoadedError} when called on a ref-only glyph.` | +| `@example` | Call order, setup, or output carries the lesson. | See below — fenced ts, imports, `// Output:` line. | +| `@remarks` | Long explanation that would bloat the summary. | One short paragraph; not multi-paragraph essays. | +| `@see {@link Foo}` | One tag per related sibling API. | `@see {@link createDraft}` | +| `@deprecated ` | Always with replacement or removal reason. | `@deprecated Use draft.setPositions instead — avoids NAPI per frame.` | +| `@template T - desc` | Generic with a _semantic_ constraint that isn't obvious from the signature. | `@template T - coordinate-space tag; controls bound interpretation.` | + +### Avoid in TypeScript + +These re-encode information TS already owns. Including them is noise and risks drift. + +- `@type`, `@typedef`, `@property` — TS variable annotations, `type`, and `interface` are the source of truth. +- `@class`, `@constructor`, `@extends`, `@implements`, `@function`, `@method` — the declaration shape says this. +- `{Type}` annotations inside `@param` / `@returns` — never write `@param {string} name`. Document meaning; TS owns the type. + +### Skip in app code + +These are doc-generator ceremony for published packages. Shift is not a published package; do not write these in app code. + +- `@since`, `@public`, `@beta`, `@alpha`, `@experimental`, `@category` +- `@author`, `@version`, `@copyright` +- date-fns-style `@name`/`@summary`/`@description` triples + +## Tag Format + +In TypeScript files, omit JSDoc type annotations. Let TypeScript own the type; let JSDoc own meaning. + +```ts +/** + * Snapshot of state required to redraw the scene layer. + * + * Building this frame establishes the reactive dependencies for the scene + * output. Drawing code consumes the frame as plain data. + * + * @param dependencies - Values that invalidate or describe one scene redraw. + */ +constructor(dependencies: SceneFrameDependencies) {} +``` + +For functions with multiple parameters, document each parameter by role: + +```ts +/** + * Converts a screen-space pointer into editor coordinate spaces. + * + * @param screen - Pointer position in canvas pixels. + * @param drawOffset - Glyph-local offset applied by the current editor view. + * @returns Coordinates in screen, scene, and glyph-local space. + */ +function resolveCoordinates( + screen: Point2D, + drawOffset: Point2D, +): Coordinates {} +``` + +When failure paths are observable, document them with `@throws`: + +```ts +/** + * Loads a glyph by handle. Resolves once the source is hydrated. + * + * @param handle - identity returned by {@link glyphHandleForUnicode}. + * @returns the loaded glyph; never a {@link GlyphRef}. + * @throws {GlyphNotFoundError} when the handle does not resolve in the active font. + * @see {@link glyphHandleForUnicode} + */ +async function loadGlyph(handle: GlyphHandle): Promise {} +``` + +When deprecating, name the replacement: + +```ts +/** + * @deprecated Use {@link draft.setPositions} — `bridge.setNodePositions` sends + * one NAPI call per point and causes ~450ms frames on dense glyphs. + */ +function setNodePositions(updates: NodePositionUpdate[]): void {} +``` + +## Examples — the rules + +Examples must be runnable assertions, not decoration. + +- **Always fenced and language-tagged** with ` ```ts `. VS Code highlights inside fences. +- **Self-contained.** Include imports. The reader should be able to paste the snippet and have it compile. +- **Show expected output** with a `// Output:` or `// =>` comment when the value carries the lesson. +- **Short.** 8–12 lines is the typical good length; 25 is the ceiling. If it doesn't fit, the example is the wrong shape. +- **One concept per `@example`.** Multiple `@example` blocks are fine and better than one mega-block. + +A good example for a Shift API: + +````ts +/** + * Begins a JS-only edit of the active glyph. Pair with {@link GlyphDraft.finish} + * to persist, or {@link GlyphDraft.discard} to revert. + * + * @returns a draft scoped to the active glyph; `null` when no glyph is loaded. + * + * @example + * ```ts + * const draft = editor.createDraft(); + * if (!draft) return; + * + * for (const update of dragFrame) { + * draft.setPositions(update); // JS-only; no NAPI + * } + * + * draft.finish("translate"); // syncs once, records undo + * ``` + */ +createDraft(): GlyphDraft | null {} +```` + +When TS inference is non-obvious, annotate the type position inline (Effect pattern): + +````ts +/** + * @example + * ```ts + * // ┌─── Option + * // ▼ + * const result = font.glyphForUnicode(0x41); + * ``` + */ +```` + +Avoid examples that depend on hidden setup, test fixtures, or implicit globals. + +## Overloads + +- **Per-overload JSDoc** when parameter _meanings_ differ. (This is the TS standard-library convention.) Copy the contract on each signature; do not put one block on the implementation signature. +- **Top-overload JSDoc only** when the contract is identical and only the type shape differs. The implementation signature stays bare. + +```ts +/** + * Resolves a glyph from its Unicode codepoint. + * @param codepoint - the Unicode scalar value. + */ +function glyphFor(codepoint: number): Glyph | null; +/** + * Resolves a glyph from its handle. + * @param handle - identity from a prior lookup; cheaper than codepoint resolution. + */ +function glyphFor(handle: GlyphHandle): Glyph | null; +function glyphFor(arg: number | GlyphHandle): Glyph | null { + /* impl */ +} +``` + +## Anti-Patterns (bad → good) + +### `@returns void` + +```ts +// ❌ +/** @returns void */ +clear(): void {} + +// ✅ describe the side effect; drop @returns entirely +/** Clears all queued render frames. Idempotent. */ +clear(): void {} +``` + +### Re-stating the signature in prose + +```ts +// ❌ +/** + * @param glyph - the glyph + * @param index - the index + */ + +// ✅ document the constraint +/** + * @param glyph - must be loaded (not a {@link GlyphRef}). + * @param index - zero-based contour index; -1 selects the outer hull. + */ +``` + +### Multi-paragraph summary + +```ts +// ❌ VS Code hover becomes a wall of text +/** + * This function is used to set positions. It is part of the GlyphDraft API and + * is used during drag operations. It does not call NAPI. It must be paired + * with either finish() or discard(). + */ + +// ✅ one-line contract + @remarks +/** + * Updates JS-side glyph positions; pair with {@link finish} or {@link discard}. + * + * @remarks + * JS-only — does not call NAPI. Use during drag hot path; call `finish()` once + * at gesture end to sync to Rust, or `discard()` to revert. + */ +``` + +### Naming current callers + +```ts +// ❌ rots fast +/** Called by GlyphSidebar and TransformPanel. */ + +// ✅ describe what it produces +/** Returns the active glyph's tight bounds, accounting for sidebearings. */ +``` + +### Bare `@deprecated` + +```ts +// ❌ +/** @deprecated */ +function oldThing() {} + +// ✅ name the replacement or the reason +/** @deprecated Use {@link newThing} — removes the legacy 2D-only path. */ +function oldThing() {} +``` + +### `@example` for trivial calls + +````ts +// ❌ noise +/** + * @example + * ```ts + * const id = glyph.id; + * ``` + */ +get id(): string {} + +// ✅ no @example; the name is the whole story +get id(): string {} +```` + +### Examples that depend on hidden setup + +````ts +// ❌ what is `editor`? +/** @example editor.commit(); */ + +// ✅ self-contained +/** + * @example + * ```ts + * const editor = createTestEditor(); + * await editor.loadGlyph("A"); + * editor.commit(); + * ``` + */ +```` + +### General "do not" list + +- API dumps covering every accessor. +- Long examples that obscure the method being documented. +- Compat wrappers or aliases that hide which API should be used. +- Rewriting behavior while documenting unless the user requested the API fix too. +- Describing bugs, migrations, or "currently used by X" in API docs. +- Listing concrete state variants as examples when the actual contract is broader. +- Documenting private helpers with full `@param`/`@returns`/`@example` blocks — a one-line summary is enough. +- Mixing `{Type}`-style JSDoc with TSDoc tags in the same module; pick one (in TS, drop `{Type}`). + +## Quick Checklist + +Before saving, scan your doc against this list: + +- [ ] First line is one sentence, verb-phrase, ends with a period. +- [ ] No `@type`, `@typedef`, or `{Type}` annotations. +- [ ] No `@returns void`. Side effect described in summary. +- [ ] Every observable failure has `@throws {Type} when …`. +- [ ] `@example` (if present) has imports, is fenced ` ```ts `, and shows output when it carries the lesson. +- [ ] No current-caller name-drops, no migration notes, no TODOs. +- [ ] No ceremony tags (`@since`, `@public`, `@category`, `@author`). +- [ ] If deprecated, the replacement or reason is named. diff --git a/.claude/skills/writing-tests/SKILL.md b/.claude/skills/writing-tests/SKILL.md index da89a9cc..16419079 100644 --- a/.claude/skills/writing-tests/SKILL.md +++ b/.claude/skills/writing-tests/SKILL.md @@ -147,6 +147,12 @@ Before committing a test, run through these. Any miss means the test is wrong. - No wrapper factories around `TestEditor`. If you need a reusable helper, it belongs as a method on `TestEditor` itself (see `pointerMove`, `click`, `escape`). - If your test needs a pre-drawn shape, draw it with the pen/shape tool in `beforeEach` — don't construct glyph snapshots inline. +## Naming + +- `describe()` names should explain the behavior or contract under test, not just repeat a class or file name. +- Prefer `describe("glyph source geometry follows coordinate patches", ...)` over `describe("GlyphSourceState", ...)`. +- A class name can appear inside a larger phrase when it adds clarity, but the phrase should still tell the reader what invariant is being protected. + ## When testing is genuinely hard Some code resists clean unit testing — DOM event handlers, focus management, IME composition, React effect lifecycle. diff --git a/.codex/skills/jsdoc/SKILL.md b/.codex/skills/jsdoc/SKILL.md new file mode 100644 index 00000000..5521445d --- /dev/null +++ b/.codex/skills/jsdoc/SKILL.md @@ -0,0 +1,343 @@ +--- +name: jsdoc +description: Add or revise source-level JSDoc for Shift APIs. Use this skill before writing or editing documentation comments for exported classes, methods, constructors, domain data structures, render frames, reactive state, or any API where caller intent, side effects, lifetime, ownership, or nullability are easy to misunderstand. +--- + +# /jsdoc — Source API Contracts + +Write JSDoc as the stable public contract a caller needs without reading the implementation. + +JSDoc comments must sit immediately before the documented symbol and use `/** ... */` so tooling can parse them. Follow the standard tag vocabulary from , adapted for TypeScript source. + +## Why this matters + +- **VS Code hover** renders the first line in bold and the rest as body. A one-sentence contract on line 1 is the single highest-leverage thing you can write. +- **TypeScript already encodes shape.** JSDoc is for what types can't say: ownership, lifetime, mutation, side effects, side-channel reactivity, nullability semantics, performance class, call ordering. +- **Code review** is the second reader. Reviewers should be able to judge a call site against the doc without opening the implementation. + +## Runbook + +1. Identify the audience: caller, implementer, renderer, tool author, or maintainer. +2. State the stable contract in one short opening sentence. +3. Add details only for ownership, lifetime, reactivity, mutation, side effects, nullability, performance, or call ordering. Put long detail under `@remarks`. +4. Use tags for callable APIs: + - Add `@param` for every public constructor, method, or function parameter, and make the text describe the parameter's role, constraint, ownership, or valid range. + - Add `@returns` when a method returns a value, nullable result, created object, snapshot, or read-only view. + - Add `@throws {ErrorType} when …` for every observable failure mode (custom error class, semantic Error). + - Do not add `@returns void`; describe the side effect in prose instead. +5. Cross-reference siblings with `@see {@link OtherSymbol}` (one tag per related symbol). +6. Add `@example` only when the intended call flow, ordering, or output is not obvious from the signature. +7. Re-read the comment and delete implementation trivia, current call-site anecdotes, and unstable examples. + +## Hard Style Rules + +- **One-line contract.** First sentence is a verb phrase (`Returns…`, `Applies…`, `Triggers…`), ending with a period. No "This function…". +- **`@remarks` for the long explanation.** If you need more than one sentence of context, demote it under `@remarks` so the hover summary stays clean. +- **`@param name - description` documents meaning, not type.** For public callable APIs, include the tag and make it earn its place by describing role, constraint, ownership, valid range, or call-order semantics. +- **`@returns` documents _meaning_, never "void".** Drop the tag entirely for void returns; describe the side effect in the summary instead. Use `@returns` to clarify ownership ("a fresh array; caller owns it"), nullability semantics ("null when the glyph has no contours, not when it's missing"), or that the result is a snapshot vs a live reactive view. +- **Document side effects, lifetime, and reactivity.** TypeScript can't encode "runs after render", "mutates the Glyph signal", "JS-only — does not call NAPI", "transfers ownership of the buffer". That is exactly what JSDoc is for. +- **Stable terms over current implementation names.** Document the concept, not today's wiring. +- **No warnings, no scolding.** State the contract directly. +- **Do not document private helpers** unless they encode a non-obvious invariant. +- **Do not name current callers** ("used by FooManager") — rots fast. +- **Never use JSDoc as a TODO list.** That belongs in commits, issues, or `// TODO` comments. + +## What To Document + +Document where the type signature is silent. If the type fully encodes the contract, write nothing. Otherwise, prioritize these dimensions: + +- **Effects** — purity, mutation of arguments, mutation of shared state, I/O. +- **Ownership** — who owns the return value, who may mutate it, aliasing with internal state. +- **Identity vs value** — handles/refs/IDs that look like the loaded object but aren't; snapshots vs live views. +- **Nullability semantics** — what `null` / `undefined` / empty actually _means_ (absent, error, not-yet-loaded, end-of-stream). +- **Resolution semantics** — strict vs fallback, find vs find-or-create, exact vs nearest. +- **Lifetime and ordering** — preconditions, disposal, idempotence, what makes the result go stale. +- **Failure modes** — which errors, under which conditions; whether failure is observable or swallowed. +- **Concurrency and context** — thread, render phase, re-entrancy, async cancellation behavior. +- **Performance class** — Big-O, hot-path safety, sync-vs-async cost, when a convenient method is wrong. + +Pick only the dimensions that apply. Do not force every doc to address all of them. + +## Tag Reference Card (TS-first) + +| Tag | Use it when | Example | +| ------------------------- | --------------------------------------------------------------------------- | --------------------------------------------------------------------- | +| `@param name - desc` | Every public param. Document the constraint, not the type. | `@param glyph - must be loaded (not a GlyphRef)` | +| `@returns desc` | Nullable / created / snapshot / read-only / non-obvious return. | `@returns null when no source is active; never throws.` | +| `@throws {Err} when …` | Every observable failure mode. Always type + condition. | `@throws {GlyphNotLoadedError} when called on a ref-only glyph.` | +| `@example` | Call order, setup, or output carries the lesson. | See below — fenced ts, imports, `// Output:` line. | +| `@remarks` | Long explanation that would bloat the summary. | One short paragraph; not multi-paragraph essays. | +| `@see {@link Foo}` | One tag per related sibling API. | `@see {@link createDraft}` | +| `@deprecated ` | Always with replacement or removal reason. | `@deprecated Use draft.setPositions instead — avoids NAPI per frame.` | +| `@template T - desc` | Generic with a _semantic_ constraint that isn't obvious from the signature. | `@template T - coordinate-space tag; controls bound interpretation.` | + +### Avoid in TypeScript + +These re-encode information TS already owns. Including them is noise and risks drift. + +- `@type`, `@typedef`, `@property` — TS variable annotations, `type`, and `interface` are the source of truth. +- `@class`, `@constructor`, `@extends`, `@implements`, `@function`, `@method` — the declaration shape says this. +- `{Type}` annotations inside `@param` / `@returns` — never write `@param {string} name`. Document meaning; TS owns the type. + +### Skip in app code + +These are doc-generator ceremony for published packages. Shift is not a published package; do not write these in app code. + +- `@since`, `@public`, `@beta`, `@alpha`, `@experimental`, `@category` +- `@author`, `@version`, `@copyright` +- date-fns-style `@name`/`@summary`/`@description` triples + +## Tag Format + +In TypeScript files, omit JSDoc type annotations. Let TypeScript own the type; let JSDoc own meaning. + +```ts +/** + * Snapshot of state required to redraw the scene layer. + * + * Building this frame establishes the reactive dependencies for the scene + * output. Drawing code consumes the frame as plain data. + * + * @param dependencies - Values that invalidate or describe one scene redraw. + */ +constructor(dependencies: SceneFrameDependencies) {} +``` + +For functions with multiple parameters, document each parameter by role: + +```ts +/** + * Converts a screen-space pointer into editor coordinate spaces. + * + * @param screen - Pointer position in canvas pixels. + * @param drawOffset - Glyph-local offset applied by the current editor view. + * @returns Coordinates in screen, scene, and glyph-local space. + */ +function resolveCoordinates( + screen: Point2D, + drawOffset: Point2D, +): Coordinates {} +``` + +When failure paths are observable, document them with `@throws`: + +```ts +/** + * Loads a glyph by handle. Resolves once the source is hydrated. + * + * @param handle - identity returned by {@link glyphHandleForUnicode}. + * @returns the loaded glyph; never a {@link GlyphRef}. + * @throws {GlyphNotFoundError} when the handle does not resolve in the active font. + * @see {@link glyphHandleForUnicode} + */ +async function loadGlyph(handle: GlyphHandle): Promise {} +``` + +When deprecating, name the replacement: + +```ts +/** + * @deprecated Use {@link draft.setPositions} — `bridge.setNodePositions` sends + * one NAPI call per point and causes ~450ms frames on dense glyphs. + */ +function setNodePositions(updates: NodePositionUpdate[]): void {} +``` + +## Examples — the rules + +Examples must be runnable assertions, not decoration. + +- **Always fenced and language-tagged** with ` ```ts `. VS Code highlights inside fences. +- **Self-contained.** Include imports. The reader should be able to paste the snippet and have it compile. +- **Show expected output** with a `// Output:` or `// =>` comment when the value carries the lesson. +- **Short.** 8–12 lines is the typical good length; 25 is the ceiling. If it doesn't fit, the example is the wrong shape. +- **One concept per `@example`.** Multiple `@example` blocks are fine and better than one mega-block. + +A good example for a Shift API: + +````ts +/** + * Begins a JS-only edit of the active glyph. Pair with {@link GlyphDraft.finish} + * to persist, or {@link GlyphDraft.discard} to revert. + * + * @returns a draft scoped to the active glyph; `null` when no glyph is loaded. + * + * @example + * ```ts + * const draft = editor.createDraft(); + * if (!draft) return; + * + * for (const update of dragFrame) { + * draft.setPositions(update); // JS-only; no NAPI + * } + * + * draft.finish("translate"); // syncs once, records undo + * ``` + */ +createDraft(): GlyphDraft | null {} +```` + +When TS inference is non-obvious, annotate the type position inline (Effect pattern): + +````ts +/** + * @example + * ```ts + * // ┌─── Option + * // ▼ + * const result = font.glyphForUnicode(0x41); + * ``` + */ +```` + +Avoid examples that depend on hidden setup, test fixtures, or implicit globals. + +## Overloads + +- **Per-overload JSDoc** when parameter _meanings_ differ. (This is the TS standard-library convention.) Copy the contract on each signature; do not put one block on the implementation signature. +- **Top-overload JSDoc only** when the contract is identical and only the type shape differs. The implementation signature stays bare. + +```ts +/** + * Resolves a glyph from its Unicode codepoint. + * @param codepoint - the Unicode scalar value. + */ +function glyphFor(codepoint: number): Glyph | null; +/** + * Resolves a glyph from its handle. + * @param handle - identity from a prior lookup; cheaper than codepoint resolution. + */ +function glyphFor(handle: GlyphHandle): Glyph | null; +function glyphFor(arg: number | GlyphHandle): Glyph | null { + /* impl */ +} +``` + +## Anti-Patterns (bad → good) + +### `@returns void` + +```ts +// ❌ +/** @returns void */ +clear(): void {} + +// ✅ describe the side effect; drop @returns entirely +/** Clears all queued render frames. Idempotent. */ +clear(): void {} +``` + +### Re-stating the signature in prose + +```ts +// ❌ +/** + * @param glyph - the glyph + * @param index - the index + */ + +// ✅ document the constraint +/** + * @param glyph - must be loaded (not a {@link GlyphRef}). + * @param index - zero-based contour index; -1 selects the outer hull. + */ +``` + +### Multi-paragraph summary + +```ts +// ❌ VS Code hover becomes a wall of text +/** + * This function is used to set positions. It is part of the GlyphDraft API and + * is used during drag operations. It does not call NAPI. It must be paired + * with either finish() or discard(). + */ + +// ✅ one-line contract + @remarks +/** + * Updates JS-side glyph positions; pair with {@link finish} or {@link discard}. + * + * @remarks + * JS-only — does not call NAPI. Use during drag hot path; call `finish()` once + * at gesture end to sync to Rust, or `discard()` to revert. + */ +``` + +### Naming current callers + +```ts +// ❌ rots fast +/** Called by GlyphSidebar and TransformPanel. */ + +// ✅ describe what it produces +/** Returns the active glyph's tight bounds, accounting for sidebearings. */ +``` + +### Bare `@deprecated` + +```ts +// ❌ +/** @deprecated */ +function oldThing() {} + +// ✅ name the replacement or the reason +/** @deprecated Use {@link newThing} — removes the legacy 2D-only path. */ +function oldThing() {} +``` + +### `@example` for trivial calls + +````ts +// ❌ noise +/** + * @example + * ```ts + * const id = glyph.id; + * ``` + */ +get id(): string {} + +// ✅ no @example; the name is the whole story +get id(): string {} +```` + +### Examples that depend on hidden setup + +````ts +// ❌ what is `editor`? +/** @example editor.commit(); */ + +// ✅ self-contained +/** + * @example + * ```ts + * const editor = createTestEditor(); + * await editor.loadGlyph("A"); + * editor.commit(); + * ``` + */ +```` + +### General "do not" list + +- API dumps covering every accessor. +- Long examples that obscure the method being documented. +- Compat wrappers or aliases that hide which API should be used. +- Rewriting behavior while documenting unless the user requested the API fix too. +- Describing bugs, migrations, or "currently used by X" in API docs. +- Listing concrete state variants as examples when the actual contract is broader. +- Documenting private helpers with full `@param`/`@returns`/`@example` blocks — a one-line summary is enough. +- Mixing `{Type}`-style JSDoc with TSDoc tags in the same module; pick one (in TS, drop `{Type}`). + +## Quick Checklist + +Before saving, scan your doc against this list: + +- [ ] First line is one sentence, verb-phrase, ends with a period. +- [ ] No `@type`, `@typedef`, or `{Type}` annotations. +- [ ] No `@returns void`. Side effect described in summary. +- [ ] Every observable failure has `@throws {Type} when …`. +- [ ] `@example` (if present) has imports, is fenced ` ```ts `, and shows output when it carries the lesson. +- [ ] No current-caller name-drops, no migration notes, no TODOs. +- [ ] No ceremony tags (`@since`, `@public`, `@category`, `@author`). +- [ ] If deprecated, the replacement or reason is named. diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 00000000..b6a839a1 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,15 @@ +root = true + +[*] +indent_style = space +indent_size = 2 +end_of_line = lf +charset = utf-8 +trim_trailing_whitespace = true +insert_final_newline = true + +[*.md] +trim_trailing_whitespace = false + +[*.{rs,toml}] +indent_size = 4 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ed0ec7dc..ca4992b7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,7 +89,7 @@ jobs: - name: Lint (oxlint) run: pnpm test:lint - - name: Format check (prettier) + - name: Format check (oxfmt) run: pnpm format:check typecheck: @@ -171,7 +171,7 @@ jobs: id: native-cache uses: actions/cache@v4 with: - path: crates/shift-node/*.node + path: crates/shift-bridge/*.node key: native-module-${{ runner.os }}-${{ hashFiles('crates/**', 'Cargo.lock') }} - name: Build native module @@ -262,7 +262,7 @@ jobs: run: cargo clippy --workspace --all-targets -- -D warnings - name: Run tests - run: cargo test --workspace --exclude shift-node + run: cargo test --workspace --exclude shift-bridge integration: name: Integration Tests @@ -308,7 +308,7 @@ jobs: - name: Build native module run: pnpm build:native - - name: Run shift-node tests + - name: Run shift-bridge tests run: pnpm test:native # --------------------------------------------------------------------------- @@ -368,7 +368,7 @@ jobs: id: native-cache uses: actions/cache@v4 with: - path: crates/shift-node/*.node + path: crates/shift-bridge/*.node key: native-module-${{ runner.os }}-${{ hashFiles('crates/**', 'Cargo.lock') }} - name: Build native module diff --git a/.github/workflows/perf.yml b/.github/workflows/perf.yml index c54f6e7c..56ede728 100644 --- a/.github/workflows/perf.yml +++ b/.github/workflows/perf.yml @@ -28,8 +28,8 @@ on: - "apps/desktop/src/renderer/src/bridge/NativeBridge.ts" - "apps/desktop/src/renderer/src/lib/commands/**" - "apps/desktop/src/renderer/src/perf/**" - - "crates/shift-core/src/**" - - "crates/shift-node/src/**" + - "crates/shift-edit/src/**" + - "crates/shift-bridge/src/**" permissions: {} @@ -79,7 +79,7 @@ jobs: id: native-cache uses: actions/cache@v4 with: - path: crates/shift-node/*.node + path: crates/shift-bridge/*.node key: native-module-${{ runner.os }}-${{ hashFiles('crates/**', 'Cargo.lock') }} - name: Build native module @@ -142,7 +142,7 @@ jobs: id: native-cache uses: actions/cache@v4 with: - path: crates/shift-node/*.node + path: crates/shift-bridge/*.node key: native-module-${{ runner.os }}-${{ hashFiles('crates/**', 'Cargo.lock') }} - name: Build native module diff --git a/.gitignore b/.gitignore index 14b98712..1da1c171 100644 --- a/.gitignore +++ b/.gitignore @@ -35,6 +35,7 @@ build/Release # Dependency directories node_modules/ jspm_packages/ +.pnpm-store/ # TypeScript v1 declaration files typings/ diff --git a/.oxfmtrc.json b/.oxfmtrc.json new file mode 100644 index 00000000..ec702262 --- /dev/null +++ b/.oxfmtrc.json @@ -0,0 +1,28 @@ +{ + "$schema": "./node_modules/oxfmt/configuration_schema.json", + "semi": true, + "singleQuote": false, + "tabWidth": 2, + "useTabs": false, + "trailingComma": "all", + "printWidth": 100, + "bracketSpacing": true, + "arrowParens": "always", + "sortPackageJson": false, + "ignorePatterns": [ + "node_modules", + "dist", + "out", + ".turbo", + "*.min.js", + "*.min.css", + "pnpm-lock.yaml", + "packages/types/src/generated", + "packages/types/src/bridge/generated.ts", + "packages/types/__fixtures__", + "crates/shift-bridge/index.js", + "crates/shift-bridge/index.d.ts", + "**/vendor", + "packages/glyph-info/resources" + ] +} diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 15015e77..2571ae8b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,15 +1,15 @@ repos: # Standard file hygiene - # Generated files (ts-rs output, NAPI .d.ts, parity fixtures) are excluded + # Generated files (bridge DTO typegen, NAPI .d.ts, parity fixtures) are excluded # because their format is owned by the generator. Re-formatting them here - # creates a loop with the cargo-test hook that regenerates them. + # creates churn with the hooks that regenerate them. - repo: https://github.com/pre-commit/pre-commit-hooks rev: v5.0.0 hooks: - id: trailing-whitespace - exclude: '^(packages/types/src/generated/|packages/types/__fixtures__/|crates/shift-node/index\.(d\.ts|js))' + exclude: '^(packages/types/src/bridge/generated\.ts|packages/types/__fixtures__/|crates/shift-bridge/index\.(d\.ts|js))' - id: end-of-file-fixer - exclude: '^(packages/types/src/generated/|packages/types/__fixtures__/|crates/shift-node/index\.(d\.ts|js))' + exclude: '^(packages/types/src/bridge/generated\.ts|packages/types/__fixtures__/|crates/shift-bridge/index\.(d\.ts|js))' - id: check-yaml - id: check-added-large-files - id: check-merge-conflict @@ -41,7 +41,7 @@ repos: - id: cargo-check-tests name: cargo check (including tests) - entry: bash -c 'cargo check --package shift-node --tests' + entry: cargo check --workspace --tests --exclude shift-bridge language: system types: [rust] pass_filenames: false @@ -62,7 +62,7 @@ repos: - id: cargo-test name: cargo test - entry: bash -c 'cargo test --workspace --exclude shift-node && npx tsx scripts/patch-generated-types.ts && git add packages/types/src/generated/*.ts' + entry: cargo test --workspace --exclude shift-bridge language: system types: [rust] pass_filenames: false @@ -70,11 +70,13 @@ repos: # TypeScript/JavaScript hooks - repo: local hooks: - - id: prettier - name: prettier - entry: pnpm prettier --write --ignore-unknown + - id: oxfmt + name: oxfmt + entry: pnpm format language: system types_or: [javascript, jsx, ts, tsx, css, json, markdown] + pass_filenames: false + exclude: '^(packages/types/src/bridge/generated\.ts|packages/types/__fixtures__/|crates/shift-bridge/index\.(d\.ts|js))' - id: oxlint name: oxlint diff --git a/.prettierignore b/.prettierignore deleted file mode 100644 index a8a5b92b..00000000 --- a/.prettierignore +++ /dev/null @@ -1,12 +0,0 @@ -node_modules -dist -out -.turbo -*.min.js -*.min.css -pnpm-lock.yaml -packages/types/src/generated -packages/types/__fixtures__ -crates/shift-node/index.js -crates/shift-node/index.d.ts -**/vendor diff --git a/.prettierrc b/.prettierrc deleted file mode 100644 index edcafc22..00000000 --- a/.prettierrc +++ /dev/null @@ -1,10 +0,0 @@ -{ - "semi": true, - "singleQuote": false, - "tabWidth": 2, - "useTabs": false, - "trailingComma": "all", - "printWidth": 100, - "bracketSpacing": true, - "arrowParens": "always" -} diff --git a/Cargo.lock b/Cargo.lock index 2541747c..2fee5bfa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -209,7 +209,7 @@ dependencies = [ "semver", "serde", "serde_json", - "thiserror 2.0.12", + "thiserror 2.0.18", ] [[package]] @@ -355,19 +355,9 @@ checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28" [[package]] name = "ctor" -version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "352d39c2f7bef1d6ad73db6f5160efcaed66d94ef8c6c573a8410c00bf909a98" -dependencies = [ - "ctor-proc-macro", - "dtor", -] - -[[package]] -name = "ctor-proc-macro" -version = "0.0.7" +version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "52560adf09603e58c9a7ee1fe1dcb95a16927b17c127f0ac02d6e768a0e25bc1" +checksum = "400a21f1014a968ec518c7ccdf9b4a4ed0cac8c56ccb6d604f8b91f00110501e" [[package]] name = "darling" @@ -455,21 +445,6 @@ dependencies = [ "syn", ] -[[package]] -name = "dtor" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1057d6c64987086ff8ed0fd3fbf377a6b7d205cc7715868cd401705f715cbe4" -dependencies = [ - "dtor-proc-macro", -] - -[[package]] -name = "dtor-proc-macro" -version = "0.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f678cf4a922c215c63e0de95eb1ff08a958a81d47e485cf9da1e27bf6305cfa5" - [[package]] name = "either" version = "1.15.0" @@ -665,7 +640,7 @@ dependencies = [ "ordered-float 5.3.0", "serde", "smol_str 0.3.6", - "thiserror 2.0.12", + "thiserror 2.0.18", "write-fonts 0.44.1", ] @@ -1142,9 +1117,9 @@ checksum = "e94e1e6445d314f972ff7395df2de295fe51b71821694f0b0e1e79c4f12c8577" [[package]] name = "napi" -version = "3.8.4" +version = "3.8.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fb7848c221fb7bb789e02f01875287ebb1e078b92a6566a34de01ef8806e7c2b" +checksum = "8e55037284865448ecf329baa86a4d05401f647ebde99f5747b640d32c2c5226" dependencies = [ "bitflags", "ctor", @@ -1163,9 +1138,9 @@ checksum = "d376940fd5b723c6893cd1ee3f33abbfd86acb1cd1ec079f3ab04a2a3bc4d3b1" [[package]] name = "napi-derive" -version = "3.5.3" +version = "3.5.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "60867ff9a6f76e82350e0c3420cb0736f5866091b61d7d8a024baa54b0ec17dd" +checksum = "a4ba740fe4c9524d86fd90798fd8ccdb23402b3eef7e7c30897a8a369b529fcf" dependencies = [ "convert_case", "ctor", @@ -1177,9 +1152,9 @@ dependencies = [ [[package]] name = "napi-derive-backend" -version = "5.0.2" +version = "5.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0864cf6a82e2cfb69067374b64c9253d7e910e5b34db833ed7495dda56ccb18" +checksum = "0d5af30503edf933ce7377cf6d4c877a62b0f1107ea05585f1b5e430e88d5baf" dependencies = [ "convert_case", "proc-macro2", @@ -1217,7 +1192,7 @@ dependencies = [ "serde", "serde_derive", "serde_repr", - "thiserror 2.0.12", + "thiserror 2.0.18", ] [[package]] @@ -1234,7 +1209,7 @@ dependencies = [ "serde", "serde_derive", "serde_repr", - "thiserror 2.0.12", + "thiserror 2.0.18", "uuid", ] @@ -1661,10 +1636,28 @@ dependencies = [ "plist", "shift-ir", "skrifa", + "tempfile", + "thiserror 2.0.18", ] [[package]] -name = "shift-core" +name = "shift-bridge" +version = "0.0.1" +dependencies = [ + "napi", + "napi-build", + "napi-derive", + "serde", + "serde_json", + "shift-backends", + "shift-edit", + "shift-ir", + "shift-wire", + "thiserror 2.0.18", +] + +[[package]] +name = "shift-edit" version = "0.0.0" dependencies = [ "bitflags", @@ -1675,9 +1668,9 @@ dependencies = [ "serde_json", "shift-backends", "shift-ir", + "shift-wire", "skrifa", - "tempfile", - "ts-rs", + "thiserror 2.0.18", ] [[package]] @@ -1689,19 +1682,16 @@ dependencies = [ "kurbo 0.13.0", "linesweeper", "serde", - "ts-rs", ] [[package]] -name = "shift-node" -version = "0.0.1" +name = "shift-wire" +version = "0.0.0" dependencies = [ "napi", - "napi-build", "napi-derive", "serde", - "serde_json", - "shift-core", + "shift-ir", ] [[package]] @@ -1801,15 +1791,6 @@ dependencies = [ "windows-sys", ] -[[package]] -name = "termcolor" -version = "1.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "06794f8f6c5c898b3275aebefa6b8a1cb24cd2c6c79397ab15774837a0bc5755" -dependencies = [ - "winapi-util", -] - [[package]] name = "thiserror" version = "1.0.69" @@ -1821,11 +1802,11 @@ dependencies = [ [[package]] name = "thiserror" -version = "2.0.12" +version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "567b8a2dae586314f7be2a752ec7474332959c6460e02bde30d702a66d488708" +checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" dependencies = [ - "thiserror-impl 2.0.12", + "thiserror-impl 2.0.18", ] [[package]] @@ -1841,9 +1822,9 @@ dependencies = [ [[package]] name = "thiserror-impl" -version = "2.0.12" +version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f7cf42b4507d8ea322120659672cf1b9dbb93f8f2d4ecfd6e51350ff5b17a1d" +checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" dependencies = [ "proc-macro2", "quote", @@ -1903,28 +1884,6 @@ dependencies = [ "zerovec", ] -[[package]] -name = "ts-rs" -version = "11.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ef1b7a6d914a34127ed8e1fa927eb7088903787bcded4fa3eef8f85ee1568be" -dependencies = [ - "thiserror 2.0.12", - "ts-rs-macros", -] - -[[package]] -name = "ts-rs-macros" -version = "11.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e9d4ed7b4c18cc150a6a0a1e9ea1ecfa688791220781af6e119f9599a8502a0a" -dependencies = [ - "proc-macro2", - "quote", - "syn", - "termcolor", -] - [[package]] name = "ufo2fontir" version = "0.2.0" @@ -2106,15 +2065,6 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" -[[package]] -name = "winapi-util" -version = "0.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" -dependencies = [ - "windows-sys", -] - [[package]] name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" diff --git a/Cargo.toml b/Cargo.toml index b7156032..eee6f173 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,5 +4,8 @@ members = ["crates/*"] [workspace.dependencies] -shift-core = { path = "crates/shift-core" } -shift-node = { path = "crates/shift-node" } +shift-wire = { path = "crates/shift-wire" } +shift-backends = { path = "crates/shift-backends" } +shift-bridge = { path = "crates/shift-bridge" } +shift-edit = { path = "crates/shift-edit" } +shift-ir = { path = "crates/shift-ir" } diff --git a/ROADMAP.md b/ROADMAP.md index c439bc36..d8e52b46 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,4 +1,222 @@ -# Shift Feature Roadmap +# Shift Roadmap + +This roadmap has two layers: + +- **Release roadmap:** the default order of work for shipping usable versions. +- **Feature inventory:** the broader backlog of implemented, planned, and experimental work. + +The release roadmap wins when priorities conflict. Exploration is still useful, but every release should have one clear promise and a small set of acceptance tests. + +## Current Stage + +Current repo state: `0.0.1-dev` / pre-alpha source development. + +Current capability level: roughly `0.2.x-alpha` feature maturity. Core editing work is significantly ahead of release infrastructure, so the first public binary does not need to start at `0.1.0` if the changelog honestly explains what already exists. + +Recommended first public binary: `0.2.0-alpha.1`. + +Why: + +- Basic vector editing, selection, undo/redo, delete, clipboard, segment hover, snapping pieces, transform tools, glyph thumbnails/search, boolean operations, and some variable/text work already exist. +- Public release basics are not done yet: no release tags, no root changelog, no release workflow, no signed/notarized binary, no published artifacts, and version files disagree. + +## Release Roadmap + +### 0.2.0-alpha.1 — First Installable Editing Alpha + +Promise: a tester can install Shift, launch it, open a font, and try the existing core editor without building from source. + +Scope: + +- GitHub Release with release notes and checksums. +- Version files aligned. +- Root `CHANGELOG.md`. +- macOS artifact, signed and notarized if possible. +- Windows and Linux artifacts if CI can produce them without derailing the release. +- README and `shift.graphics` use the same alpha language. +- Known limitations documented prominently. +- Existing editor workflows are not blocked by obvious packaging/native-module issues. + +Acceptance tests: + +- Download the release artifact on a clean macOS machine. +- Launch the app without Gatekeeper workarounds, or clearly label the build as unsigned if signing is deferred. +- Open a UFO/TTF/OTF. +- Select a glyph, edit points, undo/redo, copy/paste, and close the app. + +### 0.3.0-alpha — Persistence And Export Alpha + +Promise: a tester can use Shift on a toy font and verify save/reopen/export behavior. + +Scope: + +- UFO save and Save As are reliable enough for alpha testing. +- Dirty state and close/quit save prompts are trustworthy. +- Reopen after save preserves expected glyph edits. +- TTF/OTF export path exists for simple fonts. +- Export errors are visible and actionable. +- Round-trip tests cover representative UFO edits. +- Data-loss risks are documented. + +Acceptance tests: + +- Open UFO, edit glyph, save, quit, reopen, verify edit. +- Save As to a new UFO and reopen it. +- Export a simple TTF/OTF and install or inspect it externally. +- Try saving a non-writable source format and verify Shift forces Save As. + +### 0.4.0-alpha — Glyph Workflow Alpha + +Promise: a tester can work across multiple glyphs without fighting navigation or glyph metadata. + +Scope: + +- Recent files are functional. +- Glyph grid supports the common navigation path. +- Glyph add, duplicate, delete, and rename basics. +- Unicode/name editing basics. +- Open Recent and core File/Glyph menu items. +- Keyboard navigation in the grid. +- Basic validation for empty/missing/problem glyphs if cheap. + +Acceptance tests: + +- Open a font, find a glyph by name/unicode/character, edit it, move to another glyph, return to the first. +- Add or duplicate a glyph and save/reopen. +- Rename or edit unicode metadata and verify the result survives save/reopen where supported. + +### 0.5.0-alpha — Drawing Workflow Depth + +Promise: contour editing feels useful beyond simple point movement. + +Scope: + +- Boolean operations are stabilized in the UI. +- Remove overlap or path direction cleanup, whichever is more valuable first. +- Shape tools for rectangle/ellipse if they support real glyph work. +- Better point/segment indicators: extrema, open endpoints, smooth tangent lines, or equivalent. +- Measurement/guides if they unblock precision work. +- Zoom to selection / center glyph. + +Acceptance tests: + +- Draw overlapping contours, run boolean/remove-overlap workflow, save/reopen. +- Build a simple glyph from shapes and manual point edits. +- Use precision aids to align or measure a contour without guessing. + +### 0.6.0-alpha — Components And Accents Alpha + +Promise: Shift can represent and edit composite glyph workflows at an alpha level. + +Scope: + +- Component data model and snapshots. +- Add component to glyph. +- Move/transform component. +- Render component bounds/ghosting. +- Decompose component. +- Basic anchors. +- Simple accented glyph generation path. + +Acceptance tests: + +- Build an accented glyph from a base and mark component. +- Move/transform a component and save/reopen. +- Decompose a component and continue editing outlines. + +### 0.7.0-alpha — Variable Font Alpha + +Promise: Shift can inspect and test variable font/designspace workflows, even if editing is incomplete. + +Scope: + +- Designspace loading is user-facing. +- Master switching. +- Add/remove or copy master workflow if feasible. +- Interpolation preview is reliable enough for tester feedback. +- Named instances. +- Instance export for simple cases. +- Compatibility errors are understandable. + +Acceptance tests: + +- Open designspace, switch masters, preview interpolation. +- Detect incompatible glyphs and show a useful message. +- Export or generate a simple instance. + +### 0.8.0-alpha — Spacing And Proofing Alpha + +Promise: a tester can evaluate glyphs in text context. + +Scope: + +- Sidebearing handles or numeric sidebearing editing. +- Spacing strings and presets. +- Preview/proofing panel. +- Waterfall view. +- Basic kerning preview or editing if ready. +- HarfBuzz shaping preview if the plumbing is ready. + +Acceptance tests: + +- Edit spacing for a glyph while viewing it in context. +- Save/reopen spacing changes. +- Preview a sample string at multiple sizes. + +### 0.9.0-beta.1 — Beta Candidate + +Promise: a type designer can complete a small real task end-to-end, and the beta line is primarily about fixing bugs. + +Scope: + +- Feature freeze for the 1.0 core workflow. +- Packaging works on the supported platforms. +- macOS signing/notarization is mandatory. +- Windows/Linux packaging status is clearly documented. +- Documentation for install, open, edit, save, export, and known limitations. +- Crash/diagnostic story or at least useful error reporting. +- File-format/data-loss risks have explicit tests. + +Acceptance tests: + +- Complete a small real project from install to exported font. +- Verify clean install on each supported platform. +- Verify release notes, changelog, and website match the actual release state. + +### 1.0.0 — Stable + +Promise: Shift is a production-quality font editor for the documented core workflow. + +Scope: + +- Main workflow is dependable: install, open/create, edit, save/reopen, export. +- Documentation is sufficient for non-contributors. +- Compatibility and file-format expectations are explicit. +- Update path exists or the absence of auto-update is intentional and documented. +- Known critical data-loss issues are fixed. + +## Priority Rules + +Use these rules when deciding what to work on next: + +1. If the current milestone has a broken acceptance test, fix that before adding unrelated feature surface. +2. Prefer work that completes an end-to-end workflow over work that adds isolated capability. +3. Keep experimental work behind the current release promise unless it directly reduces release risk. +4. Patch releases fix regressions only; minor releases add a new workflow promise. +5. Beta means feature freeze for the beta line, not a larger feature bucket. + +## Flexible Exploration Backlog + +These are allowed to jump around when energy is high, but they should not silently become release blockers: + +- Components and accents. +- Variable fonts. +- Spacing and kerning. +- OpenType feature editing. +- Scripting. +- AI/MCP integration. +- Collaboration. +- Advanced rendering/performance work. ## Current Implementation Status @@ -687,54 +905,67 @@ interface ShiftScriptContext { ## 📊 Version Milestones -### v0.1 - Alpha (MVP) +The authoritative milestone plan is the release roadmap at the top of this file. This section is a compact index. + +### v0.2-alpha — First Installable Editing Alpha + +- Release infrastructure and installable binaries. +- Existing core vector editing exposed to testers. +- Clear alpha limitations. + +### v0.3-alpha — Persistence And Export Alpha + +- Save/reopen loop. +- Save As and dirty state. +- Basic TTF/OTF export. +- Round-trip tests. + +### v0.4-alpha — Glyph Workflow Alpha -- Basic bezier drawing ✓ -- Point selection and manipulation ✓ -- Undo/redo ✓ -- Delete points ✓ -- Open/save UFO files -- Export to TTF +- Recent files. +- Better glyph grid navigation. +- Glyph add/duplicate/delete/rename. +- Unicode/name editing basics. -### v0.2 - Beta +### v0.5-alpha — Drawing Workflow Depth -- Full editing toolkit (copy/paste ✓, snapping) -- Segment hover/highlighting -- Grid panel improvements (thumbnails, search) -- Glyph add/delete/rename +- Stabilized boolean/path operations. +- Shape and precision workflow improvements. +- Better contour indicators and validation basics. -### v0.3 - Components +### v0.6-alpha — Components And Accents Alpha -- Component system -- Anchors -- Accented glyph generation +- Component data model. +- Component editing/rendering/decomposition. +- Anchors. +- Accented glyph generation basics. -### v0.4 - Variable Fonts +### v0.7-alpha — Variable Font Alpha -- Designspace support -- Master switching -- Interpolation preview +- User-facing designspace workflow. +- Master switching. +- Interpolation preview. +- Named instances and simple instance export. -### v0.5 - Professional +### v0.8-alpha — Spacing And Proofing Alpha -- Full menu system -- Preferences -- Incremental compilation -- HarfBuzz preview +- Sidebearing editing. +- Spacing strings. +- Preview/proofing panel. +- Waterfall view. -### v1.0 - Stable Release +### v0.9-beta — Beta Candidate -- Production ready -- Full documentation -- Signed distributions -- Auto-updates -- Scripting v1 +- Feature freeze for the 1.0 core workflow. +- Cross-platform packaging. +- Documentation. +- File-format and data-loss hardening. -### Post-1.0 +### v1.0 — Stable Release -- Collaboration features -- AI integration -- Plugin ecosystem +- Production-quality documented core workflow. +- Signed distributions. +- Clear compatibility and update posture. --- diff --git a/apps/desktop/.oxlintrc.json b/apps/desktop/.oxlintrc.json index e312cbbf..71f3be5c 100644 --- a/apps/desktop/.oxlintrc.json +++ b/apps/desktop/.oxlintrc.json @@ -33,10 +33,10 @@ "message": "Do not use export *. Use named exports to keep the public API explicit." } ], - "shift/no-raw-contour-access": "error", "shift/prefer-instance-method-on-glyph": "error", "shift/no-raw-segment-parse": "error", "shift/no-get-signal-value-method": "error", + "shift/no-reactive-value-outside-boundary": "error", "shift/no-unbranded-ids": "error", "shift/no-snapshot-in-domain": "error", "shift/no-raw-point-type-check": "error", diff --git a/apps/desktop/e2e/perf.spec.ts b/apps/desktop/e2e/perf.spec.ts index 049c22b1..4c4eeeb0 100644 --- a/apps/desktop/e2e/perf.spec.ts +++ b/apps/desktop/e2e/perf.spec.ts @@ -157,9 +157,7 @@ test.describe("Performance — 50K points", () => { if (!result.success) throw new Error("pasteContours failed"); - return editor.bridge - .getEditingSnapshot() - ?.contours.reduce((sum: number, c: any) => sum + c.points.length, 0); + return editor.editGlyphSource?.allPoints.length ?? 0; }, contours); expect(pointCount).toBeGreaterThanOrEqual(TARGET_POINTS); @@ -174,25 +172,25 @@ test.describe("Performance — 50K points", () => { const editor = (window as any).__shift.getEditor(); editor.bridge.pasteContours(contours, 0, 0); - const snapshot = editor.bridge.getEditingSnapshot(); - const pointIds = snapshot.contours[0].points.slice(0, 5).map((p: any) => p.id); + const pointIds = editor.editGlyphSource.allPoints.slice(0, 5).map((p: any) => p.id); editor.selection.select(pointIds.map((id: string) => ({ kind: "point", id }))); - const draft = editor.createDraft(); + const draft = editor.beginSourceEditDraft({ points: pointIds }); const times: number[] = []; for (let i = 0; i < frames; i++) { const start = performance.now(); const updates = pointIds.map((id: string, idx: number) => ({ - node: { kind: "point" as const, id }, + kind: "point" as const, + id, x: 100 + i + idx, y: 200 + i + idx, })); - draft.setPositions(updates); + draft.previewPositionPatch(updates); times.push(performance.now() - start); } - draft.finish("translate-few"); + draft.commit("translate-few"); return times; }, { contours, frames: DRAG_FRAMES }, @@ -211,26 +209,26 @@ test.describe("Performance — 50K points", () => { const editor = (window as any).__shift.getEditor(); editor.bridge.pasteContours(contours, 0, 0); - const snapshot = editor.bridge.getEditingSnapshot(); - const allPoints = snapshot.contours.flatMap((c: any) => c.points); + const allPoints = editor.editGlyphSource.allPoints; const pointIds = allPoints.slice(0, 1000).map((p: any) => p.id); editor.selection.select(pointIds.map((id: string) => ({ kind: "point", id }))); - const draft = editor.createDraft(); + const draft = editor.beginSourceEditDraft({ points: pointIds }); const times: number[] = []; for (let i = 0; i < frames; i++) { const start = performance.now(); const updates = pointIds.map((id: string, idx: number) => ({ - node: { kind: "point" as const, id }, + kind: "point" as const, + id, x: idx + i, y: idx + i, })); - draft.setPositions(updates); + draft.previewPositionPatch(updates); times.push(performance.now() - start); } - draft.finish("translate-many"); + draft.commit("translate-many"); return times; }, { contours, frames: DRAG_FRAMES }, @@ -251,25 +249,25 @@ test.describe("Performance — 50K points", () => { editor.selectAll(); - const snapshot = editor.bridge.getEditingSnapshot(); - const allPoints = snapshot.contours.flatMap((c: any) => c.points); + const allPoints = editor.editGlyphSource.allPoints; const pointIds = allPoints.map((p: any) => p.id); - const draft = editor.createDraft(); + const draft = editor.beginSourceEditDraft({ points: pointIds }); const times: number[] = []; for (let i = 0; i < frames; i++) { const start = performance.now(); const updates = pointIds.map((id: string, idx: number) => ({ - node: { kind: "point" as const, id }, + kind: "point" as const, + id, x: idx + i, y: idx + i, })); - draft.setPositions(updates); + draft.previewPositionPatch(updates); times.push(performance.now() - start); } - draft.finish("translate-all"); + draft.commit("translate-all"); return times; }, { contours, frames: DRAG_FRAMES }, @@ -289,8 +287,7 @@ test.describe("Performance — 50K points", () => { editor.bridge.pasteContours(contours, 0, 0); editor.selectAll(); - const snapshot = editor.bridge.getEditingSnapshot(); - const allPoints = snapshot.contours.flatMap((c: any) => c.points); + const allPoints = editor.editGlyphSource.allPoints; const pointIds = allPoints.map((p: any) => p.id); const times: number[] = []; @@ -320,18 +317,18 @@ test.describe("Performance — 50K points", () => { editor.bridge.pasteContours(contours, 0, 0); editor.selectAll(); - const snapshot = editor.bridge.getEditingSnapshot(); - const allPoints = snapshot.contours.flatMap((c: any) => c.points); + const allPoints = editor.editGlyphSource.allPoints; const pointIds = allPoints.map((p: any) => p.id); - const draft = editor.createDraft(); + const draft = editor.beginSourceEditDraft({ points: pointIds }); const updates = pointIds.map((id: string, idx: number) => ({ - node: { kind: "point" as const, id }, + kind: "point" as const, + id, x: idx + 10, y: idx + 10, })); - draft.setPositions(updates); - draft.finish("pre-undo-translate"); + draft.previewPositionPatch(updates); + draft.commit("pre-undo-translate"); const undoTimes: number[] = []; const redoTimes: number[] = []; diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 0a0eb627..9a22ac1b 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -52,7 +52,8 @@ }, "dependencies": { "@base-ui-components/react": "1.0.0-rc.0", - "@shift/font": "workspace:*", + "@shift/bridge": "workspace:*", + "@shift/glyph-state": "workspace:*", "@shift/geo": "workspace:*", "@shift/glyph-info": "workspace:*", "@shift/rules": "workspace:*", @@ -70,7 +71,7 @@ "react-dom": "^19.1.0", "react-router-dom": "^7.12.0", "regl": "^2.1.1", - "shift-node": "workspace:*", + "shift-bridge": "workspace:*", "tailwind-merge": "^3.3.1", "throttle-debounce": "^5.0.2", "zustand": "^5.0.6" diff --git a/apps/desktop/src/main/managers/WindowManager.ts b/apps/desktop/src/main/managers/WindowManager.ts index 2244c859..acee5721 100644 --- a/apps/desktop/src/main/managers/WindowManager.ts +++ b/apps/desktop/src/main/managers/WindowManager.ts @@ -27,6 +27,7 @@ export class WindowManager { this.window = new BrowserWindow({ width: 800, height: 600, + minWidth: 1200, title: "Shift", titleBarStyle: "hidden", trafficLightPosition: { x: -100, y: -100 }, diff --git a/apps/desktop/src/preload/docs/DOCS.md b/apps/desktop/src/preload/docs/DOCS.md index f91bf794..34fd3a58 100644 --- a/apps/desktop/src/preload/docs/DOCS.md +++ b/apps/desktop/src/preload/docs/DOCS.md @@ -1,86 +1,53 @@ # Preload -Electron preload script that bridges the native Rust `FontEngine` and typed IPC channels to the renderer via `contextBridge`. +Electron preload script that exposes the native Rust bridge and typed IPC channels to the renderer through `contextBridge`. ## Architecture Invariants -- **Architecture Invariant:** `sandbox: false` is required in `WindowManager.create` `webPreferences` because the preload uses `require("shift-node")` to load the native NAPI addon. **CRITICAL**: enabling the sandbox silently breaks font engine access with no error at bridge creation time -- it only fails when the renderer calls a method. - -- **Architecture Invariant:** `buildBridgeAPI` dynamically wraps every prototype method of the `FontEngine` instance. This means the exposed API surface automatically tracks whatever `#[napi]` methods exist on the Rust side -- no manual method listing required. **CRITICAL**: if a native method is removed from Rust, the preload will still expose a wrapper that throws at call time, not at startup. - -- **Architecture Invariant:** Two separate `contextBridge.exposeInMainWorld` calls create two non-overlapping namespaces: `window.shiftFont` (native font engine) and `window.electronAPI` (IPC + system). These must stay separate because `shiftFont` is synchronous NAPI calls while `electronAPI` is async IPC. - -- **Architecture Invariant:** The `electronAPI` object must satisfy the `ElectronAPI` interface exactly. TypeScript enforces this at compile time. Adding a new IPC channel requires updating `IpcEvents` or `IpcCommands` first, then wiring it here. +- **Architecture Invariant:** The native bridge is created through `@shift/bridge`, not by importing the raw `shift-bridge` NAPI package here. **WHY:** native loading and native-module typing stay in one package boundary. +- **Architecture Invariant:** `buildContextBridgeApi` flattens prototype methods into a plain object before exposing them. **WHY:** `contextBridge` does not preserve class prototype semantics across the isolated context boundary. +- **Architecture Invariant:** Two separate globals are exposed: `window.shiftBridge` for Rust bridge calls and `window.electronAPI` for IPC/system access. **WHY:** native bridge calls and Electron IPC have different lifecycles and failure modes. +- **Architecture Invariant:** The `electronAPI` object must satisfy the `ElectronAPI` interface exactly. **WHY:** adding IPC channels should fail at typecheck time unless preload wiring is updated. ## Codemap ``` preload/ - preload.ts -- single entry point; creates FontEngine instance, - builds bridge API, wires IPC, exposes both namespaces + preload.ts -- creates BridgeApi, flattens it for contextBridge, wires IPC globals ``` ## Key Types -- `FontEngineAPI` -- derived as `Omit`, defined in the bridge module. Single source of truth for the native API surface. -- `ElectronAPI` -- typed interface for all IPC commands, event listeners, system utilities, and clipboard access. Defined in the ipc module. -- `IpcEvents` -- main-to-renderer broadcast channel map (menu actions, theme, debug). -- `IpcCommands` -- renderer-to-main request/response channel map (dialogs, window control, document state). - -## How it works - -The preload runs once before the renderer loads. It does three things: - -1. **Native font engine bridge.** Creates a single `FontEngine` instance from `shift-node`. `buildBridgeAPI` walks the prototype, wrapping each method in a forwarding closure so `contextBridge` can serialize/deserialize arguments correctly. The result is exposed as `window.shiftFont`. - -2. **Typed IPC bridge.** Uses the `listener` and `command` helpers from the ipc module to create typed wrappers around `ipcRenderer.on` and `ipcRenderer.invoke`. Each IPC channel is wired by name to a property on the `electronAPI` object, then exposed as `window.electronAPI`. +- `BridgeApi` -- native bridge API generated from Rust declarations and exposed by `@shift/bridge`. +- `ElectronAPI` -- typed interface for IPC commands, event listeners, system utilities, and clipboard access. +- `IpcEvents` -- main-to-renderer broadcast channel map. +- `IpcCommands` -- renderer-to-main request/response channel map. -3. **Direct system access.** `homePath` is captured once from `os.homedir()`. Clipboard read/write goes through Electron's `clipboard` module directly (no IPC round-trip). +## How It Works -The renderer accesses the font engine through `getNative()` in the bridge module, which caches `window.shiftFont`. All mutation calls go through `NativeBridge`, which wraps `getNative()` with session management, snapshot parsing, and reactive state. +The preload runs once before the renderer loads: -## Workflow recipes - -### Add a new IPC command (renderer-to-main) - -1. Add the channel signature to `IpcCommands` in the ipc channels module. -2. Add the corresponding property to `ElectronAPI` using the `CommandInvoker` type. -3. Add `yourCommand: invoke("your:channel")` to the `electronAPI` object in `preload.ts`. -4. Handle the channel in the main process with `ipcMain.handle`. - -### Add a new IPC event (main-to-renderer) - -1. Add the channel signature to `IpcEvents` in the ipc channels module. -2. Add the corresponding property to `ElectronAPI` using the `EventListener` type. -3. Add `onYourEvent: on("your:channel")` to the `electronAPI` object in `preload.ts`. -4. Send from main process with `webContents.send`. - -### New native FontEngine method appears after Rust rebuild - -Nothing to do in the preload. `buildBridgeAPI` auto-discovers prototype methods. The `FontEngineAPI` type updates automatically since it derives from the napi-generated `FontEngine` class. +1. Calls `createBridge()` from `@shift/bridge`. +2. Converts the bridge class instance into a plain method object with `buildContextBridgeApi`. +3. Exposes that object as `window.shiftBridge`. +4. Builds typed IPC helpers and exposes them as `window.electronAPI`. ## Gotchas -- `buildBridgeAPI` only wraps own prototype methods (skips `constructor` and non-function properties). If a native method is defined as an own property rather than on the prototype, it will not be exposed. -- `contextBridge` serialization means you cannot pass functions, Promises, or class instances through `window.shiftFont` -- only plain data. The native methods already return strings/numbers/booleans so this works, but keep it in mind if extending. -- The `listener` helper returns an unsubscribe function. If the renderer does not call it, the listener leaks. This is the renderer's responsibility, not the preload's. +- `buildContextBridgeApi` only wraps prototype methods. If a native method is added as an own property, it will not be exposed. +- `contextBridge` values must be plain data/functions. Do not expose the native class instance directly. +- `window.shiftBridge` is the raw bridge boundary. Editor/reactive behavior belongs in renderer-side model code, not preload. ## Verification ```bash -# Type-check (catches mismatches between ElectronAPI interface and preload wiring) pnpm --filter @shift/desktop typecheck - -# Lint pnpm --filter @shift/desktop lint ``` ## Related -- `FontEngineAPI` (bridge module) -- type definition for the native API surface -- `ElectronAPI` (ipc module) -- type definition for the IPC/system API surface -- `IpcEvents`, `IpcCommands` (ipc channels module) -- channel maps -- `listener`, `command` (ipc preload module) -- typed IPC wrapper factories -- `NativeBridge` (renderer bridge) -- renderer-side wrapper that consumes `window.shiftFont` -- `getNative` (renderer bridge/native) -- cached accessor for `window.shiftFont` -- `WindowManager` (main module) -- loads the preload script via `webPreferences.preload` +- `@shift/bridge` -- runtime native bridge loader and bridge type exports. +- `@shift/types` -- generated bridge DTO/API facade plus shared primitive DTO types. +- `ElectronAPI` -- IPC/system API surface exposed as `window.electronAPI`. +- `WindowManager` -- loads this preload script through `webPreferences.preload`. diff --git a/apps/desktop/src/preload/preload.ts b/apps/desktop/src/preload/preload.ts index 7f991668..f55abdde 100644 --- a/apps/desktop/src/preload/preload.ts +++ b/apps/desktop/src/preload/preload.ts @@ -2,29 +2,29 @@ // https://www.electronjs.org/docs/latest/tutorial/process-model#preload-scripts const { contextBridge, ipcRenderer, clipboard } = require("electron"); -const { FontEngine } = require("shift-node"); const os = require("os"); -import type { FontEngineAPI } from "../shared/bridge/FontEngineAPI"; +import { createBridge, type BridgeApi } from "@shift/bridge"; import type { IpcEvents, IpcCommands } from "../shared/ipc/channels"; import type { ElectronAPI } from "../shared/ipc/electronAPI"; import { listener, command } from "../shared/ipc/preload"; -const fontEngineInstance = new FontEngine(); +const bridge = createBridge(); -function buildBridgeAPI(instance: Record): T { +function buildContextBridgeApi(instance: T): T { const api: Record = {}; + const target = instance as Record; const proto = Object.getPrototypeOf(instance); for (const name of Object.getOwnPropertyNames(proto)) { - if (name === "constructor" || typeof instance[name] !== "function") continue; - api[name] = (...args: unknown[]) => (instance[name] as Function)(...args); + if (name === "constructor" || typeof target[name] !== "function") continue; + api[name] = (...args: unknown[]) => (target[name] as Function)(...args); } - return api as unknown as T; + return api as T; } -const fontEngineAPI = buildBridgeAPI(fontEngineInstance); +const bridgeApi = buildContextBridgeApi(bridge); // Expose to renderer via contextBridge -contextBridge.exposeInMainWorld("shiftFont", fontEngineAPI); +contextBridge.exposeInMainWorld("shiftBridge", bridgeApi); const on = (ch: K) => listener(ipcRenderer, ch); const invoke = (ch: K) => command(ipcRenderer, ch); diff --git a/apps/desktop/src/renderer/index.css b/apps/desktop/src/renderer/index.css index e4f6a7fb..0f2da78a 100644 --- a/apps/desktop/src/renderer/index.css +++ b/apps/desktop/src/renderer/index.css @@ -61,6 +61,15 @@ --text-xs: 0.625rem /* 10px */; } +/* Hide scrollbar while preserving scroll behavior */ +@utility scrollbar-hidden { + scrollbar-width: none; + -ms-overflow-style: none; + &::-webkit-scrollbar { + display: none; + } +} + /* Custom titlebar styles */ .titlebar-drag { -webkit-app-region: drag; diff --git a/apps/desktop/src/renderer/src/app/App.tsx b/apps/desktop/src/renderer/src/app/App.tsx index c707e404..de4bd1aa 100644 --- a/apps/desktop/src/renderer/src/app/App.tsx +++ b/apps/desktop/src/renderer/src/app/App.tsx @@ -8,7 +8,7 @@ import { DebugProvider } from "@/context/DebugContext"; import { ZoomToast } from "@/components/chrome/ZoomToast"; import { isDev } from "@/lib/utils/utils"; import { dumpSelectionPatternsToConsole } from "@/lib/debug/dumpSelectionPatterns"; -import { clearDirty, getEditor, setFilePath } from "@/store/store"; +import { getDocument } from "@/store/store"; import { documentPersistence } from "@/persistence"; import { RouteDispatcher } from "./RouteDispatcher"; @@ -26,6 +26,10 @@ function isLandingHash(hash: string): boolean { return hash === "" || hash === "#" || hash === "#/"; } +function needsLoadedDocument(hash: string): boolean { + return !isLandingHash(hash); +} + function parseEditorUnicodeFromHash(hash: string): number | null { const match = hash.match(EDITOR_HASH_RE); if (!match) return null; @@ -35,7 +39,8 @@ function parseEditorUnicodeFromHash(hash: string): number | null { export const App = () => { useEffect(() => { - const editor = getEditor(); + const fontDocument = getDocument(); + const editor = fontDocument.editor; documentPersistence.init(editor); let didOpenFont = false; @@ -50,19 +55,14 @@ export const App = () => { } try { - editor.loadFont(filePath); - editor.updateMetricsFromFont(); - setFilePath(filePath); - clearDirty(); - documentPersistence.openDocument(filePath); + fontDocument.openFont(filePath); didOpenFont = true; + if (source === "restore") { const unicode = parseEditorUnicodeFromHash(window.location.hash); if (unicode !== null) { - const glyphName = editor.font.glyphName(unicode); - editor.setMainGlyphUnicode(unicode); - editor.open(glyphName); - editor.setDrawOffsetForGlyph({ x: 0, y: 0 }, glyphName, unicode); + const handle = editor.font.glyphHandleForUnicode(unicode); + editor.getGlyph(handle); } } else { navigateToHome(); @@ -83,16 +83,20 @@ export const App = () => { const mostRecentDocId = state.registry.lruDocIds[0]; const mostRecentPath = mostRecentDocId ? state.registry.docIdToPath[mostRecentDocId] : null; if (!mostRecentPath) { + fontDocument.createFont(); return; } const exists = await window.electronAPI?.pathsExist([mostRecentPath]); if (!exists?.[0]) { + fontDocument.createFont(); return; } handleOpenFont(mostRecentPath, "restore"); })(); + } else if (needsLoadedDocument(window.location.hash) && !fontDocument.loaded) { + fontDocument.createFont(); } const unsubscribeOpen = window.electronAPI?.onMenuOpenFont(handleOpenFont); @@ -100,12 +104,7 @@ export const App = () => { const unsubscribeSave = window.electronAPI?.onMenuSaveFont(async (savePath) => { try { - await editor.saveFont(savePath); - setFilePath(savePath); - clearDirty(); - documentPersistence.onDocumentPathChanged(savePath); - documentPersistence.flushNow(); - await window.electronAPI?.saveCompleted(savePath); + await fontDocument.saveFont(savePath); } catch (error) { console.error("Failed to save font:", error); } diff --git a/apps/desktop/src/renderer/src/app/Document.test.ts b/apps/desktop/src/renderer/src/app/Document.test.ts new file mode 100644 index 00000000..35f9020d --- /dev/null +++ b/apps/desktop/src/renderer/src/app/Document.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it } from "vitest"; +import { TestEditor } from "@/testing/TestEditor"; +import { MUTATORSANS_DESIGNSPACE } from "@/testing/fixtures"; +import { Document, type DocumentPersistence } from "./Document"; + +class InMemoryDocumentPersistence implements DocumentPersistence { + currentDocId: string | null = null; + currentPath: string | null = null; + + closeDocument(): void { + this.currentDocId = null; + this.currentPath = null; + } + + openDocument(filePath: string): void { + this.currentPath = filePath; + this.currentDocId = `file:${filePath}`; + } + + openUntitledDocument(docId: string): void { + this.currentDocId = docId; + this.currentPath = null; + } + + onDocumentPathChanged(filePath: string | null): void { + this.currentPath = filePath; + if (filePath) this.currentDocId ??= `file:${filePath}`; + } + + flushNow(): void {} +} + +function testDocument() { + const editor = new TestEditor(); + const persistence = new InMemoryDocumentPersistence(); + let filePath: string | null = "stale.ufo"; + let dirty = true; + const document = new Document(editor, { + persistence, + createUntitledId: () => "untitled-1", + setFilePath: (nextPath) => { + filePath = nextPath; + }, + clearDirty: () => { + dirty = false; + }, + }); + + return { + document, + editor, + persistence, + get filePath() { + return filePath; + }, + get dirty() { + return dirty; + }, + }; +} + +describe("Document", () => { + it("creates a loaded untitled font document with a default source", () => { + const state = testDocument(); + + state.document.createFont(); + + expect(state.document.identity).toEqual({ + kind: "untitled", + id: "untitled-1", + }); + expect(state.editor.font.loaded).toBe(true); + expect(state.editor.font.defaultSource.name).toBe("Regular"); + expect(state.persistence.currentDocId).toBe("untitled-1"); + expect(state.filePath).toBeNull(); + expect(state.dirty).toBe(false); + }); + + it("opens a file-backed font document", () => { + const state = testDocument(); + + state.document.openFont(MUTATORSANS_DESIGNSPACE); + + expect(state.document.identity).toEqual({ + kind: "file", + path: MUTATORSANS_DESIGNSPACE, + }); + expect(state.editor.font.loaded).toBe(true); + expect(state.editor.font.glyphHandleForName("A")).toEqual({ + name: "A", + unicode: 65, + }); + expect(state.persistence.currentPath).toBe(MUTATORSANS_DESIGNSPACE); + expect(state.filePath).toBe(MUTATORSANS_DESIGNSPACE); + expect(state.dirty).toBe(false); + }); + + it("closes the current document", () => { + const state = testDocument(); + state.document.createFont(); + + state.document.close(); + + expect(state.document.identity).toBeNull(); + expect(state.editor.font.loaded).toBe(false); + expect(state.editor.font.sources).toEqual([]); + expect(state.persistence.currentDocId).toBeNull(); + expect(state.filePath).toBeNull(); + }); + + it("requires a save path for untitled documents", async () => { + const state = testDocument(); + state.document.createFont(); + + await expect(state.document.saveFont()).rejects.toThrow( + "Cannot save an untitled document without a file path", + ); + }); +}); diff --git a/apps/desktop/src/renderer/src/app/Document.ts b/apps/desktop/src/renderer/src/app/Document.ts new file mode 100644 index 00000000..b5f23147 --- /dev/null +++ b/apps/desktop/src/renderer/src/app/Document.ts @@ -0,0 +1,113 @@ +import type { Editor } from "@/lib/editor/Editor"; + +export type DocumentIdentity = + | { readonly kind: "untitled"; readonly id: string } + | { readonly kind: "file"; readonly path: string }; + +export interface DocumentServices { + readonly persistence: DocumentPersistence; + readonly setFilePath: (filePath: string | null) => void; + readonly clearDirty: () => void; + readonly createUntitledId?: () => string; + readonly notifySaveCompleted?: (path: string) => Promise | void; +} + +export interface DocumentPersistence { + closeDocument(): void; + openDocument(filePath: string): void; + openUntitledDocument(docId: string): void; + onDocumentPathChanged(filePath: string | null): void; + flushNow(): void; +} + +/** + * App-level lifecycle for the current font document. + * + * `Document` owns the distinction between no document, a new untitled font, + * and a file-backed font. It coordinates editor font lifecycle, file identity, + * dirty state, and document-scoped persistence. + */ +export class Document { + readonly editor: Editor; + + readonly #persistence: DocumentPersistence; + readonly #setFilePath: (filePath: string | null) => void; + readonly #clearDirty: () => void; + readonly #createUntitledId: () => string; + readonly #notifySaveCompleted: (path: string) => Promise | void; + + #identity: DocumentIdentity | null = null; + + constructor(editor: Editor, services: DocumentServices) { + this.editor = editor; + this.#persistence = services.persistence; + this.#setFilePath = services.setFilePath; + this.#clearDirty = services.clearDirty; + this.#createUntitledId = services.createUntitledId ?? createUntitledId; + this.#notifySaveCompleted = services.notifySaveCompleted ?? (() => undefined); + } + + get identity(): DocumentIdentity | null { + return this.#identity; + } + + get loaded(): boolean { + return this.editor.font.loaded; + } + + createFont(): void { + this.#persistence.closeDocument(); + + const id = this.#createUntitledId(); + this.editor.createFont(); + this.#identity = { kind: "untitled", id }; + + this.#setFilePath(null); + this.#clearDirty(); + + this.#persistence.openUntitledDocument(id); + this.#persistence.flushNow(); + } + + openFont(path: string): void { + this.#persistence.closeDocument(); + + this.editor.loadFont(path); + this.#identity = { kind: "file", path }; + + this.#setFilePath(path); + this.#clearDirty(); + + this.#persistence.openDocument(path); + this.#persistence.flushNow(); + } + + async saveFont(path?: string): Promise { + const savePath = path ?? (this.#identity?.kind === "file" ? this.#identity.path : null); + if (!savePath) { + throw new Error("Cannot save an untitled document without a file path"); + } + + await this.editor.saveFont(savePath); + this.#identity = { kind: "file", path: savePath }; + + this.#setFilePath(savePath); + this.#clearDirty(); + + this.#persistence.onDocumentPathChanged(savePath); + this.#persistence.flushNow(); + await this.#notifySaveCompleted(savePath); + } + + close(): void { + this.#persistence.closeDocument(); + this.editor.closeFont(); + this.#identity = null; + this.#setFilePath(null); + this.#clearDirty(); + } +} + +function createUntitledId(): string { + return globalThis.crypto?.randomUUID?.() ?? `untitled-${Date.now().toString(36)}`; +} diff --git a/apps/desktop/src/renderer/src/app/WorkspaceLayout.tsx b/apps/desktop/src/renderer/src/app/WorkspaceLayout.tsx index 8221159e..d9a0dc79 100644 --- a/apps/desktop/src/renderer/src/app/WorkspaceLayout.tsx +++ b/apps/desktop/src/renderer/src/app/WorkspaceLayout.tsx @@ -1,40 +1,15 @@ -import { useLocation } from "react-router-dom"; +import { Navigate, Route, Routes } from "react-router-dom"; import { Home } from "@/views/Home"; import { Editor } from "@/views/Editor"; -import { FontInfo } from "@/views/FontInfo"; - -const EDITOR_PATH = /^\/editor\/([^/]+)$/; -const FONT_INFO_PATH = "/info"; export const WorkspaceLayout = () => { - const { pathname } = useLocation(); - const editorMatch = pathname.match(EDITOR_PATH); - const glyphId = editorMatch?.[1]; - const isEditor = !!glyphId; - const isFontInfo = pathname === FONT_INFO_PATH; - const showHome = !isEditor && !isFontInfo; - return (
-
- -
-
- -
- {isEditor && ( -
- -
- )} + + } /> + } /> + } /> +
); }; diff --git a/apps/desktop/src/renderer/src/app/routes.ts b/apps/desktop/src/renderer/src/app/routes.ts index 1743ec44..5c8365b3 100644 --- a/apps/desktop/src/renderer/src/app/routes.ts +++ b/apps/desktop/src/renderer/src/app/routes.ts @@ -2,23 +2,27 @@ import GridSvg from "@assets/toolbar/grid.svg"; import InfoSvg from "@assets/toolbar/info.svg"; import type { SVG } from "@/types/common"; -export interface NavRoute { +type NavItemBase = { id: string; - path: string; description: string; icon?: SVG; -} +}; + +export type NavRoute = NavItemBase & + ({ kind: "route"; path: string } | { kind: "dialog"; dialogId: "font-info" }); export const routes: NavRoute[] = [ { id: "home", + kind: "route", path: "/home", icon: GridSvg, description: "Display all glyphs", }, { id: "info", - path: "/info", + kind: "dialog", + dialogId: "font-info", icon: InfoSvg, description: "Display and edit font information, such as family name, weight, style, etc.", }, diff --git a/apps/desktop/src/renderer/src/assets/sidebar-left/sidebar-left.svg b/apps/desktop/src/renderer/src/assets/sidebar-left/sidebar-left.svg new file mode 100644 index 00000000..d729a1d3 --- /dev/null +++ b/apps/desktop/src/renderer/src/assets/sidebar-left/sidebar-left.svg @@ -0,0 +1,4 @@ + + + + diff --git a/apps/desktop/src/renderer/src/bridge/NativeBridge.test.ts b/apps/desktop/src/renderer/src/bridge/NativeBridge.test.ts deleted file mode 100644 index c5ecee6a..00000000 --- a/apps/desktop/src/renderer/src/bridge/NativeBridge.test.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { describe, it, expect, beforeEach } from "vitest"; -import { NativeBridge } from "./NativeBridge"; -import { createBridge } from "@/testing/engine"; - -describe("NativeBridge session lifecycle", () => { - let bridge: NativeBridge; - - beforeEach(() => { - bridge = createBridge(); - }); - - it("has no session and a null $glyph before any start", () => { - expect(bridge.hasSession()).toBe(false); - expect(bridge.$glyph.peek()).toBe(null); - }); - - it("startEditSession opens a session and populates $glyph", () => { - bridge.startEditSession("A"); - - expect(bridge.hasSession()).toBe(true); - expect(bridge.$glyph.peek()).not.toBe(null); - expect(bridge.getEditingGlyphName()).toBe("A"); - }); - - it("endEditSession clears the session and nulls $glyph", () => { - bridge.startEditSession("A"); - bridge.endEditSession(); - - expect(bridge.hasSession()).toBe(false); - expect(bridge.$glyph.peek()).toBe(null); - }); - - it("starting the same glyph again is a no-op — $glyph reference is preserved", () => { - bridge.startEditSession("A"); - const first = bridge.$glyph.peek(); - - bridge.startEditSession("A"); - const second = bridge.$glyph.peek(); - - expect(second).toBe(first); - }); - - it("switching to a different glyph replaces the Glyph instance", () => { - bridge.startEditSession("A"); - const first = bridge.$glyph.peek(); - - bridge.startEditSession("B"); - const second = bridge.$glyph.peek(); - - expect(bridge.getEditingGlyphName()).toBe("B"); - expect(second).not.toBe(first); - }); -}); diff --git a/apps/desktop/src/renderer/src/bridge/NativeBridge.ts b/apps/desktop/src/renderer/src/bridge/NativeBridge.ts deleted file mode 100644 index 822f5e3f..00000000 --- a/apps/desktop/src/renderer/src/bridge/NativeBridge.ts +++ /dev/null @@ -1,498 +0,0 @@ -import type { - GlyphSnapshot, - FontMetadata, - FontMetrics, - PointId, - ContourId, - Point2D, - AnchorId, - Axis, - AxisLocation, - Source, - GlyphData, - GlyphVariationData, - MasterSnapshot, -} from "@shift/types"; -type RawSourceLocation = { values: { [tag in string]?: number } }; -type RawSource = Omit & { location: RawSourceLocation }; -import { signal, type WritableSignal, type Signal } from "@/lib/reactive/signal"; -import type { Bounds } from "@shift/geo"; -import { Bounds as BoundsUtil } from "@shift/geo"; -import { getNative } from "./native"; -import { NoEditSessionError, NativeOperationError } from "./errors"; -import { constrainDrag } from "@shift/rules"; -import { ValidateSnapshot } from "@shift/validation"; -import { Glyphs } from "@shift/font"; -import type { FontEngineAPI } from "@shared/bridge/FontEngineAPI"; -import type { CompositeComponents } from "@shared/bridge/FontEngineAPI"; -import type { CommandResult, PasteResult, PointEdit } from "@/types/engine"; -import { ContourContent } from "@/lib/clipboard"; -import type { NodePositionUpdateList } from "@/types/positionUpdate"; -import { Glyph, type GlyphChange } from "@/lib/model/Glyph"; - -export interface InterpolationResult { - instance: GlyphSnapshot; - errors: Array<{ sourceIndex: number; sourceName: string; message: string }>; -} - -/** - * Owns the raw NAPI bridge and the reactive {@link $glyph} signal. - * All font queries, session lifecycle, and glyph mutations live here. - * - * The $glyph signal holds a reactive {@link Glyph} with per-contour signals. - * All mutations go through {@link Glyph.apply} — structural edits pass a - * snapshot, position updates pass a NodePositionUpdateList. - */ -export class NativeBridge { - readonly #$glyph: WritableSignal; - #raw: FontEngineAPI; - - constructor(raw?: FontEngineAPI) { - this.#raw = raw ?? getNative(); - this.#$glyph = signal(null, { equals: () => false }); - } - - get $glyph(): Signal { - return this.#$glyph; - } - - getEditingSnapshot(): GlyphSnapshot | null { - const glyph = this.#$glyph.peek(); - return glyph ? glyph.toSnapshot() : null; - } - - hasSession(): boolean { - return this.#raw.hasEditSession(); - } - - startEditSession(glyphName: string, unicode?: number): void { - if (this.hasSession()) { - const currentName = this.getEditingGlyphName(); - if (currentName === glyphName) return; - this.endEditSession(); - } - const ref = unicode !== undefined ? { glyphName, unicode } : { glyphName }; - this.#raw.startEditSession(ref); - this.#$glyph.set(this.hasSession() ? new Glyph(this) : null); - } - - endEditSession(): void { - this.#raw.endEditSession(); - this.#$glyph.set(null); - } - - getEditingUnicode(): number | null { - return this.#raw.getEditingUnicode(); - } - - getEditingGlyphName(): string | null { - return this.#raw.getEditingGlyphName(); - } - - loadFont(path: string): void { - this.#raw.loadFont(path); - } - - saveFontAsync(path: string): Promise { - return this.#raw.saveFontAsync(path); - } - - /** @knipclassignore — satisfies Font interface */ - getMetadata(): FontMetadata { - return JSON.parse(this.#raw.getMetadata()); - } - - getMetrics(): FontMetrics { - return JSON.parse(this.#raw.getMetrics()); - } - - getGlyphUnicodes(): number[] { - return this.#raw.getGlyphUnicodes(); - } - - nameForUnicode(unicode: number): string | null { - return this.#raw.getGlyphNameForUnicode(unicode); - } - - /** @knipclassignore — used by glyph grid for dependent lookups */ - getDependentUnicodesByName(glyphName: string): number[] { - return this.#raw.getDependentUnicodesByName(glyphName); - } - - getSvgPath(name: string): string | null { - return this.#raw.getGlyphSvgPathByName(name) ?? null; - } - - /** @knipclassignore — satisfies Font interface */ - getAdvance(name: string): number | null { - return this.#raw.getGlyphAdvanceByName(name) ?? null; - } - - getBbox(name: string): Bounds | null { - const b = this.#raw.getGlyphBboxByName(name); - if (b == null || b.length !== 4) return null; - return BoundsUtil.create({ x: b[0], y: b[1] }, { x: b[2], y: b[3] }); - } - - /** @knipclassignore — satisfies Font interface */ - getPath(name: string): Path2D | null { - const editing = this.#$glyph.peek(); - if (editing?.name === name) return editing.path; - const svg = this.getSvgPath(name); - return svg ? new Path2D(svg) : null; - } - - getGlyphCompositeComponents(glyphName: string): CompositeComponents | null { - const payload = this.#raw.getGlyphCompositeComponents(glyphName); - if (!payload) return null; - return JSON.parse(payload) as CompositeComponents; - } - - /** - * Bundled per-glyph fetch for the render-side `GlyphView` model. One FFI - * returns geometry, variation deltas, and component refs (names + transforms, - * not pre-flattened — the renderer recurses into composites at iteration time). - */ - getGlyphData(name: string): GlyphData | null { - const json = this.#raw.getGlyphData(name); - if (!json) return null; - return JSON.parse(json) as GlyphData; - } - - /** @knipclassignore — used by VariationPanel component */ - isVariable(): boolean { - return this.#raw.isVariable(); - } - - /** @knipclassignore — used by VariationPanel component */ - getAxes(): Axis[] { - return JSON.parse(this.#raw.getAxes()) as Axis[]; - } - - /** @knipclassignore — used by VariationPanel component */ - getSources(): Source[] { - const raw = JSON.parse(this.#raw.getSources()) as RawSource[]; - const axes = this.getAxes(); - return raw.map((source) => ({ - ...source, - location: resolveAxisLocation(source.location.values, axes), - })); - } - - /** @knipclassignore — used by VariationPanel component */ - getGlyphMasterSnapshots(glyphName: string): MasterSnapshot[] | null { - const json = this.#raw.getGlyphMasterSnapshots(glyphName); - if (!json) return null; - return JSON.parse(json) as MasterSnapshot[]; - } - - getGlyphVariationData(glyphName: string): GlyphVariationData | null { - const json = this.#raw.getGlyphVariationData(glyphName); - if (!json) return null; - return JSON.parse(json) as GlyphVariationData; - } - - getSnapshot(): GlyphSnapshot { - return JSON.parse(this.#raw.getSnapshotData()) as GlyphSnapshot; - } - - #execute(json: string): CommandResult { - const raw = JSON.parse(json); - if (!raw.success) { - throw new NativeOperationError(raw.error ?? "Unknown native error"); - } - if (!raw.snapshot) { - throw new NativeOperationError("Native operation succeeded but returned no snapshot"); - } - return { snapshot: raw.snapshot as GlyphSnapshot, affectedPointIds: raw.affectedPointIds }; - } - - #dispatch(json: string): PointId[] { - this.#requireSession(); - const response = this.#execute(json); - this.#syncFromResponse(response.snapshot); - return response.affectedPointIds; - } - - #dispatchVoid(json: string): void { - this.#requireSession(); - const response = this.#execute(json); - this.#syncFromResponse(response.snapshot); - } - - #syncFromResponse(snapshot: GlyphSnapshot): void { - const glyph = this.#$glyph.peek(); - if (glyph) { - glyph.apply(snapshot); - this.#$glyph.set(glyph); - } else { - this.#$glyph.set(new Glyph(this)); - } - } - - #requireSession(): void { - if (!this.hasSession()) { - throw new NoEditSessionError(); - } - } - - addPoint(edit: PointEdit): PointId { - const ids = this.#dispatch(this.#raw.addPoint(edit.x, edit.y, edit.pointType, edit.smooth)); - const pointId = ids[0]; - if (pointId) return pointId; - - const glyph = this.#$glyph.peek(); - if (!glyph) throw new NativeOperationError("Native addPoint returned no point ID"); - const lastContour = glyph.contours[glyph.contours.length - 1]; - const lastPoint = lastContour?.points[lastContour.points.length - 1]; - if (!lastPoint) { - throw new NativeOperationError("Native addPoint returned no point ID"); - } - return lastPoint.id; - } - - /** @knipclassignore — used via Editor delegation or Glyph */ - addPointToContour(contourId: ContourId, edit: PointEdit): PointId { - const ids = this.#dispatch( - this.#raw.addPointToContour(contourId, edit.x, edit.y, edit.pointType, edit.smooth), - ); - const pointId = ids[0]; - if (pointId) return pointId; - throw new NativeOperationError("Native addPointToContour returned no point ID"); - } - - /** @knipclassignore — used via Editor delegation or Glyph */ - insertPointBefore(beforePointId: PointId, edit: PointEdit): PointId { - const ids = this.#dispatch( - this.#raw.insertPointBefore(beforePointId, edit.x, edit.y, edit.pointType, edit.smooth), - ); - const pointId = ids[0]; - if (pointId) return pointId; - throw new NativeOperationError("Native insertPointBefore returned no point ID"); - } - - movePoints(pointIds: PointId[], delta: Point2D): PointId[] { - if (pointIds.length === 0) return []; - return this.#dispatch( - this.#raw.moveNodes( - pointIds.map((id) => ({ kind: "point", id })), - delta.x, - delta.y, - ), - ); - } - - moveAnchors(anchorIds: AnchorId[], delta: Point2D): void { - if (anchorIds.length === 0) return; - this.#dispatchVoid( - this.#raw.moveNodes( - anchorIds.map((id) => ({ kind: "anchor", id })), - delta.x, - delta.y, - ), - ); - } - - movePointTo(pointId: PointId, x: number, y: number): void { - this.#requireSession(); - - const snapshot = this.getEditingSnapshot(); - if (!snapshot) throw new NativeOperationError("No glyph available"); - - const found = Glyphs.findPoint(snapshot, pointId); - if (!found) throw new NativeOperationError(`Point ${pointId} not found`); - - this.movePoints([pointId], { x: x - found.point.x, y: y - found.point.y }); - } - - removePoints(pointIds: PointId[]): void { - if (pointIds.length === 0) return; - this.#dispatchVoid(this.#raw.removePoints(pointIds)); - } - - /** @knipclassignore — used via Editor delegation or Glyph */ - toggleSmooth(pointId: PointId): void { - this.#dispatchVoid(this.#raw.toggleSmooth(pointId)); - } - - addContour(): ContourId { - this.#requireSession(); - const response = this.#execute(this.#raw.addContour()); - this.#syncFromResponse(response.snapshot); - return response.snapshot.activeContourId!; - } - - closeContour(): void { - this.#dispatchVoid(this.#raw.closeContour()); - } - - getActiveContourId(): ContourId | null { - if (!this.hasSession()) return null; - return this.#raw.getActiveContourId(); - } - - setActiveContour(contourId: ContourId): void { - this.#dispatchVoid(this.#raw.setActiveContour(contourId)); - } - - clearActiveContour(): void { - if (!this.hasSession()) return; - this.#dispatchVoid(this.#raw.clearActiveContour()); - } - - /** @knipclassignore — used via Editor delegation or Glyph */ - reverseContour(contourId: ContourId): void { - this.#dispatchVoid(this.#raw.reverseContour(contourId)); - } - - /** @knipclassignore — used via Editor delegation */ - applyBooleanOp( - contourIdA: ContourId, - contourIdB: ContourId, - operation: "union" | "subtract" | "intersect" | "difference", - ): void { - this.#dispatchVoid(this.#raw.applyBooleanOp(contourIdA, contourIdB, operation)); - } - - /** @knipclassignore — used via Editor delegation or Glyph */ - openContour(contourId: ContourId): void { - this.#dispatchVoid(this.#raw.openContour(contourId)); - } - - /** @knipclassignore — used via Editor delegation or Glyph */ - setXAdvance(width: number): void { - this.#dispatchVoid(this.#raw.setXAdvance(width)); - } - - /** @knipclassignore — used via Editor delegation or Glyph */ - translateLayer(dx: number, dy: number): void { - this.#dispatchVoid(this.#raw.translateLayer(dx, dy)); - } - - setNodePositions(updates: NodePositionUpdateList): void { - if (!this.hasSession()) return; - if (updates.length === 0) return; - - const glyph = this.#$glyph.peek(); - if (!glyph) return; - - glyph.apply(updates); - this.#$glyph.set(glyph); - this.#syncPositions(updates); - } - - /** @knipclassignore — used by Editor for smart drag constraints */ - applySmartEdits(selectedPoints: ReadonlySet, dx: number, dy: number): PointId[] { - if (!this.hasSession()) return []; - const reactive = this.#$glyph.peek(); - if (!reactive) return []; - const glyph = reactive.toSnapshot(); - - const patch = constrainDrag( - { glyph, selectedIds: selectedPoints, mousePosition: { x: dx, y: dy } }, - { includeMatchedRules: false }, - ); - - const updates: NodePositionUpdateList = patch.pointUpdates.map( - (update: (typeof patch.pointUpdates)[number]) => ({ - node: { kind: "point", id: update.id }, - x: update.x, - y: update.y, - }), - ); - if (updates.length > 0) { - this.setNodePositions(updates); - } - return patch.pointUpdates.map((u: (typeof patch.pointUpdates)[number]) => u.id); - } - - pasteContours(contours: ContourContent[], offsetX: number, offsetY: number): PasteResult { - this.#requireSession(); - const contoursJson = JSON.stringify(contours); - const raw = JSON.parse(this.#raw.pasteContours(contoursJson, offsetX, offsetY)); - - if (!raw.success) { - throw new NativeOperationError(raw.error ?? "pasteContours failed"); - } - - const snapshot = this.getSnapshot(); - this.#syncFromResponse(snapshot); - - return { - success: raw.success, - createdPointIds: raw.createdPointIds, - createdContourIds: raw.createdContourIds, - error: raw.error, - }; - } - - /** @knipclassignore — used via Editor delegation or Glyph */ - restoreSnapshot(snapshot: GlyphSnapshot): void { - this.#requireSession(); - if (!ValidateSnapshot.isGlyphSnapshot(snapshot)) { - throw new NativeOperationError("Cannot restore invalid snapshot"); - } - const success = this.#raw.restoreSnapshot(JSON.stringify(snapshot)); - if (!success) { - throw new NativeOperationError("Failed to restore snapshot"); - } - this.#syncFromResponse(snapshot); - } - - /** - * Rust-side mirror of glyph.apply(). Syncs a change to Rust without - * updating the JS reactive model. Position updates use Float64Array - * (zero-copy), snapshots use JSON. - */ - sync(change: GlyphChange): void { - this.#requireSession(); - - if (Array.isArray(change)) { - this.#syncPositions(change); - return; - } - - const success = this.#raw.restoreSnapshot(JSON.stringify(change)); - if (!success) { - throw new NativeOperationError("Failed to sync snapshot to native"); - } - } - - #syncPositions(updates: NodePositionUpdateList): void { - if (updates.length === 0) return; - - const pointIds: number[] = []; - const pointCoords: number[] = []; - const anchorIds: number[] = []; - const anchorCoords: number[] = []; - - for (const u of updates) { - switch (u.node.kind) { - case "point": - pointIds.push(Number(u.node.id)); - pointCoords.push(u.x, u.y); - break; - case "anchor": - anchorIds.push(Number(u.node.id)); - anchorCoords.push(u.x, u.y); - break; - } - } - - this.#raw.setPositions( - pointIds.length > 0 ? new Float64Array(pointIds) : null, - pointCoords.length > 0 ? new Float64Array(pointCoords) : null, - anchorIds.length > 0 ? new Float64Array(anchorIds) : null, - anchorCoords.length > 0 ? new Float64Array(anchorCoords) : null, - ); - } -} - -function resolveAxisLocation( - values: RawSourceLocation["values"] | undefined, - axes: Axis[], -): AxisLocation { - const out: AxisLocation = {}; - for (const axis of axes) out[axis.tag] = values?.[axis.tag] ?? axis.default; - return out; -} diff --git a/apps/desktop/src/renderer/src/bridge/docs/DOCS.md b/apps/desktop/src/renderer/src/bridge/docs/DOCS.md deleted file mode 100644 index 1a5826c6..00000000 --- a/apps/desktop/src/renderer/src/bridge/docs/DOCS.md +++ /dev/null @@ -1,119 +0,0 @@ -# NativeBridge - -Reactive wrapper over the Rust NAPI bindings that owns the `Glyph` lifecycle and provides the sole API boundary between the JS editor and the native font engine. - -## Architecture Invariants - -- **Architecture Invariant:** All glyph mutations must go through `Glyph.apply` for the JS-side reactive model. Direct signal writes on `Contour` or `Glyph` internals will bypass the path/bounds recomputation pipeline. -- **Architecture Invariant:** **CRITICAL**: `GlyphDraft.setPositions` must only call `Glyph.apply` -- never `NativeBridge` methods. Crossing the NAPI boundary on every drag frame kills performance. Rust sync happens once on `GlyphDraft.finish`. -- **Architecture Invariant:** **CRITICAL**: `#syncPositions` passes `null` (not a zero-length `Float64Array`) when a category has no updates. napi-rs panics on zero-length `Float64Array`. -- **Architecture Invariant:** `$glyph` is an identity signal (`equals: () => false`). It fires on glyph open/close, not on data changes. Render effects must track `Glyph` internal signals (`contours`, `anchors`, `path`) to respond to mutations. -- **Architecture Invariant:** `Glyph` holds a back-reference to `NativeBridge` so it can delegate structural mutation methods (`addPoint`, `removePoints`, etc.) directly. This keeps callers from needing both a bridge and a glyph reference. -- **Architecture Invariant:** Snapshot restore validates via `ValidateSnapshot.isGlyphSnapshot` before sending to Rust. Invalid snapshots throw `NativeOperationError` without touching native state. - -## Codemap - -``` -bridge/ - NativeBridge.ts — NAPI wrapper, session lifecycle, mutation dispatch - native.ts — cached `window.shiftFont` accessor (FontEngineAPI) - errors.ts — FontEngineError, NoEditSessionError, NativeOperationError - index.ts — re-exports NativeBridge, Glyph, Contour, errors -``` - -## Key Types - -- `NativeBridge` -- owns `#raw` (FontEngineAPI) and `#$glyph` (WritableSignal). All font queries and mutations live here. -- `Glyph` -- reactive mirror of a Rust glyph with per-contour signal granularity. Property getters auto-unwrap signals. All mutations enter through `Glyph.apply`. -- `Contour` -- reactive contour with `#points`, `#closed`, computed `#path` and `#bounds`. Updated via `_update` (snapshot) or `_setPoints` (position patch). -- `GlyphChange` -- union of `GlyphSnapshot | NodePositionUpdateList`. Snapshot for structural edits; update list for position-only changes. -- `GlyphDraft` -- interface with `setPositions`, `finish`, `discard`. Created by `Editor.createDraft`, implements immer-style preview/commit separation. -- `NodePositionUpdateList` -- `readonly NodePositionUpdate[]`, each entry is a `NodeRef` (point, anchor, or guideline) with absolute x/y. -- `FontEngineAPI` -- derived from the napi-rs generated `FontEngine` class, exposed via `window.shiftFont` through Electron's contextBridge. -- `NoEditSessionError` / `NativeOperationError` -- thrown when an operation requires a session or the Rust side fails. - -## How it works - -### Two-tier mutation model - -The bridge separates JS-side reactivity from Rust-side persistence. Not every mutation crosses the NAPI boundary immediately. - -**Tier 1 -- JS-only (`Glyph.apply`):** Updates reactive signals (`#contours`, `#anchors`). Render effects auto-track these signals and schedule redraws. Rust is untouched. Used by `GlyphDraft.setPositions` during drag (every frame). - -**Tier 2 -- Rust sync (`NativeBridge.sync`):** Pushes a `GlyphChange` to Rust without updating the JS model (already correct from Tier 1). Position updates go through `#syncPositions` (flat `Float64Array` arrays); snapshots go through `restoreSnapshot` (JSON). Used by `GlyphDraft.finish` on drag end (once). - -**Combined -- JS + Rust:** Methods like `setNodePositions` and `restoreSnapshot` update both sides atomically. Used by commands (`SetNodePositionsCommand`, `SnapshotCommand`) and sidebar edits where the mutation is immediate. - -### Dispatch pipeline - -`#dispatch` / `#dispatchVoid` are the standard Rust command path for structural mutations (addPoint, removePoints, closeContour, etc.): - -1. Call NAPI method, receive JSON string -2. `#execute` parses JSON, checks `success`, extracts `CommandResult` (snapshot + affectedPointIds) -3. `#syncFromResponse` applies the returned snapshot to the reactive `Glyph` via `Glyph.apply` - -### NAPI position sync - -`#syncPositions` converts `NodePositionUpdateList` into four flat arrays (pointIds, pointCoords, anchorIds, anchorCoords) packed as `Float64Array`, then calls `#raw.setPositions`. Cost is proportional to the number of changed nodes, not the glyph size. - -### GlyphDraft lifecycle - -Created by `Editor.createDraft`. Captures a base snapshot, then: - -- `setPositions(updates)` -- calls `Glyph.apply(updates)` (JS-only, no Rust) -- `finish(label)` -- calls `NativeBridge.sync(updates)` for Rust sync, then records a `SetNodePositionsCommand` for undo (stores the diff, not two full snapshots) -- `discard()` -- calls `Glyph.apply(base)` to revert JS state (Rust was never touched) - -A `finished` flag prevents double-finish or post-finish mutation. - -### Glyph reactive model - -`Glyph` wraps each data dimension in its own signal. Computed signals (`#path`, `#bbox`) derive from these automatically. `Glyph.apply` dispatches on change type: - -- Snapshot: `#syncFromSnapshot` reconciles contours by ID (reuses existing `Contour` instances, creates new ones as needed), updates all scalar signals inside a `batch`. -- Position updates: `#patchPositions` maps point/anchor IDs to new coordinates and patches only the affected `Contour` instances. - -## Workflow recipes - -### Add a new NAPI-backed mutation - -1. Add the `#[napi]` method in Rust, rebuild -- it appears on `FontEngineAPI` automatically -2. Add a public method on `NativeBridge` that calls `#dispatch` or `#dispatchVoid` with the raw result -3. If callers should access it through `Glyph`, add a delegation method on `Glyph` that calls `this.#bridge.` - -### Add a new position-based operation - -1. Build a `NodePositionUpdateList` with the target absolute positions -2. For immediate apply: call `NativeBridge.setNodePositions(updates)` -3. For draft-based (drag): call `GlyphDraft.setPositions(updates)` per frame, then `GlyphDraft.finish(label)` on end - -### Support undo for a structural edit - -Wrap the mutation in a `SnapshotCommand` -- capture a before-snapshot, execute the bridge method, the after-snapshot is the current state. - -## Gotchas - -- `Float64Array` of length 0 panics napi-rs. `#syncPositions` guards this by passing `null` instead. -- `$glyph` has `equals: () => false`, so every `.set()` fires subscribers even if the instance is the same. This is intentional for session open/close detection, but means you should never use `$glyph` as a change-tracking signal for data. -- `Glyph.apply` with a snapshot reconciles contours by ID. If a Rust mutation changes contour IDs (e.g., boolean operations), old `Contour` instances are dropped and new ones created. Effects holding stale `Contour` references will read the old signals. -- `getPath` for the currently-editing glyph returns the reactive `Glyph.path` (computed from JS signals), not a fresh Rust SVG path. This ensures composites render with live edits. -- `#execute` throws `NativeOperationError` if the Rust response has `success: false` or is missing a snapshot. Callers of `#dispatch` do not need to check for errors -- they propagate as exceptions. - -## Verification - -```bash -# Unit tests (draft lifecycle, position sync) -npx vitest run apps/desktop/src/renderer/src/lib/editor/draft.test.ts - -# Type check -npx tsc --noEmit -p apps/desktop/tsconfig.json -``` - -## Related - -- `Editor.createDraft` -- constructs `GlyphDraft` instances and records undo via `SetNodePositionsCommand` -- `SetNodePositionsCommand` / `SnapshotCommand` -- undo/redo primitives that call back into `NativeBridge` -- `FontEngineAPI` -- the NAPI type surface, derived from `shift-node` -- `ValidateSnapshot` -- validates `GlyphSnapshot` shape before Rust restore -- `constrainDrag` -- smart edit rules invoked by `NativeBridge.applySmartEdits` -- `TestEditor` -- test harness that wraps `NativeBridge` with a mock `FontEngineAPI` diff --git a/apps/desktop/src/renderer/src/bridge/errors.ts b/apps/desktop/src/renderer/src/bridge/errors.ts deleted file mode 100644 index a52ced9b..00000000 --- a/apps/desktop/src/renderer/src/bridge/errors.ts +++ /dev/null @@ -1,29 +0,0 @@ -/** - * Custom error class for FontEngine operations. - */ -export class FontEngineError extends Error { - constructor(message: string) { - super(message); - this.name = "FontEngineError"; - } -} - -/** - * Error thrown when an operation requires an active edit session but none exists. - */ -export class NoEditSessionError extends FontEngineError { - constructor() { - super("No active edit session. Call session.start() first."); - this.name = "NoEditSessionError"; - } -} - -/** - * Error thrown when an operation fails on the Rust side. - */ -export class NativeOperationError extends FontEngineError { - constructor(message: string) { - super(message); - this.name = "NativeOperationError"; - } -} diff --git a/apps/desktop/src/renderer/src/bridge/index.ts b/apps/desktop/src/renderer/src/bridge/index.ts deleted file mode 100644 index 24ebe764..00000000 --- a/apps/desktop/src/renderer/src/bridge/index.ts +++ /dev/null @@ -1,9 +0,0 @@ -export { NativeBridge } from "./NativeBridge"; -export type { FontMetadata, FontMetrics } from "@shift/types"; - -export { FontEngineError, NoEditSessionError, NativeOperationError } from "./errors"; - -export type { FontEngineAPI } from "./native"; - -export type { GlyphDraft } from "@/types/draft"; -export { Glyph, Contour } from "@/lib/model/Glyph"; diff --git a/apps/desktop/src/renderer/src/bridge/native.ts b/apps/desktop/src/renderer/src/bridge/native.ts deleted file mode 100644 index 64719494..00000000 --- a/apps/desktop/src/renderer/src/bridge/native.ts +++ /dev/null @@ -1,15 +0,0 @@ -import type { FontEngineAPI } from "@shared/bridge/FontEngineAPI"; - -export type { FontEngineAPI }; -export type NativeFontEngine = FontEngineAPI; - -let cached: FontEngineAPI | null = null; - -export function getNative(): FontEngineAPI { - if (cached) return cached; - if (!window.shiftFont) { - throw new Error("Native FontEngine not available"); - } - cached = window.shiftFont; - return cached; -} diff --git a/apps/desktop/src/renderer/src/components/chrome/FontInfoDialog.tsx b/apps/desktop/src/renderer/src/components/chrome/FontInfoDialog.tsx new file mode 100644 index 00000000..f67d97cc --- /dev/null +++ b/apps/desktop/src/renderer/src/components/chrome/FontInfoDialog.tsx @@ -0,0 +1,145 @@ +import { useState } from "react"; +import { + Button, + Dialog, + DialogBackdrop, + DialogClose, + DialogPopup, + DialogPortal, + DialogTitle, + Input, + X, + cn, +} from "@shift/ui"; +import { getEditor } from "@/store/store"; + +interface FontInfoDialogProps { + open: boolean; + onOpenChange: (open: boolean) => void; +} + +type CategoryId = "general" | "font" | "sources" | "features" | "shortcuts"; + +const CATEGORIES: { id: CategoryId; label: string }[] = [ + { id: "general", label: "General" }, + { id: "font", label: "Font" }, + { id: "sources", label: "Sources" }, + { id: "features", label: "Features" }, + { id: "shortcuts", label: "Shortcuts" }, +]; + +export const FontInfoDialog = ({ open, onOpenChange }: FontInfoDialogProps) => { + const editor = getEditor(); + const [category, setCategory] = useState("font"); + + if (!editor.font.loaded) return null; + + return ( + + + + + Font information + + + +
+ + + + + +
+ + + + ); +}; + +const CategoryPanel = ({ category }: { category: CategoryId }) => { + switch (category) { + case "font": + return ; + case "general": + case "sources": + case "features": + case "shortcuts": + return c.id === category)!.label} />; + } +}; + +const FontPanel = () => { + const editor = getEditor(); + const m = editor.font.metadata; + const version = m.versionMajor !== undefined ? `${m.versionMajor}.${m.versionMinor ?? 0}` : ""; + + return ( +
+

Font

+ +
+ ); +}; + +interface Field { + label: string; + text: string; +} + +const FieldGrid = ({ fields }: { fields: Field[] }) => ( +
+ {fields.map((f) => ( + + ))} +
+); + +const FieldRow = ({ field }: { field: Field }) => ( + <> + + + +); + +const PlaceholderPanel = ({ label }: { label: string }) => ( +
+

{label}

+

Coming soon.

+
+); diff --git a/apps/desktop/src/renderer/src/components/chrome/NavigationPane.tsx b/apps/desktop/src/renderer/src/components/chrome/NavigationPane.tsx index 0b78244d..1ccbae82 100644 --- a/apps/desktop/src/renderer/src/components/chrome/NavigationPane.tsx +++ b/apps/desktop/src/renderer/src/components/chrome/NavigationPane.tsx @@ -1,33 +1,47 @@ +import { useState } from "react"; import { useNavigate } from "react-router-dom"; import { Button } from "@shift/ui"; import { routes } from "@/app/routes"; +import { FontInfoDialog } from "./FontInfoDialog"; export const NavigationPane = () => { const navigate = useNavigate(); + const [fontInfoOpen, setFontInfoOpen] = useState(false); return (
{routes.map((route) => { - if (route.icon) { - const Icon = route.icon; - return ( -
+
); }; diff --git a/apps/desktop/src/renderer/src/components/debug/DebugPanel.tsx b/apps/desktop/src/renderer/src/components/debug/DebugPanel.tsx index 6a5a8b7a..4203780a 100644 --- a/apps/desktop/src/renderer/src/components/debug/DebugPanel.tsx +++ b/apps/desktop/src/renderer/src/components/debug/DebugPanel.tsx @@ -1,15 +1,36 @@ -import { useRef, useEffect } from "react"; +import { useEffect, useMemo, useRef } from "react"; import { useSignalText } from "@/hooks/useSignalText"; import { getEditor } from "@/store/store"; import { Separator } from "@shift/ui"; -import { effect } from "@/lib/reactive"; +import { effect } from "@/lib/signals"; +import { useSignalState, useSignalTrigger } from "@/lib/signals/useSignal"; function formatCoords(x: number, y: number): string { return `(${Math.round(x)}, ${Math.round(y)})`; } +function formatBytes(bytes: number): string { + if (bytes < 1024) return `${bytes} B`; + if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} KB`; + return `${(bytes / (1024 * 1024)).toFixed(1)} MB`; +} + export function DebugPanel() { const editor = getEditor(); + const instance = useSignalState(editor.glyphInstanceCell); + useSignalTrigger(instance?.xAdvanceCell); + + const glyphStats = useMemo(() => { + if (!instance) return { pointCount: "0", snapshotSize: "—" }; + + const snapshot = instance.edit?.state ?? null; + const bytes = snapshot ? new Blob([JSON.stringify(snapshot)]).size : null; + + return { + pointCount: `${instance.geometry.allPoints.length}`, + snapshotSize: bytes === null ? "—" : formatBytes(bytes), + }; + }, [instance]); useEffect(() => { editor.startFpsMonitor(); @@ -28,33 +49,13 @@ export function DebugPanel() { return `${editor.fps.value}`; }); - const pointCountRef = useSignalText(() => { - const glyph = editor.glyph.value; - if (!glyph) return "0"; - - return `${glyph.allPoints.length}`; - }); - - const glyphMemoryRef = useSignalText(() => { - const glyph = editor.glyph.value; - if (!glyph) return "—"; - - const snapshot = glyph.toSnapshot(); - const json = JSON.stringify(snapshot); - const bytes = new Blob([json]).size; - - if (bytes < 1024) return `${bytes} B`; - if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} KB`; - return `${(bytes / (1024 * 1024)).toFixed(1)} MB`; - }); - const upmRef = useRef(null); const screenRef = useRef(null); const worldRef = useRef(null); useEffect(() => { const fx = effect(() => { - const screen = editor.screenMousePosition.value; + const screen = editor.screenMousePositionCell.value; const coords = editor.fromScreen(screen); if (upmRef.current) upmRef.current.textContent = formatCoords(coords.scene.x, coords.scene.y); if (screenRef.current) screenRef.current.textContent = formatCoords(screen.x, screen.y); @@ -64,10 +65,10 @@ export function DebugPanel() { return () => fx.dispose(); }, [editor]); - const cellClass = "px-2 py-1 border border-line-subtle"; + const cellClass = "px-2 py-1 border"; return ( -
+
Debug Panel @@ -89,14 +90,15 @@ export function DebugPanel() {

Canvas

Total Points - + {glyphStats.pointCount}
Snapshot Size - + {glyphStats.snapshotSize}
+

Coordinates

Mouse

diff --git a/apps/desktop/src/renderer/src/components/editor/BooleanOps.tsx b/apps/desktop/src/renderer/src/components/editor/BooleanOps.tsx index 049f633e..f791d244 100644 --- a/apps/desktop/src/renderer/src/components/editor/BooleanOps.tsx +++ b/apps/desktop/src/renderer/src/components/editor/BooleanOps.tsx @@ -19,19 +19,19 @@ export const BooleanOps = () => { { - editor.applyBooleanOp(contourIdA, contourIdB, "union"); + editor.boolean(contourIdA, contourIdB, "union"); }} /> { - editor.applyBooleanOp(contourIdA, contourIdB, "intersect"); + editor.boolean(contourIdA, contourIdB, "intersect"); }} /> { - editor.applyBooleanOp(contourIdA, contourIdB, "subtract"); + editor.boolean(contourIdA, contourIdB, "subtract"); }} />
diff --git a/apps/desktop/src/renderer/src/components/editor/EditorView.tsx b/apps/desktop/src/renderer/src/components/editor/EditorView.tsx index 6084843e..0dab5842 100644 --- a/apps/desktop/src/renderer/src/components/editor/EditorView.tsx +++ b/apps/desktop/src/renderer/src/components/editor/EditorView.tsx @@ -2,15 +2,18 @@ import { FC, useEffect, useRef, useState } from "react"; import { CanvasContextProvider } from "@/context/CanvasContext"; import { useDebugSafe } from "@/context/DebugContext"; -import { effect } from "@/lib/reactive/signal"; +import { effect } from "@/lib/signals/signal"; +import { useSignalState } from "@/lib/signals"; import { getEditor } from "@/store/store"; import { zoomMultiplierFromWheel } from "@/lib/transform"; import { InteractiveScene } from "./InteractiveScene"; import { StaticScene } from "./StaticScene"; import { DebugPanel } from "../debug/DebugPanel"; -import { HiddenTextInput } from "./HiddenTextInput"; +import { TextInput } from "../text/HiddenTextInput"; import { Vec2 } from "@shift/geo"; +const GLYPH_ID_RE = /^[0-9a-f]+$/i; + interface EditorViewProps { glyphId: string; } @@ -19,34 +22,33 @@ export const EditorView: FC = ({ glyphId }) => { const editor = getEditor(); const debug = useDebugSafe(); const containerRef = useRef(null); + const fontLoaded = useSignalState(editor.font.$loaded); const [cursorStyle, setCursorStyle] = useState(() => editor.cursor); useEffect(() => { const fx = effect(() => { - setCursorStyle(editor.cursor); + setCursorStyle(editor.cursorCell.value); }); return () => fx.dispose(); }, [editor]); useEffect(() => { - const parsed = Number.parseInt(glyphId, 16); - const unicode = Number.isNaN(parsed) ? 0x41 : parsed; - const glyphName = editor.font.glyphName(unicode); + if (!fontLoaded) return undefined; + if (!GLYPH_ID_RE.test(glyphId)) return undefined; - const initEditor = () => { - editor.setMainGlyphUnicode(unicode); - editor.open(glyphName); + const parsed = Number.parseInt(glyphId, 16); + if (Number.isNaN(parsed)) return undefined; - editor.setDrawOffsetForGlyph({ x: 0, y: 0 }, glyphName, unicode); + const unicode = parsed; + const handle = editor.font.glyphHandleForUnicode(unicode); - // Update viewport with actual font metrics (UPM, descender, guides) - editor.updateMetricsFromFont(); + const source = editor.font.sourceAtOrDefault(editor.font.defaultLocation()); - editor.requestRedraw(); - }; + editor.openGlyph(handle); + editor.openGlyphSource(handle, source.id); - initEditor(); + editor.updateMetricsFromFont(); const toolManager = editor.toolManager; const activeToolId = editor.getActiveTool(); @@ -56,7 +58,7 @@ export const EditorView: FC = ({ glyphId }) => { toolManager.reset(); editor.close(); }; - }, [glyphId]); + }, [editor, fontLoaded, glyphId]); useEffect(() => { const element = containerRef.current; @@ -71,7 +73,6 @@ export const EditorView: FC = ({ glyphId }) => { e.preventDefault(); const zoomFactor = zoomMultiplierFromWheel(e.deltaY, e.deltaMode); editor.zoomToPoint(screenPos.x, screenPos.y, zoomFactor); - editor.requestRedraw(); } else { const currentPan = editor.pan; const newPan = Vec2.sub(currentPan, { x: e.deltaX, y: e.deltaY }); @@ -84,9 +85,8 @@ export const EditorView: FC = ({ glyphId }) => { altKey: e.altKey, metaKey: e.metaKey, }, - { force: true, skipHover: true }, + { force: true }, ); - editor.requestRedraw(); } }; @@ -107,7 +107,7 @@ export const EditorView: FC = ({ glyphId }) => { - + {debug?.debugPanelOpen && }
); diff --git a/apps/desktop/src/renderer/src/components/editor/GlyphFinder.tsx b/apps/desktop/src/renderer/src/components/editor/GlyphFinder.tsx index 2386cfd8..1c88d140 100644 --- a/apps/desktop/src/renderer/src/components/editor/GlyphFinder.tsx +++ b/apps/desktop/src/renderer/src/components/editor/GlyphFinder.tsx @@ -9,9 +9,9 @@ import { Separator, } from "@shift/ui"; import { formatCodepointAsUPlus } from "@/lib/utils/unicode"; -import { getGlyphInfo } from "@/store/glyphInfo"; import type { SearchResult } from "@shift/glyph-info"; import { useFocusZone } from "@/context/FocusZoneContext"; +import { getGlyphInfo } from "@/store/store"; interface GlyphFinderProps { open: boolean; @@ -29,9 +29,12 @@ function glyphChar(codepoint: number): string { export function GlyphFinder({ open, onOpenChange, onSelect }: GlyphFinderProps) { const { lockToZone, unlock } = useFocusZone(); + const [query, setQuery] = useState(""); + const [results, setResults] = useState([]); const [selectedIndex, setSelectedIndex] = useState(0); + const inputRef = useRef(null); const listRef = useRef(null); @@ -141,7 +144,7 @@ export function GlyphFinder({ open, onOpenChange, onSelect }: GlyphFinderProps)
{results.map((result, index) => (
( -