diff --git a/COHERENCE.md b/COHERENCE.md index 1af9981..670b235 100644 --- a/COHERENCE.md +++ b/COHERENCE.md @@ -95,7 +95,7 @@ remain open to them. | § | Concern | Grade | One-line state | |---|---|---|---| -| 2 | Golden product journey | **Runs to step 5** | `pmacs .` opens the directory (Journey Stage 1a); thin from step 6 on | +| 2 | Golden product journey | **Runs to step 10** | `pmacs .` opens the directory (1a); the interface introduces itself (1b-3); a missing language server says so (1b-2, #204); a build is bound and prefilled (1b-1, #203). Steps 1, 11 and 12 remain the thin end | | 3 | Zero-configuration state | **Partial** | Defaults genuinely strong; missing-tool failure is silent, not graceful | | 4 | Progressive disclosure | **Inverted** | The advanced level is real; the beginner level is the missing one | | 5 | Unified discoverability | **Substrate without surface** | Best-in-class registration metadata; almost no way for a user to reach it | @@ -111,8 +111,8 @@ remain open to them. | 15 | Contextual affordances | **Weak** | Right-click menu only; code actions apply first-blindly; no git integration at all | | 16 | Semantic frontend | **Strong** | v6..=v21 schema support; production attach remains v20 during the dark panel slice; degradation practiced | | 17 | Distribution | **Missing** | CI is test-only; no binaries, channels, checksums, or update path | -| 18 | Onboarding | **Missing** | No welcome, no tutorial; `C-h` deletes a word; `M-x` is the only door in | -| 19 | Coherence acceptance tests | **Started** | `tests/journey_acceptance.rs` exists (steps 2, 3, 5); the other five scenarios are still unwritten | +| 18 | Onboarding | **Partial** | Journey Stage 1b-3: an unconfigured launch greets in `*scratch*` naming `M-x` and four real bindings, and `M-x help` renders a cheat sheet. Still no tutorial and `C-h` still deletes a word — deliberately, see §18 | +| 19 | Coherence acceptance tests | **Started** | `tests/journey_acceptance.rs` carries 45 pins over steps 2, 3, 4, 5, 6 and 9 — the ratchet is real and stages add rows to it. The other five §19 scenarios (workspace lifecycle, worker ownership, config provenance, package lifecycle, extension isolation) are still unwritten | Three cross-cutting patterns explain most of the table; they are detailed in §1.1–§1.3: **substrate without surface**, **the silence @@ -392,9 +392,9 @@ Full verdict table: | 1 | Install | **Partial** | Source build only: `cargo build --release --workspace --features pmacs/crdt` (`README.md`). No binaries, no packaging. Runtime deps (`/bin/sh`, git, tar, coreutils) documented, never checked at runtime | | 2 | Launch unconfigured | **Works** | `EditorState::new()` → empty `*scratch*`; missing config is not an error (`src/config.rs:7-9`); recentf/saveplace/autosave default-on | | 3 | Open real project | **Works at the CLI** | Journey Stage 1a: `resolve_target_buffer` answers `ResolvedTarget::Directory` before the EISDIR-producing load, and `EditorState::open` / the daemon bootstrap dispatch the `path.open-directory` chain, whose fallback is dired (#165's buffer, reached rather than duplicated). Startup no longer fails: an unreadable directory, a crashed resolver, and a cleared handler all report on the status line and leave the session running. Because the listing is async and the bootstrap is synchronous, the commit runs against a destination captured at request time (`pmacs.window.commit_to`) rather than against the ambient frontend | -| 4 | Understand interface | **Partial** | Mode line gives name/modified/L:C/scroll + mode/LSP/terminal segments; but no welcome text (`EditorCore::new` sets `status: String::new()`), no cheat sheet, and `C-h` deletes a word (§18) | +| 4 | Understand interface | **Partial** | Mode line gives name/modified/L:C/scroll + mode/LSP/terminal segments. Journey Stage 1b-3 adds a welcome in `*scratch*` and `M-x help`; **still Partial** because `C-h` deletes a word (deliberately — §18) and there is no tutorial | | 5 | Edit | **Works** | Full CUA + Emacs keymap in 161 lines (`builtin/keymaps/default.lua`); isearch, query-replace, kill ring, undo/redo, auto-indent/pair/comment, atomic save. Genuinely excellent zero-config | -| 6 | Language intelligence | **Partial** | Rust grammar bundled and auto-attaches; rust-analyzer preconfigured (`builtin/runtime/lsp.lua`). **Journey Stage 1b-2 (PR open) ends the silence** for a server that fails to *start*: the status line names the command, language and errno once per `(language, root, command)`; the modeline reads `LSP:!` instead of nothing; and `M-x lsp.status` renders `*lsp*` over the `status_buffer_text()` renderer that had existed since M4.8 with no caller. **Still Partial**, and flips only on merge (§25): a server that starts and then *crashes* is still unsurfaced | +| 6 | Language intelligence | **Partial** | Rust grammar bundled and auto-attaches; rust-analyzer preconfigured (`builtin/runtime/lsp.lua`). **Journey Stage 1b-2 (#204) ended the silence** for a server that fails to *start*: the status line names the command, language and errno once per `(language, root, command)`; the modeline reads `LSP:!` instead of nothing; and `M-x lsp.status` renders `*lsp*` over the `status_buffer_text()` renderer that had existed since M4.8 with no caller. **Still Partial**, for a reason unaffected by that landing: a server that starts and then *crashes* is still unsurfaced — `LspEventKind::Crashed` is pushed and no builtin subscriber handles it | | 7 | Find symbol / file | **File: fixed (open by path merged #162; browsing #165). Symbol: works but undiscoverable** | No find-file/dired/picker existed at audit. Now `C-x C-f` opens a known path and `C-x d` / `C-x C-j` browse (flat listing, `dired` mode keymap); `M-.`/`M-?`/`C-c o` still bound but advertised nowhere and server-gated; no workspace-symbol command; `pmacs.index.*` has no UI | | 8 | Open terminal | **Works** | Full PTY with scrollback + modeline segment, bound to `C-c t` and configurable through three registered settings (`terminal.default-profile`, `terminal.scrollback-rows`, `terminal.escape-key`) plus named `pmacs.terminal.profiles` (PR #173), and searchable through `M-x terminal.copy-mode` / `C-c C-t`, which materializes the retained scrollback into an ordinary read-only buffer (Stage 2). Named limitations: `C-c t` is unreachable from *inside* a terminal window, where `C-c` is consumed as the escape — `M-x terminal` still works there; and there is still **no close/kill command**, which is the remaining half of this step's discoverability gap. *Was broken outright on the GPU frontend until the double terminal-layout sync was fixed: the child took a `SIGWINCH` storm at tick cadence, so typing into it was impossible while output still flowed.* | | 9 | Build / test | **Works** | Journey Stage 1b-1 (#203): `C-c c` runs `compile.run`, and the first prompt is prefilled from the detected project kind (`pmacs.compile.defaults`, seeded `rust = "cargo build"`, extensible from `init.lua`) via `ProjectKind::Rust` — **not** `Cargo`, see §24. The prompt **captures** its directory rather than re-resolving at accept time, so the command it offers and the directory it runs in cannot drift while the minibuffer waits. Still defaults cwd to the detected project root and parses Rust `-->` errors. Named limitation: after `pmacs ` the active buffer is dired's and pathless, so the cwd falls back to the process cwd — §8's execution-location model owns that, and the degradation stays coherent (no suggestion is offered for a directory with no detected Cargo project) | @@ -1482,16 +1482,26 @@ restartable help workspace, not a one-time modal wizard. ### Ground truth -**Grade: missing entirely.** +**Grade: partial — the cheap floor's first two items landed with Journey +Stage 1b-3.** -No welcome buffer, no tutorial, no first-run detection, no cheat sheet -reachable from inside the editor (`docs/keybindings.md` exists on disk -only). `C-h` is `buffer.delete-word-backward`; there is no help prefix -key and no `F1`. The sole discovery affordance is knowing to press -`M-x` (`builtin/keymaps/default.lua:141` — whose own header comment -calls it the "command palette"). The empty `*scratch*` buffer that -greets a new user says nothing (`EditorCore::new` sets an empty -status). +An unconfigured launch now greets in `*scratch*` naming `M-x` and four +real bindings, and `M-x help` renders a cheat sheet through the existing +`*help*` mechanism. The greeting happens in a launch-finalization seam +(`prepare_startup` → `EditorState::finalize_local_launch`) that runs +after config, after attach dispatch resolves to local, and after desktop +restore — no constructor is the right hook, because `EditorState::open` +calls `new` before resolving its target and the daemon constructs one +too. + +Still missing: no tutorial, no first-run detection, and **`C-h` still +deletes a word — deliberately.** It is bound to +`buffer.delete-word-backward` because non-kitty terminals cannot +disambiguate Ctrl+Backspace from Ctrl+H (both produce byte 0x08, +`builtin/keymaps/default.lua:78-86`), so rebinding it to a help prefix +would break Ctrl+Backspace on every legacy terminal. The help-prefix +question is a real trade for §20 Priority 4's discovery arc to weigh +across the whole command family, not an oversight. Note the dependency: five of the ten onboarding steps above currently lead somewhere broken or invisible (find a file — the mechanism is fixed @@ -1500,7 +1510,9 @@ diagnostic — silent-failure risk; view workers — undiscoverable; setting provenance — unanswerable). Onboarding is correctly sequenced *after* the P1/P4 fixes, but the cheap floor — a welcome buffer in `*scratch*` naming `M-x`, the keybinding cheat sheet as a help buffer, -and a help prefix decision — has no prerequisites at all. +and a help prefix decision — had no prerequisites at all. **The first +two are done** (Stage 1b-3); the third is deferred with its reason +recorded above. --- @@ -1563,19 +1575,20 @@ Establish the end-to-end workflow; treat regressions as release blockers. **State: runs to step 5; thin from step 6 (§2). Mostly wiring, and unusually cheap:** directory-argument handling (**done**: Journey Stage 1a); a find-file surface (**done**: #162 open-by-path, #165 -browsing); surfacing the LSP spawn failure with guidance (**in flight**: -Journey Stage 1b-2, §1.2); a compile keybinding + `cargo build`/`test` -default (**done**: Journey Stage 1b-1, #203, from the existing +browsing); surfacing the LSP spawn failure with guidance (**done**: Journey Stage +1b-2, #204, §1.2); a compile keybinding + `cargo build`/`test` default +(**done**: Journey Stage 1b-1, #203, from the existing `ProjectKind::Rust` — **not** `Cargo`, see §24); a terminal keybinding -(**done**: `C-c t`, #173); a welcome buffer. The journey acceptance +(**done**: `C-c t`, #173); a welcome buffer (**in flight**: Journey +Stage 1b-3). The journey acceptance suite (§19) is the ratchet that keeps it fixed — it **exists now** (`tests/journey_acceptance.rs`, Stage 1a), seeded with steps 2, 3 and 5, and carrying step 9 since #203. -Journey Stage 1b is the named remainder, and it splits: **1b-1 — the -compile binding + project-kind defaults — landed as #203**; **1b-2** -(LSP spawn guidance, step 6) is in flight; **1b-3**, the welcome buffer -(step 4), remains. +Journey Stage 1b is the named remainder, and it splits: **1b-1** (the +compile binding + project-kind defaults) landed as #203 and **1b-2** +(LSP spawn guidance, step 6) as #204; **1b-3**, the welcome buffer +(step 4), is in flight and completes the split. ### Priority 2: Make workspace and location explicit @@ -1648,8 +1661,9 @@ implementation — this list is direction, not commitment): #165's dired buffer rather than growing a second directory surface. **Stage 1b-1 — landed (#203)**: the compile binding and project-kind defaults, with the prompt capturing its directory rather than - re-resolving it at accept time. **Stage 1b-2 — in flight**: - LSP-failure surfacing. **Stage 1b-3 — remaining**: welcome buffer. + re-resolving it at accept time. **Stage 1b-2 — landed (#204)**: + LSP-failure surfacing. **Stage 1b-3 — in flight**: the welcome + buffer and `M-x help`. With it the 1b split is complete. 2. **Discovery surface** (P4): the describe/list/where-is command family, M-x rich rows, help unification, help prefix. 3. **Transient keymap layer** (§6): the overlay scope + lifetime diff --git a/builtin/commands/default.lua b/builtin/commands/default.lua index 2a13c21..48d6384 100644 --- a/builtin/commands/default.lua +++ b/builtin/commands/default.lua @@ -1259,6 +1259,21 @@ local function show_help_text(text) pmacs.window.switch_buffer(buf) end +--- Internal seam (journey Stage 1b-3): render `text` into the shared +--- `*help*` buffer. Exposed under the underscore convention so +--- `runtime/welcome.lua`'s `M-x help` renders through THIS mechanism +--- rather than growing a second help surface — `commands/default.lua` +--- loads before the runtime chunks, so the seam is present by then. +--- +--- Inherits this mechanism's two known gaps, recorded rather than +--- papered over: it writes with `delete`/`insert` instead of +--- `set_generated_contents` (so the buffer stays ordinarily editable and +--- keeps its undo history), and it finds `*help*` BY NAME, so a foreign +--- buffer of that name would be cleared. +function pmacs.editor._show_help(text) + show_help_text(text) +end + cmd { name = "editor.describe-command", description = "Prompt for a command name and render its description in *help*.", fn = function() diff --git a/builtin/runtime/welcome.lua b/builtin/runtime/welcome.lua new file mode 100644 index 0000000..fa55b3c --- /dev/null +++ b/builtin/runtime/welcome.lua @@ -0,0 +1,102 @@ +-- welcome.lua --- journey step 4: say something when the editor opens. +-- Framing: docs/journey-stage1b3-welcome-framing.md. +-- +-- `COHERENCE.md` §18 graded onboarding "missing entirely": no welcome, +-- no tutorial, no cheat sheet reachable from inside the editor. The sole +-- discovery affordance was knowing to press `M-x`. +-- +-- Split of responsibility with Rust: this file owns WHAT is said (the +-- entries and their rendering) and the `help` command; the Rust seam +-- `EditorState::finalize_local_launch` owns WHEN and WHERE — it alone +-- decides that this is a local, no-target launch whose `*scratch*` is +-- still untouched, and it clears the modified flag afterwards (there is +-- no Lua API for that, deliberately). + +pmacs.welcome = pmacs.welcome or {} + +--- The keys the welcome advertises, in display order. +--- +--- `keys` is EXACTLY what `pmacs.keymap.lookup` accepts, which is the +--- whole point of the shape: the acceptance suite checks every entry +--- resolves, so the welcome can never advertise a binding a later stage +--- removed. Scraping the rendered prose instead would be ambiguous — +--- `C-c c` is two chords and nothing in the text marks the boundary. +--- +--- Public so a user who rebinds can rebuild it from `init.lua`. +pmacs.welcome.entries = { + { keys = "C-x C-f", label = "open a file" }, + { keys = "C-c t", label = "terminal" }, + { keys = "C-c c", label = "build" }, + { keys = "C-x b", label = "switch buffer" }, +} + +-- Two entries per line, padded so the labels align. Kept deliberately +-- small: three lines total, because the greeting a user must delete +-- before typing should not be chrome. +local function entry_columns() + local width = 0 + for _, e in ipairs(pmacs.welcome.entries) do + if #e.keys > width then width = #e.keys end + end + local lines, pending = {}, nil + for _, e in ipairs(pmacs.welcome.entries) do + local cell = string.format("%-" .. width .. "s %s", e.keys, e.label) + if pending then + lines[#lines + 1] = " " .. string.format("%-24s", pending) .. cell + pending = nil + else + pending = cell + end + end + if pending then lines[#lines + 1] = " " .. pending end + return lines +end + +--- The welcome text, as written into an untouched `*scratch*`. +--- +--- `M-x` and `M-x help` are prose rather than entries: `M-x` is the +--- palette itself and `help` is a command name, so neither is a keymap +--- lookup. The acceptance checks the command exists instead. +function pmacs.welcome.text() + local lines = { "Welcome to pmacs. M-x runs any command; M-x help lists the keys." } + for _, line in ipairs(entry_columns()) do + lines[#lines + 1] = line + end + return table.concat(lines, "\n") .. "\n" +end + +-- --------------------------------------------------------------------- +-- M-x help +-- --------------------------------------------------------------------- +-- +-- The smallest version of §18's second item, included because the +-- welcome would otherwise point at nothing. It is the ROOT of the +-- eventual family: when the discovery arc adds `help.keys` and friends, +-- `help` stays the index they are reached from, so no rename is owed. +-- +-- Renders through `editor.describe-command`'s existing `*help*` +-- mechanism rather than growing a second help surface. +local function help_text() + local lines = { + "pmacs help", + "", + " M-x run a command by name", + } + for _, e in ipairs(pmacs.welcome.entries) do + lines[#lines + 1] = string.format(" %-18s %s", e.keys, e.label) + end + lines[#lines + 1] = "" + lines[#lines + 1] = " M-x editor.describe-command what a command does" + lines[#lines + 1] = " M-x editor.list-buffers every open buffer" + lines[#lines + 1] = "" + lines[#lines + 1] = "The full keymap reference is docs/keybindings.md." + return table.concat(lines, "\n") .. "\n" +end + +pmacs.command.define { + name = "help", + description = "Show the pmacs key and command cheat sheet.", + fn = function() + pmacs.editor._show_help(help_text()) + end, +} diff --git a/docs/active-work.md b/docs/active-work.md index 696ff0f..147f568 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -255,72 +255,129 @@ If it does not, stop and repair the remote/fetch configuration. never been enforced. Any CI job that compiles the `crdt` targets has to fix them first or it will be red on arrival. -## Journey lane (P1) — 1a and 1b-1 MERGED; 1b-2 PR OPEN; 1b-3 REMAINS +## Journey lane (P1) — 1a, 1b-1, 1b-2 MERGED; 1b-3 PR #205 OPEN -**Rewritten, not removed, at #203's merge.** Rule 4 removes a lane when -its ARC is done; the journey arc is not — 1b-2 is in flight and 1b-3 is -unframed. Stage 1a (#182/#183) and Stage 1b-1 (#203) are on `main` and -their durable facts are in `docs/agent-handoff.md` §1, which is rule 4's -precondition satisfied rather than deferred. +**Rewritten, not removed.** Rule 4 removes a lane when its ARC is done; +the journey arc is not — 1b-3 is the last stage of the 1b split and is +still open. Stages 1a (#182/#183), 1b-1 (#203) and **1b-2 (#204)** are +all on `main` and their durable facts are in `docs/agent-handoff.md` §1, +which is rule 4's precondition satisfied rather than deferred — so their +per-stage blocks are gone from here rather than left to rot as +"PR OPEN". -**#203's merge obligation is DISCHARGED**: `COHERENCE.md` §2's step-9 -row now reads **Works**, §2's keybinding-inversion paragraph records all -three examples answered (the quote itself deliberately unchanged), §20 -Priority 1 and its arc list say "landed", and the handoff bullet says -LANDED. That flip rides *this* branch rather than a standalone docs PR, -because #204 already touches all three files and a separate PR would -re-conflict on every merge. +**#203's merge obligation is DISCHARGED** on this branch: `COHERENCE.md` +§2's step-9 row reads **Works**, §2's keybinding-inversion paragraph +records all three examples answered (the quote itself deliberately +unchanged), §20 Priority 1 and its arc list say "landed", and the +handoff bullet says LANDED. **#204's is discharged too**: §2's step-6 +row names it as landed and stays **Partial** for a reason that landing +did not touch — a server that starts and then *crashes* is still +unsurfaced. Both rode this branch rather than standalone docs PRs, +which would have re-conflicted on every merge. -- **Branch `journey-stage1b2-lsp-guidance`**, worktree - `../pmacs-journey-1b2`, based on `githubsucks/main` @ `fbcf235`, - **integrated with `main` @ `1f290d5` (#203)**. - `docs/journey-stage1b2-lsp-guidance-framing.md` revision 4, three - review rounds closed (round 1: two blocking, three major, one minor; - round 2: two blocking, two cleanups; round 3: one blocking; all - accepted). **Implemented; PR #204 open.** -- **What it is.** `COHERENCE.md` §1.2's canonical silence, journey step - 6: a preconfigured-but-missing language server now reports with - guidance, marks the modeline `LSP:!`, and appears in `M-x lsp.status`. -- **Half of it was already built and unwired.** - `LspManager::status_buffer_text()` and `last_error()` have existed - since M4.8, exposed to Lua and tested, with **no production caller** - and no `*lsp*` buffer, while several `src/lsp.rs` and `src/project.rs` - doc comments refer to that buffer as though it existed. -- **The reporting shape was already adopted twice in `lsp.lua` itself** - (root resolvers, notification subscribers). The canonical case was - silent because nobody had converted it — this finishes an adoption. -- **`COHERENCE.md` §1.2's frequency note was wrong, and it decided the - design.** `LspManager::spawn` returns early *before* both - `status_tracker.ensure` and `clients.insert`, so a failed spawn leaves - **no record**, `pmacs.lsp.list()` cannot see it, and the affinity loop - re-spawns: the real rate is **once per file open**, not once per - project root. Hence **memoize the report, not the failure**. -- **The affinity key is `(language, key_uri)`, and `key_uri` is nil for - markerless files**, which deliberately share one server per language. - Lua cannot index by nil (`t[nil]` raises), so one encoding function - serves both tables with a `u`/`n` discriminator no URI can collide - with. -- **Three tables, three lifetimes**, plus a buffer-keyed projection for - the modeline — that provider runs for every window on every paint, so - deriving an affinity key inside it would invoke user root resolvers - during painting. **A success sweeps every projection sharing the key**, - and the projection has its own `pmacs.buffer.on_removed` teardown - because nothing existing reaches it (`attachments_under` iterates - `attachments`, and a failed buffer has none by construction). -- **ON MERGE of #204, flip the step-6 grade.** §2's step-6 row stays - **Partial** while the PR is open, per §25's landed-evidence rule, and - says so in the row. -- **Stage 1b-3 (welcome buffer, step 4) is unframed** — the last of the - 1b split. -- Recovery from a clean checkout — **the two-argument form does not - work** (`git worktree add ` fails with - `fatal: invalid reference`): +### Stage 1b-3 — IMPLEMENTED, PR #205 open + +- **Branch `journey-stage1b3-welcome`**, worktree `../pmacs-journey-1b3`, + based on `githubsucks/main` @ `1f290d5`, **integrated with `main` @ + `5376af1`** (#204). **Implemented; PR #205 open.** + `docs/journey-stage1b3-welcome-framing.md` revision 4, three + review rounds closed (round 1: four findings; round 2: two acceptance + holes plus a doc correction; round 3: a visibility mismatch and an + unobservable assertion; all accepted). The last of the 1b split + (1b-1 landed #203, 1b-2 landed #204 at `5376af1`). +- **What it is.** Journey step 4 / `COHERENCE.md` §18: a fresh `pmacs` + greets the user with an empty buffer, an empty status line, and no + indication that `M-x` exists. The stage renders a three-line welcome + into `*scratch*` and adds a minimal `M-x help`. +- **`EditorState::new()` is NOT the no-argument entry point**, and + revision 1 rested every criterion on the assumption that it was. + `EditorState::open` calls it *before* resolving the target + (`src/editor.rs:944`), the daemon constructs one too, user config runs + *inside* it, and desktop restore happens much later + (`restore_desktop_if_armed`, `:3637`, inside `run()`'s `RunLocal` + arm). The stage now adds a **launch-finalization seam** called right + after desktop restore, with `had_file` — already threaded to that + point for the same kind of question — as the no-target signal. + **Round 2 closed the wiring hole rather than disclaiming it.** + Revision 2 called `run()`'s call to the seam an untestable residual — + but deleting that one line left every proposed pin green while + shipping no welcome, because the pins called the seam by hand. The + terminal-free prefix of `run()` (everything before `Frontend::new()`) + is now extracted into `prepare_startup`, which `run()` delegates to + and the pins drive. It is **`pub`**, not `pub(crate)`: the journey + suite is a separate integration crate and cannot reach crate-private + items, and `run`/`new`/`open`/`install_state_dirs`/ + `restore_desktop_if_armed` are already public, so this completes that + surface rather than widening it for a test. The pin must assert + desktop restore was **unarmed** (an ambient `init.lua` calling + `desktop_mode(true)` would otherwise change the result) and must + assert buffer content only, since `install_state_dirs` resolves real + XDG/`PMACS_STATE_HOME` roots that tests cannot override + (`set_var` is unsafe and forbidden). +- **The step-2 pin is NOT amended.** Revision 1 analysed a status-line + welcome and then chose `*scratch*`, but kept the amendment — an + internal contradiction. The status stays empty, so the pin stays true, + and "no error text" has no defined predicate over an unstructured + status string anyway. +- **`C-h` is NOT free, and the reason is load-bearing.** It is bound to + `buffer.delete-word-backward` because non-kitty terminals cannot + disambiguate Ctrl+Backspace from Ctrl+H — both produce byte 0x08 + (`builtin/keymaps/default.lua:78-86`). Rebinding it to a help prefix + would break Ctrl+Backspace on every legacy terminal, so §2's step-4 + row calling it an oversight is wrong: it is a deliberate trade. The + help-prefix decision is deferred to §20 Priority 4's discovery arc + with the constraint recorded. +- **`*help*` already exists** (`builtin/commands/default.lua:1226`, + `show_help_text`, reused buffer, buffer-local `q`), used by + `editor.describe-command` / `-setting`, over `src/help.rs`'s + renderers and link resolution. Two gaps recorded rather than + inherited silently: it writes with `buf:delete`/`buf:insert` instead + of `set_generated_contents`, and it is **found by name**, so a + foreign `*help*` would be cleared. +- **Deliberately NOT `set_generated_contents` for the welcome** — that + lifts read-only, discards history and marks the buffer generated, all + wrong for a buffer step 5 requires the user to type into immediately. + The one place not adopting that invariant is correct, stated so a + later audit does not "fix" it. +- **Step 4 stays Partial, but there IS a §25 obligation** — revision 1 + claimed there was none. The scorecard's row 18 and §18's ground truth + both read **Missing**, and both become false on merge: they move to + **Partial**. A stage can be too small to flip its journey step while + still falsifying a "missing entirely" grade. +- **The welcome renders from a structured entry list**, not from scraped + prose: `M-x help` mixes a chord with a command name and `C-c c` is two + chords whose boundary prose does not mark, so the binding checks run + `pmacs.keymap.lookup` over the same list that renders the text. +- **`pmacs.command.invoke` is NOT the M-x path** — it is the + programmatic API. M-x is `editor.execute-command`, a minibuffer with + the `commands` completion source that calls `invoke_interactive` on + accept. The `M-x help` pin dispatches the chord, enters the name and + accepts — and asserts **`pmacs.minibuffer.selected() == "help"` + BEFORE RET**, because a selected candidate shadows typed text (dired + refused a completion source for exactly this reason) and `accept()` + does `session.take()`, so nothing about the accepted value survives + afterwards. +- **Integrated with `main` @ `5376af1`** (#204). The journey suite's + conflict was additive on both sides — step 4 (this lane) and step 6 + (#204) — and both are kept: **44 pins, covering steps 2, 3, 4, 5, 6 + and 9**. +- **§18 and the scorecard move Missing → Partial ON THIS PR**, per §25; + §2's step-4 row stays Partial because `C-h` and the tutorial remain. + No deferred flip is owed at merge — unlike 1b-1's. +- **Bites, all directed.** Deleting the production `run()` wiring fails + the two greeting pins — **the mutation revision 2's design would have + survived entirely**. Removing `mark_clean` fails the editable/clean + pin; the `had_file` guard fails the directory pin (not the file pin, + because a file buffer is active by then while the dired listing is + async); the emptiness guard fails the existing-content pin; the + active-buffer requirement fails the backgrounded pin; and advertising + an unbound key fails the binding pin. ```sh git fetch githubsucks - git worktree add ../pmacs-journey-1b2 \ - -b journey-stage1b2-lsp-guidance \ - githubsucks/journey-stage1b2-lsp-guidance + git worktree add ../pmacs-journey-1b3 \ + -b journey-stage1b3-welcome \ + githubsucks/journey-stage1b3-welcome ``` ## Generated-buffer immutability lane (Arc: workbench primitives) — STAGE 1 MERGED; STAGE 2 IS NEXT @@ -403,7 +460,7 @@ compatible. githubsucks/test-ambient-config-isolation ``` -## Reap-ledger silent failures — IMPLEMENTED, PR OPEN +## Reap-ledger silent failures — MERGED (#202); kept for its parked follow-ons - **Branch `reap-ledger-silent-failures`**, worktree `../pmacs-reap-ledger`, based on `githubsucks/main` @ `22df6ab`. diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index f7058d0..d4542f4 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -300,12 +300,45 @@ commands, read `docs/active-work.md` immediately after this file. disagree — and it still establishes no identity, because it is read inside the same read-then-act window and no portable mechanism closes that for a *group* (`pidfd` covers a process; macOS has neither). -- **Journey arc (P1) — Stage 1b-2 IMPLEMENTED, PR open** +- **Journey arc (P1) — Stage 1b-3 IMPLEMENTED, PR open** + (`docs/journey-stage1b3-welcome-framing.md`, rev 4, three review + rounds). The last of the 1b split. An unconfigured launch greets in + `*scratch*`; `M-x help` renders a cheat sheet. **§18 and the scorecard + move Missing → Partial**; §2's step-4 row stays Partial. + - **No constructor is the startup hook.** `EditorState::open` calls + `new` *before* resolving its target, the daemon constructs one too, + `init.lua` runs inside `new`, and desktop restore happens later + still. `run()`'s terminal-free prefix is now `prepare_startup`, and + the greeting is its last step. + - **Extraction is what makes wiring testable.** With the seam called + by hand from tests, deleting the production call left every + assertion green while shipping no welcome — the "guard with no + production caller" shape. `prepare_startup` is `pub` because the + journey suite is a separate crate and the rest of the sequence + (`run`, `new`, `open`, `install_state_dirs`, + `restore_desktop_if_armed`) is already public. + - **`C-h` is not free**, and §2's step-4 row used to imply it was: it + deletes a word because non-kitty terminals cannot disambiguate + Ctrl+Backspace from Ctrl+H (both byte 0x08). Rebinding it to a help + prefix breaks Ctrl+Backspace on every legacy terminal. Deferred to + the discovery arc **with the reason recorded**. + - **Deliberately NOT `set_generated_contents`** — it would lift + read-only, discard history and mark the buffer generated, all wrong + for a buffer step 5 requires the user to type into immediately. The + one place not adopting that invariant is correct. + - **`pmacs.command.invoke` is not the M-x path.** M-x is + `editor.execute-command`, a minibuffer with the `commands` + completion source calling `invoke_interactive` on accept — and + because a selected candidate shadows typed text while `accept()` + does `session.take()`, the pin asserts + `pmacs.minibuffer.selected() == "help"` **before** RET. +- **Journey arc (P1) — Stage 1b-2 LANDED (#204)** (`docs/journey-stage1b2-lsp-guidance-framing.md`, rev 4, three review rounds). `COHERENCE.md` §1.2's canonical silence: a preconfigured language server that is not installed now reports with guidance, marks - the modeline `LSP:!`, and appears in `M-x lsp.status`. Per §25 the - step-6 grade flips only on merge. + the modeline `LSP:!`, and appears in `M-x lsp.status`. §2's step-6 row + **stays Partial** for a reason that landing did not touch: a server + that starts and then *crashes* is still unsurfaced. - **`status_buffer_text()` had existed since M4.8, exposed to Lua and tested, with no production caller and no `*lsp*` buffer** — several `src/lsp.rs` and `src/project.rs` doc comments referred to that diff --git a/docs/journey-stage1b3-welcome-framing.md b/docs/journey-stage1b3-welcome-framing.md new file mode 100644 index 0000000..4b234b0 --- /dev/null +++ b/docs/journey-stage1b3-welcome-framing.md @@ -0,0 +1,601 @@ +# Journey Stage 1b-3 — say something when the editor opens + +**Status: framing, rev 4 — awaiting review round 4.** +**Serves `COHERENCE.md` §2 (the golden journey, step 4), §18 +(onboarding), §19, §20 Priority 1.** + +## 0. Revision history + +- rev 4 (2026-07-31) — review round 3. Two findings, both accepted. + - **The pinned API was crate-private but the pin was external.** + `tests/journey_acceptance.rs` is a separate integration crate and + cannot call `pub(crate) fn prepare_startup` or match a + `pub(crate) enum Startup`, so rev 3's acceptance 1 was + uncompilable. Resolved by making both **`pub`** — which is + consistent with the startup surface already exported (`run`, + `new`, `open`, `install_state_dirs`, `restore_desktop_if_armed` + are all `pub`), not a widening invented for a test. §3.2b also now + states the **isolation** the pin needs, since `prepare_startup` + deliberately calls `install_state_dirs` and can reach desktop + restore. + - **The M-x pin still had an "or", and half of it was impossible.** + `Minibuffer::accept` does `self.session.take()` and resolves + against the *selected candidate* (`src/minibuffer.rs:334-337`), so + after RET neither the session nor the typed contents survive to be + asserted. §4.4 now specifies **one** observable, available before + RET: `pmacs.minibuffer.selected()` + (`src/lua_bindings/mod.rs:14030`) must equal exactly `"help"`. +- rev 3 (2026-07-31) — review round 2. Two acceptance holes and one doc + correction; all accepted. + - **The real startup wiring was still unpinned, and rev 2 knew it.** + Rev 2 named the gap in §3.2a and then accepted it as "residual", + which is worse than missing it: deleting the sole `run()` call to + `finalize_local_launch` would leave **every** proposed pin green + while shipping no welcome at all. Pin 1 called the seam by hand and + pin 10 only proved constructors were blank. §3.2b now **extracts the + non-terminal part of `run()`** into a production helper that `run()` + delegates to, and the pins drive that helper — terminal takeover + stays outside the test, which is the only part that ever needed to + be. + - **Acceptance 4 was not the M-x path.** `pmacs.command.invoke` + (`src/lua_bindings/mod.rs:6062`) is the *programmatic* API. M-x is + `editor.execute-command`, which opens a minibuffer with the + `commands` completion source and calls **`invoke_interactive`** only + once the user accepts (`builtin/commands/default.lua:743-755`). The + pin now dispatches `M-x`, enters `help`, and accepts — and §4 names + the candidate-shadowing hazard that makes "type and press RET" + non-trivial with a completion source attached. + - **1b-2 has landed.** Rev 2's history said so while §8 and the ledger + still called #204 open; `githubsucks/main` @ `5376af1` is the merge. + Corrected. +- rev 2 (2026-07-31) — review round 1. Four findings, all accepted, all + verified in the tree first. §7's three questions are answered and + folded into the design. + - **The startup seam was wrong.** `EditorState::new()` is not the + no-argument entry point: `EditorState::open` calls it + (`src/editor.rs:944`) *before* handling the file or directory, the + daemon constructs it too, user config runs *inside* it, and desktop + restore happens later still (`:3637`). Greeting from `new()` would + greet a daemon, greet before a file argument replaced the buffer, + and run before config or restore could put anything in `*scratch*`. + §3.2 now defines a **launch-finalization seam** that runs after + config, after attach dispatch resolves to local, and after desktop + restore — and §4 pins the three paths that must *not* greet. + - **The status-pin amendment was unnecessary and weakened a stronger + ratchet.** Rev 1 analysed a status-line welcome in §2.2 and then + chose `*scratch*` in §3.2, but kept the amendment — an internal + contradiction. With the chosen design + `journey_step2_launches_unconfigured_into_scratch` stays correct as + written, and "no error text" has no defined predicate over an + unstructured status string anyway. **The pin is untouched**; the + welcome gets its own row. + - **"No §25 landed-evidence obligation" was false.** The scorecard + row 18 reads **Missing** and §18's ground truth says "missing + entirely" — both are audited claims that a landed welcome plus + interactive help changes. §6 now records the obligation: §18 and the + scorecard move **Missing → Partial** on merge, while §2's step-4 row + stays Partial. + - **Acceptance 2 could not be implemented as written.** "Parse the key + sequences out of the rendered text" is ambiguous: `M-x help` mixes a + chord with a command name, and multi-chord sequences like `C-c c` + cannot be scraped unambiguously from prose. §3.1 now specifies a + **structured entry list** that both renders the text and drives the + binding checks. +- rev 1 (2026-07-30) — first framing. Scouted against `githubsucks/main` + @ `1f290d5` (Journey Stage 1b-1, #203). + +## 1. What this stage is + +The last of the 1b split. `COHERENCE.md` §20 named three things: +**1b-1** (compile binding + defaults, landed as #203), **1b-2** (LSP +spawn guidance, landed as #204), and this — the welcome buffer. + +§18 is unusually specific about the size of it: + +> the cheap floor — a welcome buffer in `*scratch*` naming `M-x`, the +> keybinding cheat sheet as a help buffer, and a help prefix decision — +> has no prerequisites at all. + +**This stage takes the first of those three.** §2's step-4 row lists all +three as what is missing, so §5 is explicit that step 4 does **not** +reach Works here, and says what remains. + +## 2. Ground truth + +Verified in the tree at `1f290d5`. + +### 2.1 A fresh launch says nothing, and the audit's citation is right + +`EditorCore::new` sets `status: String::new()` (`src/editor_core.rs:659`) +and creates `*scratch*` (`:629`). A user who runs `pmacs` with no +arguments and no config gets an empty buffer, an empty status line, and +no indication that `M-x` exists. + +`builtin/keymaps/default.lua`'s own header comment calls `M-x` the +"command palette" — the sole discovery affordance in the product is +knowing to press it. + +### 2.2 `EditorState::new()` is not the no-argument entry point + +Rev 1 assumed it was, and every acceptance criterion rested on that. +It is wrong four ways, all in `src/editor.rs`: + +- **`EditorState::open` calls it first** (`:944`), *then* resolves the + target. So a greeting inside `new()` fires before the file or + directory is handled, and the `*scratch*` buffer survives that + handling — `replace_active_buffer` reassigns the window's buffer and + removes nothing. +- **The daemon constructs one too**, so a greeting there is written into + a session no human is looking at. +- **User `init.lua` runs inside `new()`**, so a greeting written there + precedes anything config might put in `*scratch*`. +- **Desktop restore happens much later** — `restore_desktop_if_armed` + at `:3637`, inside the `RunLocal` arm of `run()`, after + `install_state_dirs` and after attach dispatch. A restored session can + populate `*scratch*`, and a greeting from `new()` would already have + written into it. + +`run()`'s shape is what defines the correct seam: + +```rust +let mut state = match target { + Some(path) => EditorState::open(path)?, // config runs inside + None => EditorState::new(), // …and here +}; +state.install_state_dirs(); +let requested = state.lua_host.take_requested_attach(); +match dispatch_attach(requested) { + RunLocal => { + state.restore_desktop_if_armed(had_file); // :3637 + // ← the only correct place to greet +``` + +Note `had_file` is already threaded to exactly this point for exactly +this kind of question ("a positional argument means *open this*, not +*restore my desktop*"), so the no-target signal does not need inventing. + +### 2.2a The step-2 pin stays as it is + +Rev 1 proposed amending +`journey_step2_launches_unconfigured_into_scratch`'s +`assert!(status(&s).is_empty())`. **That was left over from a +status-line design rev 1 then rejected**, and keeping both was an +internal contradiction. + +The welcome goes into `*scratch*` (§3.2), so the status line stays empty +and the pin stays true as written. It is also the stronger assertion: +"no *error* text" has no defined predicate over an unstructured status +string, so replacing an exact check with a fuzzy one would weaken the +ratchet to buy nothing. **The pin is untouched**; the welcome gets its +own row (§4). + +### 2.3 `C-h` is not free, and the reason is load-bearing + +§18 lists "a help prefix decision" as part of the cheap floor. It is +cheap to *decide* and not cheap to get wrong: + +```lua +-- builtin/keymaps/default.lua:78-86 +-- Why we also bind C-h: most terminals (anything not implementing the +-- kitty keyboard protocol) cannot disambiguate Ctrl+Backspace from +-- Ctrl+H — both legacy paths produce byte 0x08 … +bind("C-h", "buffer.delete-word-backward") +``` + +**Rebinding `C-h` to a help prefix would break Ctrl+Backspace on every +non-kitty terminal**, because those terminals cannot tell the two apart. +§2's step-4 row says "`C-h` deletes a word" as though it were an +oversight; it is a deliberate trade with a stated rationale. + +This stage therefore **does not touch `C-h`** (§5), and the framing +records the constraint so the discovery arc inherits the reason rather +than rediscovering it. + +### 2.4 There is already a `*help*` buffer mechanism + +`builtin/commands/default.lua:1226-1260` has `show_help_text(text)`: a +single reused `*help*` buffer, found by name, with a buffer-local `q` → +`buffer.kill-this`. `editor.describe-command` and +`editor.describe-setting` both render through it. `src/help.rs` supplies +`render_command` / `render_key` / `render_buffer` / `render_mode` / +`render_view` plus link resolution (`link_at`, `follow_link_at`). + +**Two facts about it that matter to any consumer:** + +- It writes with `buf:delete` + `buf:insert`, **not** + `pmacs.buffer.set_generated_contents`. It is one of the writer + mechanisms that has not adopted the generated-buffer write invariant, + so its buffer stays ordinarily editable and keeps its undo history. + Riding it inherits that; fixing it is not this stage's job, but a + stage that renders into it should not claim read-only guarantees it + does not have. +- It is **found by name**, which is the adoption hazard `listview` and + dired both refused. A foreign `*help*` buffer would be cleared. + +### 2.5 A modified `*scratch*` does not block quitting + +`editor.quit` runs the `editor.before-quit` hook and then `ed.quit()` +(`builtin/commands/default.lua:250-256`). There is no unsaved-buffer +confirmation, so welcome text left in `*scratch*` cannot strand a user +at exit. That removes the strongest objection to putting text in the +buffer, and it is stated because it is the kind of thing that would +otherwise be assumed in either direction. + +## 3. Design + +### 3.1 What the welcome says, and the shape it is built from + +**Three lines** (Q#W1), naming `C-c c` and `C-c t` (Q#W2): + +``` +Welcome to pmacs. M-x runs any command; M-x help lists the keys. + C-x C-f open a file C-c t terminal + C-c c build C-x b switch buffer +``` + +**The text is rendered from a structured list, never scraped back out +of it.** Rev 1 said the acceptance would "parse the key sequences out of +the rendered text"; that cannot be implemented reliably — `M-x help` +puts a chord and a command name in one phrase, and `C-c c` is two chords +whose boundary prose does not mark. + +One list is the single source for both the rendering and the checks: + +```lua +-- Each entry is { keys = "", label = "" }. +-- `keys` is EXACTLY what `pmacs.keymap.lookup` accepts, so a binding +-- check is a lookup, not a guess about where a chord ends. +pmacs.welcome.entries = { + { keys = "C-x C-f", label = "open a file" }, + { keys = "C-c t", label = "terminal" }, + { keys = "C-c c", label = "build" }, + { keys = "C-x b", label = "switch buffer" }, +} +``` + +`M-x` and `M-x help` are prose in the first line rather than entries: +`M-x` is the palette itself and `help` is a command name, so neither is +a keymap lookup. The acceptance checks the command exists instead (§4.3). + +**Every entry must resolve through `pmacs.keymap.lookup`**, asserted as +a property over the list (§4.2). That is what stops the welcome becoming +documentation drift with a user attached — and it is why the list is +public: a user who rebinds can rebuild it. + +### 3.1a `M-x help` + +Named `help` (Q#W3). It renders the cheat sheet through the existing +`show_help_text` mechanism (§2.4) and is the **root of the eventual +family**: when the discovery arc adds `help.keys`, `help.commands` and +friends, `help` remains the index they are reached from, so no +deprecation is owed. + +### 3.2 Where it goes — `*scratch*`, not the status line + +The status line is the wrong vehicle even though §2's audit cites it: +it is overwritten by the next status write, and a user who blinks loses +the only pointer to `M-x`. §18 says "a welcome buffer in `*scratch*`", +and that is right for the reason the audit itself gives — *the empty +`*scratch*` buffer is what greets the user*. + +### 3.2a **When** it goes — a launch-finalization seam + +Per §2.2, no existing constructor is the right hook. The stage adds one +named seam: + +```rust +/// Final step of a LOCAL, no-target launch, after config, attach +/// dispatch and desktop restore have all had their say. The only +/// caller is `run()`'s `RunLocal` arm; the only thing it does is +/// conditionally greet an untouched `*scratch*`. +pub fn finalize_local_launch(&mut self, had_file: bool) +``` + +called from `run()` immediately after `restore_desktop_if_armed(had_file)`. + +It greets only when **all four** hold, and each excludes a case §2.2 +showed rev 1 would have got wrong: + +1. **`had_file` is false** — a positional argument means "open this". +2. **The session is local** — it is inside the `RunLocal` arm, so a + daemon or an attach hand-off never reaches it. +3. **`*scratch*` is the active buffer** — desktop restore may have put + something else in front. +4. **`*scratch*` is empty** — never overwrite config's or a restored + session's content. + +And it leaves the buffer **unmodified**, so the greeting does not look +like unsaved work in the modeline. §2.5 shows quitting is not blocked +either way; this is about not lying. + +### 3.2b The wiring is pinned by extracting it, not by disclaiming it + +Rev 2 stopped here and called `run()`'s call to the seam an untestable +residual. That is not good enough: **delete that one line and every pin +rev 2 proposed still passes**, because pin 1 called the seam by hand and +pin 10 only asserted that constructors greet nothing. The product would +ship with no welcome and a green suite — the exact shape of "a guard +with no production caller passes every direct-call test". + +The boundary is already clean. Everything in `run()` from +`install_panic_hook()` through the end of the attach-dispatch `match` is +terminal-free; `Frontend::new()?` is where takeover begins. So that +prefix becomes a helper: + +```rust +/// Everything `run` does before it touches the terminal: construct from +/// the target, install state dirs, dispatch the init-time attach +/// request, and — on the local path — restore the desktop and finalize +/// the launch. +/// +/// Extracted so the local-startup sequence is testable. `run` adds only +/// `Frontend::new()` and the event loop. +pub fn prepare_startup(file: Option) -> io::Result; + +pub enum Startup { + /// Ready to enter the local TUI loop — desktop restored, launch + /// finalized. + Local(EditorState), + /// A hand-off the caller must perform; it takes the terminal, so it + /// stays out of here. + HandOff(crate::attach_dispatch::AttachDispatch), +} +``` + +**`pub`, not `pub(crate)`.** The journey ratchet lives in +`tests/journey_acceptance.rs`, a separate integration crate that cannot +reach crate-private items — rev 3 specified `pub(crate)` and an external +pin, which does not compile. Exporting it is also consistent rather than +expedient: `run`, `EditorState::new`, `EditorState::open`, +`install_state_dirs` and `restore_desktop_if_armed` are **already** +`pub`, so the startup sequence is public surface and this is the piece +that was missing from it. (The alternative — keep it private and move +the pin into `src/editor.rs`'s unit tests — was rejected because §19 +wants the journey row in the journey suite.) + +**Isolation the pin requires, stated because `prepare_startup` reaches +real state.** It calls `install_state_dirs`, which resolves +`user_history_dir()` / `user_state_dir()` from `PMACS_STATE_HOME` and +XDG (`src/state.rs:54`). Tests cannot set those themselves — +`std::env::set_var` is `unsafe` and this crate forbids it — so the pin +inherits the same ambient-root requirement as every other +editor-constructing test: the five bootstrap-storage variables +controlled externally, which is already the standing local-gate +workaround and is what #201's framing owns fixing. + +Two consequences the pin must respect: + +- **Assert buffer content only.** No assertion may depend on a state + directory being present, absent, or writable, so an ambient root + cannot change the verdict. +- **Assert that desktop restore is unarmed**, rather than assuming it. + `restore_desktop_if_armed` is gated on `DesktopRestoreArmed`, which + only `pmacs.session.desktop_mode(true)` sets from config. A developer + whose real `init.lua` arms desktop mode would otherwise get a restored + `*scratch*` and a silently different result. + +`run()` becomes `match prepare_startup(file)? { … }` plus the loop it +already has. The attach arms keep calling `run_attach*` from `run()`, +because those take over the terminal. + +**What this buys:** the welcome pin now drives `prepare_startup(None)` — +production code, the same call `run()` makes — so deleting the +`finalize_local_launch` call inside it turns the pin red. The only thing +still outside a test is `Frontend::new()` and the event loop, which is +where terminal takeover genuinely lives and which this stage does not +touch. + +**What remains true and small:** `run()` could in principle stop calling +`prepare_startup`. But then it has no `EditorState` at all and does not +compile — the risk collapses to nothing a reviewer could miss, which is +the difference between this and rev 2's disclaimer. + +### 3.3 What it must not do + +- **Not read-only.** Step 5 is "edit immediately"; a read-only greeting + would break the journey one step after it starts. The user types over + it, as in Emacs. +- **Not a new buffer.** A `*welcome*` buffer displayed instead of + `*scratch*` would change what step 2 asserts and give the user a + buffer whose only purpose is to be closed. +- **Not `set_generated_contents`.** That lifts the rope's read-only, + discards history, and marks the buffer generated — all wrong for a + buffer the user is meant to edit immediately. This is the one place + where *not* adopting the generated-buffer invariant is correct, and it + is stated so a later audit does not "fix" it. + +### 3.4 `M-x help` is deliberately minimal + +It exists only because the welcome would otherwise point at nothing. +**This is the smallest possible version of §18's second item**: the full +cheat sheet, `where-is`, `describe-key` and the help-prefix question all +belong to §20 Priority 4's discovery arc (§5). + +## 4. Acceptance + +**N** = new behaviour, must fail on full revert. **P** = preservation, +falsified by a named mutation. + +1. **N — journey step 4, through the production startup path.** + In `tests/journey_acceptance.rs`, drive the **public** + `prepare_startup(None)` (§3.2b) — the same call `run()` makes — and + assert the returned `Startup::Local` state has `*scratch*` active, + **non-empty**, naming `M-x`. This is the ratchet row. + *Preconditions it must assert rather than assume* (§3.2b): that + desktop restore was **unarmed**, so an ambient `init.lua` calling + `desktop_mode(true)` cannot silently change the result. It asserts + buffer content only — never anything about state directories. + *Rev 1 claimed `EditorState::new()` was the entry point (§2.2 shows + it is shared with `open()` and the daemon); rev 2 then called the seam + by hand, which left the wiring unpinned (§3.2b). This drives neither + — it drives what `run()` drives.* + **Falsified by deleting the `finalize_local_launch` call inside + `prepare_startup`**, which is the mutation rev 2's pins survived. +2. **N — every entry the welcome names is bound.** For each + `pmacs.welcome.entries` item, `pmacs.keymap.lookup(entry.keys)` + resolves. A property over the **structured list** (§3.1), not a scrape + of prose — so it cannot rot when the wording changes, and it fails + loudly if a later stage unbinds something the welcome advertises. + Includes a precondition that the list is non-empty, or the loop is + vacuous. +3. **N — the rendered text contains every entry's `keys` and `label`.** + This is what ties the list to the thing the user actually sees; pin 2 + alone would pass if rendering dropped an entry. +4. **N — `M-x help` renders the cheat sheet**, reached the way a user + reaches it. `pmacs.command.invoke` is the **programmatic** API + (`src/lua_bindings/mod.rs:6062`) and is *not* the M-x path: M-x is + `editor.execute-command`, which opens a minibuffer with the + `commands` completion source and calls **`invoke_interactive`** only + on accept (`builtin/commands/default.lua:743-755`). So the pin + **dispatches `M-x` as a key**, enters `help`, and accepts — then + asserts `*help*` contains the entries' key sequences. + **One observable, chosen here rather than left to implementation.** + With a completion source attached, a selected candidate shadows typed + text and the minibuffer selects candidate 0 whenever the list is + non-empty (dired's `C-x d` comment records this as S0-1/S0-4 and + refuses a source for that reason), so typing `help` and pressing RET + can accept a *different* command. + + The accepted value cannot be recovered afterwards: + `Minibuffer::accept` does `self.session.take()` and resolves against + the selected candidate (`src/minibuffer.rs:334-337`), so after RET + neither the session nor the contents survive. + + So the assertion happens **before** RET: + + ``` + dispatch M-x → type "help" + assert pmacs.minibuffer.selected() == "help" -- exact, no fallback + dispatch RET + assert *help* contains every entry's `keys` + ``` + + `pmacs.minibuffer.selected()` returns the selected candidate string + (`src/lua_bindings/mod.rs:14030`). If the completion source has + selected something else, the pin fails **there**, naming what was + actually selected — instead of passing on a help buffer some other + command produced. +5. **P — the buffer is editable and unmodified.** After greeting, + `*scratch*` reports unmodified; typing a character inserts it, so + step 5 works from the first frame. Targeted mutation: rendering + through `set_generated_contents`, which would lift read-only, discard + history, and make the insert fail. +6. **P — a file target does not greet.** `EditorState::open(file)` then + `finalize_local_launch(true)`: no welcome anywhere, and the file is + active. Targeted mutation: dropping the `had_file` guard. +7. **P — a directory target does not greet.** Same with a directory, so + Stage 1a's dired listing is untouched. Separate pin because the + directory path reaches `*scratch*` differently — the bootstrap + replaces the window's buffer and `replace_active_buffer` removes + nothing, so the scratch buffer still exists to be wrongly greeted. +8. **P — a non-empty `*scratch*` is never overwritten.** Write to + scratch (standing in for config or a restored desktop), then run the + seam: the content survives byte for byte. Targeted mutation: dropping + the emptiness guard. +9. **P — a non-active `*scratch*` is not greeted.** Switch the active + buffer away, then run the seam: scratch stays empty. Stands in for + desktop restore having put something else in front. Targeted + mutation: dropping the active-buffer guard. +10. **P — the three constructors greet nothing on their own.** + `EditorState::new()`, `open(file)` and `open(dir)`, each with no + seam call, leave `*scratch*` empty. **This is what makes the seam the + only writer**, and it is the pin that would catch a greeting + smuggled back into a constructor — including the daemon's. + *Necessary but not sufficient on its own: pin 1 is what proves the + seam is actually reached in production.* +11. **P — the step-2 pin is unchanged and still passes.** + `journey_step2_launches_unconfigured_into_scratch` keeps asserting + an empty status, because the welcome goes to the buffer (§2.2a). Not + a new test — a stated requirement that this stage does not touch it. + +**What is not pinned, and why it no longer matters:** `Frontend::new()` +and the event loop. Those are terminal takeover, this stage does not +touch them, and everything before them is now inside `prepare_startup` +and driven by pin 1 (§3.2b). Rev 2 left the whole local-startup wiring +untested and said so; that hole is closed rather than disclaimed. + +## 5. Deferred, and why + +- **The help prefix / `C-h`.** §2.3: rebinding it breaks Ctrl+Backspace + on every non-kitty terminal. The decision belongs with §20 Priority + 4's discovery arc, which can weigh a prefix (`F1`? `C-c ?`?) across + the whole command family instead of one key at a time. +- **The full cheat sheet and the discovery command family** — + `where-is`, `describe-key`, richer M-x rows: §20 Priority 4, called + "the best payoff-per-effort in this document". +- **Making `show_help_text` adopt the generated-buffer write + invariant** (§2.4). A real gap, with its own lane. +- **Onboarding proper** — §18's ten-step teaching sequence. This stage + is the floor beneath it, not a down payment on it. + +**Step 4 therefore stays Partial**, and the PR must say so: §2's row +names a welcome, a cheat sheet *and* `C-h`, and this closes the first +plus a minimal version of the second. **§18 and the scorecard do move**, +Missing → Partial (§6) — a stage can be too small to flip its journey +step while still falsifying a "missing entirely" grade. + +## 6. Coherence impact + +- **Journey steps touched:** 4 — **and it stays Partial**, because §2's + row names a welcome, a cheat sheet *and* `C-h`, and this closes the + first plus a minimal second. Step 2 is not touched at all (§2.2a). +- **§25 landed-evidence obligation: yes, and rev 1 said otherwise.** + Two audited claims change on merge and both must move + **Missing → Partial**: + - the **scorecard** row 18, "Onboarding | **Missing** | No welcome, no + tutorial; `C-h` deletes a word; `M-x` is the only door in"; + - **§18's ground truth**, "Grade: missing entirely. No welcome buffer, + … no cheat sheet reachable from inside the editor". + + Both become false the moment this lands — a welcome buffer exists and + a cheat sheet is reachable by `M-x help` — while `C-h` and the tutorial + stay untrue, which is what makes the new grade Partial rather than + Works. §25 requires the update to ride the landing PR. Rev 1's claim + that this stage had no such obligation was simply wrong, and would + have left the document asserting "missing entirely" about a feature + the same PR shipped. +- **Interaction islands: none added.** `M-x help` renders through the + existing `*help*` mechanism rather than inventing a second help + surface — though §2.4 records that mechanism's own two gaps rather + than pretending it is exemplary. +- **Config registry:** not adopted. A `welcome.enabled` scalar would be + expressible, and is deliberately not added — a greeting the user can + delete by typing does not need a setting, and §11's adoption work + should not gain a token entry from a stage this small. +- **Background-work attribution:** unchanged; nothing here is + asynchronous. +- **Docs riding the PR:** `COHERENCE.md`'s **scorecard row 18** and + **§18's ground truth** (Missing → **Partial**), §2's **step-4 row** + (stays Partial, with the closed half named), and §18's own note that + the cheap floor's first two items are done and the help-prefix + decision is not; `docs/keybindings.md` gains `M-x help`; + `docs/agent-handoff.md` §1; the ledger. + +## 7. Questions — answered in review round 1 + +- **Q#W1 — three lines, or one? → three.** The whole complaint in §18 is + that the editor teaches nothing, so the floor is not one line. +- **Q#W2 — name `C-c c` and `C-c t`? → yes**, and verify them from the + structured entries (§3.1). Naming real bindings is the point; pin 2 + turns the maintenance risk into a caught failure rather than drift. +- **Q#W3 — is `help` the right name? → yes.** It stays the **root/index** + when `help.keys` and friends arrive under the discovery arc, so no + future rename or deprecation is owed. + +## 8. Ledger + +Branch `journey-stage1b3-welcome`, worktree `../pmacs-journey-1b3`, +based on `githubsucks/main` @ `1f290d5`. Framing only; no code, no PR. + +**#204 has landed** (`githubsucks/main` @ `5376af1`), so the conflict +this note originally warned about is now a plain rebase surface: this +lane still touches `COHERENCE.md`, `docs/agent-handoff.md` and +`docs/active-work.md`, so integrate `main` late (at PR time) rather than +opening a refresh PR. + +```sh +git fetch githubsucks +git worktree add ../pmacs-journey-1b3 \ + -b journey-stage1b3-welcome \ + githubsucks/journey-stage1b3-welcome +``` diff --git a/docs/keybindings.md b/docs/keybindings.md index 922692f..1d3cbae 100644 --- a/docs/keybindings.md +++ b/docs/keybindings.md @@ -189,6 +189,13 @@ Source: `builtin/runtime/compile.lua`. | `M-!` | `shell.command` — asynchronous output in `*shell-command*` | | `C-c c` | `compile.run` — prompts, prefilled from the detected project kind | +`M-x help` renders this file's essentials as a `*help*` buffer inside +the editor, and is what the startup welcome points at. It is the root of +the eventual help family (`help.keys` and friends arrive with the +discovery arc), so it takes no keybinding yet — `C-h` is **not** free: +it deletes a word because non-kitty terminals cannot tell Ctrl+Backspace +from Ctrl+H. + `compile.recompile` is available through `M-x`, and through `g` inside `*compilation*`; no global key is assigned to it. `C-c c` is unreachable from inside a terminal window (`C-c` is consumed as the escape key) and diff --git a/src/editor.rs b/src/editor.rs index 4be27be..b089973 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -705,6 +705,17 @@ impl EditorState { include_str!("../builtin/runtime/compile.lua"), ) .expect("load compile builtin chunk"); + // Journey Stage 1b-3: the welcome text and `M-x help`. Loaded + // after `commands/default.lua` (which `attach_editor` above ran) + // so `pmacs.editor._show_help` exists, and after the runtime + // chunks whose keys it advertises, so a binding it names is + // already registered when the acceptance suite checks them. + lua_host + .eval( + Some("@pmacs/builtin/runtime/welcome.lua"), + include_str!("../builtin/runtime/welcome.lua"), + ) + .expect("load welcome 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 @@ -878,6 +889,82 @@ impl EditorState { } } + /// Final step of a **local, no-target** launch: greet an untouched + /// `*scratch*` (journey step 4, `COHERENCE.md` §18). + /// + /// Called from [`prepare_startup`] after config has run, after + /// attach dispatch resolved to local, and after + /// [`Self::restore_desktop_if_armed`]. None of the constructors is + /// the right hook: `EditorState::open` calls `new` *before* + /// resolving its target, the daemon constructs one too, and + /// `init.lua` runs inside `new` — so a greeting written there would + /// reach a daemon session, precede the file argument that replaces + /// the buffer, and outrun anything config or a restored desktop puts + /// in `*scratch*`. + /// + /// Greets only when all four hold; each excludes one of those cases: + /// + /// 1. `had_file` is false — a positional argument means "open this". + /// 2. the session is local — guaranteed by the call site. + /// 3. `*scratch*` is the active buffer — restore may have moved it. + /// 4. `*scratch*` is empty — never overwrite config or a restore. + /// + /// Leaves the buffer **unmodified**: the greeting must not look like + /// unsaved work. It is deliberately *not* written through + /// `set_generated_contents`, which would lift read-only, discard + /// history and mark the buffer generated — all wrong for a buffer + /// journey step 5 requires the user to type into immediately. + pub fn finalize_local_launch(&mut self, had_file: bool) { + if had_file { + return; + } + let buffer_id = self.core.borrow().active_buffer_id(); + { + let registry = self.core.borrow().registry.clone(); + let reg = registry.borrow(); + let Ok(buf) = reg.get(buffer_id) else { + return; + }; + if buf.name() != "*scratch*" || !buf.is_empty() { + return; + } + } + let text: String = match self + .lua_host + .lua() + .load("return pmacs.welcome.text()") + .eval() + { + Ok(text) => text, + // A user who replaced `pmacs.welcome` with something broken + // gets no greeting, not a failed launch. + Err(_) => return, + }; + // Capture the edit and RELEASE the registry borrow before + // notifying: `notify_buffer_edit` borrows the registry itself. + let registry = self.core.borrow().registry.clone(); + let edit = { + let mut reg = registry.borrow_mut(); + let Ok(buf) = reg.get_mut(buffer_id) else { + return; + }; + let Ok(edit) = buf.apply_edit(crate::buffer::EditOp::Insert { + pos: 0, + bytes: text.as_bytes(), + }) else { + return; + }; + buf.mark_clean(); + edit + }; + // Without this the greeting renders as ONE row on the first + // frame: the window's `TextView` indexed `*scratch*` while it + // was empty, and newlines are zero-width to a painter working + // from a stale line index. Every other direct-registry writer + // notifies for the same reason. + self.core.borrow_mut().notify_buffer_edit(buffer_id, &edit); + } + /// Restore the session saved under this desktop's key, if armed /// (`pmacs.session.desktop_mode(true)` called it) and no positional /// file arg was given (Q#DS7). Called from the `RunLocal` arm of @@ -3590,6 +3677,68 @@ impl Default for EditorState { // Run loop // --------------------------------------------------------------------------- +/// Outcome of [`prepare_startup`]: either a session ready for the local +/// TUI loop, or a hand-off the caller must perform. +pub enum Startup { + /// Ready to enter the local loop — desktop restored, launch + /// finalized. + Local(Box), + /// The init-time attach request resolved to something other than + /// local. Performing it takes over the terminal, so it stays out of + /// [`prepare_startup`] and is the caller's job. + HandOff(crate::attach_dispatch::AttachDispatch), +} + +/// Everything [`run`] does **before** it touches the terminal: +/// construct from the target, install state dirs, dispatch the +/// init-time attach request, and — on the local path — restore the +/// desktop and finalize the launch. +/// +/// Extracted so the local-startup sequence is testable: `run` adds only +/// `Frontend::new()` and the event loop, which is where terminal +/// takeover genuinely lives. Without this split, deleting the +/// [`EditorState::finalize_local_launch`] call would leave every +/// direct-call test green while shipping no welcome at all. +/// +/// `pub` rather than `pub(crate)` because the journey acceptance suite +/// is a separate integration crate — and because the rest of this +/// sequence (`run`, `EditorState::new`, `EditorState::open`, +/// `install_state_dirs`, `restore_desktop_if_armed`) is already public, +/// so this completes that surface rather than widening it. +pub fn prepare_startup(file: Option) -> io::Result { + // Capture before the `match` consumes `file`: a positional file arg + // means "open this", not "restore my desktop" (Q#DS7). + let had_file = file.is_some(); + let mut state = match file { + Some(path) => EditorState::open(path)?, + None => EditorState::new(), + }; + // Real session: wire up on-disk persistence (history + pmacs.state). + state.install_state_dirs(); + // Post-init dispatch: read whatever init.lua left in the + // RequestedAttach slot and decide whether to run local or hand off + // to attach mode. `take_requested_attach` consumes the slot. + let requested = state.lua_host.take_requested_attach(); + match crate::attach_dispatch::dispatch_attach(requested) { + crate::attach_dispatch::AttachDispatch::RunLocal => { + // Committed to local mode: restore the desktop if armed and + // no file arg was given (Q#DS7). Done here, not right after + // construction, so a hand-off never populates an + // EditorState it's about to drop. + state.restore_desktop_if_armed(had_file); + // Journey step 4: the last thing before the loop, so config + // and any restored desktop have already had their say. + state.finalize_local_launch(had_file); + Ok(Startup::Local(Box::new(state))) + } + // The EditorState is dropped by the caller before it takes over + // the terminal: attach mode constructs its own Frontend, and a + // locally-built one would leak its alternate-screen / raw-mode + // setup if held across the call. + other => Ok(Startup::HandOff(other)), + } +} + /// Main run loop. Opens the file (if any), takes over the terminal, /// renders, dispatches keys, until the user quits. /// @@ -3611,54 +3760,22 @@ impl Default for EditorState { /// local-TUI for the terminal. pub fn run(file: Option) -> io::Result<()> { install_panic_hook(); - // Capture before the `match` consumes `file`: a positional file arg - // means "open this", not "restore my desktop" (Q#DS7). - let had_file = file.is_some(); - let mut state = match file { - Some(path) => EditorState::open(path)?, - None => EditorState::new(), - }; - // Real session: wire up on-disk persistence (history + pmacs.state). - state.install_state_dirs(); - - // Post-init dispatch: read whatever init.lua left in the - // RequestedAttach slot and decide whether to run local or hand - // off to attach mode. `take_requested_attach` consumes the slot - // — even on the hand-off path the local EditorState is dropped - // before attach::run_attach takes over the terminal, so the - // request is consumed exactly once. - let requested = state.lua_host.take_requested_attach(); - match crate::attach_dispatch::dispatch_attach(requested) { - crate::attach_dispatch::AttachDispatch::RunLocal => { - // Committed to local mode: restore the desktop if armed and - // no file arg was given (Q#DS7). Done here, not right after - // construction, so a hand-off to attach mode (above) never - // populates an EditorState it's about to drop. - state.restore_desktop_if_armed(had_file); - // Fall through to the local TUI loop below. - } - crate::attach_dispatch::AttachDispatch::RunAttachLocalSocket(socket) => { - // Drop the local EditorState before taking over the - // terminal: attach mode constructs its own Frontend, and - // the locally-built one would leak its alternate-screen - // / raw-mode setup if held across the call. - drop(state); + let mut state = match prepare_startup(file)? { + Startup::Local(state) => *state, + Startup::HandOff(crate::attach_dispatch::AttachDispatch::RunAttachLocalSocket(socket)) => { return crate::attach::run_attach(socket).map_err(|e| io::Error::other(format!("{e}"))); } - crate::attach_dispatch::AttachDispatch::RunAttachSsh(target) => { - // Same EditorState-drop reasoning as the local-socket - // path: SSH attach takes over the terminal. - drop(state); + Startup::HandOff(crate::attach_dispatch::AttachDispatch::RunAttachSsh(target)) => { return crate::attach::run_attach_ssh(target) .map_err(|e| io::Error::other(format!("{e}"))); } - dispatch @ crate::attach_dispatch::AttachDispatch::DeferredInV01 { .. } => { + Startup::HandOff(dispatch) => { let msg = dispatch .deferred_message() - .expect("DeferredInV01 always has a message"); + .unwrap_or_else(|| "unsupported attach dispatch".to_owned()); return Err(io::Error::other(msg)); } - } + }; let mut frontend = Frontend::new()?; let mut render_state = crate::instance_render::RenderState::new(frontend.size()); diff --git a/tests/journey_acceptance.rs b/tests/journey_acceptance.rs index 7ce2457..db88942 100644 --- a/tests/journey_acceptance.rs +++ b/tests/journey_acceptance.rs @@ -1744,6 +1744,301 @@ fn journey_step9_the_compile_directory_is_detection_canonical() { } // --------------------------------------------------------------------------- +// Step 4 — understand the visible interface (Journey Stage 1b-3) +// +// `COHERENCE.md` §18 graded onboarding "missing entirely": no welcome, +// no cheat sheet reachable from inside the editor, and `M-x` the only +// door in — which a new user has no way to learn about. +// +// These rows drive `prepare_startup`, the production call `run()` makes. +// Calling `finalize_local_launch` by hand instead would leave the wiring +// unpinned: deleting the one call inside `prepare_startup` would keep +// every other assertion here green while shipping no welcome at all. +// --------------------------------------------------------------------------- + +/// The `*scratch*` buffer's text, wherever it currently sits. +fn scratch_text(s: &EditorState) -> String { + eval( + s, + r#" + for _, id in ipairs(pmacs.buffer.list()) do + if pmacs.describe.buffer(id).name == "*scratch*" then + return id:slice(0, id:len()) + end + end + return "" + "#, + ) +} + +fn welcome_entries(s: &EditorState) -> Vec<(String, String)> { + let raw: Vec = eval( + s, + "local out = {} + for _, e in ipairs(pmacs.welcome.entries) do + out[#out + 1] = e.keys .. '\\1' .. e.label + end + return out", + ); + raw.into_iter() + .map(|row| { + let (keys, label) = row + .split_once('\u{1}') + .expect("entry encodes keys and label"); + (keys.to_owned(), label.to_owned()) + }) + .collect() +} + +/// Drive the production startup path with no target, as `pmacs` does. +fn start_local() -> EditorState { + match pmacs::editor::prepare_startup(None).expect("startup must not fail") { + pmacs::editor::Startup::Local(state) => *state, + pmacs::editor::Startup::HandOff(_) => panic!("no init.lua attach request in a test"), + } +} + +/// **N** — journey step 4: a no-target local launch greets. +/// +/// Falsified by deleting the `finalize_local_launch` call inside +/// `prepare_startup` — the mutation every by-hand pin would survive. +#[test] +fn journey_step4_a_no_target_launch_greets_in_scratch() { + let s = start_local(); + + // Preconditions asserted, not assumed (framing §3.2b): a developer + // whose real init.lua arms desktop mode would otherwise get a + // restored scratch and a silently different result. + assert!( + eval::( + &s, + "return pmacs.session == nil or pmacs.session.desktop_armed ~= true" + ), + "precondition: desktop restore must be unarmed for this pin to mean anything" + ); + assert_eq!(active_name(&s), "*scratch*"); + + let text = active_text(&s); + assert!( + !text.is_empty(), + "an unconfigured launch must say something" + ); + assert!( + text.contains("M-x"), + "and must name the one key that opens everything; got {text:?}" + ); +} + +/// **N** — every entry the welcome names is actually bound. +/// +/// A property over the structured list, not a scrape of prose: `C-c c` +/// is two chords and nothing in the rendered text marks the boundary. +#[test] +fn journey_step4_every_advertised_key_is_bound() { + let s = start_local(); + let entries = welcome_entries(&s); + assert!( + !entries.is_empty(), + "precondition: the entry list must be non-empty or this loop is vacuous" + ); + for (keys, label) in entries { + let bound: Option = eval( + &s, + &format!("local b = pmacs.keymap.lookup({keys:?}) return b and b.command"), + ); + assert!( + bound.is_some(), + "the welcome advertises {keys:?} ({label}) but nothing is bound to it" + ); + } +} + +/// **N** — the rendered text contains every entry. +/// +/// Pin 2 alone would pass if rendering silently dropped one. +#[test] +fn journey_step4_the_rendered_welcome_contains_every_entry() { + let s = start_local(); + let text = active_text(&s); + for (keys, label) in welcome_entries(&s) { + assert!( + text.contains(&keys), + "welcome text omits the key {keys:?}; got {text:?}" + ); + assert!( + text.contains(&label), + "welcome text omits the label {label:?}; got {text:?}" + ); + } +} + +/// **N** — `M-x help` renders the cheat sheet, reached the way a user +/// reaches it. +/// +/// `pmacs.command.invoke` is the *programmatic* API; M-x is +/// `editor.execute-command`, a minibuffer with the `commands` completion +/// source that calls `invoke_interactive` only on accept. +/// +/// The selection is asserted **before** RET: a selected candidate +/// shadows typed text, and `Minibuffer::accept` does `session.take()`, +/// so nothing about the accepted value survives afterwards. +#[test] +fn journey_step4_m_x_help_renders_the_cheat_sheet() { + let mut s = start_local(); + + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char('x'), KeyModifiers::ALT), + ); + assert!( + eval::(&s, "return pmacs.minibuffer.is_active()"), + "M-x must open the command palette" + ); + for ch in "help".chars() { + type_char(&mut s, ch); + } + assert_eq!( + eval::>(&s, "return pmacs.minibuffer.selected()").as_deref(), + Some("help"), + "the completion source must have `help` selected; a different \ + candidate would accept a different command" + ); + + press(&mut s, KeyCode::Enter); + pump(&mut s); + + let help = named_text(&s, "*help*"); + for (keys, _) in welcome_entries(&s) { + assert!( + help.contains(&keys), + "the cheat sheet omits {keys:?}; got:\n{help}" + ); + } +} + +/// **P** — the greeted buffer is editable and unmodified. +/// +/// Targeted mutation: rendering through `set_generated_contents`, which +/// would lift read-only, discard history, and fail the insert. +#[test] +fn journey_step4_preservation_the_greeted_scratch_is_editable_and_clean() { + let mut s = start_local(); + assert!( + eval::( + &s, + "return pmacs.describe.buffer(pmacs.window.buffer()).modified == false" + ), + "a greeting must not look like unsaved work" + ); + + let before = active_text(&s).len(); + type_char(&mut s, 'X'); + assert!( + active_text(&s).len() > before, + "step 5 must still work from the first frame: typing inserts" + ); +} + +/// **P** — a file target does not greet. +#[test] +fn journey_step4_preservation_a_file_target_does_not_greet() { + let td = project(); + let path = td.path().join("alpha.txt"); + let s = match pmacs::editor::prepare_startup(Some(path.clone())).expect("startup") { + pmacs::editor::Startup::Local(state) => *state, + pmacs::editor::Startup::HandOff(_) => panic!("no attach request"), + }; + assert_eq!(active_name(&s), path.display().to_string()); + assert_eq!( + scratch_text(&s), + "", + "a positional argument means \"open this\", not \"greet me\"" + ); +} + +/// **P** — a directory target does not greet either, so Stage 1a's +/// dired listing is what the user sees. +/// +/// Separate from the file pin because the directory path reaches +/// `*scratch*` differently: the bootstrap replaces the window's buffer +/// and `replace_active_buffer` removes nothing, so the scratch buffer +/// still exists to be wrongly greeted. +#[test] +fn journey_step4_preservation_a_directory_target_does_not_greet() { + let td = project(); + let mut s = + match pmacs::editor::prepare_startup(Some(td.path().to_path_buf())).expect("startup") { + pmacs::editor::Startup::Local(state) => *state, + pmacs::editor::Startup::HandOff(_) => panic!("no attach request"), + }; + pump(&mut s); + assert!(active_name(&s).starts_with("*dired:")); + assert_eq!(scratch_text(&s), "", "the dired listing is the greeting"); +} + +/// **P** — a non-empty `*scratch*` is never overwritten. +#[test] +fn journey_step4_preservation_existing_scratch_content_survives() { + let mut s = EditorState::new(); + exec(&s, "pmacs.window.buffer():insert(0, 'user content')"); + s.finalize_local_launch(false); + assert_eq!( + active_text(&s), + "user content", + "config or a restored desktop owns whatever is already there" + ); +} + +/// **P** — a non-active `*scratch*` is not greeted. Stands in for a +/// desktop restore having put something else in front. +#[test] +fn journey_step4_preservation_a_backgrounded_scratch_is_not_greeted() { + let td = project(); + let mut s = EditorState::new(); + exec( + &s, + &format!( + "pmacs.window.display_file({:?})", + td.path().join("alpha.txt").display().to_string() + ), + ); + pump(&mut s); + s.finalize_local_launch(false); + assert_eq!( + scratch_text(&s), + "", + "only the buffer actually greeting the user is written to" + ); +} + +/// **P** — the constructors greet nothing on their own. +/// +/// This is what makes the seam the only writer, and would catch a +/// greeting smuggled back into a constructor — including the daemon's. +/// Necessary but not sufficient: pin 1 is what proves the seam is +/// reached in production. +#[test] +fn journey_step4_preservation_constructors_never_greet() { + let bare = EditorState::new(); + assert_eq!(scratch_text(&bare), "", "EditorState::new must not greet"); + + let td = project(); + let file = EditorState::open(td.path().join("alpha.txt")).expect("open file"); + assert_eq!( + scratch_text(&file), + "", + "EditorState::open(file) must not greet" + ); + + let mut dir = EditorState::open(td.path().to_path_buf()).expect("open dir"); + pump(&mut dir); + assert_eq!( + scratch_text(&dir), + "", + "EditorState::open(dir) must not greet" + ); +} + // Step 6 — receive language intelligence (Journey Stage 1b-2) // // `COHERENCE.md` §2 graded this **Partial**: a preconfigured server that @@ -1829,3 +2124,71 @@ fn journey_step6_a_missing_language_server_is_reported_not_swallowed() { is what made highlighting able to mask this" ); } + +/// **N** — the greeting renders as separate rows on the **first frame**. +/// +/// Writing to the registry without `notify_buffer_edit` leaves the +/// window's `TextView` indexed against the empty `*scratch*` it was +/// built for. Newlines are zero-width to a painter working from a stale +/// line index, so the whole three-line greeting collapses onto row 0 — +/// visible to a user, invisible to every buffer-text assertion above. +/// +/// Falsified by dropping the `notify_buffer_edit` call in +/// `finalize_local_launch`. +#[test] +fn journey_step4_the_welcome_paints_as_multiple_rows_on_the_first_frame() { + use pmacs::cell::{Cell, CellGrid, CellSize, Glyph}; + + let s = start_local(); + let (rows, cols) = (12u32, 100u32); + let mut cells = vec![Cell::default(); (rows * cols) as usize]; + let mut grid = CellGrid { + cells: &mut cells, + stride: cols, + size: CellSize::new(rows, cols), + }; + let _ = pmacs::editor::paint_frame( + &s, + FrontendId::LOCAL, + &std::collections::HashMap::new(), + &mut grid, + CellSize::new(rows, cols), + ); + + let row_text = |row: u32| -> String { + (0..cols) + .map( + |column| match &cells[(row * cols + column) as usize].glyph { + Glyph::Char(ch) => *ch, + Glyph::Cluster(bytes) => std::str::from_utf8(bytes) + .ok() + .and_then(|t| t.chars().next()) + .unwrap_or(' '), + Glyph::Continuation => ' ', + }, + ) + .collect::() + .trim_end() + .to_owned() + }; + + assert!( + row_text(0).contains("Welcome to pmacs"), + "row 0 is the greeting's first line; got {:?}", + row_text(0) + ); + // The discriminating half: with a stale TextView these land on row 0 + // too, and row 1 is blank. + assert!( + row_text(1).contains("C-x C-f"), + "the second line must occupy its own row, not collapse into the \ + first; row 1 = {:?}, row 0 = {:?}", + row_text(1), + row_text(0) + ); + assert!( + !row_text(0).contains("C-x C-f"), + "and must not have been folded into row 0; got {:?}", + row_text(0) + ); +}