diff --git a/builtin/runtime/zoom.lua b/builtin/runtime/zoom.lua new file mode 100644 index 0000000..55f5990 --- /dev/null +++ b/builtin/runtime/zoom.lua @@ -0,0 +1,190 @@ +-- GUI zoom (QoL Stage 2, framing docs/gui-zoom-framing.md). +-- +-- Drives the font preference that already exists: `pmacs.gpu.set_font` +-- writes it, `semantic_render` relays it as `InstanceMessage::FontFacts` +-- at protocol v17, and the GPU frontend owns every pixel consequence. +-- Nothing here knows a metric, an advance, or what resolves --- the +-- no-pixels invariant (src/font_pref.rs) holds through this module. +-- +-- NO KEYBINDINGS, deliberately (Q#Z3). `keymap_stack::Scope` is +-- Buffer | Mode | Global and carries no frontend identity, so "bind +-- this on GPU frontends only" is not expressible; and `FrontendEvent` +-- has no command-invocation variant, so the GPU cannot ask for a +-- command by name either. A global binding would capture C-+/C-- in +-- the TUI and take away the terminal's own zoom --- the very thing the +-- user is pressing the key for. Commands are discoverable via M-x and +-- one line to bind in init.lua; capability-aware binding is its own +-- lane. + +pmacs.zoom = pmacs.zoom or {} + +-- Wire bounds, in logical px: `FONT_SIZE_CENTI_PX_RANGE` is 600..=7200 +-- (pmacs-gpu/src/main.rs). Mirrored rather than imported because the +-- Lua range check is a UX courtesy; the frontend re-checks on arrival +-- because that side is deserialized protocol input. +local MIN_PX = 6.0 +local MAX_PX = 72.0 + +local STATE_KEY = "gpu-zoom" + +pmacs.config.define { + name = "ui.gpu-font-size-base", + description = "Logical-pixel size the first zoom step starts from when no font size is set.", + type = "number", + default = 16.0, + min = MIN_PX, + max = MAX_PX, + mutability = "live", +} + +-- Lower bound is the QUANTIZER, not tidiness: `validate_font_size` +-- rounds to the nearest hundredth, so a step below 0.01 quantizes to +-- zero and "zoom in" silently does nothing forever. A negative step +-- would invert the commands --- zoom-in shrinking is not a malfunction +-- the user can diagnose, because the command still does something +-- coherent. Upper bound is the range span (72 - 6): a larger step can +-- only ever clamp. +pmacs.config.define { + name = "ui.gpu-zoom-step", + description = "Logical pixels added or removed per zoom step.", + type = "number", + default = 1.0, + min = 0.01, + max = 66.0, + mutability = "live", +} + +-- Round to centi-pixel, the wire's unit. Doing this here keeps every +-- comparison and every round-trip in the quantized domain, so "n in, +-- n out" is exact addition rather than float drift. +local function quantize(px) + return math.floor(px * 100 + 0.5) / 100 +end + +-- The current size in logical px, or nil when the preference is unset. +-- nil is a REAL state (the frontend's own default), never inferred from +-- silence --- Q#TH7. +local function current_px() + return pmacs.gpu.font().size +end + +-- Preserve the configured family on EVERY write. `set_font` replaces +-- both fields unconditionally, so `set_font { size = n }` alone would +-- clear a family the user set in init.lua, and they would get it back +-- only by restarting. +local function write_size(px) + local spec = { size = px } + local family = pmacs.gpu.font().family + if family then spec.family = family end + pmacs.gpu.set_font(spec) +end + +local function save(px) + if not pmacs.state.available() then return end + pmacs.state.write(STATE_KEY, string.format("%d\n", math.floor(px * 100 + 0.5))) +end + +local function forget() + if not pmacs.state.available() then return end + pmacs.state.write(STATE_KEY, "") +end + +-- Step by `delta` logical px. Returns the new size, or nil plus a +-- reason. +local function step(delta) + local base = current_px() or pmacs.config.get("ui.gpu-font-size-base") + local want = quantize(base + delta) + if want < MIN_PX or want > MAX_PX then + -- Reject the WHOLE step rather than pinning to the boundary. This + -- is what keeps "n steps in, n steps out returns exactly" true at + -- the edges, which is precisely where a user steps back and forth. + -- It also mirrors `apply_font_facts`, which rejects an + -- out-of-range message outright rather than clamping it. + return nil, string.format( + "zoom: %.2f px is outside %.2f-%.2f; size unchanged", want, MIN_PX, MAX_PX) + end + write_size(want) + save(want) + return want +end + +-- Named `increase`/`decrease` rather than `in`/`out`: `in` is a Lua +-- keyword, and `in_` reads like a workaround for one. +function pmacs.zoom.increase() + return step(pmacs.config.get("ui.gpu-zoom-step")) +end + +function pmacs.zoom.decrease() + return step(-pmacs.config.get("ui.gpu-zoom-step")) +end + +-- Reset returns the preference to NIL --- the frontend's own default --- +-- not to `ui.gpu-font-size-base`. The base is only the origin for a +-- first step; resetting to it would ship an explicit size that merely +-- happens to equal the default, making the untouched state unreachable +-- once a user has ever zoomed. Clearing the saved state too, because +-- "reset until restart" is not what the word says. +function pmacs.zoom.reset() + local family = pmacs.gpu.font().family + local spec = {} + if family then spec.family = family end + pmacs.gpu.set_font(spec) + forget() + return nil +end + +-- Restore a saved zoom. Called from the Rust side AFTER +-- `install_state_dirs`, never at module load: builtins and init.lua both +-- run before state is wired up, so a read here would return nothing, +-- always. Zoom is this project's first EAGER state consumer --- saveplace +-- and recentf both read lazily inside functions and never meet this. +-- +-- Whole-file parse, not a line iterator. `^(%d+)$` would reject the +-- newline-terminated file we write ourselves ($ anchors to end of +-- subject), and saveplace's `gmatch("([^\n]+)")` would accept the FIRST +-- line of a multi-line file --- fine for recentf, where a line is one +-- independent entry, wrong here, where the file IS the value. +function pmacs.zoom.restore() + if not pmacs.state.available() then return nil end + local text = pmacs.state.read(STATE_KEY) + if not text then return nil end + local centi = text:match("^(%d+)\n$") + if not centi then return nil end + centi = tonumber(centi) + -- Range-check before it can reach `set_font`. A syntactically fine + -- but out-of-range value --- hand-edited, or written by a future + -- version with a wider range --- would otherwise be rejected as a + -- whole message, leaving the user with neither the saved zoom nor an + -- explanation. + if centi < MIN_PX * 100 or centi > MAX_PX * 100 then return nil end + local px = centi / 100 + write_size(px) + return px +end + +pmacs.command.define { + name = "gpu.zoom-in", + description = "Increase the GPU frontend's font size by one step", + fn = function() + local px, why = pmacs.zoom.increase() + pmacs.editor.set_status(why or string.format("zoom: %.2f px", px)) + end, +} + +pmacs.command.define { + name = "gpu.zoom-out", + description = "Decrease the GPU frontend's font size by one step", + fn = function() + local px, why = pmacs.zoom.decrease() + pmacs.editor.set_status(why or string.format("zoom: %.2f px", px)) + end, +} + +pmacs.command.define { + name = "gpu.zoom-reset", + description = "Return the GPU frontend to its own default font size", + fn = function() + pmacs.zoom.reset() + pmacs.editor.set_status("zoom: reset to the frontend default") + end, +} diff --git a/docs/active-work.md b/docs/active-work.md index 38955a9..62c6c70 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -296,6 +296,81 @@ change to *when* `needs_full_grid` is set — the producer's triggers were verified correct, along with per-frame geometry sync and `view_top` reconciliation on shrink. +## GUI zoom (QoL Stage 2) — IN FLIGHT + +**Written with the lane's first commit, before the PR exists** — the +standing correction from #171 and #215. + +- **Branch `gui-zoom`**, base `githubsucks/main` @ `218d2e7` (the #219 + merge). `githubsucks/gui-zoom` is the authoritative tip. + Recover: `git fetch githubsucks && git checkout gui-zoom`. +- **Framing `docs/gui-zoom-framing.md` revision 4**, approved after + four review rounds. **Q#Z1 = (c)** configured base with `None` + preserved; **Q#Z2 = additive**; **Q#Z3 = (C)** commands only, no + default bindings; **Q#Z4** eager restore inside `install_state_dirs`. + +### What it ships + +`builtin/runtime/zoom.lua`: two settings (`ui.gpu-font-size-base`, +`ui.gpu-zoom-step`), three commands (`gpu.zoom-in` / `-out` / +`-reset`), and `pmacs.zoom.restore` called from `install_state_dirs`. +**No rendering work** — `FontMetrics::scale` already derived every GUI +dimension and `apply_font_facts` already re-metriced everything; this +drives the preference that existed. + +### The three findings review caught, none of which was in revision 1 + +- **Q#Z3 was not implementable.** `keymap_stack::Scope` is + `Buffer | Mode | Global` with no frontend identity, and + `FrontendEvent` has no command-invocation variant — so neither "bind + on GPU only" nor "the GPU asks for a command" exists. Commands ship; + the binding waits on **capability-aware keymap resolution**, now a + named follow-on. +- **The restore seam did not exist.** Builtins and `init.lua` both run + *before* `install_state_dirs`, so a `pmacs.state.read` at module load + returns nothing, always. `saveplace` and `recentf` never meet this + because **both read lazily**; zoom must apply with no user action, + making it the **first eager state consumer**. Restore lives at the + end of `install_state_dirs` — by definition when state becomes + readable, so it cannot be mis-ordered or missed by a future third + startup path. +- **Every size write clobbered the family.** `set_font` replaces both + fields unconditionally, so `{ size = n }` alone silently cleared a + configured family until restart. + +### Verification + +13 acceptance tests, three bitten: dropping family preservation fails +3; reverting to the framing's first parser `^(%d+)$` fails 4 including +the seam restore (it anchors to end-of-subject and rejects the +newline-terminated file the writer emits); hardcoding the 16.0 origin +fails the base test. + +### Not in scope + +Stage 3 (long lines). Capability-aware keymap resolution — Q#Z3's +option (A), deliberately deferred rather than half-built. Per-buffer +zoom. Any change to `FontFacts` or the wire. + +## Empty-content readiness, a fourth and fifth instance — FOR THE R6 AUDIT + +Found 2026-08-06 while gating this lane, recorded here because it +widens an existing lane's scope rather than starting one. + +A loaded `--features crdt` run failed `m6_1_pty_raw_mode_disables_kernel_echo` +and `m6_1_pty_canonical_mode_keeps_kernel_echo` with +**`stty -a output was: ""`** — read-before-write on the child's output. +That is the **same family as R4** (readiness predicate satisfied by an +empty file) and **R6** (readiness file never published), and it means +the readiness-helper audit's scope is not just three `wait_for_file` +copies under `tests/`: `src/process.rs`'s own tests carry the shape +too. + +Both passed isolated and the full suite was green on a quiet machine, +so this is load-sensitive and **undiagnosed** — recorded as a scope +note for the audit, not as a registry row: these were local, and the +registry judges red **CI** runs. + ## Tree primitive (P5) — MERGED as #217; adoption is the open work **The lane is gone, not the work.** Rule 4 removes a lane after merge, diff --git a/docs/gui-zoom-framing.md b/docs/gui-zoom-framing.md new file mode 100644 index 0000000..cf0ce43 --- /dev/null +++ b/docs/gui-zoom-framing.md @@ -0,0 +1,440 @@ +# GUI zoom — QoL Stage 2 + +**Status: revision 4 — proposed, awaiting approval.** Q#Z1 approved as +**(c)** (configured base, `None` preserved) and Q#Z2 as **additive**. +Revision 4 fixes a self-contradiction in §5a1: the specified parser +`^(%d+)$` rejects the newline-terminated format specified beside it. +Whole-file `^(%d+)\n$`, with a verified case table and a valid-restore +witness. Revision 3 added the contract details a second pass asked for: §1 and §7 +no longer claim a keybinding this stage does not ship, §3.1 bounds and +quantizes both settings, and §5a1 defines the persisted format and what +corrupted state does. Revision 2 answered four review findings that revision 1 got wrong or +left out: Z3 was **not implementable as written** (§4), Z4 had **no +working restore seam** (§5), every size write **silently clears a +configured family** (§5a), and reset/precedence semantics were +unspecified (§5b). Reported from daily-driver use +alongside Stage 1: `Ctrl +/-` has *no effect whatsoever* in the GPU +frontend. Stage 1 (#219) fixed what zoom did to the TUI; this stage is +the other half — making the GUI zoom at all. + +**Most of this already exists**, which is why it is a short stage. It +is not, however, a trivial one: relative zoom needs a starting point, +and the daemon is deliberately built never to know one (§3). + +--- + +## 1. What is already built + +The whole mechanism ships today and is exercised by tests: + +| piece | where | state | +|---|---|---| +| Scale that drives **everything** | `FontMetrics { scale, advance_ratio }`, `pmacs-gpu/src/main.rs:129`+ | code size, line height, status band, divider, menu rows, minibuffer dropdown, gutter advance all derive from it | +| Daemon-side preference | `src/font_pref.rs` — family + `size_centi_px` + monotonic `epoch` | written by `pmacs.gpu.set_font`, read per-frame by the producer | +| Wire relay | `InstanceMessage::FontFacts`, protocol v17 | bufferless; v16 peers get none | +| Frontend application | `apply_font_facts`, `pmacs-gpu/src/main.rs:8132` | one transaction: validate, resolve family, re-metric all seven buffers, reshape at retained scroll, re-follow caret | +| Getter | `pmacs.gpu.font()` | fresh table, `size` in logical px, key absent when unset | +| Validation | `600..=7200` centi-px (6.0–72.0 px), checked at both ends | Lua range-check is UX; the frontend re-checks because it is wire input | + +There is even a byte-identity test that `(None, None)` reproduces the +never-set frame exactly. + +**So this stage adds no rendering work.** It adds **commands**, a +starting point, and remembering — **and deliberately no default +keybindings**, per Q#Z3 (§4): the keymap cannot express "GPU frontends +only", and neither a protocol variant nor a rushed keymap change is +worth a default chord. + +--- + +## 2. Q#Z1 — where does a *relative* zoom start? (the real question) + +`src/font_pref.rs` is explicit: + +> *"The daemon relays a PREFERENCE: it never learns metrics, advances, +> or what resolves (the no-pixels invariant); the frontend owns +> resolution and every pixel consequence."* + +And `size_centi_px: None` is **a real state, not an absence** — "the +frontend's built-in default", which Q#TH7 established must never be +inferred from silence. + +Zoom-in is `current + step`. On a fresh session `current` is `None`, +and the daemon cannot ask what the frontend resolved. Three ways out: + +- **(a) Hard-code the frontend's default (16.0) as the origin.** One + line, and it puts a pixel constant on the daemon side — the exact + thing the no-pixels invariant forbids. It also silently breaks for + any frontend whose default differs, which is the situation the + invariant exists to allow. +- **(b) Always send an explicit size**, sourced from a config setting + defaulting to 16.0. Simple, but it **destroys the `None` state for + everyone**: a user who never zooms now gets an explicit size, the + frontend's own default query never runs, and Q#F5's always-shipped + all-default baseline stops being reachable. +- **(c) A configured *base*, with `None` preserved until the user + actually zooms.** `ui.gpu-font-size-base` (logical px, default 16.0) + is the documented origin for the *first* zoom step. Until a zoom + happens the preference stays `None` and the frontend's default is + used exactly as today. + +**Recommendation: (c).** It is the only one where the daemon still +infers nothing — the base is a *user-facing preference about zooming*, +not the daemon guessing a frontend metric. A user whose frontend +default is not 16.0 changes one setting, which is a real answer rather +than a broken assumption. And the untouched path is byte-identical to +today, so the existing `(None, None)` identity test keeps its meaning. + +**Q#Z1 — DECIDED: (c)**, on review. + +--- + +## 3. Q#Z2 — step shape + +Additive (`+1.0 px`) or multiplicative (`×1.1`)? + +**DECIDED: additive**, on review — via `ui.gpu-zoom-step` (logical px, +default 1.0). + +### 3.1 Both settings are validated, and the bounds are not cosmetic + +The registry validates at `define` time (`min`/`max`, as +`autosave.interval-ms` does), so both settings are constrained rather +than checked at use: + +| setting | bounds | why | +|---|---|---| +| `ui.gpu-font-size-base` | **6.00 – 72.00** logical px | exactly the wire range (`600..=7200` centi-px). A base outside it could never be sent, so the first zoom step would fail from a value the user was allowed to set | +| `ui.gpu-zoom-step` | **0.01 – 66.00** logical px, strictly positive | see below | + +**The step's lower bound is the quantizer.** `validate_font_size` +range-checks the original value and then rounds to the nearest +hundredth, so a step below `0.01` quantizes to **zero** — "zoom in" +would do nothing, forever, with no error anywhere. `0.01` is one +centi-pixel, the smallest representable step. + +**Zero and negative are excluded for a stronger reason than tidiness.** +A negative step **inverts** the commands: `gpu.zoom-in` would shrink. +That is not a malfunction the user can diagnose from the outside — the +command does something coherent, just the opposite of its name. + +**The upper bound is the range span** (72.00 − 6.00). A step larger +than the whole domain can only ever clamp or be rejected, so permitting +it buys a setting that cannot be used. + +**These bounds are what make the round-trip claim true.** "n steps in, +then n steps out, returns to exactly the starting value" holds because +the step is centi-pixel representable and addition is exact in that +domain — *provided no clamp occurred*, which is why §6 requires an +out-of-range zoom to leave the preference **unmutated** rather than +pinning it to the boundary. Pinning would silently break the round trip +at the edges, which is precisely where a user is most likely to be +stepping back and forth. The domain is integer hundredths of a pixel over +6.0..=72.0 — a small, bounded, quantized range where additive steps are +predictable and land on round numbers. Multiplicative stepping +accumulates rounding through the quantizer and makes "two in, two out" +fail to return to where you started, which is the property users +actually notice. + +--- + +## 4. Q#Z3 — keys. Revision 1 was not implementable + +Revision 1 said "do not install the binding on a non-GPU frontend." +**There is no such thing.** `keymap_stack::Scope` is exactly +`Buffer(BufferId) | Mode(String) | Global`, and neither `resolve()` nor +`keymap_tree` receives any frontend identity or capability. A global +binding is global — it would capture `C-+` / `C--` in the TUI, which is +the outcome §4 claimed to avoid. + +Nor is there a way for the GPU to ask for a command by name: +`FrontendEvent` is `Key | Mouse | Resize | Paste | FocusGained | +FocusLost | Detach | CrdtOp`. There is **no command-invocation +variant**, so "the GPU intercepts the chord locally" cannot reach the +daemon-side preference that owns the value and its persistence. + +So there are three real options, and none is free: + +- **(A) Capability-aware keymap resolution.** Teach `Scope` — or + `resolve()`'s inputs — about the requesting frontend. Correct and + general, and it would serve every later frontend-specific binding. + But it touches the keymap core, every resolve call site, and the Lua + `bind` surface. That is its own lane, not a step inside a zoom lane. +- **(B) A new `FrontendEvent` for it.** Narrow and direct, but it is a + **protocol addition** — schema bump and negotiation — which §8 + explicitly scopes out, and which buys a keybinding rather than a + capability. +- **(C) Ship commands now; bind later.** `gpu.zoom-in` / `-out` / + `-reset` exist and work from `M-x` and from `init.lua`, where a user + who runs the GPU can bind them themselves in one line. No core + change, no protocol change, no half-built capability. + +**Recommendation: (C), with (A) recorded as the follow-on it implies.** +The value the report asked for is *zoom working at all*; the default +keybinding is the smaller half, and buying it with either a protocol +variant or a rushed keymap change trades a large permanent surface for +a small convenience. **(A) is the right eventual answer** — "this +binding applies to frontends with capability X" is a question this +project will keep asking — and it deserves its own framing rather than +being smuggled in here. + +**What must NOT happen** is a global binding plus a command that +reports "not applicable" in the TUI. That is the Q#P3 fall-through +lesson and #217's flat-panel `TAB` finding: capturing a key to deliver +an apology removes a working behaviour — and here the working +behaviour is *the terminal's own zoom*, which is exactly what the user +is pressing the key for. + +*Tempting and rejected:* "terminals swallow `C-+` anyway, so a global +binding is harmless in practice." Most do. Relying on most is how a +binding becomes a bug report from whoever runs the terminal that +doesn't. + +## 5. Q#Z4 — remembering it, and the seam revision 1 did not have + +The config registry has **no write-back** — settings are declared, not +saved. The project's remembered-state mechanism is `pmacs.state.read` / +`write` / `available` (`docs/persistence-framing.md`), used by +`saveplace` and `recentf`. The split is the right one: + +- **`pmacs.config`** — what you *declared*: base size, step. +- **`pmacs.state`** — what you *arrived at*: the current zoom level. + +### 5.1 Reading at module load is inert, and revision 1 assumed it was not + +`EditorState::new()` / `open()` load the builtins and run `init.lua`. +**`install_state_dirs()` runs after that** — `src/editor.rs:3838` on +the local path, `src/daemon.rs:480` on the daemon path. A +`pmacs.state.read` at Lua module load therefore returns nothing, always. + +`saveplace` and `recentf` never hit this because **both read lazily**, +inside functions called long after startup (`recentf.lua:27` in +`load_list`, `saveplace.lua:33` behind its `available()` guard). **Zoom +cannot be lazy**: its whole purpose is to apply with no user action, +before the first frame the GPU paints. It is this project's **first +eager state consumer**, which is why the seam has to be named rather +than assumed. + +### 5.2 The seam + +Restore must run **after `install_state_dirs()` on both startup paths**. +Neither existing post-install hook qualifies: `restore_desktop_if_armed` +is local-only by design (Q#DS9 keeps desktop restore out of the daemon, +which has a layout per attached frontend and none at construction). + +**Recommendation: restore inside `install_state_dirs()` itself**, at its +end. Both paths already call it, exactly once, and it is by definition +the moment state becomes readable — so the restore cannot be ordered +wrongly and cannot be forgotten by a future third startup path. The +alternative, a new call added beside both existing call sites, is two +places a fourth path can miss. + +**This must be tested against production ordering, not a direct call.** +A test that calls the restore helper itself proves nothing about when +it runs — the same shape as the `prepare_startup` note at +`src/editor.rs:3820`, where splitting the sequence was what kept a +deleted call from leaving every direct-call test green while shipping +nothing. The witness asserts that a state file written before startup +is reflected in `pmacs.gpu.font()` after the real startup sequence. + +### 5a. Every size write must preserve the family + +`pmacs.gpu.set_font` replaces **both** fields unconditionally +(`src/lua_bindings/mod.rs`: `pref.family = family; pref.size_centi_px = +size_centi_px;`). So `set_font { size = 18 }` **silently clears a +configured family** — a user with `family = "Iosevka"` in `init.lua` +loses it the first time they zoom, and gets it back only by restarting. + +Zoom must therefore read the current family via `pmacs.gpu.font()` and +pass it back with every write. **Pinned by a custom-family → zoom → +`FontFacts` case** asserting the relayed message still carries the +family. This is a property of the setter's whole-message semantics, not +a zoom bug, and the test belongs with zoom because zoom is what makes +it reachable. + +### 5a1. The persisted format, and what corrupted state does + +**Format**, following `saveplace` and `recentf` rather than inventing +one: a single line of plain text holding the size in **centi-pixels** +as decimal digits, newline-terminated. + +``` +1800 +``` + +Centi-pixels, not logical px, because that is the wire unit and the +already-quantized one — writing `18.0` would reintroduce a +float-parsing step and a second place for rounding to disagree with +`validate_font_size`. + +No version prefix. The existing state files carry none, and a single +integer has no forward-compatibility question that a prefix would +answer; a future format change can be detected by the strict parse +below failing, which lands in exactly the same handling as corruption. + +**Only the size is persisted.** Not the family — that is declared +config, and §5b's precedence rule applies to size alone. + +**Validity** is two checks, both required: + +1. **Strict whole-file parse:** `^(%d+)\n$` against the **entire file + contents**, not against a line pulled out of it. + + Revision 2 specified `^(%d+)$`, which **contradicts the + newline-terminated format two paragraphs above it**: in Lua, `$` + anchors to end-of-subject, so `("1800\n"):match("^(%d+)$")` is + `nil` and the parser would reject every file it had itself written. + + Borrowing `saveplace`'s `gmatch("([^\n]+)")` line iterator fixes + that but introduces a different hole — it happily returns the + **first** line of a multi-line file, so trailing garbage would be + accepted silently. That is tolerable for `recentf`, where each line + is an independent entry and skipping a bad one loses one path. It is + not tolerable here, where the file *is* one value and extra content + means the file is not what we wrote. + + Whole-file matching closes both, verified against Lua 5.4: + + | contents | `^(%d+)$` | `^(%d+)\n$` | + |---|---|---| + | `"1800\n"` — what we write | **nil** ✗ | `1800` ✓ | + | `"1800"` — no trailing newline | `1800` | nil ✓ | + | `"1800\n1900\n"` — multi-line | nil | nil ✓ | + | `"18.0\n"` — decimal | nil | nil ✓ | + | `" 1800\n"` — leading space | nil | nil ✓ | + | `"\n"` — empty | nil | nil ✓ | + + The multi-line case needs **no separate line-count check**: the + trailing `$` after `\n` already requires the file to end there. +2. **Range:** the parsed value is within `600..=7200`. A syntactically + fine but out-of-range number — a hand-edited file, or a file written + by a future version with a wider range — must never reach + `set_font`, which would reject the whole message and leave the user + with neither the saved zoom nor an explanation. + +**Malformed, out-of-range, empty, or multi-line state is treated as +absent.** Startup proceeds exactly as if nothing had been saved: the +`init.lua` size applies if there is one, otherwise `None`. + +**Silently, and deliberately so.** It matches the precedent — both +existing consumers skip unparseable lines without comment — and the +failure is already self-evident and self-healing: the user sees their +zoom did not restore, and the next zoom rewrites the file correctly. +A startup diagnostic for a recoverable, self-announcing condition is +noise on every launch for a problem that fixes itself on the next +keystroke. + +**The corrupt file is not rewritten on read.** It is left alone until +the next successful zoom overwrites it, so a user who wants to look at +what went wrong still can. Truncating it at startup would destroy the +only evidence of a bug we would then have no way to reproduce. + +### 5b. Reset, clearing, and precedence + +Revision 1 left all three unspecified. As recommended in review: + +- **Reset returns size to `None`**, not to `ui.gpu-font-size-base`. + `None` is the real "frontend's own default" state (Q#F5/Q#TH7); the + base is only the *origin for the first step*. Resetting to the base + would send an explicit size that merely happens to equal the default, + making the untouched state unreachable once a user has ever zoomed — + and quietly breaking the `(None, None)` identity property. +- **Reset clears the saved zoom state**, so it does not resurrect on + the next launch. A reset that leaves the state file behind means + "reset until restart", which is not what the word says. +- **Valid saved state wins over an `init.lua` size.** The saved value + is a later, deliberate user action; the init value is the standing + default it was chosen against. This is the same precedence + `saveplace` already applies — a remembered position overrides where + opening the file would otherwise land. +- **The family is retained throughout**, per §5a: precedence applies to + *size only*, and an `init.lua` family is never overridden by restored + zoom state, because zoom state does not carry one. + +## 6. Verification + +- **No binding is installed at all** (Q#Z3 = C), so the witness is that + `C-+` / `C--` resolve to nothing in a default TUI session — the + terminal's own zoom is left alone. +- **Restore runs at the production seam.** Asserted through the real + startup sequence, not by calling the helper directly: a state file + written beforehand must be visible in `pmacs.gpu.font()` afterwards, + on **both** the local and daemon paths. +- **A zoom preserves a configured family** — custom family → zoom → + the relayed `FontFacts` still carries it (§5a). +- **Reset returns `None`, not the base**, and clears the saved state; + a subsequent launch is back to the frontend's own default. +- **Saved state beats an `init.lua` size, and never its family.** +- **Config bounds are enforced at `define`** — a zero, negative, or + sub-centi-pixel step is refused by the registry, not discovered as a + zoom that does nothing or runs backwards (§3.1). +- **A valid newline-terminated file restores.** `"1800\n"` — exactly + the bytes the writer produces — must round-trip through the real + startup sequence. This is the case revision 2's own parser would have + rejected, so it is asserted rather than assumed. +- **Malformed, out-of-range, empty, multi-line, and + missing-trailing-newline saved state each behave as absent**, and the + file is left intact for inspection (§5a1). The multi-line case is + pinned specifically: it is the one a line-iterator parser would have + accepted. +- **The round trip holds only where no clamp occurred**, so the + out-of-range case asserts the preference is left **unmutated** rather + than pinned to the boundary. +- **Zoom from the untouched state uses the configured base**, not a + hardcoded 16.0 — bitten by changing the base and asserting the first + step follows it. +- **Clamping at both ends of `6.0..=72.0`** reports rather than + silently pinning, and a zoom that would leave the range does not + mutate the preference at all (the `apply_font_facts` whole-message + rejection precedent). +- **`n` steps in then `n` steps out returns to the exact starting + value** — the property that fails under multiplicative stepping and + the reason Q#Z2 recommends additive. +- **The untouched path stays byte-identical**: with no zoom performed, + the existing `(None, None)` identity test must still hold. +- **Persistence round-trips**, and writes nothing when + `pmacs.state.available` is false. + +--- + +## 7. Coherence impact (§20 requirement) + +**Concern served: `COHERENCE.md` §16, Productize the Semantic Frontend +Architecture**, which names **frontend-specific typography** in its own +list of properties the semantic protocol should make visible as a +product advantage. A GPU frontend that cannot change its own type size +is that concern under-delivered. + +**No scorecard change.** Row 16 reads **Strong** (`COHERENCE.md:112`); +this neither earns nor forfeits it — it uses the mechanism the row +already credits. Per §25 the row moves only with a PR that changes what +it asserts. + +- **Journey steps touched:** none. +- **Interaction islands added:** none, and **no keybinding either**. + Revision 1 said "an ordinary global keymap binding, scoped by + frontend capability"; no such scoping exists (§4), so this stage + ships commands only, reachable from `M-x` and bindable by the user in + `init.lua`. Capability-aware binding is deferred to its own framing + and named in §8. +- **Config registry:** **yes, two settings** — + `ui.gpu-font-size-base` and `ui.gpu-zoom-step`. This is the "place + for GUI settings" the report asked for; it already exists and this + stage is its next adopter. +- **Background-work attribution:** none. + +--- + +## 8. Not in scope + +Long lines (Stage 3). **Capability-aware keymap resolution** — Q#Z3's +option (A), which this stage defers rather than half-builds, and which +should be its own framing because it decides how every future +frontend-specific binding is expressed. A default keybinding for zoom, +which waits on it. Per-buffer or per-window zoom — this is one +global preference, as `font_pref` already is. Family selection, which +`set_font` already exposes and no one has asked to bind. Any change to +`FontFacts`, the protocol, or `apply_font_facts` — this stage drives +the existing mechanism and adds nothing to the wire. Config write-back, +which does not exist and which Q#Z4 deliberately routes around rather +than inventing. diff --git a/src/editor.rs b/src/editor.rs index 4d3fd17..abd80e8 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -752,6 +752,17 @@ impl EditorState { include_str!("../builtin/runtime/help.lua"), ) .expect("load help builtin chunk"); + // GUI zoom (QoL Stage 2). Defines `ui.gpu-font-size-base` / + // `ui.gpu-zoom-step` and the `gpu.zoom-*` commands, and exposes + // `pmacs.zoom.restore` for `install_state_dirs` to call once + // `pmacs.state` is readable. Deliberately binds NO keys: the + // keymap cannot express "GPU frontends only" (framing Q#Z3). + lua_host + .eval( + Some("@pmacs/builtin/runtime/zoom.lua"), + include_str!("../builtin/runtime/zoom.lua"), + ) + .expect("load zoom builtin chunk"); // T M7.11 bundled-package bootstrap. Through M7.10 the REPL // was loaded directly via `eval(include_str!(...))`; the // M7.11 deliverable migrates it to the package system so it @@ -988,6 +999,30 @@ impl EditorState { .lua() .set_app_data(crate::lua_bindings::StateDir(dir)); } + // Restore a saved GUI zoom (QoL Stage 2, framing §5.2). HERE and + // not in a runtime module: builtins and `init.lua` both run + // during construction, BEFORE this function, so a + // `pmacs.state.read` at module load returns nothing every time. + // `saveplace` and `recentf` never meet that because both read + // lazily inside functions; zoom must apply with no user action, + // which makes it the first eager state consumer. + // + // Inside `install_state_dirs` rather than beside its two call + // sites (`prepare_startup` and the daemon) because this is by + // definition the moment state becomes readable — so it cannot be + // ordered wrongly, and a future third startup path gets it + // without knowing it had to ask. + // + // Best-effort: a failure here must not stop a session from + // starting over a font size. + if let Err(error) = self + .lua_host + .lua() + .load("if pmacs.zoom then pmacs.zoom.restore() end") + .exec() + { + eprintln!("pmacs: could not restore saved zoom: {error}"); + } } /// Final step of a **local, no-target** launch: greet an untouched diff --git a/tests/gui_zoom_acceptance.rs b/tests/gui_zoom_acceptance.rs new file mode 100644 index 0000000..2b36acb --- /dev/null +++ b/tests/gui_zoom_acceptance.rs @@ -0,0 +1,396 @@ +//! GUI zoom acceptance (`QoL` Stage 2, `docs/gui-zoom-framing.md`). +//! +//! Zoom drives the font preference that already existed: `set_font` +//! writes it, `semantic_render` relays it as `FontFacts` at v17, and the +//! GPU frontend owns every pixel consequence. Nothing here knows a +//! metric — the no-pixels invariant holds through the whole feature. +//! +//! Each test gets its **own** bootstrap roots. `iso::roots()` is a pure +//! function of the build environment by design, so every suite sharing +//! it shares one state directory — fine when nobody writes, wrong here, +//! where the state file *is* the subject and parallel tests would +//! overwrite each other's fixture. + +use std::path::{Path, PathBuf}; + +use pmacs::bootstrap::BootstrapRoots; +use pmacs::editor::EditorState; + +/// Per-test roots plus the path zoom's state file will occupy +/// (`/pmacs/gpu-zoom`, per `src/state.rs`). +fn roots_for(name: &str) -> (BootstrapRoots, PathBuf) { + let base = Path::new(env!("CARGO_TARGET_TMPDIR")) + .join("gui-zoom") + .join(name); + let _ = std::fs::remove_dir_all(&base); + let roots = BootstrapRoots::isolated_under(&base); + for (_, dir) in roots.child_env() { + std::fs::create_dir_all(&dir).expect("create controlled root"); + } + let dir = roots.state_dir().expect("isolated roots have a state dir"); + std::fs::create_dir_all(&dir).expect("create state dir"); + (roots, dir.join("gpu-zoom")) +} + +/// A session whose state dirs are installed — which is also what runs +/// `pmacs.zoom.restore`, so anything planted at `path` beforehand is +/// visible to it. +fn session(roots: &BootstrapRoots) -> EditorState { + let state = EditorState::new_with_roots(roots); + state.install_state_dirs(); + state +} + +fn eval(s: &EditorState, src: &str) -> T { + s.lua_host.lua().load(src.to_string()).eval().unwrap() +} + +fn exec(s: &EditorState, src: &str) { + s.lua_host.lua().load(src.to_string()).exec().unwrap(); +} + +/// Current preference size in logical px, or `None` when unset — the +/// real "frontend's own default" state, never inferred from silence. +fn size(s: &EditorState) -> Option { + eval(s, "return pmacs.gpu.font().size") +} + +fn family(s: &EditorState) -> Option { + eval(s, "return pmacs.gpu.font().family") +} + +// --------------------------------------------------------------------------- +// The starting point (Q#Z1 = c) +// --------------------------------------------------------------------------- + +/// The first step starts from the CONFIGURED base, not a hardcoded +/// 16.0 — which is the whole reason (c) was chosen over (a). The daemon +/// never learns what the frontend resolved; it reads a user-facing +/// preference about zooming. +#[test] +fn the_first_step_starts_from_the_configured_base_not_a_constant() { + let (roots, _) = roots_for("first_step_base"); + let s = session(&roots); + assert_eq!(size(&s), None, "premise: untouched means unset"); + + exec(&s, r#"pmacs.config.set("ui.gpu-font-size-base", 20.0)"#); + exec(&s, "pmacs.zoom.increase()"); + assert_eq!( + size(&s), + Some(21.0), + "20.0 base + 1.0 step. A hardcoded origin would give 17.0" + ); +} + +/// Until a zoom happens the preference stays unset, so the frontend's +/// own default query still runs. This is what option (b) would have +/// destroyed for every user who never zooms. +#[test] +fn defining_the_settings_does_not_itself_set_a_size() { + let (roots, _) = roots_for("no_implicit_size"); + let s = session(&roots); + assert_eq!(size(&s), None); + assert_eq!(family(&s), None); +} + +// --------------------------------------------------------------------------- +// Step arithmetic (Q#Z2 = additive) +// --------------------------------------------------------------------------- + +/// n steps in, then n steps out, returns to EXACTLY the starting value. +/// True because the step is centi-pixel representable and addition is +/// exact in that domain — the property multiplicative stepping loses. +#[test] +fn n_steps_in_then_n_out_returns_exactly() { + let (roots, _) = roots_for("round_trip"); + let s = session(&roots); + exec(&s, r#"pmacs.config.set("ui.gpu-zoom-step", 0.37)"#); + exec(&s, "pmacs.zoom.increase()"); + let start = size(&s).expect("a size exists after the first step"); + + for _ in 0..7 { + exec(&s, "pmacs.zoom.increase()"); + } + for _ in 0..7 { + exec(&s, "pmacs.zoom.decrease()"); + } + assert_eq!( + size(&s), + Some(start), + "exact return, not approximately — 0.37 is centi-pixel \ + representable and the domain is quantized" + ); +} + +/// An out-of-range step leaves the preference **unmutated** rather than +/// pinning it to the boundary. Pinning would silently break the round +/// trip precisely at the edges, where a user steps back and forth most. +#[test] +fn a_step_past_the_boundary_changes_nothing_and_says_so() { + let (roots, _) = roots_for("boundary"); + let s = session(&roots); + exec(&s, "pmacs.gpu.set_font { size = 71.5 }"); + exec(&s, r#"pmacs.config.set("ui.gpu-zoom-step", 2.0)"#); + + let why: Option = eval(&s, "local _, why = pmacs.zoom.increase() return why"); + assert_eq!( + size(&s), + Some(71.5), + "the preference is untouched, not pinned to 72.0" + ); + assert!( + why.unwrap_or_default().contains("unchanged"), + "and the caller is told why" + ); +} + +// --------------------------------------------------------------------------- +// Config bounds (§3.1) +// --------------------------------------------------------------------------- + +/// A zero, negative, or sub-centi-pixel step is refused by the REGISTRY, +/// not discovered later as a zoom that does nothing or runs backwards. +/// +/// The negative case is the sharp one: it would invert the commands, and +/// `gpu.zoom-in` shrinking is not a malfunction a user can diagnose from +/// the outside — the command still does something coherent. +#[test] +fn an_unusable_step_is_refused_at_the_registry() { + let (roots, _) = roots_for("step_bounds"); + let s = session(&roots); + for bad in ["0.0", "-1.0", "0.001"] { + let ok: bool = eval( + &s, + &format!(r#"return pcall(pmacs.config.set, "ui.gpu-zoom-step", {bad})"#), + ); + assert!(!ok, "a step of {bad} must be refused"); + } + // …and the smallest representable step is allowed. + let ok: bool = eval( + &s, + r#"return pcall(pmacs.config.set, "ui.gpu-zoom-step", 0.01)"#, + ); + assert!(ok, "0.01 is one centi-pixel — the smallest real step"); +} + +/// The base is bounded to the wire range. A base outside it could never +/// be sent, so the first step would fail from a value the registry had +/// allowed the user to set. +#[test] +fn the_base_is_bounded_to_the_wire_range() { + let (roots, _) = roots_for("base_bounds"); + let s = session(&roots); + for bad in ["5.99", "72.01"] { + let ok: bool = eval( + &s, + &format!(r#"return pcall(pmacs.config.set, "ui.gpu-font-size-base", {bad})"#), + ); + assert!(!ok, "a base of {bad} is outside 6.00-72.00"); + } +} + +// --------------------------------------------------------------------------- +// The family clobber (§5a) +// --------------------------------------------------------------------------- + +/// `set_font` replaces BOTH fields unconditionally, so a size-only write +/// would clear a family the user configured — and they would get it back +/// only by restarting. Every zoom write must carry the family through. +#[test] +fn a_zoom_preserves_a_configured_family() { + let (roots, _) = roots_for("family_kept"); + let s = session(&roots); + exec( + &s, + r#"pmacs.gpu.set_font { family = "Iosevka", size = 18.0 }"#, + ); + + exec(&s, "pmacs.zoom.increase()"); + assert_eq!(size(&s), Some(19.0)); + assert_eq!( + family(&s).as_deref(), + Some("Iosevka"), + "the family survives a zoom — set_font replaces both fields, so \ + a size-only write would silently drop it" + ); + + exec(&s, "pmacs.zoom.decrease()"); + assert_eq!( + family(&s).as_deref(), + Some("Iosevka"), + "and every step after" + ); +} + +// --------------------------------------------------------------------------- +// Reset (§5b) +// --------------------------------------------------------------------------- + +/// Reset returns the size to UNSET — the frontend's own default — not to +/// the configured base. Resetting to the base would ship an explicit +/// size that merely happens to equal the default, making the untouched +/// state unreachable once a user has ever zoomed. +#[test] +fn reset_returns_to_unset_not_to_the_base() { + let (roots, path) = roots_for("reset_unsets"); + let s = session(&roots); + exec(&s, r#"pmacs.gpu.set_font { family = "Iosevka" }"#); + exec(&s, "pmacs.zoom.increase()"); + assert!(size(&s).is_some(), "premise: a size is set"); + assert!(path.exists(), "premise: the zoom was saved"); + + exec(&s, "pmacs.zoom.reset()"); + assert_eq!(size(&s), None, "unset, not the base value"); + assert_eq!( + family(&s).as_deref(), + Some("Iosevka"), + "reset is about size; a configured family is not collateral" + ); + + // Cleared, so it does not resurrect on the next launch — "reset + // until restart" is not what the word says. + let saved = std::fs::read_to_string(&path).unwrap_or_default(); + assert!( + saved.trim().is_empty(), + "reset clears the saved zoom, found {saved:?}" + ); +} + +// --------------------------------------------------------------------------- +// Persistence and its parser (§5a1) +// --------------------------------------------------------------------------- + +/// The exact bytes the writer produces round-trip. This is the case the +/// framing's first parser (`^(%d+)$`) would have REJECTED — it anchors +/// to end-of-subject, so it fails on the trailing newline — which is why +/// it is asserted rather than assumed. +#[test] +fn a_valid_newline_terminated_file_restores() { + let (roots, path) = roots_for("restore_valid"); + std::fs::write(&path, "1800\n").expect("plant state"); + let s = session(&roots); + assert_eq!( + size(&s), + Some(18.0), + "restored at the seam, from the bytes the writer emits" + ); +} + +/// What zoom writes is what zoom reads. Pins the two halves against each +/// other so a format change cannot land in one alone. +#[test] +fn the_written_format_is_the_format_that_parses() { + let (roots, path) = roots_for("format_round_trip"); + { + let s = session(&roots); + exec(&s, "pmacs.gpu.set_font { size = 23.5 }"); + exec(&s, "pmacs.zoom.increase()"); + assert_eq!(size(&s), Some(24.5)); + } + let raw = std::fs::read_to_string(&path).expect("state written"); + assert_eq!(raw, "2450\n", "centi-pixels, newline-terminated"); + + let s2 = session(&roots); + assert_eq!(size(&s2), Some(24.5), "and a fresh session reads it back"); +} + +/// Malformed, out-of-range, and shape-wrong state all behave as absent, +/// and the file is left intact for inspection rather than truncated. +/// +/// The multi-line case is the one that matters most: `saveplace`'s +/// `gmatch("([^\n]+)")` line iterator would happily accept its FIRST +/// line. That is right for `recentf`, where a line is one independent +/// entry, and wrong here, where the file IS the value. +#[test] +fn unparseable_or_out_of_range_state_behaves_as_absent() { + for (label, contents) in [ + ("multi-line", "1800\n1900\n"), + ("no trailing newline", "1800"), + ("decimal", "18.0\n"), + ("leading space", " 1800\n"), + ("empty", ""), + ("bare newline", "\n"), + ("non-numeric", "big\n"), + ("above the wire range", "9999\n"), + ("below the wire range", "100\n"), + ] { + let (roots, path) = roots_for(&format!("bad_{}", label.replace(' ', "_"))); + std::fs::write(&path, contents).expect("plant state"); + let s = session(&roots); + assert_eq!( + size(&s), + None, + "{label}: {contents:?} must behave exactly as no saved state" + ); + assert_eq!( + std::fs::read_to_string(&path).expect("still there"), + contents, + "{label}: the file is left intact — truncating it on read \ + would destroy the only evidence of whatever wrote it" + ); + } +} + +/// Precedence: a valid saved zoom beats a size set during startup, and +/// never touches a configured family. +/// +/// The saved value is a later, deliberate user action; the init value is +/// the standing default it was chosen against — the same precedence +/// `saveplace` already applies to a remembered position. +#[test] +fn saved_state_beats_a_startup_size_but_never_the_family() { + let (roots, path) = roots_for("precedence"); + std::fs::write(&path, "2600\n").expect("plant state"); + let s = EditorState::new_with_roots(&roots); + // Stand in for init.lua, which runs before state is installed. + exec( + &s, + r#"pmacs.gpu.set_font { family = "Iosevka", size = 12.0 }"#, + ); + s.install_state_dirs(); + + assert_eq!(size(&s), Some(26.0), "the remembered zoom wins on SIZE"); + assert_eq!( + family(&s).as_deref(), + Some("Iosevka"), + "and never on family — restored state does not carry one" + ); +} + +// --------------------------------------------------------------------------- +// No keybindings (Q#Z3 = C) +// --------------------------------------------------------------------------- + +/// This stage ships commands and NO default bindings. +/// +/// The keymap has no way to say "GPU frontends only" — `Scope` is +/// `Buffer | Mode | Global` and carries no frontend identity — so a +/// global binding would capture the chord in the TUI and take away the +/// terminal's own zoom, which is the very thing the user is pressing it +/// for. Better an unbound key than one that answers with an apology. +#[test] +fn the_commands_exist_and_nothing_is_bound() { + let (roots, _) = roots_for("no_bindings"); + let s = session(&roots); + + // `pmacs.command.list()` returns plain names, as `help.lua` reads it. + let names: Vec = eval(&s, "return pmacs.command.list()"); + for want in ["gpu.zoom-in", "gpu.zoom-out", "gpu.zoom-reset"] { + assert!(names.iter().any(|n| n == want), "{want} is discoverable"); + } + + let bound: Vec = eval( + &s, + r#"local out = {} + for _, b in ipairs(pmacs.keymap.list()) do + if b.command and b.command:match("^gpu%.zoom") then + out[#out + 1] = string.format("%s -> %s (%s)", b.sequence, b.command, b.scope) + end + end + return out"#, + ); + assert!( + bound.is_empty(), + "no zoom keybinding may be installed by default, found {bound:?}" + ); +}