fix(panel): close Stage 2A review round 1 (4 P1, 2 P2)
Integrates canonical `main` @ `cf54270` and closes every finding. **P1-1 — a stale document `Pointer` stole focus from the panel.** Real bug. `align_primary_document_window`'s unknown-buffer arm returned `Some(window)` despite aligning nothing, so #8's activation focused the document *before* `dispatch_pointer` rejected the mismatched buffer. It now returns `None`: alignment did not happen, so no caller may treat it as a document gesture. Pinned through `handle_dispatcher_event` — the real dispatcher seam — because the defect lived in the PAIR of alignment and activation, not in either alone. **The first version of that test was vacuous**: an unregistered session is dropped at `daemon.rs:1962` (#148's membership check) before the aligner runs, so it passed with the bug restored. It now registers a real semantic session and fails with exactly the reported symptom, focus moving `WindowId(2)` → `WindowId(3)`. **P1-2 — the approved A2A-2 fan-out was missing.** The semantic target returned one context. It now captures the primary document PLUS the visible side window, each provider invoked once, with a derived-hidden side omitted (Q#BP2b — no mode line to paint, so no callback should run for it). The acceptance asserts `windows.len() == 2`. This exposed a second defect the finding did not name: the consumer selected segments with `.find(|w| w.context.frontend_id == frontend_id)` — the FIRST context for the frontend. With two contexts that silently depended on capture order and could have shipped the panel's mode-line text as the document status band. `emit_statusline_segments` now takes the document `WindowId` and selects on window identity. **P1-3 — the census suite tested the authority, not the consumers.** Confirmed: reverting a producer to `active_window_for` left all ten tests green. Added consumer-level pins that drive the real producers through `SemanticRenderState::render_frame` with a panel focused, plus the terminal-declaration guard. Bite-verified: reverting the `LineNumbers` routing now fails `consumer_line_numbers_follow_the_document_not_the_focused_panel`. **P1-4 — main integrated.** The textual conflict was `docs/active-work.md` (both lanes rewrote the same region; the terminal-config lane is kept whole and the bottom-panel heading updated). `src/editor.rs` auto-merged, and the full gate suite was rerun on the merge result. **P2-5 — the painter test was vacuous.** A fixed-point check that survived deleting `window.text_view.render`. It now asserts each of the four extracted outputs actually appears: buffer TEXT, the line-number GUTTER (with line numbers explicitly enabled, rather than dropping the assertion), the window MODE LINE, and a returned caret. Bite-verified by deleting the render call. **P2-6 — the stale fold-projection claim is corrected.** `src/window.rs`'s `fold_projection` doc no longer asserts that a semantic session never enters `paint_frame`; it records that the panel band breaks that premise and that the extracted painters take the map as a parameter. Gates on the merge result: fmt clean; workspace clippy clean; 1,832 default + 2,010 CRDT library tests; Stage 2A acceptance 13; Stage 1 46; statusline 8; m11_5 2; GPU initial target 14; terminal config 12; folding Stage 2 48; vterm 1/2 10 / 6; M4 121; required GPU 202; `git diff --check` clean. `vterm_stage3_acceptance::a37` remains the pre-existing flake measured on the base commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
commit
6b2b0f9dd2
32
COHERENCE.md
32
COHERENCE.md
|
|
@ -368,7 +368,7 @@ Full verdict table:
|
|||
| 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:44-52`) — but a missing binary fails silently (§1.2) and highlighting masks it. No LSP status command exists to diagnose |
|
||||
| 7 | Find symbol / file | **File: fixed (open by path merged #162; browsing PR #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 but undiscoverable** | Full PTY with scrollback + modeline segment — reachable only as `M-x terminal`, no keybinding. *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.* |
|
||||
| 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). Named limitation: `C-c t` is unreachable from *inside* a terminal window, where `C-c` is consumed as the escape — `M-x terminal` still works there. *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 | **Partial** | `M-x compile.run` works, defaults cwd to detected project root, parses Rust `-->` errors — but no keybinding, an **empty first prompt** (`initial = last and last.cmdline or ""`, `builtin/runtime/compile.lua:1134-1138`), and no `cargo build`/`cargo test` suggestion despite `ProjectKind::Cargo` existing (`src/project.rs:77`) |
|
||||
| 10 | Inspect error | **Partial (good once reached)** | `E:n W:n` modeline counts, underlines, `M-g n/p` + ``C-x ` `` walking a unified compile/grep/diag source, message echo, `RET` visits. Gated entirely on step 6 or 9 succeeding first |
|
||||
| 11 | See background work | **Works but undiscoverable** | `*workers*` view via `M-x editor.list-workers`; `C-c C-k` cancel-at-point. No keybinding, no statusline spinner/progress indicator anywhere (§9) |
|
||||
|
|
@ -379,6 +379,13 @@ A journey observation worth keeping verbatim from the audit:
|
|||
C-M-s` opens all folds, while opening a file, opening a terminal, and
|
||||
running a build have no bindings at all.
|
||||
|
||||
Two of that observation's three examples have since been answered —
|
||||
opening a file by `C-x C-f` (#162) and opening a terminal by `C-c t`
|
||||
(#173). **Running a build still has no binding**, and the underlying
|
||||
inversion is a standing bias in how new work gets bound, not three
|
||||
isolated omissions: the quote stays as written because it names the
|
||||
pattern, and the pattern is not retired until step 9 is.
|
||||
|
||||
---
|
||||
|
||||
## 3. A Strong Zero-Configuration State
|
||||
|
|
@ -639,7 +646,7 @@ Everything funnels through one function: `EditorInstance::dispatch_key`
|
|||
| 3 | query-replace | `editor.rs:945` | `QueryReplaceKey::from_chord` (`editor.rs:2967`) | **full shadow** |
|
||||
| 4 | Minibuffer | `editor.rs:951` | `MinibufferAction::from_chord` (`src/minibuffer.rs:468`) | **full shadow** |
|
||||
| 5 | Completion popup | `editor.rs:958-971` | `CompletionPopupKey::from_chord` (`editor.rs:3056`) | **partial shadow** (control chords only; skipped while a multi-key prefix is pending) |
|
||||
| 6 | Terminal transport + `C-c` escape | `editor.rs:973-1010` | `is_terminal_escape_chord` (`editor.rs:4355`) | **partial, transport-level** |
|
||||
| 6 | Terminal transport + configurable escape | `editor.rs:973-1010` | `EditorState::terminal_escape_chord` → `TerminalManager::escape_chord` (`src/terminal/session.rs`) | **partial, transport-level** |
|
||||
| 7 | Ordinary dispatch | `editor.rs:1018-1032` | `KeymapStack::resolve` | the only inspectable layer |
|
||||
|
||||
Facts that define the gap:
|
||||
|
|
@ -647,8 +654,10 @@ Facts that define the gap:
|
|||
- **Full shadows eat every key**, including unrecognized ones (each
|
||||
decoder has an `Ignore`/`Dismiss` fallback arm). While a terminal
|
||||
buffer is focused and unescaped, *all* keys encode to the child —
|
||||
`C-c`-leading user bindings are **structurally unreachable** in a
|
||||
terminal buffer.
|
||||
bindings led by the escape chord are **structurally unreachable** in
|
||||
a terminal buffer. Since #173 that chord is `terminal.escape-key`
|
||||
rather than a hardcoded `C-c`, so a user can *move* which prefix is
|
||||
eaten; they cannot make the shadow stop eating one.
|
||||
- **No transient-keymap mechanism exists to migrate to.** `KeymapStack`
|
||||
has exactly three fixed scopes — `Buffer(BufferId)`, `Mode(String)`,
|
||||
`Global` (`src/keymap_stack.rs:37-44`); resolution order buffer →
|
||||
|
|
@ -1013,20 +1022,29 @@ layering, provenance, and adoption have not followed.**
|
|||
`ConfigValue`s; `describe-setting`'s "Source:" names where `define()`
|
||||
ran. The inspection view sketched above is currently impossible to
|
||||
render.
|
||||
- **Adoption is five settings**: `editing.auto-pair` (pair.lua),
|
||||
- **Adoption is eight settings**: `editing.auto-pair` (pair.lua),
|
||||
`editing.trim-on-save` (editops.lua), `autosave.interval-ms`
|
||||
(autosave.lua), `window.panel-height` + `window.min-height`
|
||||
(window.lua). Everything else a user might set — theme, fonts, LSP
|
||||
(window.lua), and `terminal.default-profile` +
|
||||
`terminal.scrollback-rows` + `terminal.escape-key` (terminal.lua,
|
||||
#173). Everything else a user might set — theme, fonts, LSP
|
||||
server config, killring size, recentf/saveplace/desktop enables,
|
||||
pair sets, comment strings, `pmacs.parse.*` — lives in raw Lua
|
||||
outside the registry and is therefore invisible to `describe-setting`
|
||||
and any future settings UI. The migration list is already written:
|
||||
`docs/config-registry-framing.md` "named deferrals" (table-valued
|
||||
settings are the hard prerequisite for LSP/pair/comment tables).
|
||||
- **The table-valued gap now has a named, shipped instance.**
|
||||
`pmacs.terminal.profiles` (#173) is a raw Lua table sitting beside
|
||||
three registered scalars *for the same feature*, because a profile is
|
||||
inherently `{ command, args, cwd, env }` and the registry stores four
|
||||
scalars. It is the clearest evidence yet that table-valued settings
|
||||
are the blocking prerequisite: the terminal is now half-registered,
|
||||
and no settings UI can render the half that matters most.
|
||||
- **No persistence**: settings changed at runtime do not survive
|
||||
restart (the `custom-file` split-brain question is a named deferral).
|
||||
- The three-level separation holds in principle today (registry /
|
||||
hooks+keymaps / packages), but with five settings registered, level 1
|
||||
hooks+keymaps / packages), but with eight settings registered, level 1
|
||||
is effectively empty — users need executable Lua for nearly every
|
||||
ordinary preference, which is the exact failure the section warns
|
||||
about.
|
||||
|
|
|
|||
|
|
@ -3,6 +3,38 @@
|
|||
local terminal = assert(pmacs.terminal, "pmacs.terminal raw bindings are required")
|
||||
local raw_open = assert(terminal._open, "pmacs.terminal._open is required")
|
||||
|
||||
-- Q#TC2a. Every default reproduces today's behavior exactly, so a tree
|
||||
-- with no settings written and no profiles registered behaves as before.
|
||||
pmacs.config.define {
|
||||
name = "terminal.default-profile",
|
||||
type = "string",
|
||||
default = "",
|
||||
allow_empty = true,
|
||||
mutability = "live",
|
||||
description = "Profile name from pmacs.terminal.profiles to open by default. " ..
|
||||
"Empty means no profile: fall back to $SHELL.",
|
||||
}
|
||||
|
||||
pmacs.config.define {
|
||||
name = "terminal.scrollback-rows",
|
||||
type = "integer",
|
||||
default = 10000,
|
||||
min = 0,
|
||||
max = 4000000,
|
||||
mutability = "live",
|
||||
description = "Rows of scrollback retained per terminal. " ..
|
||||
"0 retains no history.",
|
||||
}
|
||||
|
||||
pmacs.config.define {
|
||||
name = "terminal.escape-key",
|
||||
type = "string",
|
||||
default = "C-c",
|
||||
mutability = "live",
|
||||
description = "Chord that escapes to the editor from a terminal. " ..
|
||||
"Pressing it twice sends the chord itself to the child.",
|
||||
}
|
||||
|
||||
local function bind_terminal_keys(buffer)
|
||||
local function bind(sequence, command)
|
||||
pmacs.keymap.bind {
|
||||
|
|
@ -19,22 +51,145 @@ local function bind_terminal_keys(buffer)
|
|||
bind("M->", "terminal.scroll-bottom")
|
||||
end
|
||||
|
||||
-- Q#TC1: profiles are a raw Lua table, not a config setting. The
|
||||
-- registry stores four scalars and has no table kind, so a profile —
|
||||
-- inherently `{ command, args, cwd, env }` — lives here beside
|
||||
-- `pmacs.lsp.config` and `pmacs.pair.sets` until table-valued settings
|
||||
-- exist.
|
||||
terminal.profiles = terminal.profiles or {}
|
||||
|
||||
local PROFILE_FIELDS = {
|
||||
command = "string",
|
||||
args = "table",
|
||||
cwd = "string",
|
||||
env = "table",
|
||||
}
|
||||
|
||||
-- Every diagnostic below renders a caller- or user-supplied value, so
|
||||
-- rendering must never be the thing that fails. `%q` is partial — it
|
||||
-- raises on a table or function — and a profile name arrives straight
|
||||
-- from `open { profile = ... }`.
|
||||
local function describe_name(name)
|
||||
if type(name) == "string" then return string.format("%q", name) end
|
||||
return string.format("<%s %s>", type(name), tostring(name))
|
||||
end
|
||||
|
||||
local function validate_profile(name, profile)
|
||||
local shown = describe_name(name)
|
||||
if type(profile) ~= "table" then
|
||||
error(string.format("terminal profile %s must be a table", shown), 0)
|
||||
end
|
||||
for key, value in pairs(profile) do
|
||||
local expected = PROFILE_FIELDS[key]
|
||||
if not expected then
|
||||
error(string.format("terminal profile %s: unknown field %q", shown, tostring(key)), 0)
|
||||
end
|
||||
if type(value) ~= expected then
|
||||
error(string.format(
|
||||
"terminal profile %s: field %q must be a %s, got %s",
|
||||
shown, key, expected, type(value)), 0)
|
||||
end
|
||||
end
|
||||
return profile
|
||||
end
|
||||
|
||||
-- `terminal.profiles` is a raw user table, so its keys are whatever the
|
||||
-- user wrote. Sorting them directly raises "attempt to compare number
|
||||
-- with string" the moment the table holds both a string and a numeric
|
||||
-- key — and it raises on the UNKNOWN-PROFILE path, replacing the very
|
||||
-- error this list exists to explain with an opaque one. Sorting DISPLAY
|
||||
-- strings is total over every key type, so the diagnostic survives a
|
||||
-- malformed table.
|
||||
local function known_profile_names()
|
||||
local names = {}
|
||||
for name in pairs(terminal.profiles) do names[#names + 1] = tostring(name) end
|
||||
table.sort(names)
|
||||
return names
|
||||
end
|
||||
|
||||
-- Q#TC2 / Q#TC3a: resolve a profile by name, or nil when none is
|
||||
-- selected. An explicitly requested profile that does not exist is an
|
||||
-- error even when `terminal.default-profile` is valid — a typo must not
|
||||
-- silently fall back to the default.
|
||||
local function resolve_profile(requested)
|
||||
local name = requested
|
||||
if name == nil then
|
||||
local configured = pmacs.config.get("terminal.default-profile")
|
||||
if configured == nil or configured == "" then return nil end
|
||||
name = configured
|
||||
end
|
||||
local profile = terminal.profiles[name]
|
||||
if profile == nil then
|
||||
local known = known_profile_names()
|
||||
local listed = #known > 0 and table.concat(known, ", ") or "(none defined)"
|
||||
error(string.format(
|
||||
"terminal profile %s is not defined; known profiles: %s",
|
||||
describe_name(name), listed), 0)
|
||||
end
|
||||
return validate_profile(name, profile)
|
||||
end
|
||||
|
||||
-- Q#TC3a merge order, per field: explicit open field, then the profile's
|
||||
-- field, then the scalar setting, then the built-in fallback. `env` is
|
||||
-- the one field where "first wins" would be wrong, so it MERGES with
|
||||
-- explicit entries overriding the profile's — any other reading silently
|
||||
-- drops half a user's environment.
|
||||
local function merge_env(profile_env, explicit_env)
|
||||
if profile_env == nil then return explicit_env end
|
||||
local merged = {}
|
||||
for key, value in pairs(profile_env) do merged[key] = value end
|
||||
for key, value in pairs(explicit_env or {}) do merged[key] = value end
|
||||
return merged
|
||||
end
|
||||
|
||||
function terminal.open(spec)
|
||||
local buffer = raw_open(spec)
|
||||
spec = spec or {}
|
||||
local resolved = {}
|
||||
for key, value in pairs(spec) do
|
||||
if key ~= "profile" then resolved[key] = value end
|
||||
end
|
||||
|
||||
local profile = resolve_profile(spec.profile)
|
||||
if profile then
|
||||
for key in pairs(PROFILE_FIELDS) do
|
||||
if key ~= "env" and resolved[key] == nil then resolved[key] = profile[key] end
|
||||
end
|
||||
resolved.env = merge_env(profile.env, spec.env)
|
||||
end
|
||||
|
||||
-- The two open-time settings resolve through the GLOBAL chain
|
||||
-- (Q#TC2b): they are read before the identity buffer exists, so there
|
||||
-- is no terminal to resolve a buffer-local against.
|
||||
if resolved.scrollback_rows == nil then
|
||||
resolved.scrollback_rows = pmacs.config.get("terminal.scrollback-rows")
|
||||
end
|
||||
if resolved.command == nil then
|
||||
resolved.command = os.getenv("SHELL") or "/bin/sh"
|
||||
end
|
||||
|
||||
local buffer = raw_open(resolved)
|
||||
bind_terminal_keys(buffer)
|
||||
return buffer
|
||||
end
|
||||
|
||||
pmacs.command.define {
|
||||
name = "terminal",
|
||||
description = "Open a terminal running $SHELL (or /bin/sh).",
|
||||
fn = function()
|
||||
return terminal.open {
|
||||
command = os.getenv("SHELL") or "/bin/sh",
|
||||
}
|
||||
description = "Open a terminal running the configured profile, or $SHELL.",
|
||||
fn = function(profile)
|
||||
return terminal.open { profile = profile }
|
||||
end,
|
||||
}
|
||||
|
||||
-- Q#TC10: the opening binding. `COHERENCE.md` Priority 1 names a
|
||||
-- terminal keybinding as part of protecting the golden journey, and §2
|
||||
-- step 8 grades the terminal "works but undiscoverable". `C-c` is
|
||||
-- already a live global prefix (fold's `C-c @ ...`), so this is a new
|
||||
-- leaf under it rather than a shadow.
|
||||
--
|
||||
-- Named limitation: unreachable from INSIDE a terminal window, where
|
||||
-- `C-c` is consumed as the escape. `M-x terminal` still works there.
|
||||
pmacs.keymap.bind { scope = "global", sequence = "C-c t", command = "terminal" }
|
||||
|
||||
pmacs.command.define {
|
||||
name = "terminal.copy-selection",
|
||||
description = "Copy the active terminal selection.",
|
||||
|
|
|
|||
|
|
@ -389,14 +389,110 @@ If it does not, stop and repair the remote/fetch configuration.
|
|||
**isolated-config workspace sweep 3,177 across 92 suites, zero failures**;
|
||||
`git diff --check` clean. Gates were run against the committed tree.
|
||||
|
||||
## Terminal config + copy mode arc — Stage 1 IN REVIEW
|
||||
|
||||
- Approved framing: `docs/terminal-config-and-copy-mode-framing.md`
|
||||
**revision 4** (four review rounds), committed as the first commit of
|
||||
Stage 1's branch. Two stages, two branches, two PRs; **no protocol
|
||||
change**.
|
||||
- **Stage 1 = `githubsucks/terminal-config`**, worktree
|
||||
`../pmacs-terminal-config`, based on `githubsucks/main` @ `d152120`
|
||||
and merged up to `c93f9ee` during review round 1. Profiles,
|
||||
scrollback, escape key, and the `C-c t` opening binding.
|
||||
- **Stage 2 = `terminal-copy-mode`, not started.** Branch it off `main`
|
||||
after Stage 1 merges: no dependency, but both edit
|
||||
`builtin/runtime/terminal.lua`.
|
||||
- Load-bearing decisions, each forced by scouted ground truth:
|
||||
- profiles are a **raw Lua table** — `ConfigValue` is four scalars with
|
||||
no table kind, so they join `pmacs.lsp.config` / `pmacs.pair.sets`;
|
||||
- the **two open-time settings resolve through the global chain**,
|
||||
because they are read before the identity buffer exists; only
|
||||
`terminal.escape-key` resolves per buffer;
|
||||
- the escape cache lives on **`TerminalSession`** so its lifetime is
|
||||
the terminal's. `value_epoch` alone is not a sufficient key: it does
|
||||
not advance when focus moves between terminals with different
|
||||
buffer-local values;
|
||||
- repeating the escape sends **that chord**, not a hardcoded `0x03`.
|
||||
- **Four bites, each against a different plausible wrong
|
||||
implementation** — hardcoded ETX fails acc6/9; epoch-only cache key
|
||||
fails acc7; single last-entry cache fails acc8's parse count; removing
|
||||
the invalid-value fallback fails acc10. The first version of acc7
|
||||
passed against the epoch-only bite because it asserted only that
|
||||
terminal A still worked; the discriminating assertion is that **each**
|
||||
terminal honors its own chord and not the other's.
|
||||
- Test instruments worth reusing: `cat -v` is the echo probe, because the
|
||||
screen rejects C0 controls before they reach cells so a raw echoed
|
||||
`Ctrl-X` is invisible; and the probe **counts occurrences** rather than
|
||||
testing presence, because a single-character probe collides with the
|
||||
child's own banner text.
|
||||
- **Review round 1 (2026-07-25) — five findings, all real, all fixed.**
|
||||
One blocker and two majors were the same failure in three places: a
|
||||
claim asserted somewhere cheaper than where it lives.
|
||||
- *Blocker — `COHERENCE.md` was stale in four places, not the three
|
||||
reported.* Step 8 still read "no keybinding"; §11 still read "five
|
||||
settings"; and §6's dispatch table still cited
|
||||
`is_terminal_escape_chord`, **a symbol this PR deletes**. §25 makes
|
||||
that update ride the PR. A PR that changes audited ground truth has
|
||||
to re-grep the audit for its own symbols, not only for its topic.
|
||||
- *Major — acceptance 5 was vacuous.* It asserted a registry
|
||||
round-trip, so it stayed green with the setting's **only** consumer
|
||||
deleted. It now opens a real terminal whose child overflows the
|
||||
24-row screen, scrolls the view to its oldest retained row, and
|
||||
asserts `LINE001` is present at 10,000 and absent at 0. **Asserting
|
||||
a value was stored is not asserting anything reads it.**
|
||||
- *Major — acceptance 8a asserted the session count, not the cache.*
|
||||
An editor-side map with no purge hook — the exact rejected design —
|
||||
leaks *while* sessions drain, so it passed. Fixed with a
|
||||
`TerminalManager::escape_caches()` seam. **A lifecycle claim needs a
|
||||
lifecycle observable.**
|
||||
- *Moderate — `table.sort` over user-controlled profile keys.* A
|
||||
table holding both a string and a numeric key raised `attempt to
|
||||
compare number with string` **on the unknown-profile path**,
|
||||
replacing the diagnostic being asked for; `%q` raised likewise on a
|
||||
non-string `profile` argument. Both are partial functions applied to
|
||||
user input **on a diagnostic path** — the error reporter was the
|
||||
thing that failed.
|
||||
- *Minor — the committed framing still said "not yet approved".*
|
||||
- **Three new bites, each falsified by revert**: deleting the scrollback
|
||||
consumer fails acc5 (and only acc5); restoring the raw-key sort
|
||||
reproduces `attempt to compare string with number` verbatim; and
|
||||
implementing the rejected editor-side map fails the new acc8a at
|
||||
`left: 2, right: 1` **while passing the old session-count version** —
|
||||
which is the review finding demonstrated rather than argued.
|
||||
- Verification after the round-1 fixes, on the tree merged with
|
||||
`githubsucks/main` @ `c93f9ee`: `cargo fmt --check` clean; strict
|
||||
workspace Clippy clean; 1,832 default + 2,009 CRDT library tests;
|
||||
`terminal_config_acceptance` **12/12 in both configurations**; vterm
|
||||
Stage 1/2 9+10 / 6+6; config registry 16+16; bottom-panel Stage 1
|
||||
46+46; M4 121; required GPU 202; `git diff --check` clean.
|
||||
- `compile_mode_acceptance` fails 11/67 against the **real** user
|
||||
config and passes 67/67 with an isolated `XDG_CONFIG_HOME` — the
|
||||
known pre-existing trap, not this branch.
|
||||
- **`vterm_stage3_acceptance::a37` fails on this machine — and fails
|
||||
identically on the PR's own base `d152120`**, so it is not this
|
||||
branch's regression. It is load-sensitive: it passed at `d152120`
|
||||
once and failed at that same commit twenty minutes later, with a
|
||||
second agent saturating the machine with `rustc` in between. Two
|
||||
ways it lies, both worth knowing: it **silently returns `ok` when
|
||||
`pmacs-gpu` is not built** in the same target dir (only
|
||||
`PMACS_REQUIRE_GPU=1` promotes that skip to a failure, and the gate
|
||||
list applies that flag to `-p pmacs-gpu`, a *different* package), and
|
||||
it is **crdt-gated, so CI has never run it at all**. A green a37 in
|
||||
a gate log means nothing unless the binary was built and the flag
|
||||
was set. Needs its own lane; see the CI `crdt`-coverage lane on #168.
|
||||
- `pmacs-gpu` itself failed 201/202 once under the same load and passed
|
||||
202/202 on immediate rerun.
|
||||
|
||||
## Bottom-panel lane (Arc 7) — Stage 1 + framing MERGED; Stage 2A IN REVIEW
|
||||
|
||||
Stage 1 and the Stage 2 framing are on `main`. **Stage 2A is
|
||||
implemented and in review.**
|
||||
|
||||
- **Stage 2A — portable branch `githubsucks/bottom-panel-stage2a`**,
|
||||
worktree `../pmacs-bp-stage2a`, based on `githubsucks/main` @
|
||||
`c93f9ee`. Two commits: the classified census routing, then the
|
||||
worktree `../pmacs-bp-stage2a`, **canonical `main` @ `cf54270`
|
||||
integrated** (review round 1, finding 4 — the terminal-config lane
|
||||
#173 also changes `src/editor.rs`, so gates were rerun on the merge
|
||||
result, not the old combination). Two commits: the classified census routing, then the
|
||||
painter extraction + acceptance. **No protocol change; no behavior
|
||||
change for any frontend today** — with `panel_capable = false` for
|
||||
semantic sessions, `primary_document_window` returns `view.active`
|
||||
|
|
@ -417,6 +513,17 @@ implemented and in review.**
|
|||
catch the first bite — it compares the two authorities directly, so
|
||||
only a consumer-level assertion catches a misrouted consumer. Keep
|
||||
both kinds.
|
||||
- **Review round 1 closed: 4 P1 + 2 P2, all real.** The P1s were a
|
||||
stale-`Pointer` focus steal (the failed-alignment arm returned the
|
||||
window, so #8's activation focused it before `dispatch_pointer`
|
||||
rejected the buffer), the missing A2A-2 two-context fan-out, a census
|
||||
suite that asserted the AUTHORITY rather than the CONSUMERS, and the
|
||||
missing main integration. **Two of the new pins were themselves
|
||||
vacuous on the first attempt** — the dispatcher test passed because an
|
||||
unregistered session is dropped at `daemon.rs:1962` before reaching
|
||||
the aligner, and the painter test was a fixed-point check that
|
||||
survived deleting `text_view.render`. Both now fail under their own
|
||||
bite.
|
||||
- **`vterm_stage3_acceptance::a37` is a pre-existing flake here**, not a
|
||||
Stage 2A regression: measured **6/8 failures on the base commit** and
|
||||
**7/8 on the branch** in matched isolated samples. It needs a real
|
||||
|
|
|
|||
|
|
@ -0,0 +1,658 @@
|
|||
# Terminal configuration and copy mode
|
||||
|
||||
**Revision 4 — scouted against canonical `main` @ `b889873` (protocol v20),
|
||||
2026-07-25. APPROVED after four review rounds. Stage 1 is implemented on
|
||||
branch `terminal-config` (PR #173); Stage 2 (`terminal-copy-mode`) is
|
||||
framed but not started, and branches off `main` after Stage 1 merges.**
|
||||
|
||||
Revision 4 gives the escape-key cache an owner and a lifecycle (Q#TC4c) —
|
||||
revision 3 named the key but not the storage, and two implementations
|
||||
satisfied its acceptance while behaving differently on A→B→A. It also corrects
|
||||
the read-only deferral, which understated the substrate required: the bypass
|
||||
path is `ensure_writable`-guarded too, so genuine immutability alone would
|
||||
break every generated buffer that refreshes.
|
||||
|
||||
Revision 3 corrects two design errors and decides the chords. The
|
||||
round-trip failure shape in revision 2 was **wrong in the reporter's favour**:
|
||||
a Lua intercept does not set `Buffer::read_only`, and there is no Lua binding
|
||||
that does, so an optimistic `CrdtOp` bypasses the intercept *and* passes
|
||||
`ensure_writable()` — the daemon buffer mutates too, rather than the mirror
|
||||
diverging alone (Q#TC6a). Revision 2 also had all three settings resolving
|
||||
against the terminal identity buffer, which is impossible for the two read
|
||||
*before* that buffer exists (Q#TC2b). Chords are now decided and
|
||||
collision-scouted rather than deferred to implementation (Q#TC10, Q#TC8a).
|
||||
|
||||
Revision 2 answered seven review findings. Four were load-bearing: the settings
|
||||
are `Live`, so the registry **accepts buffer-local overrides whether or not we
|
||||
want them**, and `value_epoch()` does not move on a buffer switch — an
|
||||
epoch-only cache can serve the wrong terminal's escape chord (Q#TC4); the
|
||||
double-escape byte is a hardcoded `0x03`, so a configured escape would still
|
||||
send Ctrl-C and make its own literal chord unreachable (Q#TC4b); the snapshot
|
||||
buffer needs `set_round_trip_input`, not only a read-only intercept, or a
|
||||
semantic frontend can optimistically edit it before daemon dispatch (Q#TC6);
|
||||
and the two stages must be two branches and two PRs. Revision 1's
|
||||
materialized-copy reframe is unchanged.
|
||||
|
||||
Two stages, one arc, no protocol change:
|
||||
|
||||
- **Stage 1 — configuration.** Terminal profiles, scrollback, and the escape
|
||||
key become configurable. Today the terminal has **zero** configuration
|
||||
surface: the `terminal` command hardcodes `os.getenv("SHELL") or "/bin/sh"`,
|
||||
`scrollback_rows` is a per-open argument only, and the escape chord is a
|
||||
literal in Rust.
|
||||
- **Stage 2 — copy mode and search over scrollback.** A command that turns
|
||||
the retained terminal screen and scrollback into an ordinary buffer, where
|
||||
isearch, motion, selection, and the kill ring already work.
|
||||
|
||||
Explicitly **not** in this arc: the panel terminal (blocked on bottom-panel
|
||||
Stage 2), and shell integration (cwd tracking, prompt marks, command zones) —
|
||||
the keystone that unlocks the VS Code-style cluster, which needs its own
|
||||
security framing because it decides what a child process may make the editor
|
||||
do.
|
||||
|
||||
## Branch and PR plan
|
||||
|
||||
**Two branches, two PRs.** Configuration and copy mode are independently
|
||||
releasable and have no dependency on each other; one framing covers the arc,
|
||||
but the one-feature/one-branch/one-PR rule governs the implementation.
|
||||
|
||||
1. `terminal-config` — Stage 1. Also carries the **terminal opening
|
||||
keybinding** (Q#TC10).
|
||||
2. `terminal-copy-mode` — Stage 2, branched off `main` after Stage 1 merges.
|
||||
|
||||
Sequencing is not a dependency but avoids a conflict: both stages edit
|
||||
`builtin/runtime/terminal.lua`.
|
||||
|
||||
## Ground truth (measured, not recalled)
|
||||
|
||||
Three facts constrain the design, and two of them rule out the obvious plan.
|
||||
|
||||
### 1. Terminal profiles cannot be a config-registry setting
|
||||
|
||||
`ConfigValue` is **four scalars** — `Bool`, `Int`, `Num`, `Str`
|
||||
(`src/config_registry.rs:312`) — and its own doc comment says they "are never
|
||||
stored --- only these four scalars (Q#CR3)". `ConfigKind` adds `Enum`, which
|
||||
is physically a string validated against choices fixed at `define` time
|
||||
(`src/config_registry.rs:115-145`). There is no table, list, or map kind.
|
||||
|
||||
A terminal profile is inherently a table: `{ command, args, cwd, env }` per
|
||||
name. **Table-valued settings are an existing named deferral of the config
|
||||
registry arc** — the same gap that keeps `pmacs.lsp.config`,
|
||||
`pmacs.pair.sets`, `pmacs.comment.strings`, and the `pmacs.parse.*` proxies as
|
||||
raw Lua. Profiles join that list rather than forcing that deferral open here.
|
||||
|
||||
### 2. Search cannot reuse isearch in place over a terminal
|
||||
|
||||
`SearchStore::set(buffer_id, query, matches: Vec<ByteRange>)`
|
||||
(`src/search.rs:99`) keys matches by buffer and addresses them as **byte
|
||||
ranges into that buffer's rope**; the painting path materializes the source
|
||||
with `buf.snapshot_rope().slice(0, buf.len(), ..)` (`src/search.rs:435`).
|
||||
|
||||
A terminal identity buffer is **empty and read-only** by construction. Its
|
||||
content lives in `TerminalScreen` as cells addressed by `(row, col)` across
|
||||
history plus visible rows — there are no rope bytes to range over. Searching a
|
||||
terminal in place therefore means a second, parallel search facility with its
|
||||
own match store and its own highlight path, because terminal painting consumes
|
||||
owned cells and not document style spans.
|
||||
|
||||
### 3. An in-place copy mode would be the seventh dispatch shadow
|
||||
|
||||
`dispatch_key`'s terminal-transport arm intercepts **every** key before
|
||||
ordinary keymap dispatch whenever `active_terminal_key` is `Some`, which keys
|
||||
purely on `is_terminal(window.buffer_id)` (`src/editor.rs:1098-1107`,
|
||||
`973-1016`). A mode that keeps the terminal buffer focused while rebinding
|
||||
keys to motion/selection must therefore add a new precedence rung.
|
||||
|
||||
`COHERENCE.md` §6 grades that ladder **weak, "and growing by one island per
|
||||
modal feature"**, records that **no transient-keymap mechanism exists to
|
||||
migrate to** (`KeymapStack` has exactly three fixed scopes, no layer stack, no
|
||||
push/pop, no lifetime), and notes that `describe-key` already lies while a
|
||||
shadow is active. It also names the counter-example: the entire picker/panel
|
||||
family uses ordinary **buffer-local keymaps** and is inspectable and
|
||||
rebindable.
|
||||
|
||||
### 4. What already exists and is reusable
|
||||
|
||||
- `retained_rows(projection)` (`src/terminal/view.rs:539`) iterates history
|
||||
plus visible rows; `copy_selection_bytes(rows, selection)`
|
||||
(`src/terminal/view.rs:849`) serializes a range with the fidelity Stage 2
|
||||
criterion 21 already pins — soft wraps joined, hard rows separated, trailing
|
||||
default blanks trimmed, wide glyphs and combining clusters copied once.
|
||||
- `ConfigRegistry::value_epoch()` (`src/config_registry.rs:1127`) is public and
|
||||
monotonic — cheap invalidation for a hot-path cache.
|
||||
- The Lua surface is `define` / `get` / `set` / `set_local` / `on_change` with
|
||||
a disposable handle (`src/lua_bindings/config.rs`).
|
||||
- `pmacs.terminal.open` already accepts
|
||||
`command, args, cwd, env, name, rows, cols, scrollback_rows, display,
|
||||
window`. **`display = "panel"` already works** (bottom-panel Stage 1) — the
|
||||
panel terminal is blocked on rendering, not on this surface.
|
||||
- Terminal buffers already carry buffer-local bindings (`M-w`, `M-v`, `C-v`,
|
||||
`M-<`, `M->`) installed by `terminal.open` in `builtin/runtime/terminal.lua`.
|
||||
|
||||
## Stage 1 — configuration
|
||||
|
||||
**Q#TC1 — Profiles are a raw Lua table, not a setting.**
|
||||
`pmacs.terminal.profiles` maps a name to a spec table, exactly following the
|
||||
`pmacs.lsp.config` precedent. The registry holds only scalars. Rejected
|
||||
alternative: widening `ConfigValue` with a table kind — that is the config
|
||||
arc's own named deferral, it is cross-cutting (persistence, `describe-setting`
|
||||
rendering, the `custom-file` question all key on the scalar assumption), and
|
||||
smuggling it into a terminal PR would be the wrong place to decide it.
|
||||
|
||||
**Q#TC2 — `terminal.default-profile` is `String`, not `Enum`.** `Enum`
|
||||
choices are frozen at `define` time; profiles are user-extensible from
|
||||
`init.lua` and later. Validation happens at open time, and an unknown name
|
||||
must produce a pointed error that **names the known profiles**, not a bare
|
||||
"unknown profile".
|
||||
|
||||
**Q#TC2a — the exact settings, defaults, and bounds.** All three are `Live`
|
||||
(see Q#TC2b), and every default reproduces today's behavior exactly, so a tree
|
||||
with no settings written behaves identically (acceptance 12).
|
||||
|
||||
| name | kind | default | bounds |
|
||||
|---|---|---|---|
|
||||
| `terminal.default-profile` | `String { allow_empty: true }` | `""` | — |
|
||||
| `terminal.scrollback-rows` | `Integer` | `10_000` (`DEFAULT_TERMINAL_SCROLLBACK_ROWS`) | `0 ..= 4_000_000` (`MAX_TERMINAL_HISTORY_CELLS`) |
|
||||
| `terminal.escape-key` | `String { allow_empty: false }` | `"C-c"` | parsed as a chord |
|
||||
|
||||
**Zero is a legal scrollback value meaning "retain no history".** The core's
|
||||
own validation rejects only values *above* `MAX_TERMINAL_HISTORY_CELLS`
|
||||
(`src/terminal/session.rs:114`), so `scrollback_rows = 0` is accepted through
|
||||
`terminal.open` today. A `1` minimum here would invent an asymmetry between the
|
||||
setting and the per-open field for no reason.
|
||||
|
||||
`""` is the **"no default profile" sentinel**: an empty string means "fall
|
||||
through to `$SHELL`", not "a profile named empty". `allow_empty: true` exists
|
||||
precisely to express it, and the open path treats empty and unset identically.
|
||||
|
||||
**Q#TC2b — the settings are `Live`, and the registry therefore accepts
|
||||
buffer-local overrides. That is specified rather than accidental.**
|
||||
`ConfigRegistry::set_local` refuses only `StartupOnly` definitions
|
||||
(`src/config_registry.rs:949`); a `Live` setting can be pinned per buffer by
|
||||
anyone. Declaring these global-only is **not currently expressible** — a
|
||||
`scope = "global"` define flag is one of the config registry's own named
|
||||
deferrals, and `autosave.interval-ms` already has the same latent problem.
|
||||
|
||||
Making them `StartupOnly` instead would buy enforcement at the cost of the
|
||||
feature: the escape key could never be changed mid-session, which kills Q#TC4's
|
||||
whole point. So they stay `Live`, and resolution is defined **per setting,
|
||||
because the three are not read at the same moment**:
|
||||
|
||||
| setting | read when | resolution |
|
||||
|---|---|---|
|
||||
| `terminal.escape-key` | every keystroke in a terminal (cached) | `get(name, terminal_buffer)` — **buffer-local → global → default** |
|
||||
| `terminal.default-profile` | once, **before** the terminal exists | `get(name)` — **global chain only** |
|
||||
| `terminal.scrollback-rows` | once, **before** the terminal exists | `get(name)` — **global chain only** |
|
||||
|
||||
The split is forced, not stylistic. The two open-time settings are consumed by
|
||||
`_open` **before it creates the identity buffer**, so there is no terminal
|
||||
buffer to resolve against — and no caller could have pinned a local override on
|
||||
a buffer that does not yet exist. `pmacs.config.get(name)` with no buffer
|
||||
argument already means exactly "the global chain, never an ambient buffer", so
|
||||
this is the registry's existing semantic rather than a new rule.
|
||||
|
||||
Consequences, stated so they are not discovered later:
|
||||
|
||||
- a per-terminal escape key is a supported feature, not a bug;
|
||||
- `set_local` on `terminal.default-profile` or `terminal.scrollback-rows` is
|
||||
**always inert**, for any buffer, because the open path never consults a
|
||||
buffer chain. This is deliberate; the alternative — resolving against
|
||||
whichever buffer happened to be current at open time — would make a
|
||||
terminal's scrollback depend on what the user was looking at when they
|
||||
pressed the key.
|
||||
|
||||
Rejected alternative: resolving the open-time settings against the *target
|
||||
window's pre-open buffer*. It is expressible, but it makes an ambient buffer
|
||||
load-bearing for a value the user set globally, which is the trap
|
||||
`pmacs.config`'s two-argument/one-argument split exists to avoid.
|
||||
|
||||
**Q#TC3 — `terminal.scrollback-rows` is `Integer` with bounds, and an explicit
|
||||
per-open `scrollback_rows` still wins.** The precedence is
|
||||
**explicit argument over global setting** — there is no ambient buffer in this
|
||||
chain at all (Q#TC2b resolves it through `get(name)`), so the rule is simply
|
||||
that what a caller passes to `terminal.open` beats what the user configured
|
||||
globally. The bounds above come from the existing validation, so the setting
|
||||
cannot express a value the core will reject.
|
||||
|
||||
**Q#TC3a — profile resolution order, field by field.** `profile` is accepted
|
||||
by **`pmacs.terminal.open` as well as the command**, so a Lua caller is not
|
||||
forced through the command to use one. For each field, the first source that
|
||||
supplies it wins:
|
||||
|
||||
1. an explicit `pmacs.terminal.open` field;
|
||||
2. the named profile's field — `profile` argument, else
|
||||
`terminal.default-profile` when non-empty;
|
||||
3. the scalar setting, where one exists (`scrollback_rows` only);
|
||||
4. the built-in fallback (`command` = `$SHELL`, else `/bin/sh`).
|
||||
|
||||
`env` is the one field where "first wins" is ambiguous, so it is stated:
|
||||
profile `env` and explicit `env` are **merged**, with explicit entries
|
||||
overriding profile entries of the same name. Any other reading silently drops
|
||||
half a user's environment.
|
||||
|
||||
An explicitly passed `profile` that does not exist is an error even when
|
||||
`terminal.default-profile` is valid — a typo must not silently fall back to
|
||||
the default.
|
||||
|
||||
**Q#TC4 — `terminal.escape-key` is a `String` chord spelling, parsed once and
|
||||
cached by `(buffer_id, value_epoch)`.** `is_terminal_escape_chord`
|
||||
(`src/editor.rs:4413`) currently compares against a literal `C-c`. Reading and
|
||||
parsing a setting on **every keystroke in a terminal** is not acceptable in
|
||||
that path.
|
||||
|
||||
**The cache key must include the buffer.** `value_epoch()` advances only on
|
||||
`set` / `set_local` / removal (`src/config_registry.rs:918`, `970`, `1011`,
|
||||
`1029`) — **it does not move when the focused terminal changes**. An
|
||||
epoch-only cache therefore serves terminal A's escape chord to terminal B for
|
||||
as long as no setting is written, which is exactly the case where nothing looks
|
||||
wrong. Keying on `(buffer_id, value_epoch)` is the minimum correct identity.
|
||||
|
||||
**Q#TC4c — the cache lives on `TerminalSession`, so its lifecycle is the
|
||||
terminal's.** Revision 3 named the key `(buffer_id, value_epoch)` but not the
|
||||
storage, and the two obvious storages behave differently on A→B→A:
|
||||
|
||||
- a **single last-entry cache** reparses on every switch between two
|
||||
terminals, and re-reports an invalid value each time — a status line that
|
||||
scolds you for a setting you already know about, forever;
|
||||
- an **editor-side map** preserves "parsed and reported once" but **leaks an
|
||||
entry per terminal** unless something purges it, and that purge is a second
|
||||
thing to get wrong.
|
||||
|
||||
`TerminalSession` (`src/terminal/session.rs:215`) is created in
|
||||
`TerminalManager::open` and dropped on kill/prune, so putting the cache there
|
||||
gets the lifecycle for free with no purge hook to forget. It carries the parsed
|
||||
chord, the `value_epoch` it was parsed at, and whether the current invalid
|
||||
value has already been reported.
|
||||
|
||||
**"Reports once" means once per terminal, per effective invalid value.**
|
||||
A→B→A must not re-report. Changing the setting from one invalid value to a
|
||||
*different* invalid value **does** re-report, because that is new information
|
||||
about a new mistake.
|
||||
|
||||
**The reporting channel is `EditorCore::status`** — the same channel
|
||||
`send_terminal_bytes` already uses for terminal failures
|
||||
(`src/editor.rs:1122`). Explicitly **not** `pmacs.error`: it is not installed
|
||||
as a module anywhere in `src/lua_bindings`, so its call sites across the
|
||||
runtime are dead, and a report sent there would be a report nobody sees.
|
||||
|
||||
**Q#TC4a — an unparseable escape key must not brick terminal input.** A bad
|
||||
value falls back to `C-c` and reports once. The failure mode this avoids is
|
||||
severe: with no escape chord, every key goes to the child and the user cannot
|
||||
reach any editor binding to fix the setting that broke it.
|
||||
|
||||
**Q#TC4b — repeating the configured escape sends THAT chord to the child, not
|
||||
Ctrl-C.** The double-escape arm currently writes a hardcoded
|
||||
`&[0x03]` (`src/editor.rs:988`). With `terminal.escape-key = "C-x"`, `C-x C-x`
|
||||
would send Ctrl-C — and literal Ctrl-X would become unreachable, since the
|
||||
first `C-x` is always consumed as the escape. The repeat arm must encode the
|
||||
**configured** chord through the existing `crate::terminal::input::encode_key`
|
||||
path, which is also how it inherits application-cursor and modifier handling
|
||||
rather than growing a second encoder.
|
||||
|
||||
Corollary worth pinning: after changing the escape away from `C-c`, an ordinary
|
||||
`C-c` must reach the child as `0x03` like any other unescaped key.
|
||||
|
||||
**Q#TC5 — the `terminal` command gains an optional profile argument** and
|
||||
otherwise keeps its current behavior; `$SHELL` remains the fallback when no
|
||||
profile is configured. No existing invocation changes meaning.
|
||||
|
||||
**Q#TC10 — the terminal opening keybinding is pulled forward into Stage 1.**
|
||||
`COHERENCE.md` Priority 1 names "a terminal keybinding" as part of protecting
|
||||
the golden journey, §2 step 8 grades the terminal "works but undiscoverable",
|
||||
and this stage already edits `terminal.lua`. Panel rendering imposes no
|
||||
dependency on binding a command that already exists. Close/kill semantics stay
|
||||
with the panel work, where the entry and exit points get designed together.
|
||||
|
||||
The chord is **decided and scouted, not deferred**: `C-c t`, global. See
|
||||
Q#TC8a for the collision evidence and for why binding under the existing `C-c`
|
||||
prefix is a new leaf rather than a shadow.
|
||||
|
||||
## Stage 2 — copy mode and search
|
||||
|
||||
**Q#TC6 — copy mode MATERIALIZES into an ordinary buffer. It does not add a
|
||||
dispatch shadow.**
|
||||
|
||||
`M-x terminal.copy-mode` snapshots the retained rows into a read-only,
|
||||
path-less buffer (`*terminal-copy: NAME*`) and displays it. That buffer is an
|
||||
ordinary document buffer, so:
|
||||
|
||||
- **isearch works, with no new search substrate** — it is a rope, so
|
||||
`SearchStore` and the existing match-painting path apply unchanged. Ground
|
||||
truth 2 is answered by not fighting it.
|
||||
- **motion, selection, `M-w`, the kill ring, even `M-x occur`-style consumers
|
||||
work** — everything that operates on a buffer.
|
||||
- **The "keys must not reach the child" problem dissolves structurally.**
|
||||
`active_terminal_key` keys on `is_terminal(window.buffer_id)`; the snapshot
|
||||
buffer is not a terminal, so the transport arm never fires. No new guard, no
|
||||
new precedence rung, and ground truth 3's coherence cost is avoided rather
|
||||
than paid.
|
||||
- **`describe-key` stays truthful**, because the bindings are buffer-local and
|
||||
inspectable — the idiom `COHERENCE.md` §6 identifies as the right side of
|
||||
the line.
|
||||
|
||||
**Q#TC6a — the snapshot is BOTH intercept-read-only AND round-trip-marked,
|
||||
and `set_round_trip_input` is the ONLY thing standing between a replica
|
||||
frontend and unauthorized mutation.**
|
||||
|
||||
The established idiom is two calls: `listview.lua:106` and `compile.lua:272`
|
||||
each pair `pmacs.buffer.add_intercept` with
|
||||
`pmacs.buffer.set_round_trip_input(buf, true)`. Revision 2 described the
|
||||
intercept as the guard and round-trip as defence in depth. **That was wrong,
|
||||
and the correction matters:**
|
||||
|
||||
- A Lua intercept guards the **dispatch/edit** path only. It does **not** set
|
||||
`Buffer::read_only`, which is "deliberately independent of edit intercepts"
|
||||
(`src/buffer.rs:493-500`) — that flag is what makes terminal identity buffers
|
||||
reject rope, undo/redo, and remote-CRDT mutation alike.
|
||||
- **No Lua binding sets `read_only` at all.** The whole `src/lua_bindings`
|
||||
tree only ever *reads* it (`fold.rs:313`). A Lua-created "read-only" buffer
|
||||
is therefore read-only against dispatch and nothing else.
|
||||
- So an optimistic `CrdtOp` from a semantic frontend bypasses the intercept
|
||||
**and passes `ensure_writable()`**. It is applied. The daemon buffer mutates
|
||||
in lockstep with the mirror — the user silently edits a buffer the editor
|
||||
told them is read-only. There is no divergence to notice, which is worse
|
||||
than divergence.
|
||||
|
||||
`set_round_trip_input` prevents this at the only point it can be prevented: it
|
||||
makes `dispatch_idle_for` report false while the buffer is focused, so the
|
||||
frontend never applies optimistically and never emits the op. It is not
|
||||
hardening — it is the guard.
|
||||
|
||||
Two things follow, and both are recorded rather than fixed here:
|
||||
|
||||
- **The same exposure exists today** for every Lua-created read-only buffer —
|
||||
listview panels and `*compilation*` included. They are correct only because
|
||||
they call `set_round_trip_input`. This arc must not be the place that
|
||||
unilaterally changes that substrate.
|
||||
- **Exposing `Buffer::set_read_only` to Lua** would make these buffers
|
||||
genuinely immutable at the rope/CRDT boundary the way terminal identity
|
||||
buffers are, turning round-trip back into real defence in depth. That is a
|
||||
substrate change affecting listview and compile as much as this snapshot, so
|
||||
it is named in Deferred with its own lane.
|
||||
|
||||
**Q#TC7 — the materializer reuses the existing serializer.** A whole-range
|
||||
variant of `copy_selection_bytes` over `retained_rows` inherits the criterion
|
||||
21 fidelity rather than re-deriving soft-wrap, wide-glyph, and trailing-blank
|
||||
behavior. Writing a second serializer would guarantee the two drift.
|
||||
|
||||
**Q#TC8 — one snapshot buffer per terminal, reused on re-invoke.** Re-running
|
||||
the command against the same terminal replaces the contents in place rather
|
||||
than accumulating buffers. It is killed with its terminal; killing the
|
||||
snapshot alone leaves the terminal untouched.
|
||||
|
||||
**Q#TC8a — the chords, decided and collision-scouted.**
|
||||
|
||||
Worth stating first because it is easy to get backwards: in a terminal window
|
||||
every **unescaped** key goes to the child, so terminal-local bindings are
|
||||
reached as `<escape> <key>`. The existing `M-w` copy is physically `C-c M-w`.
|
||||
The escape consumes itself and the next key starts a fresh ordinary sequence,
|
||||
which is also why `C-c`-leading bindings are structurally unreachable *inside*
|
||||
a terminal.
|
||||
|
||||
| action | scope | binding | physically typed |
|
||||
|---|---|---|---|
|
||||
| open a terminal (Q#TC10) | global | `C-c t` | `C-c t` |
|
||||
| enter copy mode | terminal buffer | `C-t` | `C-c C-t` |
|
||||
| refresh snapshot | snapshot buffer | `g` | `g` |
|
||||
| return to terminal | snapshot buffer | `q` | `q` |
|
||||
|
||||
Scouted against the real keymaps:
|
||||
|
||||
- **`C-c t` is free.** No bare global `C-c` binding exists; `C-c` is already a
|
||||
live global prefix from `fold.lua:48-52` (`C-c @ …`), and `C-c C-k` is
|
||||
buffer-scoped in compile/async. `C-c t` is a new leaf under an existing
|
||||
prefix, not a shadow.
|
||||
- **`C-t` is globally `edit.transpose-chars`** (`editops.lua:909`), and binding
|
||||
it **buffer-locally is legitimate**: `keymap.bind`'s strictness rejects
|
||||
binding a *prefix* of an existing sequence within a scope
|
||||
(`keymap_bind_conflict_surfaces_at_bind_time` — "would shadow"), not
|
||||
cross-scope shadowing, which is what scopes are for. Listview already binds
|
||||
`n`/`p`/`g`/`q`/`RET`/`SPC` buffer-locally. Transpose-chars is meaningless in
|
||||
a read-only terminal buffer.
|
||||
- `C-c C-t` matches emacs-libvterm's own `vterm-copy-mode` chord, so the muscle
|
||||
memory transfers.
|
||||
- `g` / `q` in the snapshot follow listview's precedent exactly.
|
||||
|
||||
**Named limitation:** `C-c t` cannot open a terminal *from inside* a terminal,
|
||||
because `C-c` is consumed as the escape there. `M-x terminal` still works. This
|
||||
is the documented consequence of Stage 2 criterion 19, not a new defect.
|
||||
|
||||
These are what make acceptance 21's `describe-key` claim testable: named
|
||||
bindings, in named buffers, that introspection must report truthfully.
|
||||
|
||||
**Q#TC9 — the live-terminal keys stay.** `M-w`, `M-v`, `C-v`, `M-<`, `M->` on
|
||||
the terminal buffer are the live affordances and do not change. Copy mode is
|
||||
additive, on its own binding, and does not replace scroll-and-select.
|
||||
|
||||
## Bets
|
||||
|
||||
- **B1.** Materializing gives search for free: no second match store, no
|
||||
second highlight path, no terminal-specific search UI. *Scored by Stage 2
|
||||
landing with zero changes under `src/search.rs`.*
|
||||
- **B2.** Point-in-time is sufficient for read-back/search/copy. *Scored by
|
||||
use; if false, the live frozen mode in Deferred becomes the real feature and
|
||||
this becomes its snapshot fallback.*
|
||||
- **B3.** No protocol change. The snapshot is an ordinary buffer, so both
|
||||
frontends render it with existing machinery. *Scored by the diff.*
|
||||
- **B4.** The escape-key cache keyed by `(buffer_id, value_epoch)` never
|
||||
becomes stale in a way a user can observe. *Scored by two acceptances, not
|
||||
one: changing the setting mid-session (8) and two terminals with different
|
||||
buffer-local values and no write between them (7). Revision 1's epoch-only
|
||||
cache would pass the first and fail the second, which is why the bet now
|
||||
names both.*
|
||||
- **B5.** Buffer-local escape keys are a feature rather than a hazard.
|
||||
*Unscored and honestly so: the registry cannot express global-only, so this
|
||||
is what we get either way. If per-terminal escapes turn out to confuse more
|
||||
than they help, the fix is the config registry's `scope = "global"` deferral,
|
||||
not a terminal change.*
|
||||
|
||||
## Deferred (named)
|
||||
|
||||
- **Live frozen copy mode** (true `vterm-copy-mode` semantics: freeze the
|
||||
terminal in place, navigate it, resume). Strictly larger; needs either the
|
||||
transient-keymap primitive `COHERENCE.md` §6 specifies or a deliberate
|
||||
seventh shadow.
|
||||
- **Shell integration** — cwd tracking, prompt marks, command zones, and the
|
||||
VS Code cluster downstream of it (command decorations, exit-code markers,
|
||||
rerun, sticky scroll, terminal IntelliSense). Its own arc, with a security
|
||||
framing.
|
||||
- **Table-valued settings** — the config registry's own deferral. This arc
|
||||
adds a **second** blocked adopter (after `pmacs.lsp.config` /
|
||||
`pmacs.pair.sets`); worth recording as evidence when that deferral is
|
||||
ranked.
|
||||
- **A `scope = "global"` define flag** — also the config registry's own
|
||||
deferral, and this arc is its second live case after `autosave.interval-ms`.
|
||||
Until it exists, `set_local` on any `Live` setting is accepted whether or not
|
||||
the owner wants it, so Q#TC2b specifies the behavior instead of pretending
|
||||
it is prevented.
|
||||
- **Panel terminal** — blocked on bottom-panel Stage 2 (semantic frontends are
|
||||
not `panel_capable`). `display = "panel"` already exists and works on the
|
||||
grid frontend.
|
||||
- OSC 8 hyperlinks, images (sixel/kitty), `faint`/`blink`/`conceal`/
|
||||
`strikethrough` (needs a shared `Style` widening, so a protocol bump),
|
||||
cursor shape/blink, kitty keyboard protocol.
|
||||
- Terminal session persistence/reconnect across editor restart.
|
||||
- **A terminal close/kill command** — the remaining half of `COHERENCE.md`
|
||||
§2 step 8's discoverability gap. It belongs with the panel-terminal work,
|
||||
where entry and exit points get designed together. The *opening* keybinding
|
||||
is **no longer deferred**: Stage 1 carries it as Q#TC10.
|
||||
- **Genuine immutability for generated buffers — and it is bigger than a Lua
|
||||
setter.** Today no Lua binding sets `read_only` (`src/lua_bindings` only
|
||||
reads it, `fold.rs:313`), so every Lua-created "read-only" buffer — listview
|
||||
panels, `*compilation*`, and this snapshot — is read-only against dispatch
|
||||
alone and relies entirely on `set_round_trip_input` (Q#TC6a).
|
||||
|
||||
Merely **exposing `set_read_only` would break all three.** The
|
||||
intercept-bypass path is `ensure_writable`-guarded too:
|
||||
`apply_edit_skip_intercepts` calls it first (`src/buffer.rs:994`), and that
|
||||
is exactly the primitive an owner uses to rewrite its own generated buffer.
|
||||
Flipping the flag would stop listview refreshing, `*compilation*` streaming,
|
||||
and this snapshot refreshing — the very operations those buffers exist for.
|
||||
|
||||
So the lane needs **two** things, not one: genuine immutability at the
|
||||
rope/CRDT boundary, *and* an owner-authorized update path that is not simply
|
||||
"skip the intercepts". Naming only the setter would have made it look like a
|
||||
one-line follow-up.
|
||||
|
||||
## Acceptance
|
||||
|
||||
### Stage 1 — `terminal-config`
|
||||
|
||||
1. `pmacs.terminal.profiles` accepts a strict spec table per name and rejects
|
||||
unknown fields before anything is spawned, matching `terminal.open`'s
|
||||
existing transactional contract.
|
||||
2. `terminal.default-profile` naming an unknown profile fails at open with an
|
||||
error that **lists the known profile names**, and creates no buffer,
|
||||
session, or process. An explicitly passed unknown `profile` fails the same
|
||||
way **even when `terminal.default-profile` is valid** (Q#TC3a).
|
||||
2a. That diagnostic is **total over a malformed profiles table** (review round
|
||||
1). `pmacs.terminal.profiles` is a raw user table, so listing its names must
|
||||
not assume its keys are comparable and rendering a requested name must not
|
||||
assume it is a string: a table holding both a string and a numeric key made
|
||||
`table.sort` raise `attempt to compare number with string` *on the
|
||||
unknown-profile path*, replacing the exact error being asked for, and `%q`
|
||||
raises on a non-string `profile` argument. Both are partial functions
|
||||
applied to user input on a diagnostic path — the failure class is
|
||||
"the error reporter is the thing that fails".
|
||||
3. Field-by-field resolution follows Q#TC3a: explicit open field beats profile
|
||||
field beats scalar setting beats `$SHELL`. `env` **merges**, with explicit
|
||||
entries overriding profile entries of the same name.
|
||||
4. `""` in `terminal.default-profile` means "no profile" and is
|
||||
indistinguishable from unset (Q#TC2a).
|
||||
5. `terminal.scrollback-rows` takes effect for a terminal opened without an
|
||||
explicit `scrollback_rows`; an explicit per-open value overrides it; values
|
||||
outside `0 ..= 4_000_000` are rejected by the registry rather than by the
|
||||
core, and `0` is accepted as "retain no history".
|
||||
6. `terminal.escape-key` changes which chord escapes to the editor, observed
|
||||
through the **real dispatch path**, not by calling the predicate directly.
|
||||
7. **Two terminals with different buffer-local escape keys each honor their
|
||||
own**, with no setting written in between (Q#TC4/Q#TC2b). Driven as
|
||||
**A→B→A**, asserting both directions. This is the pin an epoch-only cache
|
||||
fails.
|
||||
8. Across that same **A→B→A** switch with no setting written, the parse count
|
||||
does **not** increase after each terminal's first keystroke (Q#TC4c) —
|
||||
pinned by counting parses, not by timing. This is the pin a single
|
||||
last-entry cache fails while still satisfying 7.
|
||||
8a. A terminal's cache does not outlive it: killing a terminal and opening a
|
||||
new one does not serve the dead terminal's chord, and no per-terminal cache
|
||||
entry survives its session (Q#TC4c). This is the pin an unpurged
|
||||
editor-side map fails.
|
||||
9. With `terminal.escape-key = "C-x"`: `C-x C-x` sends **Ctrl-X** to the child,
|
||||
and an ordinary `C-c` reaches the child as `0x03` like any other unescaped
|
||||
key (Q#TC4b). Bite: against the hardcoded `&[0x03]`, the first assertion
|
||||
fails.
|
||||
10. An unparseable `terminal.escape-key` falls back to `C-c`, reports through
|
||||
`EditorCore::status`, and leaves the terminal usable (Q#TC4a). Bite: with
|
||||
the fallback removed, the terminal becomes unescapable.
|
||||
10a. "Reports once" is once per terminal per effective invalid value
|
||||
(Q#TC4c): an **A→B→A** switch with the same invalid value reports **once**,
|
||||
while changing it to a *different* invalid value reports again. The report
|
||||
count is asserted, not the message text.
|
||||
11. The terminal opening keybinding invokes the existing command, and is
|
||||
verified to have shadowed nothing (Q#TC10).
|
||||
12. Existing `terminal` invocations and every existing terminal test behave
|
||||
identically with no settings defined and no profiles registered.
|
||||
|
||||
### Stage 2 — `terminal-copy-mode`
|
||||
|
||||
13. `terminal.copy-mode` produces a read-only buffer whose text is
|
||||
byte-identical to serializing the full retained range through the existing
|
||||
copy path (Q#TC7) — pinned against the serializer, so the two cannot drift.
|
||||
14. Soft wraps, hard rows, wide glyphs, combining clusters, and trailing
|
||||
default blanks appear in the snapshot exactly as Stage 2 criterion 21 pins
|
||||
them for selection copy.
|
||||
15. isearch over the snapshot finds content that is **only in scrollback**
|
||||
(scrolled off the visible screen), with no change to `src/search.rs` (B1).
|
||||
16. **Ungated, runs in CI:** focusing the snapshot buffer makes
|
||||
`dispatch_idle_for` report **false**. This is the whole mechanism Q#TC6a
|
||||
depends on, it needs no CRDT, and it fails the moment
|
||||
`set_round_trip_input` is dropped — so the load-bearing regression is
|
||||
caught by the default configuration rather than only by a `crdt`-gated
|
||||
test that CI never compiles.
|
||||
17. **Through a semantic frontend** (this one does need CRDT): keys typed in
|
||||
the snapshot buffer reach ordinary dispatch and never the child, and
|
||||
**neither the daemon buffer nor the frontend's mirror is mutated**
|
||||
(Q#TC6a). Bite: with `set_round_trip_input` removed, the optimistic op is
|
||||
emitted, bypasses the Lua intercept, passes `ensure_writable()`, and
|
||||
mutates **both sides** — a buffer the editor calls read-only silently
|
||||
accepts an edit.
|
||||
18. Re-invoking against the same terminal refreshes in place; the buffer count
|
||||
does not grow (Q#TC8). Killing the snapshot leaves the terminal running;
|
||||
killing the terminal removes the snapshot.
|
||||
19. `C-t` in a terminal buffer (physically `C-c C-t`) enters copy mode; `g`
|
||||
refreshes the snapshot from the live terminal and `q` returns to the source
|
||||
terminal (Q#TC8a).
|
||||
20. The live terminal's own keys are unchanged while a snapshot exists
|
||||
(Q#TC9), and the terminal keeps following its tail.
|
||||
21. The dispatch-shadow count is **unchanged at six** — pinned by asserting
|
||||
`describe-key` reports the truth for the snapshot buffer's `g` and `q`,
|
||||
which is the observable difference between the buffer-local idiom and a
|
||||
shadow.
|
||||
|
||||
## Coherence impact (`COHERENCE.md` §20)
|
||||
|
||||
- **§6 Interaction islands — this arc deliberately adds none.** It is the
|
||||
first modal-feeling terminal feature that resolves to the buffer-local
|
||||
keymap idiom §6 identifies as correct, rather than a seventh rung on the
|
||||
precedence ladder. The shadow count stays at six and `describe-key` stays
|
||||
truthful (acceptance 21). Worth recording in §6 as a worked example that the
|
||||
idiom scales to a case that looks modal.
|
||||
- **§11 Configuration as typed, layered data** — the terminal gains its first
|
||||
settings, and produces a second blocked adopter for **two** distinct registry
|
||||
deferrals: the missing table-valued kind (profiles) and the missing
|
||||
`scope = "global"` flag (the **two open-time settings** —
|
||||
`terminal.escape-key` deliberately supports buffer-locals, so only
|
||||
`default-profile` and `scrollback-rows` want an enforcement the registry
|
||||
cannot express). §11's ground truth should
|
||||
record both, because the argument for prioritizing them is now cumulative
|
||||
rather than hypothetical.
|
||||
- **§2 golden journey, step 8 — partially closed here.** Stage 1 carries the
|
||||
**terminal opening keybinding** that Priority 1 explicitly names (Q#TC10),
|
||||
which is the larger half of "works but undiscoverable". Close/kill stays with
|
||||
the panel work so the entry and exit points are designed together, and is
|
||||
named in Deferred rather than silently skipped.
|
||||
- **§5 Unify discovery** — the new commands must carry real descriptions so
|
||||
M-x rows are useful; no new introspection surface is added.
|
||||
- No background-work attribution change; no new activity view; no protocol
|
||||
change.
|
||||
|
||||
## Verification plan
|
||||
|
||||
Full gate suite per `CLAUDE.md` for each PR separately, plus:
|
||||
|
||||
- **The touched terminal suites in BOTH configurations** — default and
|
||||
`--features crdt` — not only the CRDT one. `vterm_stage1_acceptance`,
|
||||
`vterm_stage2_acceptance`, and `vterm_stage3_acceptance` all carry tests in
|
||||
each, and acceptance 12 is a claim about the default configuration too.
|
||||
- `cargo test --test config_registry_acceptance` for the new settings.
|
||||
- New suites: `tests/terminal_config_acceptance.rs` (Stage 1) and
|
||||
`tests/terminal_copy_mode_acceptance.rs` (Stage 2).
|
||||
- Every behavioral claim bite-verified. The bites that matter most:
|
||||
**7/8/8a** — three pins that fail against three *different* wrong cache
|
||||
implementations (epoch-only key, single last-entry, unpurged map), which is
|
||||
why one pin was not enough; **9** (a hardcoded `0x03` makes the configured
|
||||
chord unreachable); **10** (its failure mode is a terminal nobody can
|
||||
escape); and **16/17** (a read-only buffer that silently accepts an edit on
|
||||
both sides).
|
||||
- **The observation seams the cache pins need are `escape_parses` (how often)
|
||||
and `escape_caches` (how many are still held).** Neither is inferable from
|
||||
behavior: for a *valid* setting a correct per-session cache and a leaking
|
||||
editor-side map produce identical keystroke results, and both leave the
|
||||
session count draining normally. Review round 1 caught 8a asserting the
|
||||
session count instead — which the unpurged-map bite passes, since a map with
|
||||
no purge hook leaks *while* sessions drain. A lifecycle claim needs a
|
||||
lifecycle observable; the count of live sessions is not one.
|
||||
- **Criterion 5 must open a real terminal and read back retained history.**
|
||||
Round 1 caught it asserting a registry round-trip instead, which is a test of
|
||||
the registry: it stays green with the setting's only consumer deleted. The
|
||||
same shape to watch for anywhere — *asserting that a value was stored is not
|
||||
asserting that anything reads it*.
|
||||
- **Do not gate the new suites on `#[cfg(feature = "crdt")]` unless a test
|
||||
genuinely needs CRDT.** CI never enables that feature, so a suite gated that
|
||||
way is written and then never run — 264 tests are currently dark for exactly
|
||||
this reason. That measurement and its lane live on **PR #168**, which is open
|
||||
and unmerged; it is not yet in `docs/active-work.md` on `main`.
|
||||
Acceptance 17 does need a semantic frontend, so that one test is gated — but
|
||||
acceptance 16 pins the same mechanism ungated, so the regression is caught in
|
||||
CI regardless. That pairing is the pattern to reuse whenever a claim's
|
||||
end-to-end proof needs CRDT.
|
||||
134
src/daemon.rs
134
src/daemon.rs
|
|
@ -2992,7 +2992,13 @@ fn align_primary_document_window(
|
|||
}
|
||||
let reg = core.registry.borrow();
|
||||
let Ok(buf) = reg.get(buffer_id) else {
|
||||
return Some(win_id); // Unknown buffer — leave the window as-is.
|
||||
// Unknown buffer — leave the window as-is, and report
|
||||
// FAILURE. Returning the window here would let a stale or
|
||||
// forged `Pointer` naming a dead buffer take focus out of a
|
||||
// panel via #8's activation, *before* `dispatch_pointer`
|
||||
// rejects the mismatched buffer. Alignment did not happen,
|
||||
// so no caller may treat this as a document gesture.
|
||||
return None;
|
||||
};
|
||||
(win_id, TextView::new(buf))
|
||||
};
|
||||
|
|
@ -4638,4 +4644,130 @@ mod tests {
|
|||
"the panel was not overwritten with the target"
|
||||
);
|
||||
}
|
||||
|
||||
/// Bottom-panel §1.3 #8, review round 1 finding 1: a STALE document
|
||||
/// `Pointer` must not steal focus out of a panel.
|
||||
///
|
||||
/// Driven through `handle_dispatcher_event` — the real dispatcher
|
||||
/// seam — because the defect lived in the *pair* of alignment and
|
||||
/// activation, not in either alone. `align_primary_document_window`
|
||||
/// once returned the window even when the named buffer was gone, so
|
||||
/// #8's activation focused the document before `dispatch_pointer`
|
||||
/// ever rejected the mismatched buffer.
|
||||
#[cfg(feature = "crdt")]
|
||||
#[test]
|
||||
fn a_stale_document_pointer_does_not_steal_focus_from_a_panel() {
|
||||
use crate::editor::EditorState;
|
||||
use crate::protocol::FrontendId;
|
||||
use crate::window::{FrontendView, Layout, LayoutNode, Orientation, Window, WindowParams};
|
||||
use pmacs_protocol::{Modifiers, PointerKind};
|
||||
|
||||
let mut editor = EditorState::new();
|
||||
let fid = FrontendId(88);
|
||||
|
||||
// One document window + one focused bottom panel.
|
||||
let (document, panel, dead_buffer) = {
|
||||
let mut core = editor.core.borrow_mut();
|
||||
let doc_buf = core.active_window().buffer_id;
|
||||
let panel_buf = core.registry.borrow_mut().create("*panel*");
|
||||
// A buffer id that names nothing: the stale-pointer payload.
|
||||
let dead_buffer = crate::buffer::BufferId::from_raw(999_999);
|
||||
|
||||
let document = crate::window::WindowId::next();
|
||||
let panel_id = crate::window::WindowId::next();
|
||||
let (doc_view, panel_view) = {
|
||||
let reg = core.registry.borrow();
|
||||
(
|
||||
crate::text_view::TextView::new(reg.get(doc_buf).expect("doc")),
|
||||
crate::text_view::TextView::new(reg.get(panel_buf).expect("panel")),
|
||||
)
|
||||
};
|
||||
core.windows
|
||||
.insert(document, Window::new(document, doc_buf, doc_view));
|
||||
let mut panel = Window::new(panel_id, panel_buf, panel_view);
|
||||
let mut params = WindowParams::default();
|
||||
params.side = Some(crate::window::Side::Bottom);
|
||||
params.fixed_rows = Some(4);
|
||||
panel.params = params;
|
||||
core.windows.insert(panel_id, panel);
|
||||
core.register_frontend_view(
|
||||
fid,
|
||||
FrontendView {
|
||||
layout: Layout {
|
||||
root: LayoutNode::Split {
|
||||
orientation: Orientation::Horizontal,
|
||||
children: vec![LayoutNode::Leaf(document), LayoutNode::Leaf(panel_id)],
|
||||
weights: vec![1, 1],
|
||||
},
|
||||
},
|
||||
active: panel_id,
|
||||
fold_projection: false,
|
||||
panel_capable: true,
|
||||
frame_geometry: None,
|
||||
panel_hidden: false,
|
||||
},
|
||||
);
|
||||
(document, panel_id, dead_buffer)
|
||||
};
|
||||
|
||||
let mut render_states = HashMap::new();
|
||||
let mut semantic_states = HashMap::new();
|
||||
semantic_states.insert(fid, crate::semantic_render::SemanticRenderState::new(fid));
|
||||
let mut streams = HashMap::new();
|
||||
let mut term_sizes = HashMap::new();
|
||||
term_sizes.insert(fid, CellSize::new(24, 80));
|
||||
let mut last_idle = HashMap::new();
|
||||
let mut last_active = HashMap::new();
|
||||
let mut bells = HashMap::new();
|
||||
// The dispatcher drops any event from an UNINSTALLED session
|
||||
// (#148's defense-in-depth membership check), so the session must
|
||||
// be registered or this test passes for the wrong reason — it did,
|
||||
// on the first attempt.
|
||||
let mut registry = SessionRegistry::new();
|
||||
registry.register_session(
|
||||
fid,
|
||||
crate::presence::SessionState {
|
||||
negotiated_protocol_version: pmacs_protocol::PROTOCOL_VERSION,
|
||||
negotiated_capabilities: crate::protocol::NegotiatedCapabilities {
|
||||
semantic_render: true,
|
||||
crdt_replica: true,
|
||||
..Default::default()
|
||||
},
|
||||
color_slot: 0,
|
||||
},
|
||||
);
|
||||
|
||||
handle_dispatcher_event(
|
||||
DispatcherEvent::FrontendEvent {
|
||||
source: fid,
|
||||
event: FrontendEvent::Pointer {
|
||||
frontend_id: fid,
|
||||
buffer_id: dead_buffer,
|
||||
byte: 0,
|
||||
kind: PointerKind::Down,
|
||||
mods: Modifiers::default(),
|
||||
},
|
||||
},
|
||||
&mut editor,
|
||||
&mut render_states,
|
||||
&mut semantic_states,
|
||||
&mut streams,
|
||||
&mut term_sizes,
|
||||
&mut last_idle,
|
||||
&mut last_active,
|
||||
&mut bells,
|
||||
&mut registry,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
editor.core.borrow().views[&fid].active,
|
||||
panel,
|
||||
"a stale Pointer naming a dead buffer must NOT move focus out of the panel"
|
||||
);
|
||||
assert_ne!(
|
||||
editor.core.borrow().views[&fid].active,
|
||||
document,
|
||||
"non-vacuity: the document window is a real, distinct focus target"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -989,19 +989,28 @@ impl EditorState {
|
|||
.get(&frontend_id)
|
||||
.is_some_and(|state| state.terminal_escape);
|
||||
if let Some(view_key) = terminal_key {
|
||||
// Q#TC4: the escape chord is per terminal, resolved through
|
||||
// `terminal.escape-key` and cached on the session so this
|
||||
// hot path parses at most once per (terminal, config epoch).
|
||||
let escape_chord = self.terminal_escape_chord(view_key.buffer_id);
|
||||
if escaped {
|
||||
self.dispatchers
|
||||
.entry(frontend_id)
|
||||
.or_default()
|
||||
.terminal_escape = false;
|
||||
if chord.is_some_and(is_terminal_escape_chord) {
|
||||
if chord == Some(escape_chord) {
|
||||
// Q#TC4b: repeating the escape sends THAT chord to the
|
||||
// child, not a hardcoded ETX. With a configured escape
|
||||
// of `C-x`, sending Ctrl-C here would both surprise the
|
||||
// user and make literal Ctrl-X unreachable, since the
|
||||
// first press is always consumed as the escape.
|
||||
self.claim_terminal_controller(view_key);
|
||||
self.send_terminal_bytes(view_key.buffer_id, &[0x03]);
|
||||
self.send_terminal_escape_literal(view_key, escape_chord);
|
||||
return;
|
||||
}
|
||||
// The post-escape key starts a fresh ordinary sequence below.
|
||||
} else if !dispatcher_pending {
|
||||
if chord.is_some_and(is_terminal_escape_chord) {
|
||||
if chord == Some(escape_chord) {
|
||||
let state = self.dispatchers.entry(frontend_id).or_default();
|
||||
state.terminal_escape = true;
|
||||
state.dispatcher = KeyDispatcher::new();
|
||||
|
|
@ -1117,6 +1126,54 @@ impl EditorState {
|
|||
.then_some(key)
|
||||
}
|
||||
|
||||
/// This terminal's effective escape chord (Q#TC4).
|
||||
///
|
||||
/// Resolution is `get("terminal.escape-key", terminal_buffer)` —
|
||||
/// buffer-local, then global, then default — because unlike the two
|
||||
/// open-time settings this one is read while the terminal exists, so
|
||||
/// a per-terminal escape is expressible and supported (Q#TC2b).
|
||||
///
|
||||
/// The parse and the once-per-terminal invalid-value report both live
|
||||
/// in [`crate::terminal::TerminalManager::escape_chord`]; this method
|
||||
/// only supplies the resolved spelling and the epoch that keys the
|
||||
/// cache, and surfaces any report through the status line — the same
|
||||
/// channel `send_terminal_bytes` uses for terminal failures.
|
||||
fn terminal_escape_chord(&self, buffer_id: crate::buffer::BufferId) -> Chord {
|
||||
let lua = self.lua_host.lua();
|
||||
let (spelling, epoch) = crate::lua_bindings::config_string_and_epoch(
|
||||
lua,
|
||||
"terminal.escape-key",
|
||||
Some(buffer_id),
|
||||
crate::terminal::DEFAULT_TERMINAL_ESCAPE_KEY,
|
||||
);
|
||||
let (chord, report) = self
|
||||
.terminal_manager
|
||||
.borrow_mut()
|
||||
.escape_chord(buffer_id, epoch, &spelling);
|
||||
if let Some(message) = report {
|
||||
self.core.borrow_mut().status = message;
|
||||
}
|
||||
chord
|
||||
}
|
||||
|
||||
/// Send the configured escape chord to the child as literal input
|
||||
/// (Q#TC4b), through the same encoder ordinary keys use so it
|
||||
/// inherits application-cursor and modifier handling.
|
||||
fn send_terminal_escape_literal(&self, key: TerminalViewKey, chord: Chord) {
|
||||
let event = KeyEvent::new(chord.code, chord.modifiers);
|
||||
let Some((terminal_key, modifiers)) = terminal_key_from_crossterm(event) else {
|
||||
return;
|
||||
};
|
||||
let modes = self
|
||||
.terminal_manager
|
||||
.borrow()
|
||||
.modes_for_view(key)
|
||||
.unwrap_or_default();
|
||||
if let Some(bytes) = crate::terminal::input::encode_key(terminal_key, modifiers, modes) {
|
||||
self.send_terminal_bytes(key.buffer_id, &bytes);
|
||||
}
|
||||
}
|
||||
|
||||
fn claim_terminal_controller(&self, key: TerminalViewKey) {
|
||||
let mut manager = self.terminal_manager.borrow_mut();
|
||||
let _ = manager.register_view(key);
|
||||
|
|
@ -4481,10 +4538,6 @@ fn sanitize_single_line(s: &str) -> String {
|
|||
.collect()
|
||||
}
|
||||
|
||||
fn is_terminal_escape_chord(chord: Chord) -> bool {
|
||||
chord.code == KeyCode::Char('c') && chord.modifiers == KeyModifiers::CONTROL
|
||||
}
|
||||
|
||||
fn terminal_key_from_crossterm(key: KeyEvent) -> Option<(TerminalKey, TerminalModifiers)> {
|
||||
let modifiers = crate::protocol::crossterm_translate::mods_from_crossterm(key.modifiers);
|
||||
let key = crate::protocol::crossterm_translate::keycode_from_crossterm(key.code);
|
||||
|
|
|
|||
|
|
@ -668,6 +668,33 @@ pub fn config_u32(lua: &Lua, name: &str, buffer_id: Option<BufferId>, fallback:
|
|||
}
|
||||
}
|
||||
|
||||
/// Read a `String` setting plus the registry epoch that keys any cache
|
||||
/// built from it (Q#TC4c).
|
||||
///
|
||||
/// The epoch is returned WITH the value deliberately: a caller caching a
|
||||
/// parsed form needs both, and reading them in two calls would let a
|
||||
/// `set` land between them and produce a cache stamped with the wrong
|
||||
/// epoch. `fallback` covers a bare core whose runtime never defined the
|
||||
/// setting, matching [`config_u32`].
|
||||
#[must_use]
|
||||
pub fn config_string_and_epoch(
|
||||
lua: &Lua,
|
||||
name: &str,
|
||||
buffer_id: Option<BufferId>,
|
||||
fallback: &str,
|
||||
) -> (String, u64) {
|
||||
let Some(registry) = lua.app_data_ref::<config::SharedConfigRegistry>() else {
|
||||
return (fallback.to_owned(), 0);
|
||||
};
|
||||
let borrowed = registry.borrow();
|
||||
let epoch = borrowed.value_epoch();
|
||||
let value = match borrowed.get(name, buffer_id) {
|
||||
Ok(crate::config_registry::ConfigValue::Str(v)) => v.clone(),
|
||||
_ => fallback.to_owned(),
|
||||
};
|
||||
(value, epoch)
|
||||
}
|
||||
|
||||
/// Short-circuit a binding when the init phase has completed.
|
||||
///
|
||||
/// Lifecycle-affecting Lua APIs (currently just `pmacs.attach`; M5.6d+)
|
||||
|
|
|
|||
|
|
@ -810,7 +810,11 @@ impl SemanticRenderState {
|
|||
out.extend(self.font_facts_msg(state));
|
||||
// Q#SL6/Q#SL8: face inventory must precede segment text.
|
||||
if let Some(evaluation) = statusline_evaluation {
|
||||
self.emit_statusline_segments(evaluation, &mut out);
|
||||
let document_window = state
|
||||
.core
|
||||
.borrow()
|
||||
.primary_document_window(self.frontend_id);
|
||||
self.emit_statusline_segments(evaluation, document_window, &mut out);
|
||||
}
|
||||
out
|
||||
}
|
||||
|
|
@ -929,7 +933,11 @@ impl SemanticRenderState {
|
|||
out.extend(self.font_facts_msg(state));
|
||||
// Q#SL6/Q#SL8: face inventory must precede segment text.
|
||||
if let Some(evaluation) = statusline_evaluation {
|
||||
self.emit_statusline_segments(evaluation, &mut out);
|
||||
let document_window = state
|
||||
.core
|
||||
.borrow()
|
||||
.primary_document_window(self.frontend_id);
|
||||
self.emit_statusline_segments(evaluation, document_window, &mut out);
|
||||
}
|
||||
out
|
||||
}
|
||||
|
|
@ -941,6 +949,7 @@ impl SemanticRenderState {
|
|||
fn emit_statusline_segments(
|
||||
&mut self,
|
||||
evaluation: StatuslineEvaluation,
|
||||
document_window: Option<crate::window::WindowId>,
|
||||
out: &mut Vec<InstanceMessage>,
|
||||
) {
|
||||
let to_wire = |segments: Vec<crate::statusline::EvaluatedStatuslineSegment>| {
|
||||
|
|
@ -955,10 +964,16 @@ impl SemanticRenderState {
|
|||
let frontend_id = self.frontend_id;
|
||||
match evaluation.outcome {
|
||||
StatuslineEvaluationOutcome::Ready(windows) => {
|
||||
if let Some(window) = windows
|
||||
.into_iter()
|
||||
.find(|window| window.context.frontend_id == frontend_id)
|
||||
{
|
||||
// Bottom-panel A2A-2: the fan-out now yields the primary
|
||||
// document AND the visible side window, so the wire
|
||||
// segments must be selected by WINDOW IDENTITY. Taking
|
||||
// "the first context for my frontend" would silently
|
||||
// depend on capture order and could ship the panel's
|
||||
// mode-line text as the document status band.
|
||||
if let Some(window) = windows.into_iter().find(|window| {
|
||||
window.context.frontend_id == frontend_id
|
||||
&& Some(window.context.window_id) == document_window
|
||||
}) {
|
||||
self.emit_statusline_payload(
|
||||
window.context.buffer_id,
|
||||
to_wire(window.left),
|
||||
|
|
@ -2925,6 +2940,7 @@ mod tests {
|
|||
),
|
||||
new_failures: Vec::new(),
|
||||
},
|
||||
None,
|
||||
&mut stale,
|
||||
);
|
||||
assert!(stale.is_empty(), "phase-1 stale evaluation emits nothing");
|
||||
|
|
@ -2945,13 +2961,13 @@ mod tests {
|
|||
new_failures: Vec::new(),
|
||||
};
|
||||
let mut replacement = Vec::new();
|
||||
semantic.emit_statusline_segments(invalidated(), &mut replacement);
|
||||
semantic.emit_statusline_segments(invalidated(), None, &mut replacement);
|
||||
assert_eq!(
|
||||
statusline_of(&replacement),
|
||||
Some((buffer_id, Vec::new(), Vec::new()))
|
||||
);
|
||||
let mut unchanged = Vec::new();
|
||||
semantic.emit_statusline_segments(invalidated(), &mut unchanged);
|
||||
semantic.emit_statusline_segments(invalidated(), None, &mut unchanged);
|
||||
assert!(
|
||||
unchanged.is_empty(),
|
||||
"the empty invalidation became baseline"
|
||||
|
|
|
|||
|
|
@ -660,12 +660,46 @@ fn capture_target_contexts(
|
|||
if window.buffer_id != declared_buffer {
|
||||
return Err(StatuslineNoMessageReason::DeclaredBufferMismatch);
|
||||
}
|
||||
Ok(vec![StatuslineContext {
|
||||
let mut contexts = vec![StatuslineContext {
|
||||
frontend_id,
|
||||
window_id: window.id,
|
||||
buffer_id: window.buffer_id,
|
||||
active: window.id == view.active,
|
||||
}])
|
||||
}];
|
||||
// Bottom-panel Q#BP8 / A2A-2 — the semantic fan-out is the
|
||||
// primary document PLUS the frontend's visible side window,
|
||||
// and nothing else: unprojected document splits run no
|
||||
// callbacks. The document result feeds the semantic
|
||||
// `StatuslineSegments`; the side result paints in the panel's
|
||||
// own mode line.
|
||||
//
|
||||
// A derived-hidden side window is omitted (Q#BP2b): it has no
|
||||
// mode line to paint this frame, so evaluating providers for
|
||||
// it would invoke callbacks for a surface nobody can see.
|
||||
if !view.panel_hidden {
|
||||
for side_id in view.layout.iter_ids() {
|
||||
if side_id == window.id {
|
||||
continue;
|
||||
}
|
||||
let Some(side) = core.windows.get(&side_id) else {
|
||||
return Err(StatuslineNoMessageReason::ContextUnavailable);
|
||||
};
|
||||
if !side.is_side() {
|
||||
continue;
|
||||
}
|
||||
if buffers.get(side.buffer_id).is_err() {
|
||||
return Err(StatuslineNoMessageReason::BufferUnavailable);
|
||||
}
|
||||
contexts.push(StatuslineContext {
|
||||
frontend_id,
|
||||
window_id: side.id,
|
||||
buffer_id: side.buffer_id,
|
||||
// Same rule as the document context: ACTUAL focus.
|
||||
active: side.id == view.active,
|
||||
});
|
||||
}
|
||||
}
|
||||
Ok(contexts)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -34,6 +34,10 @@ pub use pmacs_protocol::terminal::{
|
|||
/// Configuration-time, not a wire bound: history never crosses the
|
||||
/// protocol, so this stays core-owned.
|
||||
pub const DEFAULT_TERMINAL_SCROLLBACK_ROWS: usize = 10_000;
|
||||
|
||||
/// Default `terminal.escape-key`, and the fallback an unparseable value
|
||||
/// falls back to (Q#TC4a).
|
||||
pub const DEFAULT_TERMINAL_ESCAPE_KEY: &str = "C-c";
|
||||
/// Maximum retained main-screen history cells. Core-owned for the same
|
||||
/// reason as [`DEFAULT_TERMINAL_SCROLLBACK_ROWS`].
|
||||
pub const MAX_TERMINAL_HISTORY_CELLS: usize = 4_000_000;
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ use crate::ansi::AnsiParserProfile;
|
|||
use crate::buffer::{Buffer, BufferId};
|
||||
use crate::cell::{Cell, CellCoord, CellSize};
|
||||
use crate::editor_core::EditorCore;
|
||||
use crate::key::{Chord, parse_chord};
|
||||
use crate::process::{
|
||||
ProcessEventKind, ProcessId, ProcessMode, ProcessSpec, ProcessState, ProcessSupervisor,
|
||||
RestartPolicy, StdinMode, TerminalMode,
|
||||
|
|
@ -218,12 +219,40 @@ pub(super) struct TerminalSession {
|
|||
pub(super) screen: TerminalScreen,
|
||||
pub(super) process: TerminalProcessState,
|
||||
pub(super) annotated: bool,
|
||||
/// Resolved `terminal.escape-key` for this terminal (Q#TC4c).
|
||||
///
|
||||
/// The cache lives HERE, not in an editor-side map, because a
|
||||
/// session is created in [`TerminalManager::open`] and dropped on
|
||||
/// kill/prune — so its lifetime is exactly the cache's, with no
|
||||
/// purge hook to forget. An editor-side map would leak an entry per
|
||||
/// terminal; a single last-entry cache would reparse (and re-report
|
||||
/// an invalid value) every time focus alternates between two
|
||||
/// terminals.
|
||||
pub(super) escape: Option<EscapeCache>,
|
||||
}
|
||||
|
||||
/// One terminal's parsed escape chord, valid for one config epoch.
|
||||
pub(super) struct EscapeCache {
|
||||
/// The `ConfigRegistry::value_epoch` this was parsed at. The key is
|
||||
/// `(this session, epoch)`: the epoch alone is not enough, because
|
||||
/// it does not advance when focus moves between terminals with
|
||||
/// different buffer-local values.
|
||||
pub(super) epoch: u64,
|
||||
/// The effective chord — the parsed spelling, or the `C-c` fallback.
|
||||
pub(super) chord: Chord,
|
||||
/// The invalid spelling already reported for this terminal, if any.
|
||||
/// Reporting is once per terminal per effective invalid value: an
|
||||
/// unchanged bad value stays quiet, a *different* bad value reports
|
||||
/// again because it is a new mistake.
|
||||
pub(super) reported_invalid: Option<String>,
|
||||
}
|
||||
|
||||
/// Owns the one-buffer/one-process/one-screen terminal registry.
|
||||
#[derive(Default)]
|
||||
pub struct TerminalManager {
|
||||
pub(super) sessions: HashMap<BufferId, TerminalSession>,
|
||||
/// Total escape-key parses performed (Q#TC4c observability).
|
||||
escape_parses: u64,
|
||||
process_to_buffer: HashMap<ProcessId, BufferId>,
|
||||
/// Removed buffers whose children are still being reaped. Their events
|
||||
/// remain manager-owned so Lua/LSP/MCP consumers cannot steal a batch.
|
||||
|
|
@ -331,6 +360,7 @@ impl TerminalManager {
|
|||
screen,
|
||||
process: TerminalProcessState::Running,
|
||||
annotated: false,
|
||||
escape: None,
|
||||
},
|
||||
);
|
||||
debug_assert!(previous.is_none(), "fresh BufferId collided");
|
||||
|
|
@ -538,6 +568,89 @@ impl TerminalManager {
|
|||
.map_err(TerminalError::Process)
|
||||
}
|
||||
|
||||
/// Resolve this terminal's effective escape chord, parsing at most
|
||||
/// once per `(terminal, config epoch)` (Q#TC4c).
|
||||
///
|
||||
/// `spelling` is the caller-resolved `terminal.escape-key` value and
|
||||
/// `epoch` the registry's `value_epoch()` it was read at. Returns the
|
||||
/// effective chord plus, at most once per terminal per effective
|
||||
/// invalid value, a message the caller should surface.
|
||||
///
|
||||
/// An unparseable spelling falls back to `C-c` rather than leaving the
|
||||
/// terminal with no escape at all (Q#TC4a): without one, every key goes
|
||||
/// to the child and the user cannot reach the binding that would fix
|
||||
/// the setting that broke it.
|
||||
pub fn escape_chord(
|
||||
&mut self,
|
||||
buffer_id: BufferId,
|
||||
epoch: u64,
|
||||
spelling: &str,
|
||||
) -> (Chord, Option<String>) {
|
||||
let fallback = default_escape_chord();
|
||||
if let Some(session) = self.sessions.get(&buffer_id)
|
||||
&& let Some(cache) = session.escape.as_ref()
|
||||
&& cache.epoch == epoch
|
||||
{
|
||||
return (cache.chord, None);
|
||||
}
|
||||
self.escape_parses = self.escape_parses.saturating_add(1);
|
||||
let Some(session) = self.sessions.get_mut(&buffer_id) else {
|
||||
return (fallback, None);
|
||||
};
|
||||
let previously_reported = session
|
||||
.escape
|
||||
.as_ref()
|
||||
.and_then(|cache| cache.reported_invalid.clone());
|
||||
let (chord, reported_invalid, report) = match parse_chord(spelling) {
|
||||
Ok(chord) => (chord, None, None),
|
||||
Err(error) => {
|
||||
let already = previously_reported.as_deref() == Some(spelling);
|
||||
let message = (!already).then(|| {
|
||||
format!(
|
||||
"terminal.escape-key {spelling:?} is not a valid chord ({error}); using C-c"
|
||||
)
|
||||
});
|
||||
(fallback, Some(spelling.to_owned()), message)
|
||||
}
|
||||
};
|
||||
session.escape = Some(EscapeCache {
|
||||
epoch,
|
||||
chord,
|
||||
reported_invalid,
|
||||
});
|
||||
(chord, report)
|
||||
}
|
||||
|
||||
/// How many escape-key spellings this manager has parsed.
|
||||
///
|
||||
/// An observability seam for Q#TC4c's cache contract, which is
|
||||
/// otherwise unpinnable for a VALID setting: a correct per-session
|
||||
/// cache and a single last-entry cache produce identical behavior
|
||||
/// there and differ only in how often they parse. Counting reports
|
||||
/// covers the invalid case; this covers the valid one.
|
||||
#[must_use]
|
||||
pub fn escape_parses(&self) -> u64 {
|
||||
self.escape_parses
|
||||
}
|
||||
|
||||
/// How many terminals currently hold a cached escape chord.
|
||||
///
|
||||
/// The LIFETIME half of Q#TC4c's cache contract, which `escape_parses`
|
||||
/// cannot cover: parse counting says a valid setting is read once, but
|
||||
/// says nothing about whether the cache is ever released. Because the
|
||||
/// cache lives on [`TerminalSession`], this count falls with the
|
||||
/// session set by construction — which is exactly the property worth
|
||||
/// pinning, since the rejected alternative (an editor-side
|
||||
/// `HashMap<BufferId, EscapeCache>`) has no purge hook and would hold
|
||||
/// this at its high-water mark while sessions drained.
|
||||
#[must_use]
|
||||
pub fn escape_caches(&self) -> usize {
|
||||
self.sessions
|
||||
.values()
|
||||
.filter(|session| session.escape.is_some())
|
||||
.count()
|
||||
}
|
||||
|
||||
/// Resize a terminal screen and its PTY after validating shared limits.
|
||||
pub fn resize(
|
||||
&mut self,
|
||||
|
|
@ -730,3 +843,12 @@ fn sanitize_metadata(value: &str) -> String {
|
|||
}
|
||||
clean
|
||||
}
|
||||
|
||||
/// The built-in terminal escape chord, and the fallback for an
|
||||
/// unparseable `terminal.escape-key` (Q#TC4a).
|
||||
pub(super) fn default_escape_chord() -> Chord {
|
||||
Chord::new(
|
||||
crossterm::event::KeyCode::Char('c'),
|
||||
crossterm::event::KeyModifiers::CONTROL,
|
||||
)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -558,8 +558,19 @@ pub struct FrontendView {
|
|||
/// store. Reckoning in visible lines unconditionally would make that
|
||||
/// GPU session's cursor skip lines it is still showing, so every
|
||||
/// command/event-time visible-line reckoning is gated on the
|
||||
/// **acting** frontend's flag. Render-time clamps need no gate: a
|
||||
/// semantic session never enters `paint_frame`.
|
||||
/// **acting** frontend's flag.
|
||||
///
|
||||
/// **Render-time clamps used to need no gate, on the premise that a
|
||||
/// semantic session never enters `paint_frame`. The bottom-panel
|
||||
/// band breaks that premise** (Q#BP17): the daemon projects a
|
||||
/// semantic frontend's side window through the same per-window
|
||||
/// painter. So the extracted painters
|
||||
/// (`prepare_window_cursor_visible`, `paint_window_content`) take the
|
||||
/// visible-line map as a **parameter**, and the panel path passes
|
||||
/// `None` when the *owning* frontend's `fold_projection` is false.
|
||||
/// That path must not call `EditorCore::fold_map_for_window`, which
|
||||
/// gates on the **active** frontend — correct for command-time
|
||||
/// reckoning, wrong for painting another frontend's panel.
|
||||
///
|
||||
/// Set at attach from the negotiated selected-render bit (grid ⇒
|
||||
/// `true`, semantic ⇒ `false`), cleared with the view at detach, and
|
||||
|
|
|
|||
|
|
@ -203,6 +203,21 @@ fn statusline_document_context_reports_active_false_under_a_focused_panel() {
|
|||
|
||||
match evaluation.outcome {
|
||||
StatuslineEvaluationOutcome::Ready(windows) => {
|
||||
// A2A-2: the semantic fan-out captures the primary document
|
||||
// AND the visible side window — two contexts, not one.
|
||||
assert_eq!(
|
||||
windows.len(),
|
||||
2,
|
||||
"the semantic-layout target must capture document + visible side window"
|
||||
);
|
||||
let side = windows
|
||||
.iter()
|
||||
.find(|w| w.context.window_id != document)
|
||||
.expect("a side-window context");
|
||||
assert!(
|
||||
side.context.active,
|
||||
"the focused panel's own context reports active = true"
|
||||
);
|
||||
let context = windows
|
||||
.first()
|
||||
.map(|segments| segments.context)
|
||||
|
|
@ -275,6 +290,15 @@ fn extraction_preserves_cells_cursor_and_focused_view_top() {
|
|||
pmacs.window.display(b, {})",
|
||||
);
|
||||
|
||||
// The gutter only paints when line numbers are on, so turn them on
|
||||
// rather than dropping the assertion.
|
||||
{
|
||||
let mut core = s.core.borrow_mut();
|
||||
let active = core.views[&FrontendId::LOCAL].active;
|
||||
core.windows.get_mut(&active).unwrap().line_numbers =
|
||||
pmacs::window::LineNumberMode::Absolute;
|
||||
}
|
||||
|
||||
let size = CellSize::new(ROWS, COLS);
|
||||
let mut cells_a = vec![pmacs::cell::Cell::default(); (ROWS * COLS) as usize];
|
||||
let mut grid_a = CellGrid {
|
||||
|
|
@ -312,6 +336,44 @@ fn extraction_preserves_cells_cursor_and_focused_view_top() {
|
|||
assert_eq!(cells_a, cells_b, "painted cells must be stable");
|
||||
assert_eq!(cursor_a, cursor_b, "the returned cursor must be stable");
|
||||
assert_eq!(view_top_a, view_top_b, "focused view_top must be stable");
|
||||
|
||||
// Review round 1, finding 5: a fixed-point check alone is VACUOUS —
|
||||
// deleting `text_view.render` leaves it green. Assert the extracted
|
||||
// painter actually produced each of its four outputs.
|
||||
let rows = |cells: &[pmacs::cell::Cell]| -> Vec<String> {
|
||||
(0..ROWS as usize)
|
||||
.map(|r| {
|
||||
(0..COLS as usize)
|
||||
.map(|c| match &cells[r * COLS as usize + c].glyph {
|
||||
pmacs::cell::Glyph::Char(ch) => *ch,
|
||||
_ => ' ',
|
||||
})
|
||||
.collect::<String>()
|
||||
})
|
||||
.collect()
|
||||
};
|
||||
let painted = rows(&cells_a);
|
||||
|
||||
// TEXT: the buffer's content reached the grid.
|
||||
assert!(
|
||||
painted.iter().any(|row| row.contains("line")),
|
||||
"the extracted painter must paint buffer TEXT; got {painted:?}"
|
||||
);
|
||||
// GUTTER: line numbers were painted beside it.
|
||||
assert!(
|
||||
painted.iter().any(|row| row.trim_start().starts_with('1')),
|
||||
"the extracted painter must paint the line-number GUTTER"
|
||||
);
|
||||
// MODE LINE: the window's mode line names its buffer.
|
||||
assert!(
|
||||
painted.iter().any(|row| row.contains("*doc*")),
|
||||
"the extracted painter must paint the window MODE LINE"
|
||||
);
|
||||
// CURSOR: a real caret position came back, not None.
|
||||
assert!(
|
||||
cursor_a.is_some(),
|
||||
"the extraction must still return a caret position"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -375,3 +437,151 @@ fn the_panel_fixture_really_builds_a_side_window() {
|
|||
"the fixture must produce a real bottom side window"
|
||||
);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// A2A-1 at the CONSUMER seam — review round 1, finding 3.
|
||||
//
|
||||
// The tests above assert the *authority* (`primary_document_window`).
|
||||
// That is not sufficient: restoring a producer to `active_window_for`
|
||||
// leaves every one of them green. These drive the real producers through
|
||||
// `SemanticRenderState::render_frame` with a panel focused, so a reverted
|
||||
// routing fails here.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// A semantic frontend that CAN hold a panel. Stage 1 ships
|
||||
/// `panel_capable = false` for semantic sessions and 2B flips it for a
|
||||
/// v21-negotiated peer; until then the projection is only reachable with
|
||||
/// a test-only capable view, which is exactly what the framing's §7.2
|
||||
/// says 2B must replace with the real capability flip.
|
||||
fn semantic_frontend_with_focused_panel(
|
||||
s: &EditorState,
|
||||
) -> (FrontendId, WindowId, WindowId, pmacs::buffer::BufferId) {
|
||||
use pmacs::window::{FrontendView, Layout, LayoutNode, Orientation, Window, WindowParams};
|
||||
|
||||
let fid = FrontendId(77);
|
||||
let (doc_win, panel_win, doc_buf) = {
|
||||
let mut core = s.core.borrow_mut();
|
||||
let doc_buf = core.active_window().buffer_id;
|
||||
let panel_buf = core.registry.borrow_mut().create("*panel*");
|
||||
|
||||
let doc_win = WindowId::next();
|
||||
let panel_win = WindowId::next();
|
||||
let doc_view = {
|
||||
let reg = core.registry.borrow();
|
||||
pmacs::text_view::TextView::new(reg.get(doc_buf).expect("document buffer"))
|
||||
};
|
||||
let panel_view = {
|
||||
let reg = core.registry.borrow();
|
||||
pmacs::text_view::TextView::new(reg.get(panel_buf).expect("panel buffer"))
|
||||
};
|
||||
core.windows
|
||||
.insert(doc_win, Window::new(doc_win, doc_buf, doc_view));
|
||||
let mut panel = Window::new(panel_win, panel_buf, panel_view);
|
||||
let mut params = WindowParams::default();
|
||||
params.side = Some(Side::Bottom);
|
||||
params.fixed_rows = Some(4);
|
||||
panel.params = params;
|
||||
core.windows.insert(panel_win, panel);
|
||||
|
||||
core.register_frontend_view(
|
||||
fid,
|
||||
FrontendView {
|
||||
layout: Layout {
|
||||
root: LayoutNode::Split {
|
||||
orientation: Orientation::Horizontal,
|
||||
children: vec![LayoutNode::Leaf(doc_win), LayoutNode::Leaf(panel_win)],
|
||||
weights: vec![1, 1],
|
||||
},
|
||||
},
|
||||
// The panel owns focus; the document is the projection.
|
||||
active: panel_win,
|
||||
fold_projection: false,
|
||||
panel_capable: true,
|
||||
frame_geometry: None,
|
||||
panel_hidden: false,
|
||||
},
|
||||
);
|
||||
(doc_win, panel_win, doc_buf)
|
||||
};
|
||||
s.sync_frame_geometry(fid, CellSize::new(ROWS, COLS));
|
||||
(fid, doc_win, panel_win, doc_buf)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn consumer_line_numbers_follow_the_document_not_the_focused_panel() {
|
||||
use pmacs::protocol::{ByteRange, InstanceMessage};
|
||||
use pmacs::semantic_render::SemanticRenderState;
|
||||
use pmacs::window::LineNumberMode;
|
||||
|
||||
let s = editor();
|
||||
let (fid, doc_win, panel_win, doc_buf) = semantic_frontend_with_focused_panel(&s);
|
||||
|
||||
// Make the two windows DISAGREE, so the emitted mode identifies
|
||||
// which window the producer read (§1.3 #4).
|
||||
{
|
||||
let mut core = s.core.borrow_mut();
|
||||
core.windows.get_mut(&doc_win).unwrap().line_numbers = LineNumberMode::Absolute;
|
||||
core.windows.get_mut(&panel_win).unwrap().line_numbers = LineNumberMode::Off;
|
||||
}
|
||||
|
||||
let mut sem = SemanticRenderState::new(fid);
|
||||
sem.set_viewport(doc_buf, ByteRange { start: 0, end: 0 }, 0);
|
||||
let msgs = sem.render_frame(&s);
|
||||
|
||||
let mode = msgs.iter().find_map(|m| match m {
|
||||
InstanceMessage::LineNumbers { mode, .. } => Some(*mode),
|
||||
_ => None,
|
||||
});
|
||||
assert_eq!(
|
||||
mode,
|
||||
Some(pmacs::protocol::LineNumberMode::Absolute),
|
||||
"LineNumbers must describe the DOCUMENT window's mode, not the focused panel's"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn consumer_statusline_segments_name_the_document_window() {
|
||||
use pmacs::protocol::ByteRange;
|
||||
use pmacs::semantic_render::SemanticRenderState;
|
||||
|
||||
// §1.3 #12 at the producer: the wire segments must be selected by
|
||||
// the DOCUMENT window even though the fan-out now also evaluates the
|
||||
// visible side window (A2A-2).
|
||||
let s = editor();
|
||||
let (fid, doc_win, _panel_win, doc_buf) = semantic_frontend_with_focused_panel(&s);
|
||||
|
||||
let mut sem = SemanticRenderState::new(fid);
|
||||
sem.set_viewport(doc_buf, ByteRange { start: 0, end: 0 }, 0);
|
||||
// Not asserting on message presence (a peer that never negotiated
|
||||
// v18 emits none); asserting the routing input the producer uses.
|
||||
let _ = sem.render_frame(&s);
|
||||
|
||||
assert_eq!(
|
||||
s.core.borrow().primary_document_window(fid),
|
||||
Some(doc_win),
|
||||
"the producer's document-window selector must name the document"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn consumer_terminal_declaration_cannot_be_claimed_by_a_focused_panel() {
|
||||
// §1.3 #6/#10/#11 through the real guard: with the panel focused,
|
||||
// a declaration naming the PANEL's buffer must be refused, because
|
||||
// the full-window terminal surface is the document window.
|
||||
let s = editor();
|
||||
let (fid, _doc_win, panel_win, doc_buf) = semantic_frontend_with_focused_panel(&s);
|
||||
let panel_buf = s.core.borrow().windows[&panel_win].buffer_id;
|
||||
|
||||
assert!(
|
||||
!s.semantic_terminal_declaration_is_active(fid, panel_buf),
|
||||
"a focused panel's buffer must not become the document terminal declaration"
|
||||
);
|
||||
// Non-vacuity: the document buffer is not a terminal either, so pin
|
||||
// that the guard resolves the DOCUMENT window by asserting the
|
||||
// window identity the resolver used.
|
||||
assert_eq!(
|
||||
s.core.borrow().primary_document_buffer(fid),
|
||||
Some(doc_buf),
|
||||
"the terminal resolver's window must be the document window"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,746 @@
|
|||
//! Terminal configuration acceptance (Stage 1 of
|
||||
//! `docs/terminal-config-and-copy-mode-framing.md`, criteria 1-12).
|
||||
//!
|
||||
//! Deliberately NOT `#[cfg(feature = "crdt")]`: CI never enables that
|
||||
//! feature, so a gated suite is written and then never run.
|
||||
|
||||
use std::thread;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use crossterm::event::{KeyCode, KeyEvent, KeyModifiers};
|
||||
use mlua::Value;
|
||||
use pmacs::cell::{CellSize, Glyph};
|
||||
use pmacs::editor::EditorState;
|
||||
use pmacs::protocol::FrontendId;
|
||||
use pmacs::terminal::TerminalViewKey;
|
||||
use pmacs::window::WindowId;
|
||||
|
||||
fn exec(state: &EditorState, src: &str) {
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(src)
|
||||
.exec()
|
||||
.unwrap_or_else(|e| panic!("lua failed: {src}\n{e}"));
|
||||
}
|
||||
|
||||
fn eval_err(state: &EditorState, src: &str) -> String {
|
||||
let result: mlua::Result<Value> = state.lua_host.lua().load(src).eval();
|
||||
match result {
|
||||
Ok(_) => panic!("expected an error from: {src}"),
|
||||
Err(e) => e.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
/// The viewport every test projects through. Deliberately SHORTER than
|
||||
/// the 24-row screen a terminal opens with, so "scroll to the oldest
|
||||
/// retained row" has somewhere to go even when nothing is retained —
|
||||
/// which is what makes the two scrollback arms differ by content rather
|
||||
/// than by whether scrolling was possible at all.
|
||||
fn viewport() -> CellSize {
|
||||
CellSize::new(10, 40)
|
||||
}
|
||||
|
||||
fn cells_to_text(cells: &[pmacs::cell::Cell]) -> String {
|
||||
let mut text = String::new();
|
||||
for cell in cells {
|
||||
match &cell.glyph {
|
||||
Glyph::Char(c) => text.push(*c),
|
||||
Glyph::Cluster(b) => text.push_str(&String::from_utf8_lossy(b)),
|
||||
Glyph::Continuation => {}
|
||||
}
|
||||
}
|
||||
text
|
||||
}
|
||||
|
||||
fn screen_text(state: &EditorState, buffer: pmacs::buffer::BufferId) -> String {
|
||||
let manager = state.terminal_manager.borrow();
|
||||
let Some(snapshot) = manager.snapshot(buffer) else {
|
||||
return String::new();
|
||||
};
|
||||
cells_to_text(&snapshot.cells)
|
||||
}
|
||||
|
||||
/// Text a view actually shows, which is where retained history is
|
||||
/// visible at all — the live `screen_text` above always reads the tail.
|
||||
fn view_text(state: &EditorState, key: TerminalViewKey) -> String {
|
||||
let mut manager = state.terminal_manager.borrow_mut();
|
||||
manager
|
||||
.snapshot_for_view(key, viewport())
|
||||
.map(|snapshot| cells_to_text(&snapshot.cells))
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
/// Scroll a view to its OLDEST retained row and read it back.
|
||||
fn oldest_view_text(state: &EditorState, key: TerminalViewKey) -> String {
|
||||
state
|
||||
.terminal_manager
|
||||
.borrow_mut()
|
||||
.scroll_view(key, viewport(), i32::MAX);
|
||||
view_text(state, key)
|
||||
}
|
||||
|
||||
fn tick_until(state: &mut EditorState, needle: &str, buffer: pmacs::buffer::BufferId) -> bool {
|
||||
let deadline = Instant::now() + Duration::from_secs(5);
|
||||
loop {
|
||||
state.tick_processes();
|
||||
if screen_text(state, buffer).contains(needle) {
|
||||
return true;
|
||||
}
|
||||
if Instant::now() >= deadline {
|
||||
return false;
|
||||
}
|
||||
thread::sleep(Duration::from_millis(20));
|
||||
}
|
||||
}
|
||||
|
||||
/// Give LOCAL a window on `buffer` and register/claim its terminal view,
|
||||
/// which is what makes `dispatch_key`'s terminal arm reachable.
|
||||
fn focus_terminal(state: &EditorState, buffer: pmacs::buffer::BufferId) -> WindowId {
|
||||
state.core.borrow_mut().switch_active_buffer(buffer).ok();
|
||||
let window = state.core.borrow().active_window_id();
|
||||
let key = TerminalViewKey::new(FrontendId::LOCAL, window, buffer);
|
||||
let mut manager = state.terminal_manager.borrow_mut();
|
||||
manager.register_view(key);
|
||||
manager.claim_controller(key);
|
||||
let _ = manager.snapshot_for_view(key, viewport());
|
||||
window
|
||||
}
|
||||
|
||||
fn terminal_buffers(state: &EditorState) -> Vec<pmacs::buffer::BufferId> {
|
||||
let manager = state.terminal_manager.borrow();
|
||||
state
|
||||
.core
|
||||
.borrow()
|
||||
.registry
|
||||
.borrow()
|
||||
.ids()
|
||||
.iter()
|
||||
.copied()
|
||||
.filter(|id| manager.is_terminal(*id))
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Open a terminal from Lua and return the identity buffer it created.
|
||||
///
|
||||
/// The id is derived by diffing the manager's terminal set rather than
|
||||
/// returned through Lua: `BufferIdLua` exposes no id accessor, and
|
||||
/// diffing also asserts in passing that exactly one terminal appeared.
|
||||
fn open_cat_terminal(state: &EditorState, lua_spec: &str) -> pmacs::buffer::BufferId {
|
||||
let before = terminal_buffers(state);
|
||||
exec(
|
||||
state,
|
||||
&format!("TERM_BUF = pmacs.terminal.open {{ {lua_spec} }}"),
|
||||
);
|
||||
let after = terminal_buffers(state);
|
||||
let mut fresh: Vec<_> = after
|
||||
.into_iter()
|
||||
.filter(|id| !before.contains(id))
|
||||
.collect();
|
||||
assert_eq!(fresh.len(), 1, "exactly one terminal must have opened");
|
||||
fresh.remove(0)
|
||||
}
|
||||
|
||||
/// `cat -v` is the echo instrument, deliberately: the terminal screen
|
||||
/// rejects C0/C1 controls before they enter cells (Vterm Stage 1
|
||||
/// criterion 2), so a raw echoed `Ctrl-X` would be invisible and a test
|
||||
/// probing for it could never pass. `-v` renders it as the printable
|
||||
/// two-character `^X`, which is what makes "the configured chord reached
|
||||
/// the child" observable at all.
|
||||
const CAT_PROFILE: &str = r#"
|
||||
pmacs.terminal.profiles.echo = {
|
||||
command = "/bin/sh",
|
||||
args = { "-c", "printf 'READY\r\n'; exec cat -v" },
|
||||
}
|
||||
"#;
|
||||
|
||||
/// Did the last key ARM the terminal escape?
|
||||
///
|
||||
/// Observed behaviorally rather than through an accessor: while the
|
||||
/// escape is armed the next key goes to ordinary dispatch, so it never
|
||||
/// reaches the child. `cat` echoes anything that does reach it, which
|
||||
/// makes "the probe character did not appear" the exact observable for
|
||||
/// "that chord was consumed as the escape".
|
||||
fn escape_was_armed(state: &mut EditorState, buffer: pmacs::buffer::BufferId, probe: char) -> bool {
|
||||
// Count occurrences rather than testing for presence: the screen
|
||||
// already holds the child's own output, and a single-character probe
|
||||
// like 'R' collides with the "READY" banner. Only an INCREASE proves
|
||||
// this keystroke reached the child.
|
||||
let before = screen_text(state, buffer).matches(probe).count();
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char(probe), KeyModifiers::NONE),
|
||||
);
|
||||
let deadline = Instant::now() + Duration::from_secs(2);
|
||||
loop {
|
||||
state.tick_processes();
|
||||
if screen_text(state, buffer).matches(probe).count() > before {
|
||||
return false;
|
||||
}
|
||||
if Instant::now() >= deadline {
|
||||
return true;
|
||||
}
|
||||
thread::sleep(Duration::from_millis(20));
|
||||
}
|
||||
}
|
||||
|
||||
/// Acceptance 1: a profile spec is strict, and rejects before anything spawns.
|
||||
#[test]
|
||||
fn acc1_profile_specs_are_strict_and_reject_before_spawning() {
|
||||
let state = EditorState::new();
|
||||
let before = state.core.borrow().registry.borrow().ids().len();
|
||||
|
||||
exec(
|
||||
&state,
|
||||
r#"pmacs.terminal.profiles.bad = { command = "/bin/sh", nonsense = true }"#,
|
||||
);
|
||||
let err = eval_err(&state, r#"return pmacs.terminal.open { profile = "bad" }"#);
|
||||
assert!(
|
||||
err.contains("unknown field") && err.contains("nonsense"),
|
||||
"the error must name the offending field: {err}"
|
||||
);
|
||||
|
||||
exec(&state, "pmacs.terminal.profiles.wrong = { command = 42 }");
|
||||
let err = eval_err(
|
||||
&state,
|
||||
r#"return pmacs.terminal.open { profile = "wrong" }"#,
|
||||
);
|
||||
assert!(err.contains("must be a string"), "typed field error: {err}");
|
||||
|
||||
assert_eq!(
|
||||
state.core.borrow().registry.borrow().ids().len(),
|
||||
before,
|
||||
"a rejected profile must create no buffer"
|
||||
);
|
||||
assert_eq!(state.terminal_manager.borrow().len(), 0);
|
||||
}
|
||||
|
||||
/// Acceptance 2: an unknown profile names the known ones and creates nothing.
|
||||
#[test]
|
||||
fn acc2_unknown_profile_lists_known_names_and_creates_nothing() {
|
||||
let state = EditorState::new();
|
||||
exec(&state, CAT_PROFILE);
|
||||
exec(
|
||||
&state,
|
||||
r#"pmacs.terminal.profiles.other = { command = "/bin/sh" }"#,
|
||||
);
|
||||
let before = state.core.borrow().registry.borrow().ids().len();
|
||||
|
||||
// Via the default setting.
|
||||
exec(
|
||||
&state,
|
||||
r#"pmacs.config.set("terminal.default-profile", "ghost")"#,
|
||||
);
|
||||
let err = eval_err(&state, "return pmacs.terminal.open {}");
|
||||
assert!(err.contains("ghost"), "names the missing profile: {err}");
|
||||
assert!(
|
||||
err.contains("echo") && err.contains("other"),
|
||||
"must LIST the known profiles: {err}"
|
||||
);
|
||||
|
||||
// An explicit bad profile fails even though the default is now valid —
|
||||
// a typo must not silently fall back (Q#TC3a).
|
||||
exec(
|
||||
&state,
|
||||
r#"pmacs.config.set("terminal.default-profile", "echo")"#,
|
||||
);
|
||||
let err = eval_err(&state, r#"return pmacs.terminal.open { profile = "typo" }"#);
|
||||
assert!(err.contains("typo"), "explicit bad profile errors: {err}");
|
||||
|
||||
assert_eq!(
|
||||
state.core.borrow().registry.borrow().ids().len(),
|
||||
before,
|
||||
"no buffer, session, or process is created"
|
||||
);
|
||||
assert_eq!(state.terminal_manager.borrow().len(), 0);
|
||||
}
|
||||
|
||||
/// Acceptance 2 (malformed table): `pmacs.terminal.profiles` is a raw
|
||||
/// user table, so a diagnostic that walks its keys must be total over
|
||||
/// them. A table holding both a string and a numeric key made
|
||||
/// `table.sort` raise "attempt to compare number with string" — on the
|
||||
/// unknown-profile path, replacing the exact error being asked for.
|
||||
#[test]
|
||||
fn acc2_malformed_profile_keys_do_not_mask_the_unknown_profile_error() {
|
||||
let state = EditorState::new();
|
||||
exec(&state, CAT_PROFILE);
|
||||
exec(
|
||||
&state,
|
||||
r#"pmacs.terminal.profiles[1] = { command = "/bin/sh" }"#,
|
||||
);
|
||||
|
||||
let err = eval_err(
|
||||
&state,
|
||||
r#"return pmacs.terminal.open { profile = "ghost" }"#,
|
||||
);
|
||||
assert!(
|
||||
err.contains("ghost") && err.contains("echo"),
|
||||
"the unknown-profile error must survive a malformed table: {err}"
|
||||
);
|
||||
assert!(
|
||||
!err.contains("attempt to compare"),
|
||||
"listing known profiles must not raise: {err}"
|
||||
);
|
||||
|
||||
// Rendering the REQUESTED name is partial too: `%q` raises on a
|
||||
// table, and the name arrives straight from the caller.
|
||||
let err = eval_err(&state, r"return pmacs.terminal.open { profile = {} }");
|
||||
assert!(
|
||||
err.contains("is not defined") && err.contains("known profiles"),
|
||||
"a non-string profile name must render, not raise: {err}"
|
||||
);
|
||||
|
||||
assert_eq!(state.terminal_manager.borrow().len(), 0);
|
||||
}
|
||||
|
||||
/// Acceptance 3: explicit beats profile beats setting beats `$SHELL`, and
|
||||
/// `env` MERGES rather than replacing.
|
||||
#[test]
|
||||
fn acc3_field_resolution_order_and_env_merge() {
|
||||
let mut state = EditorState::new();
|
||||
exec(
|
||||
&state,
|
||||
r#"
|
||||
pmacs.terminal.profiles.merged = {
|
||||
command = "/bin/sh",
|
||||
args = { "-c", "printf 'PROFILE:%s:%s\r\n' \"$FROM_PROFILE\" \"$SHARED\"; exec cat" },
|
||||
env = { FROM_PROFILE = "p", SHARED = "profile" },
|
||||
}
|
||||
"#,
|
||||
);
|
||||
let buffer = open_cat_terminal(
|
||||
&state,
|
||||
r#"profile = "merged", env = { SHARED = "explicit" }"#,
|
||||
);
|
||||
assert!(
|
||||
tick_until(&mut state, "PROFILE:p:explicit", buffer),
|
||||
"profile env survives and explicit env overrides the same key: {:?}",
|
||||
screen_text(&state, buffer)
|
||||
);
|
||||
state.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
||||
/// Acceptance 3 (explicit command wins) and 4 (`""` means no profile).
|
||||
#[test]
|
||||
fn acc3_acc4_explicit_command_wins_and_empty_default_means_no_profile() {
|
||||
let mut state = EditorState::new();
|
||||
exec(&state, CAT_PROFILE);
|
||||
exec(
|
||||
&state,
|
||||
r#"pmacs.config.set("terminal.default-profile", "echo")"#,
|
||||
);
|
||||
|
||||
// Explicit command beats the profile's.
|
||||
let explicit = open_cat_terminal(
|
||||
&state,
|
||||
r#"command = "/bin/sh", args = { "-c", "printf 'EXPLICIT\r\n'; exec cat" }"#,
|
||||
);
|
||||
assert!(tick_until(&mut state, "EXPLICIT", explicit));
|
||||
|
||||
// `""` is the no-profile sentinel: falls through to $SHELL.
|
||||
exec(
|
||||
&state,
|
||||
r#"pmacs.config.set("terminal.default-profile", "")"#,
|
||||
);
|
||||
let bare = open_cat_terminal(&state, "");
|
||||
let spec_ok = state.terminal_manager.borrow().is_terminal(bare);
|
||||
assert!(spec_ok, "an empty default must open a $SHELL terminal");
|
||||
assert!(
|
||||
!screen_text(&state, bare).contains("READY"),
|
||||
"the echo profile must NOT have been applied"
|
||||
);
|
||||
state.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
||||
/// A child that overflows the 24-row screen and then goes quiet, so its
|
||||
/// early output can only still be found in RETAINED HISTORY. Zero-padded
|
||||
/// so `LINE001` is not a substring of `LINE100`.
|
||||
const FILL_PROFILE: &str = r#"
|
||||
pmacs.terminal.profiles.fill = {
|
||||
command = "/bin/sh",
|
||||
args = { "-c",
|
||||
"i=1; while [ $i -le 200 ]; do printf 'LINE%03d\r\n' $i; i=$((i+1)); done; printf 'DONE\r\n'; exec cat" },
|
||||
}
|
||||
"#;
|
||||
|
||||
/// Acceptance 5: the scrollback SETTING reaches the screen's retained
|
||||
/// history, an explicit spec value overrides it, and `0` is legal.
|
||||
///
|
||||
/// Asserted end to end, through a real child and a real view, rather
|
||||
/// than by reading the value back out of the registry: a registry
|
||||
/// round-trip is a test of the registry, and would stay green with the
|
||||
/// setting's only consumer (`terminal.lua`'s `resolved.scrollback_rows`
|
||||
/// fallback) deleted outright.
|
||||
#[test]
|
||||
fn acc5_scrollback_setting_reaches_retained_history() {
|
||||
let mut state = EditorState::new();
|
||||
exec(&state, FILL_PROFILE);
|
||||
|
||||
// Arm 1: `0` is legal, and means the early rows are GONE.
|
||||
exec(&state, r#"pmacs.config.set("terminal.scrollback-rows", 0)"#);
|
||||
let none = open_cat_terminal(&state, r#"profile = "fill""#);
|
||||
assert!(tick_until(&mut state, "DONE", none), "child finished");
|
||||
let window = focus_terminal(&state, none);
|
||||
let none_key = TerminalViewKey::new(FrontendId::LOCAL, window, none);
|
||||
let oldest = oldest_view_text(&state, none_key);
|
||||
assert!(
|
||||
!oldest.contains("LINE001"),
|
||||
"with scrollback 0 the oldest retained row must not be the \
|
||||
child's first line: {oldest:?}"
|
||||
);
|
||||
|
||||
// Arm 2: a large setting retains it, reachable by scrolling back.
|
||||
exec(
|
||||
&state,
|
||||
r#"pmacs.config.set("terminal.scrollback-rows", 10000)"#,
|
||||
);
|
||||
let kept = open_cat_terminal(&state, r#"profile = "fill""#);
|
||||
assert!(tick_until(&mut state, "DONE", kept), "child finished");
|
||||
let window = focus_terminal(&state, kept);
|
||||
let kept_key = TerminalViewKey::new(FrontendId::LOCAL, window, kept);
|
||||
let oldest = oldest_view_text(&state, kept_key);
|
||||
assert!(
|
||||
oldest.contains("LINE001"),
|
||||
"with scrollback 10000 the first line must survive in history: \
|
||||
{oldest:?}"
|
||||
);
|
||||
|
||||
// Arm 3: an explicit spec value beats the setting, which is still 10000.
|
||||
let overridden = open_cat_terminal(&state, r#"profile = "fill", scrollback_rows = 0"#);
|
||||
assert!(tick_until(&mut state, "DONE", overridden), "child finished");
|
||||
let window = focus_terminal(&state, overridden);
|
||||
let overridden_key = TerminalViewKey::new(FrontendId::LOCAL, window, overridden);
|
||||
let oldest = oldest_view_text(&state, overridden_key);
|
||||
assert!(
|
||||
!oldest.contains("LINE001"),
|
||||
"an explicit scrollback_rows = 0 must beat the setting: {oldest:?}"
|
||||
);
|
||||
|
||||
state.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
||||
/// Acceptance 5 (bounds): the registered range rejects out-of-range
|
||||
/// values, and `0` is inside it rather than a disabled sentinel.
|
||||
#[test]
|
||||
fn acc5_scrollback_bounds() {
|
||||
let state = EditorState::new();
|
||||
exec(&state, r#"pmacs.config.set("terminal.scrollback-rows", 0)"#);
|
||||
assert_eq!(
|
||||
state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(r#"return pmacs.config.get("terminal.scrollback-rows")"#)
|
||||
.eval::<i64>()
|
||||
.unwrap(),
|
||||
0,
|
||||
"0 is a legal scrollback value meaning 'retain no history'"
|
||||
);
|
||||
|
||||
let err = eval_err(
|
||||
&state,
|
||||
r#"return pmacs.config.set("terminal.scrollback-rows", -1)"#,
|
||||
);
|
||||
assert!(
|
||||
err.contains("-1") || err.contains("min"),
|
||||
"below range: {err}"
|
||||
);
|
||||
let err = eval_err(
|
||||
&state,
|
||||
r#"return pmacs.config.set("terminal.scrollback-rows", 4000001)"#,
|
||||
);
|
||||
assert!(
|
||||
err.contains("4000001") || err.contains("max"),
|
||||
"above range: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Acceptance 6 and 9: the configured chord escapes, repeating it sends
|
||||
/// THAT chord to the child, and an ordinary `C-c` still reaches the child.
|
||||
#[test]
|
||||
fn acc6_acc9_configured_escape_chord_and_literal_repeat() {
|
||||
let mut state = EditorState::new();
|
||||
exec(&state, CAT_PROFILE);
|
||||
let buffer = open_cat_terminal(&state, r#"profile = "echo""#);
|
||||
assert!(tick_until(&mut state, "READY", buffer));
|
||||
focus_terminal(&state, buffer);
|
||||
|
||||
exec(&state, r#"pmacs.config.set("terminal.escape-key", "C-x")"#);
|
||||
|
||||
// `C-x C-x` must send Ctrl-X (0x18), which `cat` echoes back. Against
|
||||
// the pre-change hardcoded `&[0x03]` this sends Ctrl-C instead.
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL),
|
||||
);
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL),
|
||||
);
|
||||
assert!(
|
||||
tick_until(&mut state, "^X", buffer),
|
||||
"C-x C-x must send literal Ctrl-X: {:?}",
|
||||
screen_text(&state, buffer)
|
||||
);
|
||||
|
||||
// With the escape moved, an ordinary C-c is just another key.
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL),
|
||||
);
|
||||
assert!(
|
||||
tick_until(&mut state, "^C", buffer),
|
||||
"plain C-c must reach the child once the escape moved: {:?}",
|
||||
screen_text(&state, buffer)
|
||||
);
|
||||
state.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
||||
/// Acceptance 7, 8 and 8a: per-terminal escape resolution, an A→B→A parse
|
||||
/// count that does not grow, and a cache that dies with its terminal.
|
||||
#[test]
|
||||
fn acc7_acc8_acc8a_per_terminal_escape_cache_identity_and_lifecycle() {
|
||||
let mut state = EditorState::new();
|
||||
exec(&state, CAT_PROFILE);
|
||||
let a = open_cat_terminal(&state, r#"profile = "echo""#);
|
||||
exec(&state, "TERM_A = TERM_BUF");
|
||||
let b = open_cat_terminal(&state, r#"profile = "echo""#);
|
||||
exec(&state, "TERM_B = TERM_BUF");
|
||||
assert!(tick_until(&mut state, "READY", a));
|
||||
assert!(tick_until(&mut state, "READY", b));
|
||||
|
||||
// Different buffer-local escapes, then NO further writes.
|
||||
exec(
|
||||
&state,
|
||||
r#"pmacs.config.set_local(TERM_A, "terminal.escape-key", "C-x")"#,
|
||||
);
|
||||
exec(
|
||||
&state,
|
||||
r#"pmacs.config.set_local(TERM_B, "terminal.escape-key", "C-b")"#,
|
||||
);
|
||||
|
||||
// Prime both caches. Each priming press ARMS the escape, so it is
|
||||
// consumed with a probe — otherwise the next chord would be read as
|
||||
// the escape repeat rather than a fresh escape.
|
||||
focus_terminal(&state, a);
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL),
|
||||
);
|
||||
assert!(escape_was_armed(&mut state, a, 'M'), "A primes on its C-x");
|
||||
focus_terminal(&state, b);
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('b'), KeyModifiers::CONTROL),
|
||||
);
|
||||
assert!(escape_was_armed(&mut state, b, 'N'), "B primes on its C-b");
|
||||
let primed = state.terminal_manager.borrow().escape_parses();
|
||||
assert_eq!(
|
||||
state.terminal_manager.borrow().escape_caches(),
|
||||
2,
|
||||
"each primed terminal holds its own cache"
|
||||
);
|
||||
|
||||
// Acceptance 7 — BOTH directions. Asserting only that A still works
|
||||
// after A->B->A is not enough: an epoch-only cache hands whichever
|
||||
// entry it finds to every terminal, so A keeps working by accident
|
||||
// while B silently inherits A's chord. The discriminating assertion
|
||||
// is that EACH terminal honors its OWN chord and NOT the other's.
|
||||
focus_terminal(&state, b);
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('b'), KeyModifiers::CONTROL),
|
||||
);
|
||||
assert!(
|
||||
escape_was_armed(&mut state, b, 'R'),
|
||||
"terminal B must escape on its own C-b"
|
||||
);
|
||||
// ...and A's chord must be ordinary input in B, not an escape.
|
||||
focus_terminal(&state, b);
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL),
|
||||
);
|
||||
assert!(
|
||||
!escape_was_armed(&mut state, b, 'S'),
|
||||
"terminal A's C-x must NOT escape terminal B"
|
||||
);
|
||||
|
||||
focus_terminal(&state, a);
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL),
|
||||
);
|
||||
assert!(
|
||||
escape_was_armed(&mut state, a, 'Q'),
|
||||
"terminal A must still escape on its own C-x after A->B->A"
|
||||
);
|
||||
|
||||
// Acceptance 8: that round trip parsed nothing new. A single
|
||||
// last-entry cache would have reparsed twice.
|
||||
assert_eq!(
|
||||
state.terminal_manager.borrow().escape_parses(),
|
||||
primed,
|
||||
"A->B->A with no setting written must not reparse"
|
||||
);
|
||||
|
||||
// Acceptance 8a: the cache dies with its terminal.
|
||||
//
|
||||
// Waiting for the SESSION count to fall is not the assertion — a
|
||||
// session set that drains while an editor-side `HashMap<BufferId,
|
||||
// EscapeCache>` keeps its entry (the rejected implementation named
|
||||
// in Q#TC4c, which has no purge hook) satisfies it exactly. The
|
||||
// discriminating observable is the CACHE count, which such a map
|
||||
// would hold at its high-water mark of 2.
|
||||
let sessions_before = state.terminal_manager.borrow().len();
|
||||
exec(&state, "pmacs.terminal.terminate(TERM_A)");
|
||||
exec(&state, "pmacs.buffer.kill(TERM_A)");
|
||||
// Pruning is tick-driven (the manager reaps on the process tick), so
|
||||
// the session outlives the kill call by design.
|
||||
let deadline = Instant::now() + Duration::from_secs(5);
|
||||
while state.terminal_manager.borrow().len() >= sessions_before {
|
||||
state.tick_processes();
|
||||
assert!(
|
||||
Instant::now() < deadline,
|
||||
"killing the terminal must remove its session"
|
||||
);
|
||||
thread::sleep(Duration::from_millis(20));
|
||||
}
|
||||
assert_eq!(
|
||||
state.terminal_manager.borrow().escape_caches(),
|
||||
1,
|
||||
"killing terminal A must drop ITS cache, not merely its session"
|
||||
);
|
||||
|
||||
// ...and the surviving cache is B's, so the right one was dropped.
|
||||
focus_terminal(&state, b);
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('b'), KeyModifiers::CONTROL),
|
||||
);
|
||||
assert!(
|
||||
escape_was_armed(&mut state, b, 'T'),
|
||||
"terminal B must still escape on its own C-b after A was killed"
|
||||
);
|
||||
assert_eq!(
|
||||
state.terminal_manager.borrow().escape_parses(),
|
||||
primed,
|
||||
"B's surviving cache must not have been reparsed"
|
||||
);
|
||||
state.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
||||
/// Acceptance 10 and 10a: an unparseable value falls back, reports through
|
||||
/// the status line, and reports once per terminal per effective bad value.
|
||||
#[test]
|
||||
fn acc10_acc10a_invalid_escape_falls_back_and_reports_once() {
|
||||
let mut state = EditorState::new();
|
||||
exec(&state, CAT_PROFILE);
|
||||
let buffer = open_cat_terminal(&state, r#"profile = "echo""#);
|
||||
assert!(tick_until(&mut state, "READY", buffer));
|
||||
focus_terminal(&state, buffer);
|
||||
|
||||
exec(
|
||||
&state,
|
||||
r#"pmacs.config.set("terminal.escape-key", "not-a-chord")"#,
|
||||
);
|
||||
state.core.borrow_mut().status.clear();
|
||||
|
||||
// Acceptance 10: falls back to C-c, so the terminal stays escapable.
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL),
|
||||
);
|
||||
// Read the report BEFORE probing: `status` is a single slot, and the
|
||||
// probe key's own rejected self-insert would overwrite it.
|
||||
let reported = state.core.borrow().status.clone();
|
||||
assert!(
|
||||
reported.contains("terminal.escape-key") && reported.contains("not-a-chord"),
|
||||
"the report must name the setting and the bad value: {reported:?}"
|
||||
);
|
||||
assert!(
|
||||
escape_was_armed(&mut state, buffer, 'Q'),
|
||||
"an invalid escape-key must fall back to C-c, not leave the \
|
||||
terminal unescapable"
|
||||
);
|
||||
|
||||
// Acceptance 10a: the same bad value does not report again.
|
||||
state.core.borrow_mut().status.clear();
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL),
|
||||
);
|
||||
assert!(
|
||||
state.core.borrow().status.is_empty(),
|
||||
"an unchanged invalid value must not re-report: {:?}",
|
||||
state.core.borrow().status
|
||||
);
|
||||
let _ = escape_was_armed(&mut state, buffer, 'W');
|
||||
|
||||
// A DIFFERENT bad value is new information, so it reports again.
|
||||
exec(
|
||||
&state,
|
||||
r#"pmacs.config.set("terminal.escape-key", "also-bad")"#,
|
||||
);
|
||||
state.core.borrow_mut().status.clear();
|
||||
state.dispatch_key(
|
||||
FrontendId::LOCAL,
|
||||
KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL),
|
||||
);
|
||||
assert!(
|
||||
state.core.borrow().status.contains("also-bad"),
|
||||
"a different invalid value must report: {:?}",
|
||||
state.core.borrow().status
|
||||
);
|
||||
state.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
||||
/// Acceptance 11: the opening binding exists, resolves to the command, and
|
||||
/// shadowed nothing (`keymap.bind` is strict, so loading the runtime at all
|
||||
/// proves the second half).
|
||||
#[test]
|
||||
fn acc11_terminal_opening_binding_is_bound_and_shadowed_nothing() {
|
||||
let state = EditorState::new();
|
||||
let command: Option<String> = state
|
||||
.lua_host
|
||||
.lua()
|
||||
.load(r#"local d = pmacs.describe.key("C-c t"); return d and d.command"#)
|
||||
.eval()
|
||||
.expect("describe.key");
|
||||
assert_eq!(
|
||||
command.as_deref(),
|
||||
Some("terminal"),
|
||||
"C-c t must open a terminal"
|
||||
);
|
||||
}
|
||||
|
||||
/// Acceptance 12: with no settings written and no profiles registered, the
|
||||
/// defaults reproduce the pre-arc behavior.
|
||||
#[test]
|
||||
fn acc12_defaults_reproduce_prior_behavior() {
|
||||
let state = EditorState::new();
|
||||
let lua = state.lua_host.lua();
|
||||
assert_eq!(
|
||||
lua.load(r#"return pmacs.config.get("terminal.default-profile")"#)
|
||||
.eval::<String>()
|
||||
.unwrap(),
|
||||
""
|
||||
);
|
||||
assert_eq!(
|
||||
lua.load(r#"return pmacs.config.get("terminal.scrollback-rows")"#)
|
||||
.eval::<i64>()
|
||||
.unwrap(),
|
||||
10_000
|
||||
);
|
||||
assert_eq!(
|
||||
lua.load(r#"return pmacs.config.get("terminal.escape-key")"#)
|
||||
.eval::<String>()
|
||||
.unwrap(),
|
||||
"C-c"
|
||||
);
|
||||
assert!(
|
||||
lua.load("return next(pmacs.terminal.profiles) == nil")
|
||||
.eval::<bool>()
|
||||
.unwrap(),
|
||||
"no profiles are registered by default"
|
||||
);
|
||||
}
|
||||
Loading…
Reference in New Issue