fix(terminal): close review round 1 on copy mode
Four findings, all real, and they rhyme in pairs. Two implementation defects and two vacuous pins, all four tracing to one root: a name is not an identity, and a context-free readout is not a state observation. A foreign buffer carrying the snapshot's name was adopted and then overwritten. `pmacs.buffer.create` accepts any caller-chosen name and snapshot writes use bypass_intercept, so found-by-name adoption clobbered user data — the reviewer reproduced "do not clobber" becoming 23 newlines. Now follows dired's F7 rule: ownership means "in copy mode's own handle table", never "found by name", and a taken name yields a `<2>` variant. Snapshot identity was keyed by terminal NAME. `TerminalManager::open` uniquifies only the derived name — an explicit `name = ...` is inserted verbatim — so two valid terminals can share one, and a name-keyed table handed them a single snapshot: the second invocation retargeted it, `q` returned to the wrong terminal, and killing either removed the shared buffer. Identity is now the terminal buffer, compared in an array, because BufferIdLua implements `__eq` but each wrapper is a distinct table key: comparison works, hashing does not. The kill-with-terminal callback now closes over its own record rather than looking the name up again. The refresh pins were vacuous. Acceptance 19 compared a quiet terminal's snapshot against itself and 18 counted buffers, so both passed with render_snapshot replaced by a no-op. The child is `exec cat`, so the tests now type a marker into the focused terminal, require it ABSENT from the existing snapshot, and only then refresh — via `g` and via re-invocation respectively. The tail-follow pin could not observe view state. `TerminalManager::snapshot(buffer_id)` is context-free and always returns the live screen, so it reported "at the tail" even for a view forced to the oldest retained row. Now read through `snapshot_for_view`'s at_bottom and its projected cells. Adds acceptance 18a (a foreign same-named buffer is never adopted or clobbered) and 18b (two same-named terminals get two independent snapshots, each `q` returns to its own source, and killing one leaves the other's snapshot alive). Four new bites, all discriminating: restoring adopt-by-name fails 18a AND 18b; restoring name-keyed identity fails 18b; making render_snapshot a no-op fails BOTH 18 and 19, which is the vacuity demonstrated rather than argued; and forcing the view off the tail fails 20. Criterion 17 stays a named follow-up, per review agreement, until the real GPU probe is non-skipping and CI-executed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016gGQC6eqHJVbZJ5Hg7aLer
This commit is contained in:
parent
1b1e599070
commit
2eb6218ccd
|
|
@ -221,11 +221,56 @@ pmacs.keymap.bind { scope = "global", sequence = "C-c t", command = "terminal" }
|
|||
local raw_copy_retained = assert(terminal._copy_retained,
|
||||
"pmacs.terminal._copy_retained is required")
|
||||
|
||||
-- snapshot buffer name -> { terminal = <buf>, buffer = <buf> }
|
||||
-- An ARRAY of `{ terminal = <buf>, buffer = <buf> }`, scanned linearly and
|
||||
-- compared with `==`, following dired's handle table (F7).
|
||||
--
|
||||
-- Keyed by NAME, not by buffer handle: handles are not stable table keys,
|
||||
-- and a name survives the user killing the snapshot (listview precedent).
|
||||
local snapshots = {}
|
||||
-- Not `snapshots[name]`, and not `snapshots[buf]`, for two separate
|
||||
-- reasons — both of which were live defects in review round 1:
|
||||
--
|
||||
-- * **A terminal name is not a unique key.** `TerminalManager::open`
|
||||
-- uniquifies only the DERIVED name; an explicitly passed
|
||||
-- `name = "*same*"` is inserted verbatim
|
||||
-- (`src/terminal/session.rs`, `if spec.name.is_some()`). Two valid
|
||||
-- terminals can therefore share a name, and a name-keyed table gives
|
||||
-- them one snapshot between them: the second invocation silently
|
||||
-- retargets it, `q` returns to the wrong terminal, and killing either
|
||||
-- one removes the shared buffer.
|
||||
-- * **A buffer handle is not a stable table key.** `BufferIdLua`
|
||||
-- implements `__eq` but each wrapper is a distinct table key, so
|
||||
-- `snapshots[buf]` would miss on a freshly minted handle for the same
|
||||
-- buffer. Comparison works; hashing does not. Hence the scan.
|
||||
local handles = {}
|
||||
|
||||
-- Compact dead entries first, so a command in a killed snapshot sees
|
||||
-- "not in copy mode" rather than operating on dead state.
|
||||
local function live_handles()
|
||||
local live = {}
|
||||
for _, h in ipairs(handles) do
|
||||
local term_ok, term_valid = pcall(h.terminal.is_valid, h.terminal)
|
||||
local snap_ok, snap_valid = pcall(h.buffer.is_valid, h.buffer)
|
||||
if term_ok and term_valid and snap_ok and snap_valid then
|
||||
live[#live + 1] = h
|
||||
end
|
||||
end
|
||||
handles = live
|
||||
return live
|
||||
end
|
||||
|
||||
local function handle_for_terminal(term_buf)
|
||||
if term_buf == nil then return nil end
|
||||
for _, h in ipairs(live_handles()) do
|
||||
if h.terminal == term_buf then return h end
|
||||
end
|
||||
return nil
|
||||
end
|
||||
|
||||
local function handle_for_snapshot(buf)
|
||||
if buf == nil then return nil end
|
||||
for _, h in ipairs(live_handles()) do
|
||||
if h.buffer == buf then return h end
|
||||
end
|
||||
return nil
|
||||
end
|
||||
|
||||
local function buffer_name(buf)
|
||||
local ok, described = pcall(pmacs.describe.buffer, buf)
|
||||
|
|
@ -233,7 +278,7 @@ local function buffer_name(buf)
|
|||
return nil
|
||||
end
|
||||
|
||||
local function find_buffer_by_name(name)
|
||||
local function buffer_named(name)
|
||||
for _, id in ipairs(pmacs.buffer.list()) do
|
||||
local ok, described = pcall(pmacs.describe.buffer, id)
|
||||
if ok and described and described.name == name then return id end
|
||||
|
|
@ -244,11 +289,31 @@ end
|
|||
-- `*terminal:bash*` -> `*terminal-copy: terminal:bash*`. The surrounding
|
||||
-- asterisks are stripped before nesting so the result reads as one
|
||||
-- generated-buffer name rather than two.
|
||||
local function snapshot_name_for(term_buf)
|
||||
local function snapshot_base_name(term_buf)
|
||||
local name = buffer_name(term_buf) or "terminal"
|
||||
return string.format("*terminal-copy: %s*", (name:gsub("^%*", ""):gsub("%*$", "")))
|
||||
end
|
||||
|
||||
-- How far the `<2>`, `<3>`, ... disambiguation walks before giving up.
|
||||
local NAME_VARIANT_LIMIT = 99
|
||||
|
||||
-- `pmacs.buffer.create` takes any caller-chosen name, so a foreign buffer
|
||||
-- may already be called `*terminal-copy: sh*` — and two same-named
|
||||
-- terminals legitimately produce the same base name. Painting into a
|
||||
-- buffer we did not create would clobber a user's data through
|
||||
-- `bypass_intercept`, so **found-by-name is NOT adoption**: ownership
|
||||
-- means "this buffer is in the handle table above", exactly as in dired.
|
||||
local function unique_snapshot_name(term_buf)
|
||||
local name = snapshot_base_name(term_buf)
|
||||
if buffer_named(name) == nil then return name end
|
||||
for i = 2, NAME_VARIANT_LIMIT do
|
||||
local candidate = string.format("%s<%d>", name, i)
|
||||
if buffer_named(candidate) == nil then return candidate end
|
||||
end
|
||||
error(string.format(
|
||||
"terminal.copy-mode: %s is taken and no free variant remains", name), 0)
|
||||
end
|
||||
|
||||
-- Q#TC7: the snapshot text comes from the SAME serializer selection-copy
|
||||
-- uses, so soft wraps, wide glyphs, clusters and trailing blanks cannot
|
||||
-- drift between the two.
|
||||
|
|
@ -262,19 +327,17 @@ local function render_snapshot(record)
|
|||
if #text > 0 then buf:insert(0, text, { bypass_intercept = true }) end
|
||||
end
|
||||
|
||||
local function ensure_snapshot(term_buf)
|
||||
local name = snapshot_name_for(term_buf)
|
||||
local record = snapshots[name]
|
||||
if record and record.buffer:is_valid() then
|
||||
-- Q#TC8: re-invoking refreshes IN PLACE. Retarget the terminal too,
|
||||
-- in case a terminal buffer was recreated under the same name.
|
||||
record.terminal = term_buf
|
||||
return record
|
||||
end
|
||||
local function claim_snapshot(term_buf)
|
||||
-- Q#TC8: re-invoking against the same terminal refreshes IN PLACE.
|
||||
-- Identity is the terminal BUFFER, so two same-named terminals get two
|
||||
-- snapshots and neither can retarget the other's.
|
||||
local existing = handle_for_terminal(term_buf)
|
||||
if existing then return existing end
|
||||
|
||||
local buf = find_buffer_by_name(name) or pmacs.buffer.create(name)
|
||||
record = { terminal = term_buf, buffer = buf }
|
||||
snapshots[name] = record
|
||||
local name = unique_snapshot_name(term_buf)
|
||||
local buf = pmacs.buffer.create(name)
|
||||
local record = { terminal = term_buf, buffer = buf }
|
||||
handles[#handles + 1] = record
|
||||
|
||||
-- Q#TC6a — BOTH calls, and the second is the load-bearing one.
|
||||
--
|
||||
|
|
@ -298,9 +361,11 @@ local function ensure_snapshot(term_buf)
|
|||
pmacs.keymap.bind { scope = "buffer", buffer = buf,
|
||||
sequence = "q", command = "terminal.copy-quit" }
|
||||
|
||||
-- Q#TC8 lifecycle, both directions. Killing the terminal takes its
|
||||
-- snapshot with it; killing the snapshot alone leaves the terminal
|
||||
-- running and merely forgets the record, so a later invoke rebuilds.
|
||||
-- Q#TC8 lifecycle, both directions. Killing the terminal takes ITS
|
||||
-- snapshot with it — `record`, captured here, not "whatever is
|
||||
-- currently filed under this name"; killing the snapshot alone leaves
|
||||
-- the terminal running, and `live_handles` compacts the entry out so a
|
||||
-- later invoke rebuilds.
|
||||
--
|
||||
-- `on_removed` is sound here because every user-facing kill path
|
||||
-- routes through `pmacs.buffer.kill`, which fires the callbacks. The
|
||||
|
|
@ -310,14 +375,8 @@ local function ensure_snapshot(term_buf)
|
|||
-- alive, which is what makes reading back a finished command's output
|
||||
-- work at all.
|
||||
pcall(pmacs.buffer.on_removed, term_buf, function()
|
||||
local current = snapshots[name]
|
||||
if current and current.buffer:is_valid() then
|
||||
pcall(pmacs.buffer.kill, current.buffer)
|
||||
end
|
||||
snapshots[name] = nil
|
||||
end)
|
||||
pcall(pmacs.buffer.on_removed, buf, function()
|
||||
snapshots[name] = nil
|
||||
local ok, valid = pcall(record.buffer.is_valid, record.buffer)
|
||||
if ok and valid then pcall(pmacs.buffer.kill, record.buffer) end
|
||||
end)
|
||||
|
||||
return record
|
||||
|
|
@ -325,11 +384,7 @@ end
|
|||
|
||||
-- The snapshot record whose buffer the active window shows, or nil.
|
||||
local function snapshot_for_current_buffer()
|
||||
local buf = pmacs.window.buffer()
|
||||
if not buf then return nil end
|
||||
local name = buffer_name(buf)
|
||||
if not name then return nil end
|
||||
return snapshots[name]
|
||||
return handle_for_snapshot(pmacs.window.buffer())
|
||||
end
|
||||
|
||||
function terminal.copy_mode(term_buf)
|
||||
|
|
@ -338,7 +393,7 @@ function terminal.copy_mode(term_buf)
|
|||
if not terminal.is_terminal(term_buf) then
|
||||
error("terminal.copy-mode: the current buffer is not a terminal", 0)
|
||||
end
|
||||
local record = ensure_snapshot(term_buf)
|
||||
local record = claim_snapshot(term_buf)
|
||||
render_snapshot(record)
|
||||
pmacs.window.switch_buffer(record.buffer)
|
||||
return record.buffer
|
||||
|
|
|
|||
|
|
@ -435,6 +435,35 @@ If it does not, stop and repair the remote/fetch configuration.
|
|||
trailing newline); making re-invoke create a fresh buffer fails 18;
|
||||
dropping the kill-with-terminal teardown fails 18; removing the
|
||||
intercept fails 16b. Each failed exactly one test.
|
||||
- **Review round 1 — four findings, all real, and they rhyme in pairs.**
|
||||
Two P1 implementation defects and two P2 vacuous pins, all four tracing
|
||||
to one root: **a name is not an identity, and a context-free readout is
|
||||
not a state observation.**
|
||||
- *P1 — a foreign same-named buffer was adopted and clobbered.* Snapshot
|
||||
writes use `bypass_intercept`, so found-by-name adoption overwrote a
|
||||
user's buffer; the reviewer reproduced "do not clobber" becoming 23
|
||||
newlines. Fixed by dired's F7 rule: **ownership means "in our own
|
||||
handle table"**, and a taken name yields a `<2>` variant.
|
||||
- *P1 — snapshot identity was keyed by terminal NAME.*
|
||||
`TerminalManager::open` uniquifies only the *derived* name, so an
|
||||
explicit `name = "*same*"` lets two valid terminals share one; they
|
||||
then shared a snapshot, `q` returned to the wrong terminal, and
|
||||
killing either removed it. Now keyed by comparing buffer handles in an
|
||||
array — `BufferIdLua` implements `__eq` but each wrapper is a distinct
|
||||
table key, so **comparison works and hashing does not**.
|
||||
- *P2 — the refresh pins were vacuous.* 19 compared a quiet terminal's
|
||||
snapshot against itself and 18 counted buffers, so both passed with
|
||||
`render_snapshot` replaced by a no-op. Now the test types a marker
|
||||
into the `cat` child, requires it **absent** first, then refreshes.
|
||||
- *P2 — the tail-follow pin could not observe view state.*
|
||||
`manager.snapshot(buffer_id)` is context-free and always reads the
|
||||
live screen, so it reported "at the tail" for a view forced to the
|
||||
oldest retained row. Now read through `snapshot_for_view`'s
|
||||
`at_bottom` and projected cells.
|
||||
- **Four more bites, all discriminating.** Restoring adopt-by-name fails
|
||||
18a *and* 18b; restoring name-keyed identity fails 18b; making
|
||||
`render_snapshot` a no-op fails **both** 18 and 19 (the vacuity,
|
||||
demonstrated); and forcing the view off the tail fails 20.
|
||||
- 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`;
|
||||
|
|
|
|||
|
|
@ -5,12 +5,19 @@
|
|||
(`main` @ `cf54270`, 2026-07-26). Stage 2 implemented on branch
|
||||
`terminal-copy-mode` off `main` @ `cf54270`; no protocol change.**
|
||||
|
||||
**Stage 2 ships eight of its nine criteria.** Criterion 17's semantic-frontend
|
||||
end-to-end pin is deliberately absent — see the note under it — because a
|
||||
faithful version requires the real `pmacs-gpu` optimistic path, and therefore
|
||||
the `a37` foundation, which CI never compiles and which skips silently. Both
|
||||
halves of the *mechanism* it guards are pinned ungated instead (16, 16b). No
|
||||
other criterion is partial.
|
||||
**Stage 2 ships eight of its nine criteria, plus 18a and 18b added in review
|
||||
round 1.** Criterion 17's semantic-frontend end-to-end pin is deliberately
|
||||
absent — see the note under it — because a faithful version requires the real
|
||||
`pmacs-gpu` optimistic path, and therefore the `a37` foundation, which CI never
|
||||
compiles and which skips silently. Both halves of the *mechanism* it guards are
|
||||
pinned ungated instead (16, 16b). No other criterion is partial.
|
||||
|
||||
**Review round 1 found four defects, and the pair of them rhymes.** Two were
|
||||
implementation (18a's foreign-buffer clobber, 18b's name-keyed identity) and
|
||||
two were vacuous pins (18/19's refresh, 20's tail-follow) — and all four trace
|
||||
to the same root: **a name is not an identity, and a context-free readout is
|
||||
not a state observation.** The name mistake produced both P1s; the readout
|
||||
mistake produced both P2s.
|
||||
|
||||
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
|
||||
|
|
@ -608,11 +615,40 @@ additive, on its own binding, and does not replace scroll-and-select.
|
|||
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.
|
||||
|
||||
**The refresh half must be observed by CONTENT, not by buffer count**
|
||||
(review round 1). Counting buffers, or comparing a quiet terminal's
|
||||
snapshot against itself, passes with `render_snapshot` replaced by a
|
||||
no-op. The child is `exec cat`, so the test types a marker into the
|
||||
focused terminal, requires it **absent** from the existing snapshot, and
|
||||
only then re-invokes — the "advance the world" discipline.
|
||||
18a. **A foreign buffer carrying the snapshot's name is never adopted.**
|
||||
`pmacs.buffer.create` accepts any caller-chosen name, and snapshot writes
|
||||
use `bypass_intercept`, so found-by-name adoption silently overwrites a
|
||||
user's data — reproduced in review round 1 as "do not clobber" becoming
|
||||
23 newlines. Ownership means **"in copy mode's own handle table"**, which
|
||||
is dired's F7 rule; a taken name yields a `<2>` variant.
|
||||
18b. **Snapshot identity is the terminal BUFFER, not its name.**
|
||||
`TerminalManager::open` uniquifies only the *derived* name — an explicit
|
||||
`name = ...` is inserted verbatim — so two valid terminals can share one.
|
||||
A name-keyed table hands them a single snapshot: the second invocation
|
||||
retargets it, `q` returns to the wrong terminal, and killing either one
|
||||
removes the shared buffer. Keyed instead by comparing buffer handles in
|
||||
an array, because `BufferIdLua` implements `__eq` but each wrapper is a
|
||||
distinct table key — comparison works, hashing does not.
|
||||
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.
|
||||
|
||||
**Tail-following must be read through the registered VIEW.** Review
|
||||
round 1: `TerminalManager::snapshot(buffer_id)` is context-free and
|
||||
always returns the live screen, so it reports "at the tail" even for a
|
||||
view forced to the oldest retained row — falsified by doing exactly
|
||||
that and watching the assertion still pass. `snapshot_for_view`'s
|
||||
`at_bottom` plus its projected cells are the only observables that can
|
||||
tell the two apart.
|
||||
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
|
||||
|
|
|
|||
|
|
@ -123,16 +123,66 @@ fn open_fill_terminal(state: &mut EditorState) -> pmacs::buffer::BufferId {
|
|||
buffer
|
||||
}
|
||||
|
||||
fn viewport() -> CellSize {
|
||||
CellSize::new(10, 40)
|
||||
}
|
||||
|
||||
/// Give LOCAL a window on the terminal and register/claim its view, which
|
||||
/// is what makes `dispatch_key`'s terminal transport arm reachable.
|
||||
fn focus_terminal(state: &EditorState, buffer: pmacs::buffer::BufferId) {
|
||||
/// Returns the view key, so assertions can read the *projected* view
|
||||
/// rather than the context-free live screen.
|
||||
fn focus_terminal(state: &EditorState, buffer: pmacs::buffer::BufferId) -> TerminalViewKey {
|
||||
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, CellSize::new(10, 40));
|
||||
let _ = manager.snapshot_for_view(key, viewport());
|
||||
key
|
||||
}
|
||||
|
||||
/// Make the child produce NEW output, so a refresh has something to find.
|
||||
///
|
||||
/// The child is `exec cat`, so typing into the focused terminal echoes
|
||||
/// back. Without this, "refresh" tests compare a quiet terminal against
|
||||
/// itself and pass with the render replaced by a no-op — the defect review
|
||||
/// round 1 found in acceptance 18 and 19.
|
||||
fn emit_into_child(state: &mut EditorState, terminal: pmacs::buffer::BufferId, marker: &str) {
|
||||
focus_terminal(state, terminal);
|
||||
for ch in marker.chars() {
|
||||
press(state, KeyCode::Char(ch), KeyModifiers::NONE);
|
||||
}
|
||||
assert!(
|
||||
tick_until(state, marker, terminal),
|
||||
"the child must echo {marker:?} back onto the live screen"
|
||||
);
|
||||
}
|
||||
|
||||
/// What the registered VIEW currently projects — which, unlike
|
||||
/// `manager.snapshot(buffer)`, depends on where the view is anchored.
|
||||
fn view_text(state: &EditorState, key: TerminalViewKey) -> String {
|
||||
let mut manager = state.terminal_manager.borrow_mut();
|
||||
let Some(snapshot) = manager.snapshot_for_view(key, viewport()) else {
|
||||
return String::new();
|
||||
};
|
||||
let mut text = String::new();
|
||||
for cell in &snapshot.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 view_at_bottom(state: &EditorState, key: TerminalViewKey) -> bool {
|
||||
state
|
||||
.terminal_manager
|
||||
.borrow_mut()
|
||||
.snapshot_for_view(key, viewport())
|
||||
.is_some_and(|snapshot| snapshot.at_bottom)
|
||||
}
|
||||
|
||||
fn buffer_text_by_name(state: &EditorState, name: &str) -> Option<String> {
|
||||
|
|
@ -336,13 +386,30 @@ fn acc18_reinvoke_refreshes_in_place_and_lifecycle_runs_both_ways() {
|
|||
|
||||
exec(&state, "pmacs.terminal.copy_mode(TERM_BUF)");
|
||||
let count_after_first = buffer_count(&state);
|
||||
assert!(
|
||||
!buffer_text_by_name(&state, SNAPSHOT_NAME)
|
||||
.expect("snapshot")
|
||||
.contains("REINVOKE"),
|
||||
"precondition: the marker has not been emitted yet"
|
||||
);
|
||||
|
||||
// Advance the world, then re-invoke. Counting buffers alone is
|
||||
// vacuous: it passes with the render replaced by a no-op, so the
|
||||
// refresh must be observed by CONTENT that only exists after the
|
||||
// first snapshot was taken.
|
||||
emit_into_child(&mut state, terminal, "REINVOKE");
|
||||
exec(&state, "pmacs.terminal.copy_mode(TERM_BUF)");
|
||||
assert!(
|
||||
buffer_text_by_name(&state, SNAPSHOT_NAME)
|
||||
.expect("snapshot")
|
||||
.contains("REINVOKE"),
|
||||
"re-invoking must actually re-serialize, not just reuse the buffer"
|
||||
);
|
||||
exec(&state, "pmacs.terminal.copy_mode(TERM_BUF)");
|
||||
assert_eq!(
|
||||
buffer_count(&state),
|
||||
count_after_first,
|
||||
"re-invoking must refresh in place, not accumulate buffers"
|
||||
"...and it must refresh IN PLACE, not accumulate buffers"
|
||||
);
|
||||
|
||||
// Killing the snapshot alone leaves the terminal running.
|
||||
|
|
@ -402,17 +469,6 @@ fn acc19_escape_c_t_enters_copy_mode_and_g_and_q_work() {
|
|||
"C-c C-t must enter copy mode"
|
||||
);
|
||||
|
||||
// `g` re-snapshots in place.
|
||||
let before = buffer_text_by_name(&state, SNAPSHOT_NAME).expect("snapshot");
|
||||
press(&mut state, KeyCode::Char('g'), KeyModifiers::NONE);
|
||||
let after = buffer_text_by_name(&state, SNAPSHOT_NAME).expect("snapshot");
|
||||
assert_eq!(before, after, "a quiet terminal re-snapshots identically");
|
||||
assert_eq!(
|
||||
active_buffer_name(&state),
|
||||
SNAPSHOT_NAME,
|
||||
"g must not move us"
|
||||
);
|
||||
|
||||
// `q` returns to the source terminal.
|
||||
press(&mut state, KeyCode::Char('q'), KeyModifiers::NONE);
|
||||
assert_eq!(
|
||||
|
|
@ -420,6 +476,45 @@ fn acc19_escape_c_t_enters_copy_mode_and_g_and_q_work() {
|
|||
terminal_name,
|
||||
"q must return to the terminal the snapshot was taken from"
|
||||
);
|
||||
|
||||
// Now advance the world and come back WITHOUT re-invoking copy mode,
|
||||
// so the snapshot is genuinely stale. Comparing a quiet terminal's
|
||||
// snapshot against itself is vacuous — it passes with `render_snapshot`
|
||||
// replaced by a no-op.
|
||||
emit_into_child(&mut state, terminal, "AFTER-G");
|
||||
exec(
|
||||
&state,
|
||||
&format!(
|
||||
r"
|
||||
for _, id in ipairs(pmacs.buffer.list()) do
|
||||
local ok, d = pcall(pmacs.describe.buffer, id)
|
||||
if ok and d and d.name == {SNAPSHOT_NAME:?} then
|
||||
pmacs.window.switch_buffer(id)
|
||||
end
|
||||
end
|
||||
"
|
||||
),
|
||||
);
|
||||
assert!(
|
||||
!buffer_text_by_name(&state, SNAPSHOT_NAME)
|
||||
.expect("snapshot")
|
||||
.contains("AFTER-G"),
|
||||
"the snapshot must still be stale before `g` — otherwise the next \
|
||||
assertion proves nothing"
|
||||
);
|
||||
|
||||
press(&mut state, KeyCode::Char('g'), KeyModifiers::NONE);
|
||||
assert!(
|
||||
buffer_text_by_name(&state, SNAPSHOT_NAME)
|
||||
.expect("snapshot")
|
||||
.contains("AFTER-G"),
|
||||
"`g` must re-snapshot from the live terminal"
|
||||
);
|
||||
assert_eq!(
|
||||
active_buffer_name(&state),
|
||||
SNAPSHOT_NAME,
|
||||
"g must not move us"
|
||||
);
|
||||
state.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
||||
|
|
@ -430,7 +525,7 @@ fn acc19_escape_c_t_enters_copy_mode_and_g_and_q_work() {
|
|||
fn acc20_live_terminal_keys_are_unchanged_while_a_snapshot_exists() {
|
||||
let mut state = EditorState::new();
|
||||
let terminal = open_fill_terminal(&mut state);
|
||||
focus_terminal(&state, terminal);
|
||||
let key = focus_terminal(&state, terminal);
|
||||
exec(&state, "pmacs.terminal.copy_mode(TERM_BUF)");
|
||||
|
||||
// Back to the terminal; its five live bindings must still resolve.
|
||||
|
|
@ -453,11 +548,25 @@ fn acc20_live_terminal_keys_are_unchanged_while_a_snapshot_exists() {
|
|||
);
|
||||
}
|
||||
|
||||
// The terminal is still following its tail: the child's last output is
|
||||
// visible without scrolling.
|
||||
// The terminal still FOLLOWS ITS TAIL while a snapshot exists.
|
||||
//
|
||||
// Read through the registered view, not `manager.snapshot(buffer)`:
|
||||
// that call is context-free and always returns the live screen, so it
|
||||
// reports "at the tail" even for a view forced to the oldest retained
|
||||
// row. The projected view is the only thing that can distinguish them.
|
||||
assert!(
|
||||
screen_text(&state, terminal).contains("DONE"),
|
||||
"the live terminal keeps following its tail"
|
||||
view_at_bottom(&state, key),
|
||||
"precondition: the view starts at the tail"
|
||||
);
|
||||
emit_into_child(&mut state, terminal, "TAILMARK");
|
||||
assert!(
|
||||
view_at_bottom(&state, key),
|
||||
"new child output must not knock the view off the tail"
|
||||
);
|
||||
assert!(
|
||||
view_text(&state, key).contains("TAILMARK"),
|
||||
"the freshest output must be visible in the PROJECTED view: {:?}",
|
||||
view_text(&state, key)
|
||||
);
|
||||
state.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
|
@ -503,6 +612,149 @@ fn acc21_describe_key_reports_the_truth_for_the_snapshot_bindings() {
|
|||
state.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
||||
/// Acceptance 18a (review round 1, P1): a foreign buffer that happens to
|
||||
/// carry the snapshot's name is **never adopted**.
|
||||
///
|
||||
/// `pmacs.buffer.create` takes any caller-chosen name, and snapshot writes
|
||||
/// use `bypass_intercept`, so found-by-name adoption clobbers a user's
|
||||
/// data outright. Ownership means "in copy mode's own handle table"
|
||||
/// (dired's F7 rule); a taken name gets a `<2>` variant instead.
|
||||
#[test]
|
||||
fn acc18a_a_foreign_same_named_buffer_is_never_adopted_or_clobbered() {
|
||||
let mut state = EditorState::new();
|
||||
let terminal = open_fill_terminal(&mut state);
|
||||
focus_terminal(&state, terminal);
|
||||
|
||||
// A user's buffer, sitting exactly where the snapshot wants to go.
|
||||
exec(
|
||||
&state,
|
||||
&format!(
|
||||
r"
|
||||
FOREIGN = pmacs.buffer.create({SNAPSHOT_NAME:?})
|
||||
FOREIGN:insert(0, 'do not clobber')
|
||||
"
|
||||
),
|
||||
);
|
||||
|
||||
exec(&state, "pmacs.terminal.copy_mode(TERM_BUF)");
|
||||
|
||||
let foreign_text: String = eval(&state, r"return FOREIGN:slice(0, FOREIGN:len())");
|
||||
assert_eq!(
|
||||
foreign_text, "do not clobber",
|
||||
"the foreign buffer must be untouched"
|
||||
);
|
||||
assert_ne!(
|
||||
active_buffer_name(&state),
|
||||
SNAPSHOT_NAME,
|
||||
"copy mode must not display the foreign buffer"
|
||||
);
|
||||
assert_eq!(
|
||||
active_buffer_name(&state),
|
||||
format!("{SNAPSHOT_NAME}<2>"),
|
||||
"a taken name must yield a unique variant"
|
||||
);
|
||||
assert!(
|
||||
buffer_text_by_name(&state, &format!("{SNAPSHOT_NAME}<2>"))
|
||||
.expect("variant snapshot")
|
||||
.contains("LINE200"),
|
||||
"the variant is the real snapshot"
|
||||
);
|
||||
state.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
||||
/// Acceptance 18b (review round 1, P1): snapshot identity is the terminal
|
||||
/// BUFFER, not its name.
|
||||
///
|
||||
/// `TerminalManager::open` uniquifies only the *derived* name — an
|
||||
/// explicit `name = ...` is inserted verbatim — so two valid terminals can
|
||||
/// share a name. Keying snapshots by name gives them one buffer between
|
||||
/// them: the second invocation retargets it, `q` returns to the wrong
|
||||
/// terminal, and killing either one removes the shared snapshot.
|
||||
#[test]
|
||||
fn acc18b_two_same_named_terminals_get_two_independent_snapshots() {
|
||||
let mut state = EditorState::new();
|
||||
exec(&state, FILL_PROFILE);
|
||||
|
||||
let before = terminal_buffers(&state);
|
||||
exec(
|
||||
&state,
|
||||
r#"TERM_A = pmacs.terminal.open { profile = "fill", name = "*same*" }"#,
|
||||
);
|
||||
exec(
|
||||
&state,
|
||||
r#"TERM_B = pmacs.terminal.open { profile = "fill", name = "*same*" }"#,
|
||||
);
|
||||
let fresh: Vec<_> = terminal_buffers(&state)
|
||||
.into_iter()
|
||||
.filter(|id| !before.contains(id))
|
||||
.collect();
|
||||
assert_eq!(fresh.len(), 2, "two terminals opened under one name");
|
||||
|
||||
// Distinguish them by content, since their names are identical.
|
||||
emit_into_child(&mut state, fresh[0], "AAAA");
|
||||
emit_into_child(&mut state, fresh[1], "BBBB");
|
||||
|
||||
focus_terminal(&state, fresh[0]);
|
||||
let snap_a: String = eval(
|
||||
&state,
|
||||
r"local b = pmacs.terminal.copy_mode(TERM_A); return (pmacs.describe.buffer(b)).name",
|
||||
);
|
||||
focus_terminal(&state, fresh[1]);
|
||||
let snap_b: String = eval(
|
||||
&state,
|
||||
r"local b = pmacs.terminal.copy_mode(TERM_B); return (pmacs.describe.buffer(b)).name",
|
||||
);
|
||||
|
||||
assert_ne!(
|
||||
snap_a, snap_b,
|
||||
"two terminals must not share one snapshot buffer"
|
||||
);
|
||||
let text_a = buffer_text_by_name(&state, &snap_a).expect("snapshot A");
|
||||
let text_b = buffer_text_by_name(&state, &snap_b).expect("snapshot B");
|
||||
assert!(
|
||||
text_a.contains("AAAA") && !text_a.contains("BBBB"),
|
||||
"snapshot A must hold only A's output: {:?}",
|
||||
&text_a[text_a.len().saturating_sub(60)..]
|
||||
);
|
||||
assert!(
|
||||
text_b.contains("BBBB") && !text_b.contains("AAAA"),
|
||||
"snapshot B must hold only B's output"
|
||||
);
|
||||
|
||||
// `q` from each snapshot returns to ITS OWN terminal, which is only
|
||||
// observable through the buffer id — the two names are the same.
|
||||
exec(
|
||||
&state,
|
||||
&format!(
|
||||
r"
|
||||
for _, id in ipairs(pmacs.buffer.list()) do
|
||||
local ok, d = pcall(pmacs.describe.buffer, id)
|
||||
if ok and d and d.name == {snap_b:?} then pmacs.window.switch_buffer(id) end
|
||||
end
|
||||
"
|
||||
),
|
||||
);
|
||||
press(&mut state, KeyCode::Char('q'), KeyModifiers::NONE);
|
||||
let returned_is_b: bool = eval(&state, r"return pmacs.window.buffer() == TERM_B");
|
||||
assert!(
|
||||
returned_is_b,
|
||||
"q from B's snapshot must return to terminal B"
|
||||
);
|
||||
|
||||
// Killing terminal A removes only A's snapshot.
|
||||
exec(&state, "pmacs.terminal.terminate(TERM_A)");
|
||||
exec(&state, "pmacs.buffer.kill(TERM_A)");
|
||||
assert!(
|
||||
buffer_text_by_name(&state, &snap_a).is_none(),
|
||||
"A's snapshot dies with A"
|
||||
);
|
||||
assert!(
|
||||
buffer_text_by_name(&state, &snap_b).is_some(),
|
||||
"B's snapshot must SURVIVE — a shared buffer would have gone too"
|
||||
);
|
||||
state.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
||||
/// Copy mode refuses a non-terminal buffer rather than producing an empty
|
||||
/// snapshot of nothing.
|
||||
#[test]
|
||||
|
|
|
|||
Loading…
Reference in New Issue