diff --git a/COHERENCE.md b/COHERENCE.md index 670b235..8a71b90 100644 --- a/COHERENCE.md +++ b/COHERENCE.md @@ -98,7 +98,7 @@ remain open to them. | 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 | +| 5 | Unified discoverability | **Partial** | Discovery Stage 1: eleven `help.*` commands (describe key/mode/hook/buffer/command/setting, where-is, list commands/keybindings/settings, apropos) over the existing registries, indexed by `M-x help`. Commands, keys, modes, hooks and settings are now reachable, and `*workers*` already was (`M-x editor.list-workers`); **packages have no comparable surface** (§13), and workers still lack owner/purpose/hierarchy and any indicator (§9). `Command` still has no title/category/flags, M-x rows are still bare names, and the Rust help layer is still orphaned | | 6 | Interaction islands | **Weak, and growing** | Six hardcoded key-interception shadows; no transient-keymap mechanism exists | | 7 | First-class workspaces | **Missing (conventions only)** | Marker walk + four independent consumers; no workspace object | | 8 | Execution locations | **Missing (architecture ready)** | SSH attach works; "location" is not a value anywhere | @@ -509,9 +509,10 @@ build actions, menus, missing-tool guidance): settings, profiles, package management, task definitions, frontend/language settings): -- palette △ (`M-x` fuzzy over bare names, §5) · keybinding search ✗ (no - list-keybindings/where-is commands) · workspace settings ✗ (no - workspace scope, §11) · profiles ✗ (§12) · package management ✗ +- palette △ (`M-x` fuzzy over bare names — the rows are still bare, §5) · + keybinding search ✓ (`help.list-keybindings`, `help.where-is`, + `help.describe-key`; still no key to reach them) · workspace settings + ✗ (no workspace scope, §11) · profiles ✗ (§12) · package management ✗ in-session (§13) · task definitions ✗ · frontend customization △ (themes, `pmacs.gpu.set_font`, statusline providers — all Lua-only) · language settings △ (raw Lua tables, outside the registry). @@ -523,8 +524,11 @@ orchestrate workers, replace interaction models): - inspect ✓ (SourceLocation on everything; no jump-to-source command though) · redefine live ✓ (`unregister` + `define`) · packages ✓ (authoring is real, §13) · new views ✓ (listview is Lua-usable) · - providers ✓ (statusline; completion/minibuffer sources are a fixed - Rust vocabulary) · keymap layers ✗ (§6 — the mechanism does not + providers ✓ (statusline; and minibuffer completion is **not** a fixed + Rust vocabulary — `parse_completion_source` accepts a Lua function as + `CompletionSource::Custom`, which is how Discovery Stage 1 gave + `help.describe-setting` completion with no Rust at all) · keymap + layers ✗ (§6 — the mechanism does not exist) · workspace policy ✗ · orchestrate workers △ (`pmacs.workers.register` funnels into builtin dispatchers, §9) · replace interaction models ✗ (the shadows, §6). @@ -557,7 +561,27 @@ This suggests a general pmacs principle: ### Ground truth -**Grade: substrate without surface — the sharpest instance of §1.1.** +**Grade: partial — Discovery Stage 1 built the first surface.** + +Eleven `help.*` commands now render the registries that were already +there: describe-key/mode/hook/buffer/command/setting, where-is, list +commands/keybindings/settings, and a substring apropos over names and +descriptions, indexed by `M-x help`. It needed **no Rust** — the data +was all reachable from Lua, and even the settings completion source is a +Lua function through `CompletionSource::Custom`. + +**What is still missing** is itemized below and unchanged by that stage: +`Command` has no title/category/aliases/flags/arg-schema; the predicate +is still never evaluated; M-x rows are still bare name strings; the Rust +help layer is still orphaned (Stage 1 funnels every command through one +Lua seam so the eventual migration is enumerated per subject rather than +per call site); **packages** have no discovery surface, and workers, +though `M-x editor.list-workers` opens `*workers*`, still lack the +ownership model and the activity indicator §9 asks for; settings +value provenance is still absent; and there is still no help prefix key. + +*The original audit grade, for reference:* **substrate without surface — +the sharpest instance of §1.1.** **What the substrate already has (genuinely strong):** @@ -609,18 +633,31 @@ This suggests a general pmacs principle: `builtin/commands/default.lua:1103-1136` — and the one users can actually reach (`M-x editor.describe-command`) renders **less** than the unreachable one (no source, no scope, no predicate note). -- **Missing as commands entirely:** describe-key, describe-mode, - describe-hook, describe-buffer, where-is, list-commands, - list-settings, list-keybindings, apropos. What exists: - `editor.describe-command`, `editor.describe-setting`, - `editor.describe-instance[-buffer]`, `editor.list-buffers`, - `editor.list-workers`. `M-x describe-setting` prompts **free-text - with no completion source** (deliberately skipped — - `builtin/commands/default.lua:1180-1185`); a typo yields a status - line error. -- **No help prefix key.** `C-h` is `buffer.delete-word-backward` - (`builtin/keymaps/default.lua:86`, with a comment noting the key "was - free"). No `F1`, no `C-h k/f/b`. +- **The command family now exists** (Discovery Stage 1): eleven `help.*` + commands — describe-key/mode/hook/buffer/command/setting, where-is, + list-commands/keybindings/settings, apropos — indexed by `M-x help`, + with `editor.describe-command` / `editor.describe-setting` retained as + forwarders. `help.describe-setting` **now completes**, through a Lua + function passed as `CompletionSource::Custom`. + **What remains missing here:** a discovery surface for **packages** + (§13) — `*workers*` already has one, reachable by + `M-x editor.list-workers`, though §9's ownership model and activity + indicator are still absent — and **no key reaches any of this**: the + family is `M-x`-only by design until the help-prefix decision is taken + (see the prefix bullet below). + *Completion is assistance, not validation:* `resolve_accepted_value` + returns the literal typed text when no candidate is selected, so a + typo still reaches the handler; refusing a non-candidate is unbuilt + Rust work. +- **No help prefix key**, and `C-h` is **not** available for one. + It is `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 would break + Ctrl+Backspace on every legacy terminal. That file's own comment says + the key "was free", which reads as an oversight and is not one; §18 + records the trade. No `F1`, no `C-h k/f/b`. **This is why Discovery + Stage 1 shipped the family unbound**: the prefix is one decision for + the whole family, not nine. - **Settings value provenance is absent.** Overrides are stored as bare values (`global: HashMap`, `src/config_registry.rs:693-708`); `describe-setting`'s "Source:" is diff --git a/builtin/commands/default.lua b/builtin/commands/default.lua index 48d6384..bcf54df 100644 --- a/builtin/commands/default.lua +++ b/builtin/commands/default.lua @@ -1274,7 +1274,15 @@ function pmacs.editor._show_help(text) show_help_text(text) end -cmd { name = "editor.describe-command", +-- The two family commands below call `pmacs.editor._show_help`, NOT the +-- local `show_help_text`, even though they are in the same file and the +-- local is in scope. That is deliberate: discovery Stage 1's funnel +-- ("one owner for `*help*` writes") is only real if every command goes +-- through the PUBLIC seam — a command calling the local bypasses any +-- later change made at the seam, and bypassed the acceptance pin that +-- counts seam calls, which is how this was caught. + +cmd { name = "help.describe-command", description = "Prompt for a command name and render its description in *help*.", fn = function() pmacs.minibuffer.read { @@ -1303,7 +1311,7 @@ cmd { name = "editor.describe-command", lines[#lines + 1] = " " .. tostring(seq) end end - show_help_text(table.concat(lines, "\n")) + pmacs.editor._show_help(table.concat(lines, "\n")) end, } end } @@ -1315,12 +1323,17 @@ cmd { name = "editor.describe-command", -- way in, modeled on `editor.describe-command` directly above and sharing -- its `*help*` buffer handling. -- --- The prompt takes free text: `pmacs.minibuffer.read`'s `source` is a --- fixed vocabulary ("commands", "buffers") resolved in Rust, and adding a --- settings source means touching the minibuffer candidate machinery, --- which this arc deliberately stays out of. `pmacs.config.list()` is the --- programmatic way to enumerate names meanwhile; a completion source (and --- an M-x list-settings panel) are named deferrals in the framing. +-- The prompt now completes. That comment used to say `source` is "a fixed +-- vocabulary ("commands", "buffers") resolved in Rust" — it is not: +-- `parse_completion_source` also accepts a Lua **function**, which +-- becomes `CompletionSource::Custom` and is called for candidates. So a +-- settings source needs no Rust at all (discovery Stage 1). +-- +-- **Completion here is assistance, not validation.** +-- `resolve_accepted_value` returns the literal typed text whenever no +-- candidate is selected, so a non-matching typo still reaches +-- `on_accept` and the `no such setting` path below still earns its +-- keep. Refusing a non-candidate outright is Rust work and is deferred. local function describe_setting_lines(name, info) -- Header block mirrors help.rs's `format_hook_text`: aligned label @@ -1354,12 +1367,28 @@ local function describe_setting_lines(name, info) return lines end -cmd { name = "editor.describe-setting", +cmd { name = "help.describe-setting", description = "Prompt for a setting name and render its definition in *help*.", fn = function() pmacs.minibuffer.read { prompt = "Describe setting: ", history = "command", + -- Sorted for DETERMINISTIC POOL CONSTRUCTION, not display + -- order: `recompute_candidates` runs `filter_and_sort`, which + -- ranks by fuzzy score and tie-breaks lexically, so this order + -- never reaches the user. It matters because + -- `.take(CANDIDATE_LIMIT)` is applied to the filtered iterator + -- BEFORE that sort, so pool order decides which candidates + -- survive truncation; registration order would make that vary + -- with an unrelated config edit. + source = function() + local names = {} + for _, d in ipairs(pmacs.config.list()) do + names[#names + 1] = d.name + end + table.sort(names) + return names + end, on_accept = function(name) if name == nil or name == "" then return end -- An undefined name raises NotFound rather than returning nil @@ -1370,7 +1399,7 @@ cmd { name = "editor.describe-setting", pmacs.editor.set_status("describe-setting: no such setting: " .. name) return end - show_help_text(table.concat(describe_setting_lines(name, info), "\n")) + pmacs.editor._show_help(table.concat(describe_setting_lines(name, info), "\n")) end, } end } diff --git a/builtin/runtime/help.lua b/builtin/runtime/help.lua new file mode 100644 index 0000000..0e31b56 --- /dev/null +++ b/builtin/runtime/help.lua @@ -0,0 +1,402 @@ +-- help.lua --- the discovery command family (P4 Stage 1). +-- Framing: docs/discovery-stage1-command-family-framing.md. +-- +-- `COHERENCE.md` §5 grades discoverability "substrate without surface — +-- the sharpest instance of §1.1": the registries already carry +-- descriptions, source locations and reverse key lookup, and almost none +-- of it was reachable. This file is the surface. It adds NO Rust: every +-- command below renders data `pmacs.describe.*`, `pmacs.keymap.list()`, +-- `pmacs.command.list()` and `pmacs.config.list()` already return. +-- +-- ORDERING CONTRACT: loads after `commands/default.lua` (for +-- `pmacs.editor._show_help` and the two renamed commands it forwards to) +-- and after `runtime/welcome.lua` (whose `pmacs.welcome.entries` the +-- index reads). +-- +-- TWO DISCIPLINES THIS FILE KEEPS +-- +-- 1. **One owner for `*help*` writes.** Every command renders through +-- `pmacs.editor._show_help` and never touches a buffer itself. That +-- does NOT make a later migration to `src/help.rs` a one-site change +-- — that layer has renderers for command/key/buffer/mode/hook/view +-- and none for settings, lists or apropos, and `_show_help` takes +-- already-flattened text. What the funnel buys is the shared policy +-- in one place: reuse-by-name, wholesale replacement, the `q` +-- binding, and the foreign-`*help*` hazard (found-by-name is not +-- ownership — a user's own `*help*` is adopted and cleared; the +-- missing guarantee is ownership identity, which `listview` has as +-- `panels` and dired as its handle table, and this does not). +-- +-- 2. **Rendering is a named per-subject function**, and the command body +-- does nothing but call it and hand the result to `_show_help`. That +-- keeps the semantics addressable, so the future help-unification +-- stage is enumerated per subject — replace the four `src/help.rs` +-- already covers, write three new Rust renderers for settings, lists +-- and apropos — rather than discovered per call site. + +pmacs.help = pmacs.help or {} + +local function show(text) + pmacs.editor._show_help(text) +end + +-- Sorted command names. `pmacs.command.list()` returns registration +-- order, which is not meaningful to a reader. +local function sorted_command_names() + local names = {} + for _, n in ipairs(pmacs.command.list()) do names[#names + 1] = n end + table.sort(names) + return names +end + +local function description_of(name) + local ok, info = pcall(pmacs.describe.command, name) + if ok and type(info) == "table" and type(info.description) == "string" then + return info.description + end + return "(no description)" +end + +-- --------------------------------------------------------------------- +-- Per-subject renderers (discipline 2) +-- --------------------------------------------------------------------- + +function pmacs.help.render_key(seq, info) + if type(info) ~= "table" then + return string.format("Key: %s\n\n (unbound in this buffer)\n", seq) + end + local lines = { + "Key: " .. seq, + "", + " Command: " .. tostring(info.command), + " Scope: " .. tostring(info.scope), + } + if info.source then lines[#lines + 1] = " Source: " .. tostring(info.source) end + lines[#lines + 1] = "" + lines[#lines + 1] = description_of(info.command) + return table.concat(lines, "\n") .. "\n" +end + +function pmacs.help.render_mode(info) + if type(info) ~= "table" then return "Mode: (none)\n" end + local lines = { "Mode: " .. tostring(info.name or "(none)"), "" } + for k, v in pairs(info) do + if k ~= "name" then + lines[#lines + 1] = string.format(" %-12s %s", k .. ":", tostring(v)) + end + end + return table.concat(lines, "\n") .. "\n" +end + +function pmacs.help.render_buffer(info) + if type(info) ~= "table" then return "Buffer: (none)\n" end + local lines = { "Buffer: " .. tostring(info.name), "" } + for _, k in ipairs({ "path", "major_mode", "modified", "read_only", "length" }) do + if info[k] ~= nil then + lines[#lines + 1] = string.format(" %-12s %s", k .. ":", tostring(info[k])) + end + end + return table.concat(lines, "\n") .. "\n" +end + +function pmacs.help.render_hook(name, info) + local lines = { "Hook: " .. name, "" } + if type(info) ~= "table" then + lines[#lines + 1] = " (no listeners)" + return table.concat(lines, "\n") .. "\n" + end + lines[#lines + 1] = string.format(" %-12s %s", "kind:", tostring(info.kind)) + local listeners = info.listeners + if type(listeners) == "table" then + lines[#lines + 1] = string.format(" %-12s %d", "listeners:", #listeners) + for _, l in ipairs(listeners) do + local src = (type(l) == "table" and l.source) or l + lines[#lines + 1] = " " .. tostring(src) + end + end + return table.concat(lines, "\n") .. "\n" +end + +function pmacs.help.render_where_is(name, bindings) + local lines = { "Where is: " .. name, "" } + if type(bindings) ~= "table" or #bindings == 0 then + lines[#lines + 1] = " (not bound to any key)" + lines[#lines + 1] = "" + lines[#lines + 1] = " Run it with M-x " .. name + return table.concat(lines, "\n") .. "\n" + end + for _, b in ipairs(bindings) do + local seq = (type(b) == "table" and b.sequence) or tostring(b) + local scope = (type(b) == "table" and b.scope) and (" (" .. tostring(b.scope) .. ")") or "" + lines[#lines + 1] = " " .. tostring(seq) .. scope + end + return table.concat(lines, "\n") .. "\n" +end + +function pmacs.help.render_command_list(names) + local lines = { string.format("Commands (%d)", #names), "" } + for _, n in ipairs(names) do + lines[#lines + 1] = string.format(" %-34s %s", n, description_of(n)) + end + return table.concat(lines, "\n") .. "\n" +end + +function pmacs.help.render_keybinding_list(rows) + -- Grouped by scope so buffer-local bindings are not mixed in with the + -- global map; sorted within a group by sequence. + local by_scope = {} + local scopes = {} + for _, r in ipairs(rows) do + local scope = tostring(r.scope) + if not by_scope[scope] then + by_scope[scope] = {} + scopes[#scopes + 1] = scope + end + table.insert(by_scope[scope], r) + end + table.sort(scopes) + local lines = { string.format("Key bindings (%d)", #rows), "" } + for _, scope in ipairs(scopes) do + local group = by_scope[scope] + table.sort(group, function(a, b) return tostring(a.sequence) < tostring(b.sequence) end) + lines[#lines + 1] = scope .. ":" + for _, r in ipairs(group) do + lines[#lines + 1] = string.format(" %-18s %s", tostring(r.sequence), tostring(r.command)) + end + lines[#lines + 1] = "" + end + return table.concat(lines, "\n") +end + +function pmacs.help.render_settings_list(rows) + local lines = { string.format("Settings (%d)", #rows), "" } + for _, d in ipairs(rows) do + lines[#lines + 1] = string.format(" %-34s %s", tostring(d.name), + tostring(d.description or "(no description)")) + end + return table.concat(lines, "\n") .. "\n" +end + +function pmacs.help.render_apropos(needle, hits) + local lines = { string.format("Apropos %q (%d)", needle, #hits), "" } + if #hits == 0 then + lines[#lines + 1] = " (nothing matched)" + return table.concat(lines, "\n") .. "\n" + end + for _, h in ipairs(hits) do + lines[#lines + 1] = string.format(" %-34s %s", h.name, h.description) + end + return table.concat(lines, "\n") .. "\n" +end + +--- Commands whose name or description CONTAINS `needle`, case-insensitively. +--- +--- **Substring, deliberately, not fuzzy** (framing Q#D3). `fuzzy_score` +--- is subsequence-based and descriptions are long sentences, so a short +--- query's letters almost always appear in order — fuzzy here would match +--- nearly every command and destroy the precision that makes apropos +--- worth having. +function pmacs.help.apropos_hits(needle) + local lowered = tostring(needle):lower() + local hits = {} + if lowered == "" then return hits end + for _, name in ipairs(sorted_command_names()) do + local desc = description_of(name) + if name:lower():find(lowered, 1, true) or desc:lower():find(lowered, 1, true) then + hits[#hits + 1] = { name = name, description = desc } + end + end + return hits +end + +-- --------------------------------------------------------------------- +-- The index +-- --------------------------------------------------------------------- + +--- Every command in the family, in the order the index lists them. +--- Public so the acceptance suite can assert the index is complete as a +--- PROPERTY — adding a twelfth canonical command without indexing it +--- must fail, not silently pass. +pmacs.help.family = { + "help.describe-command", + "help.describe-setting", + "help.describe-key", + "help.describe-mode", + "help.describe-buffer", + "help.describe-hook", + "help.where-is", + "help.list-commands", + "help.list-keybindings", + "help.list-settings", + "help.apropos", +} + +local function index_text() + local lines = { "pmacs help", "" } + if type(pmacs.welcome) == "table" and type(pmacs.welcome.entries) == "table" then + lines[#lines + 1] = "Keys" + lines[#lines + 1] = "" + lines[#lines + 1] = string.format(" %-18s %s", "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] = "" + end + lines[#lines + 1] = "Discovery commands" + lines[#lines + 1] = "" + for _, name in ipairs(pmacs.help.family) do + lines[#lines + 1] = string.format(" %-26s %s", name, description_of(name)) + end + 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 = "Index of the pmacs help and discovery commands.", + fn = function() show(index_text()) end, +} + +-- --------------------------------------------------------------------- +-- The family +-- --------------------------------------------------------------------- + +pmacs.command.define { + name = "help.describe-key", + description = "Describe what a key sequence is bound to in this buffer.", + fn = function() + pmacs.minibuffer.read { + prompt = "Describe key: ", + history = "command", + on_accept = function(seq) + if seq == nil or seq == "" then return end + local ok, info = pcall(pmacs.describe.key, seq) + show(pmacs.help.render_key(seq, ok and info or nil)) + end, + } + end, +} + +pmacs.command.define { + name = "help.describe-mode", + description = "Describe the active buffer's major mode.", + fn = function() + local buf = pmacs.window.buffer() + local ok, info = pcall(pmacs.describe.mode, buf) + show(pmacs.help.render_mode(ok and info or nil)) + end, +} + +pmacs.command.define { + name = "help.describe-buffer", + description = "Describe the active buffer.", + fn = function() + local buf = pmacs.window.buffer() + local ok, info = pcall(pmacs.describe.buffer, buf) + show(pmacs.help.render_buffer(ok and info or nil)) + end, +} + +pmacs.command.define { + name = "help.describe-hook", + description = "Describe a hook and list its listeners.", + fn = function() + pmacs.minibuffer.read { + prompt = "Describe hook: ", + history = "command", + on_accept = function(name) + if name == nil or name == "" then return end + local ok, info = pcall(pmacs.describe.hook, name) + show(pmacs.help.render_hook(name, ok and info or nil)) + end, + } + end, +} + +pmacs.command.define { + name = "help.where-is", + description = "Show which keys run a command.", + fn = function() + pmacs.minibuffer.read { + prompt = "Where is command: ", + source = "commands", + history = "command", + on_accept = function(name) + if name == nil or name == "" then return end + local ok, info = pcall(pmacs.describe.command, name) + if not ok or type(info) ~= "table" then + pmacs.editor.set_status("where-is: no such command: " .. name) + return + end + show(pmacs.help.render_where_is(name, info.key_bindings)) + end, + } + end, +} + +pmacs.command.define { + name = "help.list-commands", + description = "List every registered command with its description.", + fn = function() show(pmacs.help.render_command_list(sorted_command_names())) end, +} + +pmacs.command.define { + name = "help.list-keybindings", + description = "List every key binding, grouped by scope.", + fn = function() show(pmacs.help.render_keybinding_list(pmacs.keymap.list())) end, +} + +pmacs.command.define { + name = "help.list-settings", + description = "List every registered setting with its description.", + fn = function() show(pmacs.help.render_settings_list(pmacs.config.list())) end, +} + +pmacs.command.define { + name = "help.apropos", + description = "Search command names and descriptions by substring.", + fn = function() + pmacs.minibuffer.read { + prompt = "Apropos (substring): ", + history = "command", + on_accept = function(needle) + if needle == nil or needle == "" then return end + show(pmacs.help.render_apropos(needle, pmacs.help.apropos_hits(needle))) + end, + } + end, +} + +-- --------------------------------------------------------------------- +-- Forwarders (framing Q#D2) +-- --------------------------------------------------------------------- +-- +-- `help.*` is canonical, so typing `help` at M-x surfaces the whole +-- family. These two keep the documented names working for users whose +-- muscle memory and whose `docs/keybindings.md` predate the rename. +-- +-- Two names for one thing is the duplication §5 complains about; it is +-- the bounded price of not breaking documented commands, and it carries +-- a deprecation path a later stage can take. + +-- `invoke`, NOT `invoke_interactive`. The forwarder must work however it +-- was itself reached, and `pmacs.command.invoke('editor.describe-setting')` +-- is a real caller (`tests/config_registry_acceptance.rs`) — CI caught +-- that, because the acceptance pin here only drove the M-x path. +-- +-- Plain `invoke` is also the correct semantics, not merely the working +-- one: the interactive-command boundary is rotated once, by whatever +-- entry point the user actually used, for the name the user actually +-- typed. Rotating again on the inner call would record a second +-- boundary for a command the user never invoked. +local function forward(old_name, new_name) + pmacs.command.define { + name = old_name, + description = string.format("Deprecated alias for `%s`.", new_name), + fn = function() pmacs.command.invoke(new_name) end, + } +end + +forward("editor.describe-command", "help.describe-command") +forward("editor.describe-setting", "help.describe-setting") diff --git a/builtin/runtime/welcome.lua b/builtin/runtime/welcome.lua index fa55b3c..e2468cf 100644 --- a/builtin/runtime/welcome.lua +++ b/builtin/runtime/welcome.lua @@ -69,34 +69,6 @@ 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, -} +-- The `help` command itself lives in `runtime/help.lua`, which owns the +-- whole discovery family and loads after this file so its index can read +-- `pmacs.welcome.entries` above. This file keeps only the greeting. diff --git a/docs/active-work.md b/docs/active-work.md index 75eed40..6663937 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -380,6 +380,106 @@ which would have re-conflicted on every merge. githubsucks/journey-stage1b3-welcome ``` +## Discovery Stage 1 (P4) — IMPLEMENTED, PR OPEN + +- **Branch `discovery-stage1-commands`**, worktree `../pmacs-p4-discovery`, + based on `githubsucks/main` @ `54a092e`. **Implemented; PR open.** + `docs/discovery-stage1-command-family-framing.md` revision 6, + three review rounds closed (round 1: two blocking, two major; round 2: + two blocking, two major; round 3: two factual corrections; all + accepted), Q#D2 / Q#D3 decided by the user, and the final review's + acceptance corrections applied. +- **What it is.** `COHERENCE.md` §20 Priority 4 — "almost pure wiring, + the best payoff-per-effort in this document". **Eleven commands under + one `help.*` prefix**: nine new (describe-key/mode/hook/buffer, + where-is, list-commands, list-keybindings, list-settings, apropos) + plus `editor.describe-command` / `editor.describe-setting` renamed, + with the old names retained as **forwarders** so nothing documented + breaks. Typing `help` at M-x surfaces the whole family, which is the + discoverability win the arc exists for (Q#D2). +- **`apropos` matches by SUBSTRING, not fuzzy** (Q#D3). `fuzzy_score` + is subsequence-based and descriptions are long sentences, so fuzzy + would match nearly every command. Pinned by a + `test.apropos-subsequence-fixture` whose `qzjx` letters occur as `q z + j x`, only after asserting no registered name or description contains + `qzjx` as a substring; it must find nothing, whereas fuzzy finds the + fixture. +- **It adds no Rust.** `pmacs.describe.*`, `pmacs.keymap.list()`, + `pmacs.command.list()` and `pmacs.config.list()` already return + everything needed, and `parse_completion_source` accepts a **Lua + `Function`** (`CompletionSource::Custom`), so even the prompts need no + new Rust. +- **Completion is ASSISTANCE, not validation.** + `resolve_accepted_value` returns the **literal typed text** whenever + no candidate is selected, so a non-matching typo still reaches + `on_accept` and the existing error path — and a fuzzy near-miss can + silently describe a *different* setting, a new failure mode. + Closed-set acceptance ("refuse a non-candidate") is Rust work and is + deferred. The custom source needs a **mapper**: `config.list()` + yields descriptor *tables* while `Custom` consumes a sequence of + strings. **It does not control display order** — + `recompute_candidates` runs `filter_and_sort` (fuzzy score, lexical + tiebreak); sorting the pool matters only because `.take( + CANDIDATE_LIMIT)` runs *before* the sort, so pool order decides which + candidates survive truncation. +- **`invoke_interactive` is NOT the M-x path** — the error #205 + corrected, repeated one PR later. The path is dispatch `M-x` → + `editor.execute-command` → assert the selected candidate **before** + RET (`accept()` does `session.take()`) → accept → `invoke_interactive`. + Six of the eleven canonical commands (`help.describe-command`, + `help.describe-setting`, `help.describe-key`, `help.describe-hook`, + `help.where-is`, and `help.apropos`) open a **second** prompt the pins + must drive too; a pin that stops after the first RET has tested the + palette. +- **One owner for `*help*` writes — NOT a one-site migration.** + `src/help.rs` has semantic renderers for command/key/buffer/mode/ + hook/view and **none for settings, lists or apropos**, and + `_show_help` takes already-flattened text, so a later migration still + changes each command's subject-specific logic. What the funnel buys is + the shared policy in one place: reuse-by-name, wholesale + delete+insert, the `q` binding, and the foreign-`*help*` hazard. + **`*help*` is ordinary editable content** — no read-only intercept, no + generated-content invariant. And read-only would **not** fix the + foreign-buffer hazard either: a user's own `*help*` carries no + intercept of ours, so the renderer still finds it by name and clears + it. The missing guarantee is **ownership identity**, which `listview` + and dired both carry and this mechanism does not. Each command's rendering + is a named per-subject function so the future Rust work is enumerated + per-subject (three new renderers) rather than discovered per-call-site. +- **Deliberately deferred, each with a reason:** richer M-x rows + (`MinibufferPrompt.candidates` is `Vec` — protocol change); + `Command` gaining title/category/flags (~147 definition sites); + predicate evaluation (**read only at `src/help.rs:76` and one test** — + a behaviour change); help-layer unification; closed-set acceptance; + and the **help prefix key**, which this stage does not touch because + #205 recorded why `C-h` is not free. +- **Implementation notes.** `runtime/help.lua` is new and owns the + family plus the `help` index, which **moved out of `welcome.lua`** so + the greeting file keeps only the greeting; it loads after welcome.lua + so the index can read `pmacs.welcome.entries`. +- **The seam-counting pin caught a real bypass immediately.** The two + renamed commands were still calling the file-local `show_help_text`, + so the funnel the framing promised was fiction for exactly the two + commands that predate the seam. They now call + `pmacs.editor._show_help`, with a comment saying why the in-scope + local is deliberately not used. +- **Bites, all directed** — six mutations, each failing exactly one pin: + fuzzy apropos, name-only apropos, static where-is, a dropped + forwarder, an unindexed family command, and one command writing + `*help*` itself (seam count 10 vs 11). +- **§5 moves substrate-without-surface → Partial ON THIS PR** per §25, + with the remaining gaps named in the row: packages and workers have no + surface, `Command` still lacks title/category/flags, M-x rows are bare + names, and the Rust help layer is still orphaned. +- Recovery: + + ```sh + git fetch githubsucks + git worktree add ../pmacs-p4-discovery \ + -b discovery-stage1-commands \ + githubsucks/discovery-stage1-commands + ``` + ## Generated-buffer immutability lane (Arc: workbench primitives) — STAGE 1 MERGED; STAGE 2 IS NEXT **Framing #188 (revision 7) and Stage 1 #191 are both on `main` @ diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index ad61320..48a6605 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -300,6 +300,37 @@ 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). +- **Discovery arc (P4) — Stage 1 IMPLEMENTED, PR open** + (`docs/discovery-stage1-command-family-framing.md`, rev 6, three + review rounds). Eleven `help.*` commands over the existing registries, + indexed by `M-x help`. **§5 moves substrate-without-surface → + Partial.** No Rust: the data was all reachable from Lua, and the + settings completion source is a Lua function through + `CompletionSource::Custom` — correcting a `default.lua` comment that + claimed `source` was a fixed Rust-side vocabulary. + - **Completion is assistance, not validation.** + `resolve_accepted_value` returns the literal typed text whenever no + candidate is selected, so a typo still reaches `on_accept`; and a + fuzzy near-miss would silently select a *different* value. Refusing + a non-candidate is Rust work, deferred. + - **`apropos` is substring, not fuzzy.** `fuzzy_score` is + subsequence-based and descriptions are long sentences, so fuzzy + matches nearly everything. Pinned by a fixture whose description + contains the needle only as a non-contiguous subsequence. + - **One owner for `*help*` writes is what the seam buys — not a + one-site migration.** `src/help.rs` has renderers for + command/key/buffer/mode/hook/view and **none** for settings, lists + or apropos, and `_show_help` takes already-flattened text. Rendering + is therefore a named per-subject function, so the future Rust work + is enumerated per subject (three new renderers) rather than + discovered per call site. + - **The seam-counting pin caught a real bypass**: the two renamed + commands were still calling the file-local `show_help_text`, so the + funnel was fiction for exactly the two that predate it. + - **`Command.predicate` is still never evaluated** — read only at + `src/help.rs:76` and one test. A preservation pin registers a + *raising* predicate and asserts the command still runs, so a stage + that starts evaluating must change that pin knowingly. - **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 diff --git a/docs/discovery-stage1-command-family-framing.md b/docs/discovery-stage1-command-family-framing.md new file mode 100644 index 0000000..fc6bd73 --- /dev/null +++ b/docs/discovery-stage1-command-family-framing.md @@ -0,0 +1,546 @@ +# Discovery Stage 1 — the describe/list command family + +**Status: framing, rev 6 — final review corrections applied; ready for +implementation approval.** +**Serves `COHERENCE.md` §5 (unify discoverability), §1.1 (substrate +without surface), §20 Priority 4.** + +## 0. Revision history + +- rev 6 (2026-07-31) — final review corrections applied. + - **The second-prompt census now includes `help.describe-command`.** + Six of the eleven canonical commands take an argument, not five: + `describe-command`, `describe-setting`, `describe-key`, + `describe-hook`, `where-is`, and `apropos`. The full M-x driver must + accept each command's second prompt before it has tested the command. + - **The negative substring pin now has a discriminating fixture.** It + registers `test.apropos-subsequence-fixture` with a description whose + `qzjx` letters occur only as the non-contiguous sequence `q z j x`, then + first asserts that no registered command name or description contains + `qzjx` as a substring. `help.apropos qzjx` finding nothing therefore + fails under a fuzzy implementation rather than passing as an ordinary + no-match. + - **The index-property arithmetic follows the eleven-command family.** + Its targeted mutation is now adding a twelfth canonical command + without indexing it. +- rev 5 (2026-07-31) — **Q#D2 and Q#D3 answered by the user**; no review + findings at `1cc9d96`. + - **Q#D2 → `help.*` is canonical, with two forwarders.** + `editor.describe-command` and `editor.describe-setting` are renamed + to `help.*` and the old names retained as thin forwarders. The + family is **eleven** commands under one prefix, so typing `help` at + M-x surfaces all of it — which is the arc's entire purpose. The + split surface rev 3 flagged as "the one outcome that should not + survive review" is gone. + - **Q#D3 → `apropos` matches by substring**, stated in its own + description. `fuzzy_score` is subsequence-based and descriptions are + long sentences, so a short query's letters almost always appear in + order — fuzzy would match nearly every command and destroy the + precision that makes apropos worth having. +- rev 4 (2026-07-31) — review round 3. Two factual corrections, both + accepted. + - **The custom source does not control display order.** Rev 3 + justified `table.sort` by claiming `Custom` candidates appear in + return order; `recompute_candidates` immediately runs + `filter_and_sort`, which ranks by fuzzy score and tie-breaks + lexically. The sort is kept for a reason that is true — `.take( + CANDIDATE_LIMIT)` is applied to the filtered iterator *before* the + sort, so pool order decides which candidates survive truncation. + - **Read-only would not mitigate the foreign-`*help*` collision.** + Rev 3 said it would. A user-created buffer of that name has no + intercept of ours; the renderer finds it by name and clears it + regardless. The missing guarantee is **ownership identity**, the + thing `listview` and dired both have. +- rev 3 (2026-07-31) — review round 2. Two blocking, two major; all four + accepted. + - **The ledger lane still said revision 1 and kept refuted claims.** + Rev 2's ledger edit **aborted on a failed assertion before writing**, + so only one paragraph of it landed while the commit message reported + all of it. The lane is rewritten from scratch and the result was + verified by re-reading the file, not inferred from an exit code. + *(Second occurrence of this failure mode in this project — an + assert-then-write block discards every earlier edit in the block.)* + - **`names_from` does not exist.** Rev 2's completion source called a + helper nobody has written, over `config.list()`'s descriptor + **tables** where `Custom` wants a sequence of **strings** — the + prompt would have raised on an undefined global the first time it + opened. §3.2 now specifies the mapper. + - **`*help*` has no read-only intercept.** Rev 2's §3.4 claimed one. + `show_help_text` writes with plain `delete`/`insert` and #205 + recorded that this mechanism has not adopted the generated-buffer + write invariant. §3.4 now names the four policies that are actually + shared. + - **The naming was underspecified.** The nine-command table contains + no `help.describe-command`, so calling the `editor.*` commands + "aliases-by-retention" was wrong on both halves. They are now stated + as explicit exceptions, and Q#D2 is sharpened to the two ways out. +- rev 2 (2026-07-31) — review round 1. Two blocking, two major; all four + accepted, all four verified in the code first. + - **Completion does not close the free-text hole**, and rev 1 said it + did. `resolve_accepted_value` (`src/minibuffer.rs:564-575`) returns + the **literal typed text** whenever `session.selected` is `None`, so + a non-matching typo still reaches `on_accept` — and a fuzzy match + can instead select a *different* setting silently. Completion here + is **assistance**, not validation. §3.2 is reframed and acceptance 5 + now pins what actually happens; closed-set acceptance is named as + Rust work in §5. + - **`invoke_interactive` is not the M-x path** — the exact error #205 + corrected, repeated one PR later. It rotates the interactive-command + boundary and calls the body (`mod.rs:6097-6110`); it does not open a + palette. The path is **dispatch `M-x` → `editor.execute-command` → + accept a command → `invoke_interactive`**. Acceptances 1, 7 and 9 + are rewritten around it, including the **second** prompt for + commands that take an argument. + - **`_show_help(text)` is an output sink, not a migration seam.** + `src/help.rs` has semantic renderers for command / key / buffer / + mode / hook / view and **none** for settings, lists or apropos, so + once Lua has flattened those to text a later migration still has to + change each command's subject-specific logic. §3.4's claim is + narrowed to what is true — one owner for Lua `*help*` writes — and + §3.4a states the structure that makes the future Rust work + per-subject rather than per-call-site. + - **Ground-truth and counting errors.** §2.2 listed eight missing + commands and omitted `list-settings` while §3.1 listed nine; §2.5's + "two sites into ten" should have been eleven; `pmacs.keymap.lookup` + does **not** return `description` (it calls `key_info_table` with + `cmd = None`, `mod.rs:6938-6940`); and the `has_predicate` / + raw-predicate sites rev 1 cited are **`MenuItem` fields**, not + `Command.predicate`. The predicate conclusion survives on correct + evidence (§2.4). +- rev 1 (2026-07-31) — first framing. Scouted against `githubsucks/main` + @ `54a092e` (Journey Stage 1b-3, #205). + +## 1. Why this, and why now + +`COHERENCE.md` §20 Priority 4 calls unified discovery **"almost pure +wiring — the best payoff-per-effort in this document"**, and §5 grades +it *"substrate without surface — the sharpest instance of §1.1"*. + +Journey Stage 1b-3 (#205) just landed `M-x help` and documented it as +**the root of this family**: *"when the discovery arc adds `help.keys` +and friends, `help` stays the index they are reached from, so no rename +is owed."* This stage is that family. It also inherits 1b-3's deferred +question — the help prefix — with the constraint already recorded (§6). + +Two journey steps are graded *works but undiscoverable* and move here +without any new machinery: step 7 (symbol search — `M-.`/`M-?`/`C-c o` +bound but "advertised nowhere") and step 11 (`*workers*` — "no +keybinding, no indicator"). + +## 2. Ground truth + +Read in the tree at `54a092e`. **The substrate is already there**; this +stage adds no Rust. + +### 2.1 What Lua can already ask + +| Surface | Returns | +|---|---| +| `pmacs.command.list()` | every command name | +| `pmacs.describe.command(name)` | description, source, **`key_bindings`** (where-is, computed on demand) | +| `pmacs.describe.key(seq)` | resolved against the **active buffer + major mode** | +| `pmacs.describe.{buffer,view,mode,hook}` | structured tables | +| `pmacs.keymap.list()` | `{ sequence, command, scope }` for **every** binding, via `KeymapStack::iter_all` | +| `pmacs.keymap.lookup(seq)` | `{ sequence, command, scope, source }` — **not** `description`: it calls `key_info_table` with `cmd = None` (`mod.rs:6938-6940`), so the description arm never fires | +| `pmacs.config.list()` / `pmacs.config.describe(name, buf)` | full typed descriptors | + +Every one of the commands in §3 is a rendering of data already +reachable, and `CompletionSource::Custom(Function)` means even the +prompts need no new Rust (§3.2). **This stage adds no Rust at all.** + +That is what "pure wiring" means here, and it is worth stating precisely +so the stage is not oversold: **the work is surface, and the risk is in +what the surface leaves out.** + +### 2.2 What exists as a command today + +`editor.describe-command`, `editor.describe-setting`, +`editor.describe-instance[-buffer]`, `editor.list-buffers`, +`editor.list-workers`, and `help` (#205). + +**Missing entirely — nine, matching §3.1 exactly:** describe-key, +describe-mode, describe-hook, describe-buffer, where-is, list-commands, +list-keybindings, **list-settings**, apropos. (Rev 1 listed eight here +and nine in §3.1.) + +### 2.3 `describe-setting` prompts free-text, deliberately + +```lua +-- builtin/commands/default.lua +pmacs.minibuffer.read { + prompt = "Describe setting: ", + history = "command", -- note: no `source` + on_accept = function(name) … end, +``` + +A typo yields a status-line error. `describe-command` **does** pass +`source = "commands"`. The asymmetry is real and this stage closes it +(§3.2) — but note *why* it was skipped: dired's `C-x d` records that a +completion source makes RET-on-empty accept whatever sorts first, and a +selected candidate shadows typed text. That is a genuine trade, not an +oversight, so §3.2 says what changes about it. + +### 2.4 `Command.predicate` is stored, exposed, and never evaluated + +`predicate: Option` (`src/command.rs:79`) is read in exactly +**two** places: `src/help.rs:76` — inside the orphaned renderer — and one +assertion past `#[cfg(test)]`. No production call site *evaluates* it: +not `invoke`, not `invoke_interactive`, not dispatch, not M-x filtering, +not the menu. Its doc comment describes palette gray-out that never +shipped (§24 already logs this). + +*(Rev 1 cited `mod.rs:6204` / `:6257` as evidence. Those are +**`MenuItem`** fields — `item.label`, `item.group`, `item.order`, +`item.predicate` — a different type with its own predicate. The +conclusion held; the evidence did not.)* + +**This stage does not evaluate it either** (§5), because doing so makes +commands stop being invocable — a behaviour change needing its own +decision about what "unavailable" means at each call site. + +### 2.5 The help layer is duplicated, and this stage would deepen it + +`src/help.rs` has `render_command` / `render_key` / `render_buffer` / +`render_mode` / `render_hook` / `render_view` plus link resolution, and +is **orphaned** — the reachable renderer is the Lua `show_help_text`, +which renders *less* (no source, no scope). + +**Eleven commands each calling `show_help_text` directly would make it +an eleven-site migration** (nine new, plus the two renamed ones that +call it today). §3.4 is the answer, and it is the +most consequential decision in this framing — but §3.4 is careful about +what it can actually promise, because `src/help.rs` has renderers for +command / key / buffer / mode / hook / view and **none for settings, +lists, or apropos**. + +## 3. Design + +### 3.1 The family — eleven commands under one prefix + +**Nine new**, all rendering data that already exists: + +| Command | Reads | +|---|---| +| `help.describe-key` | `pmacs.describe.key` — prompts for a chord, resolved against the active buffer + mode | +| `help.describe-mode` | `pmacs.describe.mode` for the active buffer | +| `help.describe-buffer` | `pmacs.describe.buffer` | +| `help.describe-hook` | `pmacs.describe.hook` | +| `help.where-is` | `describe.command(name).key_bindings` | +| `help.list-commands` | `command.list()` + each description | +| `help.list-keybindings` | `keymap.list()`, grouped by scope | +| `help.list-settings` | `config.list()` | +| `help.apropos` | **substring** over names and descriptions (Q#D3) | + +**Two renamed**, so the family is not split: + +| Command | Was | +|---|---| +| `help.describe-command` | `editor.describe-command` | +| `help.describe-setting` | `editor.describe-setting` | + +**Naming — decided (Q#D2).** `help.*` is canonical. #205 established +`help` as the index, so the family it indexes shares its prefix, and +typing `help` at M-x now surfaces the whole family rather than only the +index. That grouping *is* the discoverability win this arc exists for. + +**The two old names become forwarders, not exceptions.** +`editor.describe-command` and `editor.describe-setting` remain +registered and invoke their `help.*` counterparts, so nothing a user has +in muscle memory or that `docs/keybindings.md` names stops working. + +Two forwarders are a real, bounded cost — two names for one thing is the +duplication §5 complains about. They are accepted because the +alternative is breaking documented commands, and they carry a +deprecation path a later stage can take. **What is not accepted is a +split family**, which is what rev 3 shipped and rev 5 removes. + +### 3.2 `describe-setting` gains completion — which is assistance, not validation + +Rev 1 claimed a completion source means "a typo cannot reach +`on_accept`". **It does not.** + +```rust +// src/minibuffer.rs:564-575 +fn resolve_accepted_value(session: &MinibufferSession, typed: &str) -> String { + if matches!(session.source, CompletionSource::None) { return typed.to_owned(); } + if let Some(idx) = session.selected + && let Some(cand) = session.candidates.get(idx) { return cand.clone(); } + typed.to_owned() // <-- no selection: the literal typed text +} +``` + +So with a source attached there are **two** outcomes rev 1 conflated: + +- **No candidate selected** (a typo matching nothing) → the literal text + reaches `on_accept`, exactly as today, and the existing + `no such setting: ` status path handles it. +- **A candidate selected** → that candidate wins over the typed text. On + a fuzzy source a near-miss can therefore **silently describe a + different setting** — a new failure mode, milder than the old one but + not nothing. + +What the source genuinely buys is *assistance*: the closed set is +visible and reachable by completion instead of having to be known. That +is worth doing and is what §3.1 promises. **Closed-set acceptance +semantics — "refuse a value that is not a candidate" — is Rust work** +(`resolve_accepted_value` and a per-session flag) and is deferred to +§5 rather than smuggled in as a side effect. + +**Still no Rust in this stage**, but the source needs a mapper. +`parse_completion_source` (`mod.rs:14145-14165`) accepts `none` / +`commands` / `buffers` / `files` **and a Lua `Function`** → +`CompletionSource::Custom`, and `Custom` consumes a **sequence of +strings**. `pmacs.config.list()` returns descriptor **tables**, so +handing it over directly would not typecheck — and rev 1 wrote +`names_from(...)`, **a helper that does not exist**; the prompt would +have raised on an undefined global the first time it opened. + +The mapper is three lines and belongs to this stage: + +```lua +source = function() + local names = {} + for _, d in ipairs(pmacs.config.list()) do names[#names + 1] = d.name end + table.sort(names) + return names +end, +``` + +Sorted for **deterministic pool construction**, not for display order — +rev 2's stated reason was false. `recompute_candidates` hands the pool +straight to `filter_and_sort`, which ranks by fuzzy score descending and +breaks ties lexically (`src/minibuffer.rs:672-680`), so what the source +returns never reaches the user in that order. + +The sort still earns its place, for a subtler reason: `filter_and_sort` +applies `.take(CANDIDATE_LIMIT)` to the **filtered** iterator *before* +sorting, so when more than `CANDIDATE_LIMIT` settings match a needle, +**pool order decides which ones survive truncation**. Registration +order would make that selection vary with an unrelated config edit; +sorting makes it reproducible. + +### 3.3 `M-x help` becomes the index + +#205 shipped `help` as a static cheat sheet. It now lists the family +above, so the arc's own promise — *`help` stays the index they are +reached from* — is kept rather than merely restated. + +### 3.4 One owner for Lua `*help*` writes — the honest version of the claim + +Every new command renders through **`pmacs.editor._show_help`** — the +seam #205 added — and **not** by calling `show_help_text` directly or +building its own buffer. + +**What that buys, precisely: one owner for `*help*` writes.** Four +shared policies get decided in one place instead of eleven: + +- **reuse-by-name** — a single `*help*` buffer found by name and reused + across invocations; +- **wholesale replacement** — `buf:delete(0, len)` then `buf:insert(0, + text)`, never a diff, because `*help*` is reflowed per subject; +- the **`q` binding**, rebound per fresh buffer; +- the **foreign-`*help*` hazard** — `find_or_create_help_buffer` matches + on the *name*, so a user's own buffer called `*help*` is adopted and + cleared. + +**`*help*` is ordinary editable content.** Rev 1 wrote "the read-only +intercept"; there isn't one. `show_help_text` writes with plain +`delete`/`insert`, the buffer keeps its undo history, and #205 already +recorded that this mechanism has **not** adopted the generated-buffer +write invariant. + +**And read-only would not fix the fourth policy either** — rev 2 implied +it would. A buffer the *user* created and named `*help*` carries no +intercept of ours, so an intercept on the buffers we create protects +nothing: the renderer still finds theirs by name and clears it. The +missing protection is **ownership identity** — a private table of +buffers this module created, so found-by-name is not adoption — which +is exactly what `listview` (`panels`) and dired (its handle table) both +carry and this mechanism does not. Naming the right missing guarantee +matters, because the wrong one would send a later fix at the wrong +layer. + +**What it does not buy, and rev 1 claimed it did:** a one-site migration +to `src/help.rs`. That layer has semantic renderers for **command, key, +buffer, mode, hook and view** — and **none for settings, lists, or +apropos**. `_show_help` takes *already-flattened text*, so by the time a +subject reaches it the structure a richer renderer would need is gone. +A later migration still has to change each command's subject-specific +logic; the seam saves the plumbing, not the semantics. + +### 3.4a The structure that makes the future work per-subject + +So the funnel is paired with a shape that keeps the semantics +addressable: each command's rendering is a **named per-subject +function** returning text — `render_key_help(info)`, +`render_settings_list(rows)` — and the command body does nothing but +call it and hand the result to `_show_help`. + +Then the future help-unification stage is: replace each named renderer +whose subject `src/help.rs` already covers (key, mode, hook, buffer), +and **write new Rust renderers for the three subjects it does not +cover** (settings, lists, apropos). That work is enumerated here rather +than discovered later, which is the actual deliverable of this section. + +**Corollary this stage must respect:** where the Lua renderer is poorer +than `src/help.rs` for a subject it *does* cover (no source, no scope), +the new commands render the poorer form rather than inventing a third +shape. + +## 4. Acceptance + +**N** = new behaviour, must fail on full revert. **P** = preservation, +falsified by a named mutation. + +### 4.0 The M-x path, stated once + +Rev 1 said "driven through `pmacs.command.invoke_interactive` (the M-x +path)". **That is not the M-x path**, and #205 established as much one +PR earlier. `invoke_interactive` rotates the interactive-command +boundary and calls the body (`mod.rs:6097-6110`); it opens no palette. + +Every pin below that claims to exercise a command as a user does drives: + +``` +dispatch M-x + → editor.execute-command opens the minibuffer (source = "commands") + → type the command name + → assert pmacs.minibuffer.selected() == "" -- BEFORE RET + → dispatch RET -- accept + → editor.execute-command calls invoke_interactive +``` + +The pre-RET assertion is not decoration: `accept()` does +`session.take()`, so afterwards nothing about the accepted value +survives, and a selected candidate shadows typed text. + +**Six of the eleven canonical commands take an argument and open a SECOND +prompt** (`describe-command`, `describe-setting`, `describe-key`, +`describe-hook`, `where-is`, `apropos`). Those pins drive that prompt too, +and assert against it with the same pre-accept discipline. A pin that stops +after the first RET has tested the palette, not the command. + +### 4.1 Pins + +1. **N — each of the eleven commands runs from M-x and renders + content.** Through §4.0's full path, including the second prompt + where the command takes one. Asserts **content produced** in + `*help*`. +2. **N — `where-is` agrees with the keymap.** Bind a command to a known + chord, then assert `where-is` reports that chord. Falsified by + rendering a static string. +3. **N — `list-keybindings` covers every binding `keymap.list()` + reports.** A property over the data, with a non-empty precondition so + the loop cannot be vacuous. +4. **N — `apropos` matches descriptions, not only names, and does so by + substring.** Two assertions: a word appearing in exactly one + command's *description* and no command *name* finds that command + (what distinguishes apropos from a name filter); and a deliberately + discriminating negative: register + `test.apropos-subsequence-fixture`, whose description's `qzjx` letters + occur only as the non-contiguous sequence `q z j x`, assert that **no** + registered command name or description contains `qzjx` as a substring, + then assert `help.apropos qzjx` finds **nothing**. A fuzzy implementation + finds the fixture, so this pins Q#D3's substring decision rather than + passing as an ordinary no-match. +5. **N — `describe-setting` completes, and a non-matching typo still + reaches the existing error path.** Two assertions, because §3.2 has + two outcomes: (a) typing a real setting's prefix makes it the + selected candidate, and accepting describes it; (b) typing a string + that matches **nothing** leaves `selected()` nil, and accepting + produces the `no such setting` status — **not** a described setting. + *Rev 1 asserted a typo "cannot reach `on_accept`", which + `resolve_accepted_value` contradicts.* +6. **N — `M-x help` lists the family.** A property over the family list, + so adding a twelfth canonical command without indexing it fails. +7. **P — every command's `*help*` write goes through `_show_help`.** + Replace that function with a counting stub, drive all **eleven** + through §4.0's path, and assert the count equals eleven. Pins §3.4's + *actual* claim — one owner for `*help*` writes — rather than the + migration claim rev 1 overstated. The two renamed commands are in the + count precisely because they were the pre-existing direct callers. +8. **P — the old names still work, as forwarders.** Invoke + `editor.describe-command` and `editor.describe-setting` through + §4.0's M-x path and assert they render the same subject as their + `help.*` counterparts. `editor.list-buffers` and + `editor.list-workers` are untouched and asserted unchanged. + Targeted mutation: dropping the forwarders after the rename — which + is the failure a user with muscle memory would hit first. +9. **P — no command's predicate is evaluated.** Register a command whose + predicate **raises**, then run it through §4.0's full M-x path and + assert it **runs**. Pins §2.4's deliberate non-change, so a stage + that starts evaluating predicates must change this pin knowingly. + Driven through the palette, not `invoke_interactive` directly — + otherwise it would pass even if M-x grew predicate filtering. + +## 5. Deferred, each with its reason + +- **Richer M-x rows.** `MinibufferPrompt.candidates` is `Vec` on + the wire; `CompletionPopupRow` already carries `kind`/`detail`, so the + pattern exists — but it is a **protocol change** and belongs to its + own stage with a version bump. +- **`Command` gaining title/category/aliases/flags/arg-schema.** A Rust + type change with ~147 definition sites; own stage. +- **Predicate evaluation** (§2.4) — a behaviour change. +- **Help-layer unification.** The reason §3.4 exists; own stage, made + cheaper by this one rather than harder. +- **A help prefix key.** #205 recorded the constraint in §18: `C-h` + deletes a word because non-kitty terminals cannot disambiguate + Ctrl+Backspace from Ctrl+H. This stage adds **no keybindings at all**, + so the prefix decision is taken once, for the whole family, by the + stage that can weigh `F1` / `C-c ?` / a rebind together. +- **Settings value provenance** (§11) — `describe-setting` will still + answer "who set this?" with the *definition* site. +- **Closed-set acceptance semantics** (§3.2). Making a prompt *refuse* a + value that is not a candidate needs `resolve_accepted_value` and a + per-session flag — Rust, and a change every existing prompt with a + source would inherit. Named here because rev 1 claimed this stage + delivered it as a side effect of adding completion. + +## 6. Coherence impact + +- **Journey steps touched:** 7 and 11, both graded *works but + undiscoverable*; this makes the existing bindings and the `*workers*` + view reachable by name. No step's grade flips on this stage alone — + both also want keybindings, which §5 defers. +- **§5's grade moves** from "substrate without surface" toward Partial: + the surface exists for commands, keys, modes, hooks and settings; it + does not yet exist for packages or workers (§13, §9). **That is a + §25 obligation on the landing PR.** +- **Interaction islands: none added.** Everything renders into the one + `*help*` buffer through the one seam (§3.4) — this stage's main job + is to *avoid* becoming the next island. +- **Config registry: not adopted.** Nothing here is a tunable. +- **Background-work attribution:** unchanged. + +## 7. Questions — decided + +- **Q#D2 — `help.*` or `editor.*`? → `help.*` canonical, with two + forwarders.** Eleven commands under one prefix, so typing `help` at + M-x surfaces the family. `editor.describe-command` / + `editor.describe-setting` keep working as forwarders. The split + surface rev 3 flagged is gone; the forwarders' duplication is the + accepted, bounded price of not breaking documented names. +- **Q#D3 — should `apropos` fuzzy-match? → No, substring.** + `fuzzy_score` is subsequence-based and descriptions are long + sentences, so a short query's letters almost always appear in order — + fuzzy would match nearly every command. Substring is stated in the + command's own description, and acceptance 4 pins it with a + subsequence-that-is-not-a-substring finding nothing. + +## 8. Ledger + +Branch `discovery-stage1-commands`, worktree `../pmacs-p4-discovery`, +based on `githubsucks/main` @ `54a092e`. Framing only; no code, no PR. + +A sibling lane (`test-ambient-isolation-impl`) is in flight in another +worktree and touches `src/editor.rs` and `tests/`. This lane is +`builtin/` plus its own suite, so the surfaces are disjoint; integrate +`main` late regardless. + +```sh +git fetch githubsucks +git worktree add ../pmacs-p4-discovery \ + -b discovery-stage1-commands \ + githubsucks/discovery-stage1-commands +``` diff --git a/docs/keybindings.md b/docs/keybindings.md index 1d3cbae..b08e4f4 100644 --- a/docs/keybindings.md +++ b/docs/keybindings.md @@ -189,8 +189,26 @@ 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 +`M-x help` is the **index of the discovery family**, rendered as a +`*help*` buffer inside the editor, and is what the startup welcome +points at. The family — all reachable by name, none bound to a key: + +| Command | Shows | +|---|---| +| `help.describe-command` | a command's description and bindings | +| `help.describe-setting` | a setting's type, default, effective value | +| `help.describe-key` | what a chord runs in **this** buffer | +| `help.describe-mode` | the active buffer's major mode | +| `help.describe-buffer` | the active buffer | +| `help.describe-hook` | a hook and its listeners | +| `help.where-is` | which keys run a command | +| `help.list-commands` | every command with its description | +| `help.list-keybindings` | every binding, grouped by scope | +| `help.list-settings` | every registered setting | +| `help.apropos` | substring search over names **and** descriptions | + +`editor.describe-command` and `editor.describe-setting` still work as +deprecated aliases of their `help.*` counterparts. 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 diff --git a/src/editor.rs b/src/editor.rs index bf897b4..4d3fd17 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -742,6 +742,16 @@ impl EditorState { include_str!("../builtin/runtime/welcome.lua"), ) .expect("load welcome builtin chunk"); + // Discovery Stage 1: the help/describe/list family. After + // `welcome.lua` so its index can read `pmacs.welcome.entries`, + // and after `commands/default.lua` (run by `attach_editor`) for + // `pmacs.editor._show_help` and the two commands it forwards to. + lua_host + .eval( + Some("@pmacs/builtin/runtime/help.lua"), + include_str!("../builtin/runtime/help.lua"), + ) + .expect("load help 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 diff --git a/tests/discovery_acceptance.rs b/tests/discovery_acceptance.rs new file mode 100644 index 0000000..8385b13 --- /dev/null +++ b/tests/discovery_acceptance.rs @@ -0,0 +1,497 @@ +// tests/discovery_acceptance.rs --- P4 Stage 1, the discovery family. + +//! `COHERENCE.md` §5 graded discoverability "substrate without surface". +//! These pins cover the surface: +//! `docs/discovery-stage1-command-family-framing.md` §4. +//! +//! **Every command is driven through the real M-x path**, stated once in +//! `run_from_palette` below. `pmacs.command.invoke_interactive` is *not* +//! M-x — it rotates the interactive-command boundary and calls the body; +//! it opens no palette. Journey Stage 1b-2 established this and Stage +//! 1b-3 re-established it, so it is encoded in a helper here rather than +//! left to each pin to remember. +//! +//! Six of the eleven canonical commands open a **second** prompt. A pin +//! that stops after the first RET has tested the palette, not the +//! command. + +use crossterm::event::{KeyCode, KeyEvent, KeyEventKind, KeyEventState, KeyModifiers}; +use pmacs::editor::EditorState; +use pmacs::protocol::FrontendId; + +#[path = "common/iso.rs"] +mod iso; + +fn exec(s: &EditorState, src: &str) { + s.lua_host.lua().load(src.to_owned()).exec().unwrap(); +} + +fn eval(s: &EditorState, src: &str) -> T { + s.lua_host.lua().load(src.to_owned()).eval().unwrap() +} + +fn key(code: KeyCode, mods: KeyModifiers) -> KeyEvent { + KeyEvent { + code, + modifiers: mods, + kind: KeyEventKind::Press, + state: KeyEventState::NONE, + } +} + +fn press(s: &mut EditorState, code: KeyCode) { + s.dispatch_key(FrontendId::LOCAL, key(code, KeyModifiers::NONE)); +} + +fn type_str(s: &mut EditorState, text: &str) { + for ch in text.chars() { + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char(ch), KeyModifiers::NONE), + ); + } +} + +fn minibuffer_active(s: &EditorState) -> bool { + eval(s, "return pmacs.minibuffer.is_active()") +} + +fn named_text(s: &EditorState, name: &str) -> String { + eval( + s, + &format!( + r#" + for _, id in ipairs(pmacs.buffer.list()) do + if pmacs.describe.buffer(id).name == {name:?} then + return id:slice(0, id:len()) + end + end + return "" + "# + ), + ) +} + +fn help_text(s: &EditorState) -> String { + named_text(s, "*help*") +} + +/// Drive the **real** M-x path: dispatch `M-x`, type the command name, +/// assert the palette selected exactly that command *before* RET — the +/// only moment it is observable, since `accept()` does `session.take()` +/// and a selected candidate shadows typed text — then accept. +/// +/// `second` is the argument for the six commands that open another +/// prompt; `None` for the five that do not. +fn run_from_palette(s: &mut EditorState, command: &str, second: Option<&str>) { + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char('x'), KeyModifiers::ALT), + ); + assert!(minibuffer_active(s), "M-x must open the palette"); + type_str(s, command); + assert_eq!( + eval::>(s, "return pmacs.minibuffer.selected()").as_deref(), + Some(command), + "the palette must have {command} selected; a different candidate \ + would run a different command" + ); + press(s, KeyCode::Enter); + + if let Some(arg) = second { + assert!( + minibuffer_active(s), + "{command} takes an argument and must open a second prompt" + ); + exec(s, &format!("pmacs.minibuffer.set_contents({arg:?})")); + press(s, KeyCode::Enter); + } else { + assert!( + !minibuffer_active(s), + "{command} takes no argument; a second prompt means the census is wrong" + ); + } +} + +/// Constructed with **isolated bootstrap roots**, never ambiently. +/// +/// An integration test is compiled without `cfg(test)`, so a raw +/// `EditorState::new()` reads the developer's real `init.lua` and writes +/// bundled packages into their real data root. The adoption ratchet in +/// `ambient_isolation_acceptance` caught this suite the moment the +/// isolation lane merged — which is the ratchet doing its job against +/// brand-new code, so this file is migrated rather than allowlisted. +fn editor() -> EditorState { + EditorState::new_with_roots(&iso::roots()) +} + +/// The eleven canonical commands and the argument each needs, if any. +/// **Six take one** — `describe-command` is easy to forget, because it +/// joined the family by rename rather than by being new. +fn family() -> Vec<(&'static str, Option<&'static str>)> { + vec![ + ("help.describe-command", Some("help.list-commands")), + ("help.describe-setting", None), // supplied per-test: needs a real setting + ("help.describe-key", Some("C-x C-f")), + ("help.describe-mode", None), + ("help.describe-buffer", None), + ("help.describe-hook", Some("buffer.after-load")), + ("help.where-is", Some("help.list-commands")), + ("help.list-commands", None), + ("help.list-keybindings", None), + ("help.list-settings", None), + ("help.apropos", Some("compile")), + ] +} + +// --------------------------------------------------------------------------- +// The family runs +// --------------------------------------------------------------------------- + +/// **N (acceptance 1)** — every canonical command runs from M-x and +/// renders content, including the second prompt where it takes one. +#[test] +fn d1_every_command_runs_from_the_palette_and_renders() { + for (name, arg) in family() { + let mut s = editor(); + // `describe-setting` needs a setting that exists; take the first. + let arg = if name == "help.describe-setting" { + Some(eval::(&s, "return pmacs.config.list()[1].name")) + } else { + arg.map(str::to_owned) + }; + run_from_palette(&mut s, name, arg.as_deref()); + let text = help_text(&s); + assert!( + !text.is_empty(), + "{name} must render content into *help*; got empty" + ); + } +} + +/// **N (acceptance 2)** — `where-is` agrees with the keymap. +/// +/// Falsified by rendering a static string. +#[test] +fn d2_where_is_reports_the_real_binding() { + let mut s = editor(); + exec( + &s, + "pmacs.command.define { name = 'test.whereis-probe', + description = 'probe', fn = function() end } + pmacs.keymap.bind { scope = 'global', sequence = 'C-c Q', + command = 'test.whereis-probe' }", + ); + run_from_palette(&mut s, "help.where-is", Some("test.whereis-probe")); + let text = help_text(&s); + assert!( + text.contains("C-c Q"), + "where-is must report the chord actually bound; got:\n{text}" + ); +} + +/// **N (acceptance 3)** — `list-keybindings` covers every binding +/// `keymap.list()` reports. A property over the data, not a fixed list. +#[test] +fn d3_list_keybindings_covers_every_binding() { + let mut s = editor(); + let sequences: Vec = eval( + &s, + "local out = {} + for _, r in ipairs(pmacs.keymap.list()) do out[#out+1] = r.sequence end + return out", + ); + assert!( + !sequences.is_empty(), + "precondition: the keymap must be non-empty or this loop is vacuous" + ); + run_from_palette(&mut s, "help.list-keybindings", None); + let text = help_text(&s); + for seq in sequences { + assert!( + text.contains(&seq), + "list-keybindings omits {seq:?}; got:\n{text}" + ); + } +} + +// --------------------------------------------------------------------------- +// apropos — substring, not fuzzy +// --------------------------------------------------------------------------- + +/// **N (acceptance 4)** — apropos matches descriptions, not only names, +/// **and does so by substring**. +/// +/// The negative half is the one that pins Q#D3. A bare "a subsequence +/// finds nothing" assertion would pass as an ordinary no-match; this +/// registers a fixture whose description contains `qzjx` **only** as the +/// non-contiguous sequence `q z j x`, and first proves no registered +/// command contains `qzjx` as a substring. A fuzzy implementation finds +/// the fixture, so the pin fails under fuzzy rather than passing. +#[test] +fn d4_apropos_matches_descriptions_by_substring_not_subsequence() { + let mut s = editor(); + exec( + &s, + "pmacs.command.define { name = 'test.apropos-description-probe', + description = 'zzyzx marker for the description-search pin', + fn = function() end } + pmacs.command.define { name = 'test.apropos-subsequence-fixture', + description = 'q z j x letters spaced apart on purpose', + fn = function() end }", + ); + + // Positive: a word in exactly one DESCRIPTION and no NAME. + let name_hits: i64 = eval( + &s, + "local n = 0 + for _, c in ipairs(pmacs.command.list()) do + if c:lower():find('zzyzx', 1, true) then n = n + 1 end + end + return n", + ); + assert_eq!(name_hits, 0, "precondition: 'zzyzx' is in no command NAME"); + run_from_palette(&mut s, "help.apropos", Some("zzyzx")); + assert!( + help_text(&s).contains("test.apropos-description-probe"), + "apropos must search descriptions, not only names; got:\n{}", + help_text(&s) + ); + + // Negative, discriminating: `qzjx` is a subsequence of the fixture's + // description but a substring of nothing. + let substring_hits: i64 = eval( + &s, + "local n = 0 + for _, c in ipairs(pmacs.command.list()) do + local d = pmacs.describe.command(c) + local desc = (d and d.description) or '' + if c:lower():find('qzjx', 1, true) or desc:lower():find('qzjx', 1, true) then + n = n + 1 + end + end + return n", + ); + assert_eq!( + substring_hits, 0, + "precondition: 'qzjx' must be a substring of nothing, or the \ + negative below proves nothing" + ); + + let mut s2 = editor(); + exec( + &s2, + "pmacs.command.define { name = 'test.apropos-subsequence-fixture', + description = 'q z j x letters spaced apart on purpose', + fn = function() end }", + ); + run_from_palette(&mut s2, "help.apropos", Some("qzjx")); + assert!( + !help_text(&s2).contains("test.apropos-subsequence-fixture"), + "substring matching must NOT find a subsequence — a fuzzy \ + implementation finds the fixture here; got:\n{}", + help_text(&s2) + ); +} + +// --------------------------------------------------------------------------- +// describe-setting completion +// --------------------------------------------------------------------------- + +/// **N (acceptance 5)** — completion assists, and a non-matching typo +/// still reaches the existing error path. +/// +/// Both halves, because §3.2 has two outcomes and rev 1 asserted only a +/// third that does not exist ("a typo cannot reach `on_accept`"). +#[test] +fn d5_describe_setting_completes_and_a_typo_still_errors() { + let mut s = editor(); + let first: String = eval(&s, "return pmacs.config.list()[1].name"); + + // (a) typing a real setting's full name selects it. + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char('x'), KeyModifiers::ALT), + ); + type_str(&mut s, "help.describe-setting"); + press(&mut s, KeyCode::Enter); + assert!(minibuffer_active(&s), "the setting prompt must open"); + exec(&s, &format!("pmacs.minibuffer.set_contents({first:?})")); + assert_eq!( + eval::>(&s, "return pmacs.minibuffer.selected()").as_deref(), + Some(first.as_str()), + "a real setting name must be the selected candidate" + ); + press(&mut s, KeyCode::Enter); + assert!( + help_text(&s).contains(&first), + "accepting a completed setting describes it" + ); + + // (b) a name matching nothing still reaches `on_accept` and errors — + // completion is assistance, not validation. + let mut s2 = editor(); + run_from_palette( + &mut s2, + "help.describe-setting", + Some("qqzz-no-such-setting"), + ); + assert!( + s2.core.borrow().status.contains("no such setting"), + "a non-matching typo reaches the existing error path; status: {:?}", + s2.core.borrow().status + ); +} + +// --------------------------------------------------------------------------- +// The index +// --------------------------------------------------------------------------- + +/// **N (acceptance 6)** — `M-x help` lists the family, as a property. +/// +/// Targeted mutation: adding a twelfth canonical command without +/// indexing it. +#[test] +fn d6_the_help_index_lists_every_family_command() { + let mut s = editor(); + let family: Vec = eval(&s, "return pmacs.help.family"); + assert_eq!( + family.len(), + 11, + "the canonical family is eleven commands; update the index and \ + this pin together" + ); + run_from_palette(&mut s, "help", None); + let text = help_text(&s); + for name in family { + assert!(text.contains(&name), "the index omits {name}; got:\n{text}"); + } +} + +// --------------------------------------------------------------------------- +// Preservation +// --------------------------------------------------------------------------- + +/// **P (acceptance 7)** — every command's `*help*` write goes through +/// `_show_help`. +/// +/// Pins §3.4's actual claim: one owner for `*help*` writes. Not a +/// one-site migration claim — `src/help.rs` has no renderer for +/// settings, lists or apropos. +#[test] +fn d7_preservation_every_render_goes_through_the_one_seam() { + let mut s = editor(); + exec( + &s, + "_seam_calls = 0 + local real = pmacs.editor._show_help + pmacs.editor._show_help = function(text) + _seam_calls = _seam_calls + 1 + return real(text) + end", + ); + for (name, arg) in family() { + let arg = if name == "help.describe-setting" { + Some(eval::(&s, "return pmacs.config.list()[1].name")) + } else { + arg.map(str::to_owned) + }; + run_from_palette(&mut s, name, arg.as_deref()); + } + assert_eq!( + eval::(&s, "return _seam_calls"), + 11, + "all eleven commands must render through _show_help; a command \ + writing its own buffer would not be counted" + ); +} + +/// **P (acceptance 8)** — the old names still work, as forwarders. +/// +/// Targeted mutation: dropping the forwarders after the rename, which is +/// the failure a user with muscle memory hits first. +#[test] +fn d8_preservation_the_old_names_forward() { + let mut s = editor(); + run_from_palette( + &mut s, + "editor.describe-command", + Some("help.list-commands"), + ); + let forwarded = help_text(&s); + assert!( + forwarded.contains("help.list-commands"), + "editor.describe-command must still describe the command it is \ + given; got:\n{forwarded}" + ); + + let mut s2 = editor(); + run_from_palette(&mut s2, "help.describe-command", Some("help.list-commands")); + assert_eq!( + forwarded, + help_text(&s2), + "the forwarder must render the same subject as its target" + ); +} + +/// **P (acceptance 8, cont.)** — the forwarders also work when invoked +/// **programmatically**, not only from M-x. +/// +/// This pin exists because its absence shipped a bug: the forwarder body +/// used `invoke_interactive`, which raises when the alias is reached +/// through `pmacs.command.invoke` — a real caller in +/// `config_registry_acceptance`. The M-x pin above passed throughout, +/// because M-x is not the only way in. +#[test] +fn d8c_preservation_the_forwarders_work_programmatically() { + let s = editor(); + for old in ["editor.describe-command", "editor.describe-setting"] { + let ok: bool = eval( + &s, + &format!("return pcall(pmacs.command.invoke, {old:?}) and true or false"), + ); + assert!( + ok, + "{old} must be invocable programmatically, not only from the palette" + ); + } +} + +/// **P (acceptance 8, cont.)** — the untouched list commands still work. +#[test] +fn d8b_preservation_list_buffers_and_workers_are_untouched() { + let s = editor(); + for name in ["editor.list-buffers", "editor.list-workers"] { + assert!( + eval::(&s, &format!("return pmacs.command.exists({name:?})")), + "{name} must still be registered" + ); + } +} + +/// **P (acceptance 9)** — no command's predicate is evaluated. +/// +/// Driven through the real palette, not `invoke_interactive` directly: +/// otherwise it would pass even if M-x itself grew predicate filtering. +/// A stage that starts evaluating predicates must change this pin +/// knowingly. +#[test] +fn d9_preservation_a_raising_predicate_does_not_block_a_command() { + let mut s = editor(); + exec( + &s, + "_ran = false + pmacs.command.define { + name = 'test.predicate-probe', + description = 'probe whose predicate raises', + predicate = function() error('predicate evaluated') end, + fn = function() _ran = true end, + }", + ); + run_from_palette(&mut s, "test.predicate-probe", None); + assert!( + eval::(&s, "return _ran"), + "the command must run: predicates are stored and exposed but \ + never evaluated (framing §2.4)" + ); +}