From 8cd1da41acd5fe0eec0bde78b63711cf399768ef Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Fri, 7 Aug 2026 19:37:46 +0200 Subject: [PATCH] =?UTF-8?q?test:=20the=20report=20itself,=20at=20a=20real?= =?UTF-8?q?=20PTY=20=E2=80=94=20and=20=C2=A71.1=20was=20wrong?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes Stage 3's remaining obligations. THE PTY TEST. Every other test in this lane checks a mechanism. tests/long_line_readable_acceptance.rs checks the complaint: the shipped binary, a real 80x24 PTY, one source line 200 columns wide, and an assertion that its tail marker reaches the host. It bites — `scripts/bite HEAD~2 src/editor.rs --test long_line_readable_acceptance` against the pre-aa3cd4d editor never paints TAILZQX in 20s. Its truncate control (an isolated init.lua pinning the mode) is what makes that marker discriminating, and it is also the honest statement of what truncate costs today: those bytes are not off-screen, they are unreachable until Stage 4. What it does not prove: the workspace has no screen model and no vt100/termwiz/vte, so this shows the tail was WRITTEN to the terminal, not which row a human would point at. That is nonetheless the whole of the report — under truncation the bytes are never emitted at all. §1.1 WAS WRONG, FOR NINETEEN REVISIONS. `editing.fill-column` is not an orphaned registry setting "of the exact shape Stage 1 just fixed". Both cited occurrences are inside `#[cfg(test)] mod tests` — fixture names in round-trip tests covering one setting per ConfigKind. Two of those five names are real; three, including this one, are defined nowhere else in the tree. There is no shipped setting, so the Q#LL4 deliverable "sharpen its description" had no object. The mechanism is worth more than the correction. A grep hit at a src/ path, a genuine `r.define(...)` call that is real API usage rather than a mock, and `#[cfg(test)]` about fifty lines above the citation. Every later revision inherited the conclusion instead of the evidence, and three review rounds reasoned about the consequences of an orphaned setting rather than re-checking that it existed. A file:line citation is not a substitute for reading the scope it sits in. Had it gone unchecked into implementation, Stage 3 would have shipped an edit to a unit-test fixture believing it was rewording a user-visible setting — a no-op with a misleading commit message. §1.1 is withdrawn in place, keeping the original text and the reasoning that produced it; §6's answer is unchanged (a setting that does not exist is a stronger reason not to adopt it) and its premise corrected. Both fixture sites now say they are fixtures. The approval is not reopened: nothing else in the document rested on §1.1, which argued for a display setting separate from fill-column — which is what shipped. AND ONE UNCLASSIFIABLE RED, logged as U1 in ci-red-signatures.md. A `-p pmacs-gpu` run went 227/1 once; every run since is 228/0. The failing test name was NOT captured, because I piped that command through `tail -3` and discarded the failure block above the summary. 36 later runs are clean, 6 under deliberate concurrent load — which per the rerun rule establishes intermittence only, and without a selector not even that. Deliberately NOT matched against A1 despite A1 also being GPU-headless-under-load: matching requires an exact selector and every required fragment, and calling a shapeless red "probably the known one" is the reputation-by-adjacency that file exists to deny. Gates: fmt; clippy --workspace --all-targets -D warnings; --lib 1917/0; --lib --features crdt 2102/0; line_wrap 6/0; long_line_readable 2/0; folding 21/0; folding_stage2 48/0; full_grid_resync 1/0; config_registry 16/0; m4 150/0; PMACS_REQUIRE_GPU=1 -p pmacs-gpu 228/0 (see U1); git diff --check. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/active-work.md | 129 +++++++++++++++--- docs/ci-red-signatures.md | 30 +++++ docs/long-lines-framing.md | 176 ++++++++++++++++++------- src/config_registry.rs | 10 ++ src/lua_bindings/config.rs | 6 + tests/long_line_readable_acceptance.rs | 155 ++++++++++++++++++++++ 6 files changed, 438 insertions(+), 68 deletions(-) create mode 100644 tests/long_line_readable_acceptance.rs diff --git a/docs/active-work.md b/docs/active-work.md index 71ec691..e29f6f9 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -434,7 +434,7 @@ a job executed — **the log is**. Whether `docs/ci-red-signatures.md` should grow a short non-row section for this class is an open question for its owner, not something this lane decided. -## Long lines (QoL Stage 3) — APPROVED, implementing; no PR yet +## Long lines (QoL Stage 3) — IMPLEMENTED, gates green; PR not yet open **Branch `long-lines`**, originally based on `githubsucks/main` @ `218d2e7` (the #219 merge); **main merged in at #220's landing**, so @@ -447,12 +447,13 @@ records. Recover: `git fetch githubsucks && git checkout long-lines`. **This block was written with the lane's first commit, before any PR exists** — the standing correction from #171, #215 and #220. -- **Framing `docs/long-lines-framing.md` revision 19, APPROVED.** Q#LL1-LL7 - settled; **Q#LL8 not yet approved** --- implementation is blocked on - it, because it changes what gets built rather than how. -- **Independent of #220.** Stage 2 (GUI zoom) and Stage 3 share no - code, so this branch does not wait on that merge — which matters, - because #220 is blocked on a GitHub Actions outage, not on itself. +- **Framing `docs/long-lines-framing.md` revision 20, APPROVED.** All + eight questions settled, §5d.6 resolved. Revision 20 **withdraws + §1.1** — see "The framing error" below. +- **Independent of #220** (now merged). Stage 2 and Stage 3 share no + code; the merge in this branch is currency, not dependency. +- **Implementation complete.** Seven commits, `937544c..840a338`. + Every gate green locally, macOS unverifiable here as always. ### The defect @@ -472,9 +473,9 @@ for the user to express a preference in either. - **Q#LL2** — the mode is **buffer-local**; `Viewport` carries the *resolved* mode as it already carries `folds`, so `TextView` stays config-agnostic. -- **Q#LL4** — do **not** adopt `editing.fill-column`; it is orphaned - because its consumer (`M-q` / auto-fill) does not exist. Sharpen its - description, and name ours `ui.line-wrap` (`ConfigKind::Enum`). +- **Q#LL4** — do **not** adopt `editing.fill-column`; name ours + `ui.line-wrap` (`ConfigKind::Enum`). *(The recorded reason was wrong; + the answer was not. See "The framing error".)* - **Q#LL5** — **character wrap in both frontends**; the GPU document buffer gets its first explicit `set_wrap`, `Wrap::Glyph`. - **Q#LL6** — no global map (every vertical consumer is local, so @@ -534,6 +535,60 @@ for the user to express a preference in either. **No wire message, no version bump**, and the one-copy-fixed defect becomes unrepresentable rather than reviewer-guarded. +### The framing error, and the four defects review found in the code + +**`editing.fill-column` does not exist.** Framing §1.1 called it an +orphaned registry setting "of the exact shape Stage 1 just fixed" and +carried a deliverable to sharpen its description. Both cited +occurrences are inside `#[cfg(test)] mod tests` — +`src/config_registry.rs` and `src/lua_bindings/config.rs` — fixture +names in round-trip tests covering one setting per `ConfigKind`. Two of +those five names are real; three, including this one, are defined +nowhere else. + +Nineteen revisions and three review rounds inherited it. The mechanism +is worth keeping: a grep hit at a `src/` path, a genuine `r.define(...)` +call that is real API usage rather than a mock, and `#[cfg(test)]` about +fifty lines above the citation. **A file:line citation is not a +substitute for reading the scope it sits in** — and once a conclusion is +in a document, later rounds reason about its consequences rather than +re-check it. Cost: three paragraphs of framing and one carried +deliverable that had no object. Stage 3 ships a comment at both fixture +sites instead. + +The four code defects, each caught by user review or by the tests +written for it: + +1. **The toggle wrote the global layer.** `ui.line-wrap` is + buffer-local, so `config.get(name)`/`config.set(name, ...)` reported + and flipped the *global* value — leaving a pinned buffer untouched + while silently moving every unpinned one. Invisible in any + single-buffer test. Fixed in `4a26f00`; witnesses use two buffers. +2. **The renderer never got the mode.** The frame resolved + `ui.line-wrap`, stored it on the window, and fed it to coordinate + mapping and the indicator — while the `Viewport` literal still + carried a hard-coded `Truncate`. Every "is the mode right?" test + passed and the text stayed clipped. Fixed in `aa3cd4d`; the new + witnesses read the **grid**, not the resolved value. +3. **The GPU indicator reckoned in source lines.** Pre-existing (the + GPU has always wrapped), but nameable only once `ui.line-wrap` + decided which formula applies. Fixed in `840a338` via + `code_byte_painted` — layout decides, not arithmetic over it. The + two cheap predicates are both wrong: `view_range` includes + `SCROLL_OVERSCAN`, `scroll_top` ignores the sub-line residual. +4. **An empty `view_range` is not an empty layout.** Found *by* the + tests in 3, not confirmed by them: a file ending in a newline has a + final empty line, and a viewport parked on it is `(len, len)` with + one real row. The guard borrowed from the caret path made **every + newline-terminated file report a percentage instead of `Bot`** at + the bottom. + +**A process note that earned itself twice.** Two edits in this lane +were silently lost to `str.replace` calls that matched nothing (once +after an unrelated exception aborted the write). Both times the code +looked edited and was not — defect 2 above is one of them. Use the +Edit tool, which errors on mismatch, for anything load-bearing. + ### The two decisions most likely to be questioned later - **GUI users lose word wrap.** Character-wrap parity is cheap and @@ -547,17 +602,51 @@ for the user to express a preference in either. findable. Compiler-enforced where possible, correct-by-default where not. -### Verification (planned, from framing §7) +### Verification (delivered) -Cell-level tests at several **window widths** (not offsets — offsets -are Stage 4). Wrapped visual-row mapping with identity on cursor -boundaries and projection elsewhere. The **wrap-point** case: the -position at a soft break belongs to `(k+1, 0)`, while a **hard** line -end filling the row exactly keeps `(k, max_cols)` — a control that the -wrap work did not move it. A `truncate` control for every mapping -claim. A GPU witness that `truncate` is *honored*, since the existing -`wrapped_caret_survives_size_changes` passes against a wrap nobody -configured. +Everything framing §7 sketched, plus three groups it did not anticipate +(framing §7.1). The exact gate run, all green on this machine: + +``` +cargo fmt --check +cargo clippy --workspace --all-targets -- -D warnings +cargo test --lib # 1917 / 0 +cargo test --lib --features crdt # 2102 / 0 +cargo test --test line_wrap_acceptance # 6 / 0 +cargo test --test long_line_readable_acceptance # 2 / 0 +cargo test --test folding_acceptance # 21 / 0 +cargo test --test full_grid_resync_acceptance # 1 / 0 +cargo test --test m4_acceptance -- --skip basedpyright +PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu # 228 / 0 +git diff --check +``` + +`tests/long_line_readable_acceptance.rs` is the one that answers the +**report** rather than a mechanism: the shipped binary, a real PTY, a +line 200 columns wide in an 80-column terminal, and an assertion that +the tail marker reaches the host. It bites — +`scripts/bite HEAD~2 src/editor.rs --test long_line_readable_acceptance` +against the pre-`aa3cd4d` editor never paints `TAILZQX` in 20s. Its +`truncate` control (an isolated `init.lua` pinning the mode) is what +makes that marker discriminating. + +**What the PTY test does not prove.** The vterm suites assert on raw +output bytes; there is no screen model and no `vt100`/`termwiz`/`vte` +in the workspace. It proves the tail was *written to the terminal*, not +that it occupies the row a human would point at. That is nonetheless +the whole of the original report — under truncation those bytes are +never emitted at all. + +**One caveat on the GPU line, stated rather than smoothed over.** One +`-p pmacs-gpu` run went `227 passed; 1 failed` before every run since +went 228/0. **The failing test name was not captured** — that command +was piped through `tail -3`, which kept the summary and discarded the +failure block. 36 later full runs are clean, 6 under deliberate +concurrent load. Per `docs/ci-red-signatures.md`'s rerun rule that +establishes intermittence only, and without a selector it does not even +establish that. Logged there as **U1**, explicitly *not* matched +against A1 (also GPU-headless-under-load) because matching requires a +selector and fragments this occurrence does not have. ### Not in scope diff --git a/docs/ci-red-signatures.md b/docs/ci-red-signatures.md index 66e9949..07917f3 100644 --- a/docs/ci-red-signatures.md +++ b/docs/ci-red-signatures.md @@ -393,6 +393,36 @@ process test contributed two, and only one of them is a test bug. |---|---|---| | 2026-08-05 | R2, R4 | **retired** — mechanism removed, discriminating witness added; see "Retired rows" | +### U1 — an unclassifiable local red (long-lines lane, 2026-08-07) + +Recorded because the alternative is to not record it. It is **not** a +row, cannot be matched, and excuses nothing. + +| field | value | +|---|---| +| **selector** | `PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu` — **test name not captured** | +| **job / flavor** | local (Linux), immediately after a 60s `m4_acceptance` run | +| **required fragments** | **none captured** | +| **status** | **unclassifiable — evidence destroyed at capture time** | +| **what IS established** | `test result: FAILED. 227 passed; 1 failed` was emitted once | +| **what is NOT** | which test, why, and whether the lane caused it | +| **cause of the gap** | the command piped through `tail -3`, which kept the summary line and discarded the failure block above it | + +**Not matched against A1** despite A1 also being GPU-headless-under-load. +Matching needs an exact selector and every required fragment; this has +neither, and treating a shapeless red as "probably the known one" is +precisely the reputation-by-adjacency this file exists to deny. + +Follow-up: 36 subsequent full runs clean, 6 of them under deliberate +concurrent load (`m4_acceptance` in parallel). Per the rerun rule that +establishes **intermittence only** — and here not even that, since +without a name there is nothing to call intermittent. + +**The lesson is mechanical, not analytical: never pipe a gate through +`tail`/`head` on the run whose result you intend to report.** Filter +with `grep -E "FAILED|panicked|test result"`, which keeps failure +context, or capture the full log to a file and summarize from it. + **The retirements are not occurrences and do not close the log.** R1 and R3 stay live, and each retired row keeps its signature so a later red matching one reopens it. diff --git a/docs/long-lines-framing.md b/docs/long-lines-framing.md index 22aabb5..fe6a13c 100644 --- a/docs/long-lines-framing.md +++ b/docs/long-lines-framing.md @@ -1,7 +1,7 @@ # Long lines — QoL Stage 3 -**Status: revision 19 — APPROVED (2026-08-06). All eight questions -answered; §5d.6 resolved. Implementation begun.** +**Status: revision 20 — APPROVED (2026-08-06). All eight questions +answered; §5d.6 resolved. Implementation complete; PR pending.** **Revision 2** corrected a load-bearing error in revision 1: it claimed both frontends render from the same `CellGrid`. They do not — the GPU @@ -182,11 +182,12 @@ and moves the document to APPROVED. **Q#LL2** (§4): buffer-local, with `Viewport` carrying the *resolved* mode as it already carries `folds`, so `TextView` stays -config-agnostic. **Q#LL4** (§6): do not adopt `editing.fill-column` — -it is orphaned because its consumer (`M-q` / auto-fill) does not exist, -which is a different defect from `full_grid`'s and §1.1 should not be -read as equating them; sharpen its description and name ours -`ui.line-wrap`. **Q#LL5** (§5a): character wrap in **both** frontends, +config-agnostic. **Q#LL4** (§6): do not adopt `editing.fill-column`; +name ours `ui.line-wrap`. *(Revision 20 corrects the stated reason: +the setting is a `#[cfg(test)]` fixture name, not an orphaned +definition — see §1.1, withdrawn. The answer is unchanged, the +"sharpen its description" deliverable is void, and naming the fixtures +as fixtures replaces it.)* **Q#LL5** (§5a): character wrap in **both** frontends, accepting that GUI users lose word wrap — the analysis changed on discovering that a whitespace-based grid wrap would give only *approximate* parity against cosmic-text's UAX #14 line breaking, which @@ -302,7 +303,21 @@ condition that produced §5d.3's defect; sharing it via `pmacs-protocol` makes agreement structural but widens that crate toward presentation — a §16 layering call I am not taking alone. -**Revision 19 — the current one.** §5d.6 answered as **(b)**: +**Revision 20 — the current one.** Written *during* implementation +rather than before it, because that is when the error surfaced: §1.1's +"orphaned setting" does not exist. `editing.fill-column` is a +`#[cfg(test)]` fixture name at both cited sites, so the Q#LL4 +deliverable "sharpen its description" had no object. §1.1 is withdrawn +in place with the original text and the reasoning that produced it; +§6's answer is unchanged and its premise corrected. + +The approval is not reopened by this. Q#LL4's answer was *do not adopt +it*, and a setting that turns out not to exist is a stronger reason not +to adopt it than the one recorded. Nothing else in the document +depended on §1.1 — it argued for a display setting separate from +`fill-column`, which is what shipped. + +**Revision 19.** §5d.6 answered as **(b)**: `ScrollPosition` and `classify` go in `pmacs-protocol`, string rendering stays per-frontend. The §16 reading that decides it — each frontend computes its own local layout facts, the shared crate owns the @@ -382,25 +397,47 @@ What does **not** exist: The walk always starts at the line's first character. There is no mode, no flag, and no caller that can ask for anything else. -### 1.1 An orphaned setting, of the exact shape Stage 1 just fixed +### 1.1 ~~An orphaned setting, of the exact shape Stage 1 just fixed~~ **WITHDRAWN (revision 20)** -`editing.fill-column` is defined in the registry --- *"Preferred wrap -column."*, `ConfigKind::Number`, min 1, max 1000 -(`src/config_registry.rs:1191`) --- and is **read by nothing**. It -appears only in its own definition and in tests of the registry and the -Lua binding. +> **This section was wrong, and the error survived nineteen revisions +> and three rounds of review.** It is kept rather than deleted because +> the failure mode is reusable: a grep hit that *reads* like production +> code. +> +> `editing.fill-column` is **not defined in the registry**. Both +> occurrences are inside `#[cfg(test)] mod tests` --- +> `src/config_registry.rs:1191` and `src/lua_bindings/config.rs:898` +> --- where they are fixture names in round-trip tests that exercise +> one setting per `ConfigKind`. Two of the five names in those fixtures +> are real (`editing.auto-pair`, `autosave.interval-ms`); the other +> three, including this one, are defined nowhere else in the tree. +> +> So there is no shipped setting. No user can get it, set it, or +> discover it, and **there is no description to sharpen** --- which was +> the one Q#LL4 deliverable this document still owed. What Stage 3 does +> instead is name the fixtures as fixtures at both sites, so the next +> reader is not sent down the same path. +> +> The original claim, for the record: *"`editing.fill-column` is +> defined in the registry --- 'Preferred wrap column.', +> `ConfigKind::Number`, min 1, max 1000 --- and is read by nothing. +> This is the same shape as the `full_grid` defect Stage 1 closed."* +> +> **How it happened.** I grepped for prior art on line width, found the +> name at a `src/` path, read the `r.define(...)` call --- which is +> genuine API usage, not a mock --- and never checked the enclosing +> module. `#[cfg(test)]` sat about fifty lines above. Every later +> revision inherited the conclusion instead of the evidence, and each +> round of review reasoned about the *consequences* of an orphaned +> setting rather than re-checking that it existed. A file:line citation +> is not a substitute for reading the scope it sits in. -This is the same shape as the `full_grid` defect Stage 1 closed: a -declared contract with full definition-side coverage and zero -consumers. **Stage 3 must either give it a consumer or explicitly say -why it does not deserve one.** Leaving it orphaned a second time, in -the very lane about line width, would be the worse outcome. - -My reading: `fill-column` is a *fill* concept (where `M-q` reflows -text, editing the buffer), not a *display wrap* concept (where a long -line is shown across rows, buffer unchanged). Conflating them is a -known Emacs papercut. That argues for a separate display setting and a -note here --- but it is Q#LL4 below, not my call. +The surviving observation, which needed none of the above: `fill-column` +is a *fill* concept (where `M-q` reflows text, editing the buffer), not +a *display wrap* concept (where a long line is shown across rows, buffer +unchanged). Conflating them is a known Emacs papercut, and it is why +this lane's setting is `ui.line-wrap` rather than a reuse of that name +--- see Q#LL4 (§6). ### 1.2 The two frontends already disagree, and that is the defect @@ -1513,31 +1550,40 @@ correct fix in one place that looks complete. ## 6. Q#LL4 --- `editing.fill-column` **ANSWERED** -> **Answered 2026-08-06: do not adopt it** --- but the reason is -> sharper than "a different concept", and §1.1 overstated the finding. +> **Answered 2026-08-06: do not adopt it.** *(Revised 2026-08-07 --- the +> premise was wrong; the answer was not.)* > -> `editing.fill-column` is orphaned because **its consumer does not -> exist yet**: there is no `M-q`, no auto-fill, and no reflow command -> anywhere in this codebase. It is a setting ahead of its feature. -> That is a *different* defect from `full_grid`'s, which was a flag -> with a live consumer that ignored it, and §1.1 should not be read as -> equating them. +> The 2026-08-06 answer said the setting was orphaned because **its +> consumer does not exist yet**: no `M-q`, no auto-fill, no reflow +> command anywhere in the tree. That much is true. But the setting does +> not exist either --- it is a **test fixture name**, not a definition +> (§1.1, withdrawn). So the framing of "a setting ahead of its feature" +> described nothing real, and the `full_grid` comparison it was already +> walking back was doubly inapt: `full_grid` was a wire flag with a +> live consumer that ignored it, and this is a string in two +> `#[cfg(test)]` blocks. > -> Two things Stage 3 does owe it: +> **What changed in the deliverables.** One of the two evaporated: > -> - **Sharpen its description.** Once a wrap setting ships, -> `"Preferred wrap column."` actively invites the wrong conclusion. -> It must say it governs reflow commands, not display. +> - ~~**Sharpen its description.**~~ There is no shipped description. +> Replaced by: **name the fixtures as fixtures** at +> `src/config_registry.rs` and `src/lua_bindings/config.rs`, so the +> next reader does not repeat §1.1. That is the entire remaining cost, +> and it is a comment. > - **Name ours so confusion is impossible: `ui.line-wrap`**, > `ConfigKind::Enum { choices: ["wrap", "truncate"] }`, default -> `"wrap"`. `editing.*` is buffer-editing behavior, `ui.*` is -> display; both existing `ui.*` settings carry a `gpu-` prefix to -> mark frontend-specificity, so its **absence** here is what signals -> "both frontends". - - -Give it a consumer, or state why display wrap is a separate concept and -leave it orphaned with that reasoning recorded. See §1.1. +> `"wrap"`. Unchanged, and it never depended on the bad premise --- +> `editing.*` is buffer-editing behavior, `ui.*` is display; both +> existing `ui.*` settings carry a `gpu-` prefix to mark +> frontend-specificity, so its **absence** here is what signals "both +> frontends". The naming discipline stands on the concept split, which +> a real `fill-column` would only have made more urgent, not less. +> +> **The one place this could have bitten.** Had the premise gone +> unchecked into implementation, Stage 3 would have shipped an edit to a +> unit-test fixture believing it was rewording a user-visible setting +> --- a no-op change with a misleading commit message, and a lane +> closing on a deliverable it had not delivered. --- @@ -1718,14 +1764,48 @@ Not final --- it depends on Q#LL1. backwards --- it proves only that an explicit non-wrap mode reaches the GPU's layout. +### 7.1 What the sketch did not anticipate (revision 20) + +Three witnesses exist that §7 never asked for, each because review or +implementation found a defect the sketch had no reason to predict. +Recorded so the gap between sketch and suite is deliberate: + +- **Rendered-output witnesses** + (`the_default_actually_wraps_the_painted_text` and its `truncate` + control, `tests/line_wrap_acceptance.rs`). Every test §7 sketched + asks a *component* a question. The defect review found lived in the + **driver**: `src/editor.rs` resolved the mode, recorded it on the + window, and fed it to coordinate mapping and the indicator --- while + the `Viewport` literal it built still carried a hard-coded + `Truncate`. A test constructing its own viewport passes against that. + These read the grid reconstructed from emitted `CellDelta` spans + instead. +- **Two-buffer toggle witnesses** (same file). `ui.line-wrap` is + buffer-local, so the toggle's failure mode --- reading and writing + the *global* layer --- is invisible in any single-buffer test, and + wrong in both directions at once: it leaves a pinned buffer alone + while silently moving every unpinned one. +- **GPU scroll-endpoint witnesses** (`pmacs-gpu/src/main.rs`). §5d.6 + settled *what* the classifier consumes; it did not settle how the GPU + decides `first_visible` / `last_visible`, and the two cheap answers + are both wrong (`view_range` includes overscan; `scroll_top` ignores + the sub-line residual). The trailing-empty-line case + (`an_empty_final_line_still_counts_as_bot`) was found *by* these + tests, not confirmed by them --- it made every newline-terminated + file report a percentage instead of `Bot` at the bottom. + --- ## 8. Coherence impact (§20 requirement) - **Scorecard row 11, "Config layering + provenance --- Partial - (foundation only), 5 settings live in it."** This lane adds registry - settings against that row, and either adopts or explicitly declines - the orphaned `editing.fill-column`. + (foundation only), 5 settings live in it."** This lane adds + `ui.line-wrap` against that row --- registry-defined, enum-validated, + buffer-local, and consumed by both frontends. *(Revision 20: this + bullet previously also promised to adopt or decline "the orphaned + `editing.fill-column`". It is not orphaned and not a setting; see + §1.1, withdrawn. The count in that scorecard row was never affected + by it either way.)* - **Journey step 4, "Understand interface --- Partial."** A line that cannot be read in full is a direct hit on this step; the scorecard does not currently name it, and should. diff --git a/src/config_registry.rs b/src/config_registry.rs index 04db52a..5e2dd1a 100644 --- a/src/config_registry.rs +++ b/src/config_registry.rs @@ -1187,6 +1187,16 @@ mod tests { src(2), ) .unwrap(); + // The names here are ILLUSTRATIVE — one per `ConfigKind`, chosen + // to read plausibly. Only `editing.auto-pair` and + // `autosave.interval-ms` are real settings (`builtin/runtime/`); + // the other three are defined nowhere but this test. + // + // Worth saying out loud because `editing.fill-column` fooled the + // long-lines framing into three paragraphs about an "orphaned + // setting of the same shape as the `full_grid` defect". It is not + // orphaned and not a defect: it has never shipped, so no user can + // get it, set it, or discover it. r.define( "editing.fill-column".into(), "Preferred wrap column.".into(), diff --git a/src/lua_bindings/config.rs b/src/lua_bindings/config.rs index 4739ce7..12bddec 100644 --- a/src/lua_bindings/config.rs +++ b/src/lua_bindings/config.rs @@ -887,6 +887,12 @@ mod tests { // ---- acceptance 1: round-trip every kind, via Lua ---------------------- + /// The names below are ILLUSTRATIVE — one per `ConfigKind`, chosen + /// to read plausibly. Only `editing.auto-pair` and + /// `autosave.interval-ms` are real settings (`builtin/runtime/`); the + /// other three are defined nowhere but this test and its twin in + /// `config_registry.rs`. `editing.fill-column` in particular has + /// never shipped, so no user can get it, set it, or discover it. #[test] fn define_then_get_round_trips_every_kind_via_lua() { let (lua, _reg) = fresh(); diff --git a/tests/long_line_readable_acceptance.rs b/tests/long_line_readable_acceptance.rs new file mode 100644 index 0000000..b163842 --- /dev/null +++ b/tests/long_line_readable_acceptance.rs @@ -0,0 +1,155 @@ +//! The originating report, at the real PTY boundary. +//! +//! > "long lines need to either wrap somehow or be scrollable. Haven't +//! > tried this in GUI, but in TUI, a line that extends off screen +//! > cannot be read in full in any way." +//! +//! Every other test in this lane checks a mechanism: that the mode +//! resolves, that it reaches the viewport, that the classifier agrees +//! across frontends. This one checks the **complaint** — that the end of +//! a long line reaches a terminal at all — and it is deliberately the +//! only test here that runs the shipped binary against a real PTY. +//! +//! # Why this is not redundant with `line_wrap_acceptance.rs` +//! +//! That suite reconstructs rows from emitted `CellDelta` spans, which is +//! the right granularity for asserting *where* text lands. But it drives +//! `RenderState` in-process, so everything between the grid and a +//! terminal — the frontend, the ANSI writer, startup, the real terminal +//! size — is assumed rather than exercised. The defect being fixed was +//! reported from a terminal, so at least one test should end in one. +//! +//! # What it asserts, and why that is the honest assertion +//! +//! The vterm suites here assert on raw output bytes; there is no screen +//! model and no `vt100` / `termwiz` / `vte` dependency in the workspace +//! (`full_grid_resync_acceptance.rs` records the same limit). So this +//! proves **the tail of the line was written to the terminal**, not that +//! it occupies the row a human would point at. That is nevertheless the +//! whole of the original report: under truncation those bytes are never +//! emitted at all, because there is no column past the edge to paint +//! them into and no horizontal scrolling to reveal them. +//! +//! Hence the `truncate` control below. Without it the wrap assertion +//! would be satisfied by anything that happened to echo the fixture, and +//! the pair is what makes the marker discriminating. + +use std::time::{Duration, Instant}; + +#[path = "common/mod.rs"] +mod common; + +use common::pty::{PmacsPty, spawn_pmacs_in_pty}; + +/// Painted first, well within any terminal width — the "pmacs got this +/// far" anchor that keeps an absence assertion from passing vacuously. +const HEAD: &[u8] = b"HEADZQX"; +/// Painted only if something puts it on a row: it sits ~200 columns into +/// a single source line, past the right edge of the 80-column PTY below. +const TAIL: &[u8] = b"TAILZQX"; + +/// One source line, far wider than the terminal, marked at both ends. +fn fixture() -> String { + format!( + "{}{}{}\n", + String::from_utf8_lossy(HEAD), + "-".repeat(200), + String::from_utf8_lossy(TAIL), + ) +} + +fn contains(haystack: &[u8], needle: &[u8]) -> bool { + haystack.windows(needle.len()).any(|w| w == needle) +} + +/// Block until `needle` appears in pmacs's output. +/// +/// Content-anchored rather than timed, for the reason +/// `full_grid_resync_acceptance.rs` spells out: a settled pmacs screen +/// emits per-frame bytes forever, so "output stopped growing" never +/// becomes true and cannot mark the end of startup. +fn wait_for(pty: &PmacsPty, needle: &[u8], timeout: Duration) { + let deadline = Instant::now() + timeout; + loop { + if contains(&pty.output(), needle) { + return; + } + assert!( + Instant::now() < deadline, + "pmacs never painted {:?} within {timeout:?}; emitted {} bytes", + String::from_utf8_lossy(needle), + pty.output().len() + ); + std::thread::sleep(Duration::from_millis(20)); + } +} + +/// Spawn pmacs over the fixture in an 80x24 PTY, with an isolated +/// config root that optionally carries an `init.lua`. +fn spawn(dir: &std::path::Path, init_lua: Option<&str>) -> PmacsPty { + let file = dir.join("longline.txt"); + std::fs::write(&file, fixture()).expect("write fixture"); + if let Some(body) = init_lua { + let cfg = dir.join("pmacs"); + std::fs::create_dir_all(&cfg).expect("create config dir"); + std::fs::write(cfg.join("init.lua"), body).expect("write init.lua"); + } + spawn_pmacs_in_pty( + &[file.to_str().expect("utf-8 path")], + &[("HOME", dir), ("XDG_CONFIG_HOME", dir)], + 24, + 80, + ) +} + +fn quit(pty: &mut PmacsPty) { + let _ = pty.write_input(b"\x18\x03"); // C-x C-c + let _ = pty.wait_for_exit(Duration::from_secs(5)); +} + +/// The report, closed: opening a file whose line runs off the right edge +/// puts the end of that line on the terminal. +#[test] +fn the_end_of_a_long_line_reaches_the_terminal() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut pty = spawn(dir.path(), None); + + wait_for(&pty, HEAD, Duration::from_secs(20)); + wait_for(&pty, TAIL, Duration::from_secs(20)); + + quit(&mut pty); +} + +/// The control that makes the marker above mean something: pinned to +/// `truncate`, the same fixture in the same terminal never emits the +/// tail. +/// +/// This is also the honest statement of what `truncate` costs today. +/// Those bytes are not merely off-screen, they are unreachable — there +/// is no horizontal scrolling yet, which is why `wrap` is the default +/// and why `ui.toggle-line-wrap` says so when it turns wrapping off. +#[test] +fn truncate_leaves_the_end_of_the_line_unreachable() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut pty = spawn( + dir.path(), + Some("pmacs.config.set('ui.line-wrap', 'truncate')\n"), + ); + + // The anchor first: without it, "TAIL never appeared" would also be + // satisfied by pmacs failing to start. + wait_for(&pty, HEAD, Duration::from_secs(20)); + // Head and tail would be painted in the SAME frame if wrapping were + // on, so this settle is generous rather than load-bearing. + std::thread::sleep(Duration::from_millis(750)); + + assert!( + !contains(&pty.output(), TAIL), + "truncate must clip at the edge — emitting the tail would mean \ + the mode reached the resolver but not the renderer, which is \ + exactly the defect the rendered witnesses in \ + line_wrap_acceptance.rs guard from the other side" + ); + + quit(&mut pty); +}